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

feat(cli): Add --frozen flag to error out if lockfile is out of date #24355

Merged
merged 16 commits into from
Jul 2, 2024

Conversation

nathanwhit
Copy link
Member

Closes #18296.

Adds a --frozen (alias --frozen-lockfile) flag that errors out if the lockfile is out of date. This is useful for running in CI (where an out of date lockfile is usually a mistake) or to prevent accidental changes in dependencies.

Screenshot 2024-06-26 at 7 11 13 PM

cli/graph_util.rs Outdated Show resolved Hide resolved
nathanwhit added a commit that referenced this pull request Jun 29, 2024
As suggested in
#24355 (comment).

I wasn't able to hide the mutex stuff as much as I'd like (ended up just
adding an escape hatch `inner()` method that locks the inner mutex),
because you can't return references to the inner fields through a mutex.

This is mostly motivated by the frozen lockfile changes
@nathanwhit nathanwhit force-pushed the frozen-lockfile branch 3 times, most recently from 66d248a to 87d8ecc Compare June 29, 2024 00:41
@nathanwhit nathanwhit requested a review from dsherret July 1, 2024 16:55
Copy link
Member

@dsherret dsherret left a comment

Choose a reason for hiding this comment

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

Looks great!

Can you add one test for something that does a non-analyzable dynamic import of an npm package and then another for an https or jsr specifier? It looks like the code will handle it.

sbmsr pushed a commit to sbmsr/deno-1 that referenced this pull request Jul 2, 2024
…and#24366)

As suggested in
denoland#24355 (comment).

I wasn't able to hide the mutex stuff as much as I'd like (ended up just
adding an escape hatch `inner()` method that locks the inner mutex),
because you can't return references to the inner fields through a mutex.

This is mostly motivated by the frozen lockfile changes
Copy link
Member

@dsherret dsherret left a comment

Choose a reason for hiding this comment

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

LGTM!

Err(err) => Err(err).with_context(|| {
format!("Failed reading lockfile '{}'", filename.display())
}),
}
}
pub fn error_if_changed(&self) -> Result<(), AnyError> {
let lockfile = self.lockfile.lock();
if self.frozen && lockfile.has_content_changed {
Copy link
Member

Choose a reason for hiding this comment

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

In the future, we should probably error on insert into the lockfile.

An issue at the moment, is if a dynamic import inserts into the lockfile then all future dynamic imports will fail.

cli/args/flags.rs Outdated Show resolved Hide resolved
cli/args/lockfile.rs Outdated Show resolved Hide resolved
@nathanwhit nathanwhit merged commit c13b6d1 into denoland:main Jul 2, 2024
17 checks passed
@nathanwhit nathanwhit deleted the frozen-lockfile branch July 2, 2024 22:00
bartlomieju added a commit that referenced this pull request Jul 5, 2024
This commit deprecates `--lock-write` flag by removing it from
the help output and printing a warning message when it's used.

Users should use `--frozen=false` instead which was added
in #24355.

Towards #24167.
zebreus pushed a commit to zebreus/deno that referenced this pull request Jul 8, 2024
…and#24366)

As suggested in
denoland#24355 (comment).

I wasn't able to hide the mutex stuff as much as I'd like (ended up just
adding an escape hatch `inner()` method that locks the inner mutex),
because you can't return references to the inner fields through a mutex.

This is mostly motivated by the frozen lockfile changes
zebreus pushed a commit to zebreus/deno that referenced this pull request Jul 8, 2024
denoland#24355)

Closes denoland#18296.

Adds a `--frozen` (alias `--frozen-lockfile`) flag that errors out if
the lockfile is out of date. This is useful for running in CI (where an
out of date lockfile is usually a mistake) or to prevent accidental
changes in dependencies.

![Screenshot 2024-06-26 at 7 11
13 PM](https://github.com/denoland/deno/assets/17734409/538404b8-b422-4f05-89e8-4c9b1c248576)
zebreus pushed a commit to zebreus/deno that referenced this pull request Jul 8, 2024
This commit deprecates `--lock-write` flag by removing it from
the help output and printing a warning message when it's used.

Users should use `--frozen=false` instead which was added
in denoland#24355.

Towards denoland#24167.
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.

Make additive lock file optional
3 participants