-
Notifications
You must be signed in to change notification settings - Fork 28.6k
fix(turbopack): Use vergen-git2 instead of shadow-rs for napi and next-api crates to fix stale git lock files #76773
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,18 +1,52 @@ | ||
use std::fs; | ||
use std::{env, process::Command, str}; | ||
|
||
extern crate napi_build; | ||
|
||
fn main() { | ||
fn main() -> anyhow::Result<()> { | ||
println!("cargo:rerun-if-env-changed=CI"); | ||
let is_ci = env::var("CI").is_ok_and(|value| !value.is_empty()); | ||
|
||
// Generates, stores build-time information as static values. | ||
// There are some places relying on correct values for this (i.e telemetry), | ||
// So failing build if this fails. | ||
shadow_rs::ShadowBuilder::builder() | ||
.build() | ||
.expect("Should able to generate build time information"); | ||
let cargo = vergen_git2::CargoBuilder::default() | ||
.target_triple(true) | ||
.build()?; | ||
// We use the git dirty state to disable persistent caching (persistent caching relies on a | ||
// commit hash to be safe). One tradeoff of this is that we must invalidate the rust build more | ||
// often. | ||
// | ||
// This invalidates the build if any untracked files change. That's sufficient for the case | ||
// where we transition from dirty to clean. | ||
// | ||
// There's an edge-case here where the repository could be newly dirty, but we can't know | ||
// because our build hasn't been invalidated, since the untracked files weren't untracked last | ||
// time we ran. That will cause us to incorrectly report ourselves as clean. | ||
// | ||
// However, in practice that shouldn't be much of an issue: If no other dependency of this | ||
// top-level crate has changed (which would've triggered our rebuild), then the resulting binary | ||
// must be equivalent to a clean build anyways. Therefore, persistent caching using the HEAD | ||
// commit hash as a version is okay. | ||
let git = vergen_git2::Git2Builder::default() | ||
.dirty(/* include_untracked */ true) | ||
.describe( | ||
/* tags */ true, | ||
/* dirty */ !is_ci, // suppress the dirty suffix in CI | ||
/* matches */ Some("v[0-9]*"), // find the last version tag | ||
) | ||
.build()?; | ||
vergen_git2::Emitter::default() | ||
Comment on lines
+30
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's very critical that this That was a bug in the past and has led to corrupted persistent cache. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added this logging in line 46 to test that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The As the comment block above states, there's an edge case where we may not detect a newly-dirty repository, but because this is the top-level crate, that would mean that none of the changed files are depended on by the binary, so it shouldn't really matter? I think the problems that occurred before were related to I could add hack here (like the one For CI (the most important place to get this right), the dependency on |
||
.add_instructions(&cargo)? | ||
.add_instructions(&git)? | ||
.fail_on_error() | ||
.emit()?; | ||
|
||
let git_head = fs::read_to_string("../../.git/HEAD").unwrap_or_default(); | ||
if !git_head.is_empty() && !git_head.starts_with("ref: ") { | ||
println!("cargo:warning=git version {}", git_head); | ||
match Command::new("git").args(["rev-parse", "HEAD"]).output() { | ||
Ok(out) if out.status.success() => println!( | ||
"cargo:warning=git HEAD: {}", | ||
str::from_utf8(&out.stdout).unwrap() | ||
), | ||
_ => println!("cargo:warning=`git rev-parse HEAD` failed"), | ||
} | ||
|
||
#[cfg(not(all(target_os = "macos", target_arch = "aarch64")))] | ||
|
@@ -36,4 +70,6 @@ fn main() { | |
|
||
#[cfg(not(target_arch = "wasm32"))] | ||
turbo_tasks_build::generate_register(); | ||
|
||
Ok(()) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,18 @@ | ||
use turbo_tasks_build::generate_register; | ||
|
||
fn main() { | ||
fn main() -> anyhow::Result<()> { | ||
// Generates, stores build-time information as static values. | ||
// There are some places relying on correct values for this (i.e telemetry), | ||
// So failing build if this fails. | ||
shadow_rs::ShadowBuilder::builder() | ||
.build_pattern(shadow_rs::BuildPattern::Lazy) | ||
.build() | ||
.expect("Should able to generate build time information"); | ||
let cargo = vergen::CargoBuilder::default() | ||
.target_triple(true) | ||
.build()?; | ||
vergen::Emitter::default() | ||
.add_instructions(&cargo)? | ||
.fail_on_error() | ||
.emit()?; | ||
|
||
generate_register(); | ||
|
||
Ok(()) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,5 @@ | ||
use turbo_tasks_build::generate_register; | ||
use vergen::{vergen, Config}; | ||
|
||
fn main() { | ||
generate_register(); | ||
|
||
// Attempt to collect some build time env values but will skip if there are any | ||
// errors. | ||
let _ = vergen(Config::default()); | ||
} |
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.
The tag thing didn't work in CI. I think we only do a shallow clone without tags, so that's broken.
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.
I tested this by changing the
matches
parameter to something that won't match and usingcargo check -vv
(which shows the output ofbuild.rs
) and I get:Git falls back to just using the short commit hash, which is the same as what your code was previously doing when
LAST_TAG
didn't resolve.