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

Profile include order #6398

Merged
merged 2 commits into from Jan 29, 2020
Merged

Profile include order #6398

merged 2 commits into from Jan 29, 2020

Conversation

tru
Copy link
Contributor

@tru tru commented Jan 21, 2020

Changelog: BugFix: Avoid included profiles overwriting variables in the current profile.
Docs: Omit

Fix profile include order variable overwriting
If you had a setup like this:

myinclude

MYVAR=foo

myprofile

include(myinclude)
MYVAR=bar

The end result would be MYVAR=foo which is very confusing behavior. I think most users would expect MYVAR to be bar.

This was caused by inherited_vars being a reference and not a copy. Explicity copying the profile_parser.vars fixes the problem which leads me to think this is the original intent of this function since it matches what my expection would be as well.

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

If you had a setup like this:

myinclude
---------
MYVAR=foo
---------

myprofile
---------
include(myinclude)
MYVAR=bar
---------

The end result would be MYVAR=foo which is *very* confusing
behavior. I think most users would expect MYVAR to be bar.

This was caused by inherited_vars being a reference and not
a copy. Explicity copying the profile_parser.vars fixes the
problem which leads me to think this is the original intent
of this function since it matches what my expection would be
as well.
@tru tru force-pushed the profile_include_order branch from 365c0fc to 6dfa378 Compare Jan 21, 2020
@tru tru requested review from lasote and memsharded Jan 21, 2020
@memsharded memsharded added this to the 1.22 milestone Jan 28, 2020
@memsharded memsharded merged commit ebcfe58 into conan-io:develop Jan 29, 2020
2 checks passed
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.

None yet

2 participants