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

Integer based deterministic lockfile ids #6139

Merged

Conversation

memsharded
Copy link
Member

@memsharded memsharded commented Nov 27, 2019

Changelog: Fix: Make lockfiles invariant when the graph doesn't change. Now 2 different lockfiles captured with the same resulting graph in 2 different instants will be identical.
Docs: Omit

Close #6119

@memsharded memsharded mentioned this pull request Nov 27, 2019
1 task
@memsharded memsharded requested a review from lasote Nov 27, 2019
@DavidZemon
Copy link

@DavidZemon DavidZemon commented Nov 27, 2019

I've checked out this PR and installed locally. I certainly won't claim to have done any exhaustive regression tests, but it does seem to work as I was originally hoping when I first heard about lockfiles and it does solve the problem from #6119.

@memsharded memsharded added this to the 1.21 milestone Dec 2, 2019
@lasote lasote changed the title [POC] integer based deterministic lockfile ids Integer based deterministic lockfile ids Dec 3, 2019
@lasote lasote self-assigned this Dec 3, 2019
lasote
lasote approved these changes Dec 4, 2019
@@ -13,7 +14,7 @@


LOCKFILE = "conan.lock"
LOCKFILE_VERSION = "0.1"
LOCKFILE_VERSION = "0.2"
Copy link
Contributor

@lasote lasote Dec 4, 2019

Choose a reason for hiding this comment

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

👍

@lasote lasote merged commit c7521fc into conan-io:develop Dec 4, 2019
2 checks passed
@memsharded memsharded deleted the feature/deterministic_lockfile_ids branch Dec 4, 2019
@DavidZemon
Copy link

@DavidZemon DavidZemon commented Dec 10, 2019

I just noticed an unfortunate aspect of this change: though it is sorted and it is consistent, it's not sorted "correctly." It's using alphabetical sorting instead of numerical and therefore nodes are printed in order 1 -> 10 -> 2 -> 3 ...

Doesn't really matter, but it does look a little funny and was not expected.

@memsharded
Copy link
Member Author

@memsharded memsharded commented Dec 10, 2019

I'll try to change that in the next breaking change (version bump) of lockfiles.

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.

3 participants