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

Allow dependencies in Proto.lock to be updated when manifest changes #257

Merged
merged 4 commits into from
Jul 9, 2024

Conversation

fredrb
Copy link
Contributor

@fredrb fredrb commented Jul 5, 2024

This PR allows buffrs install to make changes to the lockfile when proto dependency versions were upgraded in the manifest.

This code change is sufficient now because buffrs only support pinned versions in the manifest, so we can rely on the manifest always having the desired version. However, whenever we start supporting semantic version upgrades this logic might need to change.

How I validated that my change works

Open Item

I wasn't able to load contents from cache in case we are ignoring the locked dependency and downloading a new version because Cache receives a FileRequirement which can be created from LockedPackage but can't be created from Package. I assume this could be fixed with impl From<Package> for FileRequirement { ... }.

src/resolver.rs Outdated Show resolved Hide resolved
@mara-schulke
Copy link
Contributor

It would be great if we could write a snapshot test for this but im happy to get this merged! Its a big QoL improvement 😊

@fredrb fredrb force-pushed the fred/lockfile-upgrade branch from bf869d5 to ef636e8 Compare July 5, 2024 14:18
@fredrb fredrb requested a review from mara-schulke July 8, 2024 09:23
@fredrb fredrb force-pushed the fred/lockfile-upgrade branch from 2c410b6 to 331b360 Compare July 8, 2024 13:06
@mara-schulke
Copy link
Contributor

Thank you!

@fredrb fredrb merged commit 72377a4 into main Jul 9, 2024
7 checks passed
@fredrb fredrb deleted the fred/lockfile-upgrade branch July 9, 2024 07:21
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.

2 participants