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

Apply general scope options to consumer Conanfile first #3361

Merged
merged 4 commits into from
Aug 22, 2018

Conversation

danimtb
Copy link
Member

@danimtb danimtb commented Aug 17, 2018

The real issue was this line in configure():

self.options["*"].shared = self.options.shared

And user input with something like conan install -o *:shared=True.

Basically, options with * operator are reserved for the graph builder to be applied to requirements when graph is complete. The option with * is not applied initially to the consumer conanfile and when it hits the line above in configure, changes the general scope option to its default value (shared=False).

  • 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. Also adding a description of the changes in the changelog.rst file. https://github.com/conan-io/docs

@ghost ghost assigned danimtb Aug 17, 2018
@ghost ghost added the stage: review label Aug 17, 2018
@danimtb danimtb changed the title NOT MERGE, CI ONLY: Apply general scope options to consumer Conanfile first Apply general scope options to consumer Conanfile first Aug 17, 2018
@danimtb danimtb assigned memsharded and unassigned danimtb Aug 17, 2018
@memsharded memsharded added this to the 1.7 milestone Aug 17, 2018
default_options= "shared=False"
requires = "libA/0.1@danimtb/testing"

def configure(self):
Copy link
Member

Choose a reason for hiding this comment

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

This test looks a bit complex for the use case. Just defining -o *:shared in the command line should be enough to check it, wouldn't it?

Copy link
Member Author

Choose a reason for hiding this comment

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

No. this is in fact the real issue this is testing. Read the description in the PR comments

@lasote
Copy link
Contributor

lasote commented Aug 20, 2018

But, why would you type self.options["*"].shared = self.options.shared in the configure? I don't get it. Without that line, does everything work as expected?

@danimtb
Copy link
Member Author

danimtb commented Aug 20, 2018

because you want to build your hole dependency graph with that option. So when the consumer project declares that option, all dependencies are the same.

I know it makes no sense to have that line and use the command:

conan install -o *:shared=True

You'd rather use:

conan install -o consumer:shared=True

First does NOT work, second does.

But the weird thing for the user is that it also works for:

conan install -o shared=True


Without that line, does everything work as expected?

Yes

@memsharded memsharded merged commit f45eacf into conan-io:develop Aug 22, 2018
@ghost ghost removed the stage: review label Aug 22, 2018
grisumbras pushed a commit to grisumbras/conan that referenced this pull request Dec 27, 2018
* Apply general scope options to consumer first

* try descop options

* improved test and changed fix

* Improved test with create and install
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.

conan install & conan build using option patterns
3 participants