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

Append CONAN_LIBS and CONAN_SYSTEM_LIBS CMake variables to avoid overwriting user-defined libs #6433

Merged
merged 6 commits into from Jan 30, 2020

Conversation

danimtb
Copy link
Member

@danimtb danimtb commented Jan 28, 2020

Changelog: Bugfix: Append CONAN_LIBS in cmake generator to avoid overwriting user-defined libs.
Docs: omit

  • Refer to the issue that supports this Pull Request: closes #6396
  • 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.

@danimtb danimtb added this to the 1.22 milestone Jan 28, 2020
@danimtb danimtb self-assigned this Jan 28, 2020
@danimtb danimtb changed the title Feature/6396 Append CONAN_LIBS and CONAN_SYSTEM_LIBS CMake variables to avoid overwriting user-defined libs Jan 28, 2020
Copy link
Member

@jgsogo jgsogo left a comment

If this is for 1.22, then it should take into account #6298

@danimtb
Copy link
Member Author

@danimtb danimtb commented Jan 28, 2020

Apparently, changes in this PR are not affecting the changes in PR #6298. I have run the tests of #6298 with changes here and it worked.

jgsogo
jgsogo approved these changes Jan 29, 2020
Copy link
Member

@jgsogo jgsogo left a comment

Thanks for taking the time to test these changes taking into account the other PR 🙌 Indeed, that one is about targets and shouldn't interfere.

Copy link
Member

@memsharded memsharded left a comment

Looks good, please create a 1.21.2 release branch and target this PR to it.
Thanks!

@memsharded memsharded removed this from the 1.22 milestone Jan 29, 2020
@memsharded memsharded added this to the 1.21.2 milestone Jan 29, 2020
@danimtb danimtb changed the base branch from develop to release/1.21.2 Jan 29, 2020
@jgsogo
Copy link
Member

@jgsogo jgsogo commented Jan 29, 2020

@memsharded This PR is running Conan v1.21, so Python 3.8 is not supported yet and CI won't pass. This is going to happen in this PR, in branch release/1.21.2 and also in master before merging 1.22.

Right now the CI uses py38 unconditionally, we could add some kind of if/else conditions in the CI source based on the branch name to remove it from the CI matrix, but I feel like it would be cleaner to backport Python 3.8 changes to this branch too (in fact we should backport it to any Conan version we consider alive). 👍 or 👎 ?

@jgsogo
Copy link
Member

@jgsogo jgsogo commented Jan 29, 2020

@danimtb, We are adding Python 3.8 to release/1.21.2, this PR should pass after those changes are available

@memsharded memsharded merged commit fe4df53 into conan-io:release/1.21.2 Jan 30, 2020
1 of 2 checks passed
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.

None yet

3 participants