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

reviewing lockfiles for 2.0 #11135

Merged

Conversation

memsharded
Copy link
Member

@memsharded memsharded commented Apr 29, 2022

Review from Tribe proposal conan-io/tribe#34:

  • Defined a default "conan.lock" filename
  • Whenever the default "conan.lock" file is found in the current folder, it will be used as --lockfile=conan.lock, automatically by commands that implement this argument
  • conan lock create will also default to output "conan.lock", even without --lockfile-out argument
  • Commands that implement a --lockfile-out will allow to save a lockfile as a result, even if a lockfile was not provided as input
  • The logic for strict vs non-strict has changed its default. Now by default, commands are "strict", and the arg --lockfile-no-strict (arg name not great, we can try to think a better one) will allow non-locked requires to not fail
  • The --clean argument that was existing only for conan lock create has been renamed to --lockfile-clean, because it now can be used by the other commands that save a --lockfile-out.

Second round of review:

  • The conan.lock is assumed by default besides the conanfile, not in the cwd
  • Renamed --lockfile-no-strict => --lockfile-partial, it seems a bit more aligned with the actual behavior

@SSE4
Copy link
Contributor

SSE4 commented Apr 29, 2022

what's the summary of the review? discussion is pretty long, it's hard to extract the final resolution

@memsharded
Copy link
Member Author

what's the summary of the review? discussion is pretty long, it's hard to extract the final resolution

It is a draft, it will be posted on the Tribe repo when ready, and most likely submitted to the mailing list

@memsharded memsharded added this to the 2.0.0-alpha7 milestone Apr 29, 2022
@memsharded memsharded marked this pull request as ready for review April 29, 2022 19:30
add_lockfile_args(parser)
parser.add_argument("-l", "--lockfile", action=OnceArgument,
help="Path to a lockfile.")
parser.add_argument("--lockfile-no-strict", action="store_true",
Copy link
Contributor

Choose a reason for hiding this comment

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

--lockfile-partial ?

Copy link
Member Author

Choose a reason for hiding this comment

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

It could be a complete lockfile, but you changed one requires = xxx in your recipe and use against the lockfile, want to "lock whatever you can, but if there is something you cannot lock, then evaluate it as normal". I am not sure -lockfile-partial would be closer to that than no-strict

Copy link
Contributor

Choose a reason for hiding this comment

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

"lock whatever you can, but if there is something you cannot lock, then evaluate it as normal"

So it applies the lockfile partially. Honestly, the strict might mean anything.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good for me. @czoido @franramirez688 ? --lockfile-partial better naming than --lockfile-no-strict?

Copy link
Contributor

Choose a reason for hiding this comment

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

I have to say I don't love --lockfile-partial either... maybe we need more ideas.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have used --lockfile-partial I agree it is slightly better than --lockfile-strict in the meaning.
If anyone else got other ideas, they are welcome.

@memsharded memsharded merged commit ac68c4a into conan-io:develop2 May 5, 2022
@memsharded memsharded deleted the feature/develop2_lockfiles_review branch May 5, 2022 09:11
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.

5 participants