Skip to content

Bugfix: add test to illustate failure when options differ between requires and build requires#9839

Closed
SSE4 wants to merge 1 commit into
conan-io:developfrom
SSE4:build_requires_different_options
Closed

Bugfix: add test to illustate failure when options differ between requires and build requires#9839
SSE4 wants to merge 1 commit into
conan-io:developfrom
SSE4:build_requires_different_options

Conversation

@SSE4
Copy link
Copy Markdown
Contributor

@SSE4 SSE4 commented Oct 20, 2021

we have found conan client may fail if options are different between requirements and build requirements:
conan-io/conan-center-index#7747 (comment)

I think the cause of the issue is:

conanfile.build_requires_options = conanfile.options.values

Changelog: omit
Docs: omit

  • 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.

… requirements

Signed-off-by: SSE4 <tomskside@gmail.com>
@jgsogo jgsogo requested a review from memsharded October 22, 2021 10:01
@memsharded memsharded added this to the 1.43 milestone Oct 28, 2021
@SSE4
Copy link
Copy Markdown
Contributor Author

SSE4 commented Oct 28, 2021

checked develop2 locally, it passes the same test, so no issue with v2

@SSE4
Copy link
Copy Markdown
Contributor Author

SSE4 commented Oct 28, 2021

I was doing a git bisect and identified a commit fixing an issue in develop2:
6a5bd19
the commit is quite big, and as it was squashed, so I can't say the exact line fixed it.
still, it identifies the only commit to be back-ported into develop to get the fix.

UPD: indeed, it removes the line doing conanfile.build_requires_options = conanfile.options.values.
@memsharded do you think this particular change can be isolated/extracted and back-ported into develop1?

@memsharded memsharded modified the milestones: 1.43, 1.44 Nov 30, 2021
@memsharded
Copy link
Copy Markdown
Member

I have checked this for a second time (sorry for the delay, I checked it some time ago, but didn't finalize report), it really looks like it is impossible to backport any of that from 2.0 to 1.X without breaking.

@jgsogo
Copy link
Copy Markdown
Contributor

jgsogo commented Dec 9, 2021

For anyone arriving at this issue, let's provide a workaround for Conan v1 (see below).

Description of the scenario

If the same recipe name is used in host and build contexts, but those contexts are using different recipes with different options (number or name, not values), for example:

name/v1/conanfile.py

class Recipe(ConanFile):
   name = "name"
   version = "v1"

   options = {
      "opt1": [True, False]
   }

name/v2/conanfile.py

class Recipe(ConanFile):
   name = "name"
   version = "v2"

   options = {
      "opt2": [True, False]
   }

And the consumer:

conanfile.txt

[requires]
name/v1

[build_requires]
name/v2

Conan v1 will compute only one set of options and it will try to apply the same set to both recipes. Conan will raise an error saying that some of those options cannot be found in the recipe (indeed, opt2 is not available in name/v1).

Workaround for Conan v1

The easiest way to workaround this issue is to match the list of options between the two versions, by adding dummy (unused) options to their recipes:

name/v1/conanfile.py

class Recipe(ConanFile):
   name = "name"
   version = "v1"

   options = {
      "opt1": [True, False],
      "opt2": [True, False],  # <-- not used in this version
   }
   
   def package_id(self):
      del self.options.opt2  # <-- removed from package-id computation

name/v2/conanfile.py

class Recipe(ConanFile):
   name = "name"
   version = "v2"

   options = {
      "opt1": [True, False],  # <-- not used in this version
      "opt2": [True, False]
   }
   
   def package_id(self):
      del self.options.opt1  # <-- removed from package-id computation

@lasote lasote modified the milestones: 1.44, 1.45 Dec 28, 2021
@memsharded memsharded modified the milestones: 1.45, 1.46 Jan 30, 2022
SpaceIm added a commit to SpaceIm/conan-center-index that referenced this pull request Feb 1, 2022
openssl is also a requirement of cmake, so we hit a bug of conan v1 related to a conflict between deleted options of different versions of openssl in requirement & build requirement.
see conan-io/conan#9839
SpaceIm added a commit to SpaceIm/conan-center-index that referenced this pull request Feb 1, 2022
openssl is also a requirement of cmake, so we hit a bug of conan v1 related to a conflict between deleted options of different versions of openssl in requirement & build requirement.
see conan-io/conan#9839
conan-center-bot pushed a commit to conan-io/conan-center-index that referenced this pull request Feb 7, 2022
* modernize

* cleanup test package

* workaround for CMake bug related to `@rpath`

* declarative patches

* tvos-watchos patch not suited for 1.1.0k to 1.1.1f

this patch has probably never been tested properly in all versions it claimed to support

* another workaround for `@rpath` in test package

openssl is also a requirement of cmake, so we hit a bug of conan v1 related to a conflict between deleted options of different versions of openssl in requirement & build requirement.
see conan-io/conan#9839
@memsharded memsharded modified the milestones: 1.46, 1.47 Mar 2, 2022
@memsharded
Copy link
Copy Markdown
Member

It will not be possible to provide a fix for 1.X, and this will need to wait until 2.0 for a solution.
Hopefully the workaround above is enough in the meantime. Thanks all for the feedback.

@memsharded memsharded closed this Mar 30, 2022
@memsharded memsharded modified the milestones: 1.47, 2.0.0-alpha6 Mar 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants