conan icon indicating copy to clipboard operation
conan copied to clipboard

[question] check_min_cppstd context

Open uilianries opened this issue 3 years ago • 6 comments

The documentation is clear about header-only packages here, but it's not helpful when a recipe may be header-only or not (e.g. fmt, spdlog). So, based on the real case https://github.com/conan-io/conan-center-index/pull/12021, I have the follow statement:

def package_id(self):
    if self.info.options.header_only:
         self.info.clear()

def validate(self):
    if self.info.settings.compiler.cppstd:
         check_min_cppstd(self, 11)
    if self.info.settings.os != "Windows" and (self.info.options.wchar_support or self.info.options.wchar_filenames):
        raise ConanInvalidConfiguration("wchar is only supported under windows")
    if not self.info.options.header_only and self.info.options.shared and is_msvc_static_runtime(self):
        raise ConanInvalidConfiguration("Visual Studio build for shared library with MT runtime is not supported")

When the option header_only=True is defined, the package ID is cleared, then self.info.settings and self.info.options are sanitized, resulting on the follow error:

ERROR: Traceback (most recent call last):
  File ".../conans/errors.py", line 34, in conanfile_exception_formatter
    yield
  File ".../conans/client/graph/graph_binaries.py", line 411, in _compute_package_id
    conanfile.validate()
  File "${HOME}/.conan/data/spdlog/1.10.0/_/_/export/conanfile.py", line 73, in validate
    if self.info.settings.compiler.cppstd:
AttributeError: 'NoneType' object has no attribute 'cppstd'

This error only occurs with Conan 1.x (I'm running 1.51.0), but with Conan 2.0-beta2 it works fine.

I see few options:

  • Update Conan 1.x to have same behavior as 2.0 for self.info
  • Add get_safe to self.info.settings and self.info.options
  • Need to mix self.options.get_safe('header_only') with self.info.

I personally prefer the first option, so we don't need to update the recipe again.

Environment

Conan Version: 1.51.0 OS: Linux Ubuntu 16.04 Architecture: x86_64 Docker image: conanio/gcc10-ubuntu16.04:1.51.0

Steps to reproduce

pip install conan==1.51.0
git clone https://github.com/uilianries/conan-center-index.git
cd conan-center-index/recipes/spdlog/all
git checkout 72df1ad8336c00ed427f864523b7037a08e7ff13
conan create . 1.10.0@  -pr:b=default -pr:h=default -tf test_v1_package -o spdlog:header_only=True 

uilianries avatar Aug 05 '22 08:08 uilianries

Hi, we introduced the validate_build() method that works with the self.settings and self.options directly. The meaning of the validate_build() is "The package cannot be built with this configuration" or ~"The package cannot be consumed with this configuration"~ <-- That is not accurate, the validate_build is only called when the package needs to be built. The validate() should be used only to check if a specific configuration of the package_id is valid or not (a package cannot exists). So, in theory, any check in the validate() should use the self.info object.


def validate(self):
    if self.info.options.get_safe("header_only") is False:
      # These configurations are impossible
      if self.info.settings.os != "Windows" and (self.info.options.wchar_support or self.info.options.wchar_filenames):
          raise ConanInvalidConfiguration("wchar is only supported under windows")
      if self.info.options.shared and is_msvc_static_runtime(self):
          raise ConanInvalidConfiguration("Visual Studio build for shared library with MT runtime is not supported")

About the check_min_cppstd(self, 11) let me think about it, I think we might need something else.

lasote avatar Aug 08 '22 06:08 lasote

Something more we have to check:

if self.info.options.get_safe("header_only") is False:
	ConanException: option 'get_safe' doesn't exist

Same for self.info.settings.get_safe

lasote avatar Aug 08 '22 07:08 lasote

@lasote Good point, but self.info.xxx.get_safe is not available for Conan 1.x. It would be great backporting that feature, otherwise, it will be impossible to use self.info.

Some log to clarify that behavior:

% conan --version     
Conan version 1.51.0

% cat conanfile.py 
from conan import ConanFile

class FooConan(ConanFile):
    options = {"shared": [True, False]}
    default_options = {"shared": False}

    def validate(self):
        if self.info.options.get_safe("shared"):
            raise Exception("Today is Friday in California.") 

% conan create . foo/0.1@ 
Exporting package recipe
foo/0.1: A new conanfile.py version was exported
foo/0.1: Folder: /home/conan/.conan/data/foo/0.1/_/_/export
foo/0.1: Using the exported files summary hash as the recipe revision: e97f2333afde5739e1018b184f602aa5 
foo/0.1: Exported revision: e97f2333afde5739e1018b184f602aa5
Configuration:
[settings]
arch=x86_64
arch_build=x86_64
build_type=Release
compiler=gcc
compiler.cppstd=17
compiler.libcxx=libstdc++11
compiler.version=12
os=Linux
os_build=Linux
[options]
[build_requires]
[env]
[conf]

ERROR: Traceback (most recent call last):
  File ".../python3.10/site-packages/conans/errors.py", line 34, in conanfile_exception_formatter
    yield
  File ".../python3.10/site-packages/conans/client/graph/graph_binaries.py", line 411, in _compute_package_id
    conanfile.validate()
  File "/home/conan/.conan/data/foo/0.1/_/_/export/conanfile.py", line 8, in validate
    if self.info.options.get_safe("shared"):
  File ".../python3.10/site-packages/conans/model/options.py", line 261, in __getattr__
    return getattr(self._package_values, attr)
  File ".../python3.10/site-packages/conans/model/options.py", line 75, in __getattr__
    raise ConanException(option_not_exist_msg(attr, list(self._dict.keys())))
conans.errors.ConanException: option 'get_safe' doesn't exist
Possible options are ['shared']

uilianries avatar Aug 08 '22 07:08 uilianries

About the check_min_cppstd(self, 11), this is a bit tricky. We could protect it (once it works) with a get_safe in the self.info.settings or using the suggested not self.info.options.get_safe("header_only") but we can not cover the case of checking the min cppstd when the package is header_only.

This is actually an old question/missing feature, what we would need to declare is "hey, consumers, I can exist for any cppstd but I need a minimum of cppstd=11 to work".

I'm opening a new issue to implement the get_safe, and let's explore again the cppstd issue.

lasote avatar Aug 08 '22 08:08 lasote

Thank you captain @lasote !

uilianries avatar Aug 08 '22 13:08 uilianries

The problem with check_min_cppstd(self, 11) is that we never clearly defined its semantics (as with most of the ConanInvalidConfiguration) if it is a package that cannot exist (cannot be built) or it is a package that cannot be used/consumed. We probably need to revisit (once more) this.

memsharded avatar Aug 08 '22 13:08 memsharded

this will be finally closed by https://github.com/conan-io/conan/pull/12555

memsharded avatar Nov 17 '22 17:11 memsharded

Closed by https://github.com/conan-io/conan/pull/12555, will be in beta.6

memsharded avatar Nov 18 '22 23:11 memsharded