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

fixing lockfile check of build_requires from 2 sources #7698

Merged
merged 1 commit into from Sep 17, 2020

Conversation

memsharded
Copy link
Member

@memsharded memsharded commented Sep 10, 2020

Changelog: BugFix: Removed lockfile checking build_requires when they come from 2 different origins: profiles and recipes.
Docs: Omit

@memsharded memsharded added this to the 1.30 milestone Sep 10, 2020
@memsharded memsharded changed the base branch from develop to release/1.29 Sep 12, 2020
@memsharded memsharded modified the milestones: 1.30, 1.29.1 Sep 12, 2020
@jgsogo
Copy link
Contributor

jgsogo commented Sep 15, 2020

When I run the proposed test, the error I get is (release 1.29.0):

⇒  conan install pkg2/1.0@ --build=pkg2 --lockfile=conan.lock
...
ERROR: 'pkg2/1.0' locked requirement 'cmake/3.18.2' not found

I see the build-requires is listed inside the lockfile: in the profile at the end, and related to the node:

   "1": {
    "ref": "pkg2/1.0#34fb7804e2c99443300f566a19ca037b",
    "options": "",
    "package_id": "5825778de2dc9312952d865df314547576f129b3",
    "requires": [
     "2"
    ],
    "build_requires": [
     "8",
     "11"  <--- this is 'cmake'
    ],
    "context": "host"
   },

# cmake is also in the profile
 "profile_host": "[settings]\narch=x86_64\narch_build=x86_64\nbuild_type=Release\ncompiler=apple-clang\ncompiler.libcxx=libc++\ncompiler.version=11.0\nos=Macos\nos_build=Macos\n[options]\n[build_requires]\n*: cmake/3.18.2\n[env]\n"

If all the information is available, why don't we read it from the profile_host before running the method lock_node?

graph_lock.lock_node(node, build_requires, build_requires=True)

@memsharded
Copy link
Member Author

memsharded commented Sep 16, 2020

That is not the problem, we have the information available. The problem was that the build_requires are passed in 2 different calls, one for the recipe defined build_requires and another call for the profile defined build_requires.

I managed to completely fix the issue while maintaining the check, by delaying the check for build_requires and doing it with both aggregated. But I felt that the code was adding a bit excessive complexity and something else could fail, and I didn't want to take the risk for a patch release. I can implement it again for 1.30.

jgsogo
jgsogo approved these changes Sep 17, 2020
Copy link
Contributor

@jgsogo jgsogo left a comment

I've opened here an issue to add the check: #7717

@jgsogo jgsogo assigned czoido and unassigned jgsogo Sep 17, 2020
@czoido czoido merged commit a39cfc5 into conan-io:release/1.29 Sep 17, 2020
2 checks passed
@memsharded memsharded deleted the fix/lockfile_check branch Sep 17, 2020
@memsharded
Copy link
Member Author

memsharded commented Sep 17, 2020

Hi @GordonJess

Released in Conan 1.29.1, please upgrade and let us know if everything is working fine now.

@memsharded memsharded restored the fix/lockfile_check branch Sep 29, 2020
@memsharded memsharded deleted the fix/lockfile_check branch Sep 29, 2020
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