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

Add a function to call conan lock create #319

Merged
merged 9 commits into from
Mar 29, 2022
Merged

Add a function to call conan lock create #319

merged 9 commits into from
Mar 29, 2022

Conversation

prince-chrismc
Copy link
Contributor

Follow up of #294

The new workflow is really good 👏 The only challenge was the base-lockfile workflow is not supported. So I modified the new conan_cmake_install to enable this feature.

@czoido
Copy link
Contributor

czoido commented Mar 8, 2021

Hi @prince-chrismc,
Thanks a lot for the contribution. Could you please give some details of the flow you want to follow using conan_cmake_lock_create?

@prince-chrismc
Copy link
Contributor Author

I am using the base lockfile workflow. conan-io/docs#1904

I build many different configuration so, I only have a base lockfile to make sure all the revisions are pinned and easily managed.
https://github.com/prince-chrismc/user-management/blob/master/backend/conan.lock

In order for this to be usable with the install command, I need to create a full lock for that profile/settings/options
This extra step is what I am trying to accomplish...

Here's the implementation I am using as a proof of concept https://github.com/prince-chrismc/user-management/blob/master/backend/cmake/conan-setup.cmake

@czoido czoido added this to the 0.17 milestone Mar 23, 2021
@czoido
Copy link
Contributor

czoido commented Mar 23, 2021

Hi @prince-chrismc,
Thanks a lot and sorry for the delay. I think this is a valid feature and we can add it for the next release.
I'll add some documentation and also tests to check the feature.

@dgoel
Copy link

dgoel commented Apr 27, 2021

This change looks great and I'm looking forward to using it. I've one question though.

Assuming both conanfile.txt (or conanfile.py) and base_conan.lock files are tracked by the version control in a repo, then there's a scenario where reproducibility feature of lockfile is lost. For example, a developer updates conanfile.txt to add a new package packageXYZ but forgets to update the base_conan.lock file. As the complete lock file is generated using the proposed conan_cmake_lock_create function, the underlying conan lock create command would happily work and only throw the following warning:

conanfile.txt: WARN: Require 'packageXYZ' cannot be found in lockfile

Is this the expected behavior? It would be really nice if the build actually fails in this case and requires the developer to manually update the base lock file to pin all the dependencies. I couldn't find a command line option to do that though. Directly using conan_cmake_install does throw this error but requires the inclusion of compiler settings etc. in the lockfile (which is less than ideal).

Any suggestions on how to handle this scenario?

Edit: I ended up implementing the following solution.

  • Given a conanfile.txt, create a (temporary) expected base lockfile 'E` inside the build directory.
  • Compare the newly generated base lockfile E with the one passed as an argument to this function L (i.e. the one committed to the version control). Aside: I had to filter out the "path": <conanfile.txt line from the file comparison since that obviously would be different.
  • If the files are the same, proceed as normal. Else, error out with a helpful message on how to update the base lockfile.

I would be happy to contribute this change upstream if it is deemed useful.

@czoido czoido modified the milestones: 0.17, 0.18 Nov 19, 2021
@prince-chrismc
Copy link
Contributor Author

@czoido I resolved some merge conflicts. it would be great to merge this 🙏

@czoido czoido modified the milestones: 0.18, 0.19 Mar 28, 2022
@czoido czoido closed this Mar 28, 2022
@czoido czoido reopened this Mar 28, 2022
@prince-chrismc
Copy link
Contributor Author

🚀

@czoido czoido merged commit c4c897e into conan-io:develop Mar 29, 2022
@SvenRoederer
Copy link

@dgoel The point you mentioned is worth keeping in mind. But the change you mention is not part of this PR, right? If not, was it PRed in some other way?

@memsharded
Copy link
Member

Hi @SvenRoederer

This thread was quite outdated, referring to the legacy cmake-conan integration that has been superseded by the new one for Conan 2 with CMake dependency providers. You might want to create new tickets instead, thanks for the feedback.

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.

5 participants