Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

- add new validate test #8383

Closed
wants to merge 1 commit into from
Closed

- add new validate test #8383

wants to merge 1 commit into from

Conversation

SSE4
Copy link
Contributor

@SSE4 SSE4 commented Jan 22, 2021

conan-io/docs#2003
#8375
conan-io/conan-center-index#4261

I think I have found a serious bug in the newly added validate function:

  1. library dep has two options, fPIC and shared
  2. shared=False is default, fPIC=True is default
  3. fPIC is removed for shared=False in config_options (according to the https://docs.conan.io/en/latest/mastering/conditional.html?highlight=conditional)
  4. consumer overrides dep:shared=True
  5. consumer checks in validate: dep:shared == True - OK
  6. consumer checks in validate: dep:fPIC == True - raises!

now we see that validates incorrectly computes value of fPIC option and shows a following error message:

ERROR: pkg/0.1: Error in validate() method, line 13
	assert self.options["dep"].fPIC == True
	ConanException: option 'fPIC' doesn't exist
Possible options are ['shared']

this is incorrect, since dep:shared=True, fPIC option exists

UPD1: validates works correctly if the option is removed in configure instead of config_options.
UPD2: validates also works incorrectly if the option is overridden from the command line (-o dep:shared=True)

Changelog: (Feature | Fix | Bugfix): Describe here your pull request
Docs: https://github.com/conan-io/docs/pull/XXXX

  • Refer to the issue that supports this Pull Request.
  • If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code.
  • I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.

Signed-off-by: SSE4 <tomskside@gmail.com>
Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

This is not a validate() error, it is a config_options or options bug. The del self.options.fPIC is being executed and the option is being deleted, it is expected that the validate() will fail.

@memsharded
Copy link
Member

Actually the documentation says it explicitly:

config_options() is used to configure or constraint the available options in a package, before they are given a value. So when a value is tried to be assigned it will raise an error. For example, let’s suppose that a certain package library cannot be built as shared library in Windows, it can be done:

def config_options(self):
if self.settings.os == "Windows":
del self.options.shared

This will be executed before the actual assignment of options (then, such options values cannot be used inside this function),

such options values cannot be used inside this function

It is documented behavior. The following test using configure() seems to work fine:

    def test_override(self):
        client = TestClient()
        conanfile = textwrap.dedent("""
           from conans import ConanFile
           class Lib(ConanFile):
               options = {"shared": [True, False], "fPIC": [True, False]}
               default_options = {"shared": False, "fPIC": True}
               def configure(self):
                   if not self.options.shared:
                       self.output.warn("REMOVING fPIC!!!!")
                       del self.options.fPIC
           """)

        client.save({"conanfile.py": conanfile})
        client.run("export . dep/1.0@")

        conanfile = textwrap.dedent("""
           from conans import ConanFile
           class App(ConanFile):
               requires = "dep/1.0"

               def configure(self):
                   self.options["dep"].shared = True

               def validate(self):
                   assert self.options["dep"].shared == True
                   assert self.options["dep"].fPIC == True
           """)

        client.save({"conanfile.py": conanfile}, clean_first=True)
        client.run("create . pkg/0.1@ --build missing")
        print(client.out)

@SSE4 SSE4 closed this Jan 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants