-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
fix: don't save lockfile if --frozen-lockfile or --no-save #17481
base: main
Are you sure you want to change the base?
Conversation
src/install/install.zig
Outdated
@@ -15439,6 +15439,9 @@ pub const PackageManager = struct { | |||
packages_len_before_install: usize, | |||
log_level: Options.LogLevel, | |||
) OOM!void { | |||
if (!this.options.do.save_lockfile) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should check for this flag before calling this function. The bug exists in the long chain of or's and and's used to create should_save_lockfile
. this.options.do.save_lockfile
is already checked, so likely it's not checked early enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved there: eb649a4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above
This reverts commit 721a23f.
What does this PR do?
When running
bun install --frozen-lockfile
against a project with apackage.json
andpackage-lock.json
, this brings back behavior of not creating abun.lock
file. (Regression?)This also gives
bun install --no-save
the same behavior. (Not sure if it ever worked when migrating.)Closes #16646
Closes #16965
How did you verify your code works?
Added some automated tests, too:
$ bun run test 16646
And verified that the tests fail against current
main
.