From 7cf5962c49cdca5f7bd5cac6aec2451d760f3814 Mon Sep 17 00:00:00 2001 From: Sebastian Holmin Date: Thu, 2 May 2024 15:50:16 +0200 Subject: [PATCH 1/5] Fix unnecessary rebuilds of `mullvad-version` --- mullvad-version/build.rs | 49 ++++++++++++++++++++++++++++------------ 1 file changed, 34 insertions(+), 15 deletions(-) diff --git a/mullvad-version/build.rs b/mullvad-version/build.rs index 08145b09246c..008ec8d104e1 100644 --- a/mullvad-version/build.rs +++ b/mullvad-version/build.rs @@ -75,6 +75,40 @@ fn get_dev_suffix(target: Target, product_version: &str) -> Option { Target::Desktop => product_version.to_owned(), }; + let git_dir = Path::new("..").join(".git"); + + // If we build our output on information about HEAD we need to re-run if HEAD moves + let head_path = git_dir.join("HEAD"); + if head_path.exists() { + println!("cargo:rerun-if-changed={}", head_path.display()); + } + + let output = Command::new("git") + .arg("branch") + .arg("--show-current") + .output() + .ok()?; + let current_branch = String::from_utf8(output.stdout).unwrap(); + // If we build our output on information about a git reference, we need to re-run + // if it moves. Instead of trying to be smart, just re-run if any git reference moves. + let git_current_branch_ref = git_dir + .join("refs") + .join("heads") + .join(current_branch.trim()); + if git_current_branch_ref.exists() { + println!( + "cargo:rerun-if-changed={}", + git_current_branch_ref.display() + ); + } + let git_current_branch_ref = git_dir.join("refs").join("tags").join(&release_tag); + if git_current_branch_ref.exists() { + println!( + "cargo:rerun-if-changed={}", + git_current_branch_ref.display() + ); + } + // Get the git commit hashes for the latest release and current HEAD // Return `None` if unable to find the hash for HEAD. let head_commit_hash = git_rev_parse_commit_hash("HEAD")?; @@ -94,21 +128,6 @@ fn get_dev_suffix(target: Target, product_version: &str) -> Option { /// /// Returns `None` if executing the `git rev-parse` command fails for some reason. fn git_rev_parse_commit_hash(git_ref: &str) -> Option { - let git_dir = Path::new("..").join(".git"); - // If we build our output on information about HEAD we need to re-run if HEAD moves - if git_ref == "HEAD" { - let head_path = git_dir.join("HEAD"); - if head_path.exists() { - println!("cargo:rerun-if-changed={}", head_path.display()); - } - } - // If we build our output on information about a git reference, we need to re-run - // if it moves. Instead of trying to be smart, just re-run if any git reference moves. - let git_refs_dir = git_dir.join("refs"); - if git_refs_dir.exists() { - println!("cargo:rerun-if-changed={}", git_refs_dir.display()); - } - let output = Command::new("git") .arg("rev-parse") .arg(format!("{git_ref}^{{commit}}")) From 57568b339518a1a8513a0e5dd8bfdf118dd4fb1e Mon Sep 17 00:00:00 2001 From: Sebastian Holmin Date: Fri, 3 May 2024 10:07:44 +0200 Subject: [PATCH 2/5] Extract `rerun-if` logic to separate fn --- mullvad-version/build.rs | 100 ++++++++++++++++++++++----------------- 1 file changed, 57 insertions(+), 43 deletions(-) diff --git a/mullvad-version/build.rs b/mullvad-version/build.rs index 008ec8d104e1..fc7eb9edbe70 100644 --- a/mullvad-version/build.rs +++ b/mullvad-version/build.rs @@ -52,15 +52,26 @@ fn get_product_version(target: Target) -> String { Target::Desktop => DESKTOP_VERSION_FILE_PATH, }; println!("cargo:rerun-if-changed={version_file_path}"); - let version = fs::read_to_string(version_file_path) + let product_version = fs::read_to_string(version_file_path) .unwrap_or_else(|_| panic!("Failed to read {version_file_path}")) .trim() .to_owned(); - if let Some(dev_suffix) = get_dev_suffix(target, &version) { - format!("{version}{dev_suffix}") + // Compute the expected tag name for the release named `product_version` + let release_tag = match target { + Target::Android => format!("android/{product_version}"), + Target::Desktop => product_version.to_owned(), + }; + + // Rerun this build script on changes to the git ref that affects the build version. + // NOTE: This must be kept up to date with the behavior of `git_rev_parse_commit_hash`. + rerun_if_git_ref_changed(&release_tag) + .expect("Failed to set 'cargo:rerun-if-changed' on git ref changes"); + + if let Some(dev_suffix) = get_dev_suffix(&product_version) { + format!("{product_version}{dev_suffix}") } else { - version + product_version } } @@ -68,13 +79,26 @@ fn get_product_version(target: Target) -> String { /// suffix if the build is not done on a git tag named `product_version`. /// This also returns `None` if the `git` command can't run, or the code does /// not live in a git repository. -fn get_dev_suffix(target: Target, product_version: &str) -> Option { - // Compute the expected tag name for the release named `product_version` - let release_tag = match target { - Target::Android => format!("android/{product_version}"), - Target::Desktop => product_version.to_owned(), - }; +fn get_dev_suffix(release_tag: &str) -> Option { + // Get the git commit hashes for the latest release and current HEAD + // Return `None` if unable to find the hash for HEAD. + let head_commit_hash = git_rev_parse_commit_hash("HEAD")?; + let product_version_commit_hash = git_rev_parse_commit_hash(release_tag); + + // If we are currently building the release tag, there is no dev suffix + if Some(&head_commit_hash) == product_version_commit_hash.as_ref() { + return None; + } + Some(format!( + "-dev-{}", + &head_commit_hash[..GIT_HASH_DEV_SUFFIX_LEN] + )) +} +/// Trigger rebuild of `mullvad-version` on changing branch (`.git/HEAD`), on changes to the ref of +/// the current branch (`.git/refs/heads/$current_branch`) and on changes to the ref of the current +/// release tag (`.git/refs/tags/$current_release_tag`). +fn rerun_if_git_ref_changed(release_tag: &str) -> std::io::Result<()> { let git_dir = Path::new("..").join(".git"); // If we build our output on information about HEAD we need to re-run if HEAD moves @@ -83,45 +107,35 @@ fn get_dev_suffix(target: Target, product_version: &str) -> Option { println!("cargo:rerun-if-changed={}", head_path.display()); } + // If we build our output on information about the ref of the current branch, we need to re-run + // on changes to it let output = Command::new("git") .arg("branch") .arg("--show-current") - .output() - .ok()?; - let current_branch = String::from_utf8(output.stdout).unwrap(); - // If we build our output on information about a git reference, we need to re-run - // if it moves. Instead of trying to be smart, just re-run if any git reference moves. - let git_current_branch_ref = git_dir - .join("refs") - .join("heads") - .join(current_branch.trim()); - if git_current_branch_ref.exists() { - println!( - "cargo:rerun-if-changed={}", - git_current_branch_ref.display() - ); - } - let git_current_branch_ref = git_dir.join("refs").join("tags").join(&release_tag); - if git_current_branch_ref.exists() { - println!( - "cargo:rerun-if-changed={}", - git_current_branch_ref.display() - ); - } + .output()?; - // Get the git commit hashes for the latest release and current HEAD - // Return `None` if unable to find the hash for HEAD. - let head_commit_hash = git_rev_parse_commit_hash("HEAD")?; - let product_version_commit_hash = git_rev_parse_commit_hash(&release_tag); + let current_branch = String::from_utf8(output.stdout).unwrap(); + let current_branch = current_branch.trim(); - // If we are currently building the release tag, there is no dev suffix - if Some(&head_commit_hash) == product_version_commit_hash.as_ref() { - return None; + // When in 'detached HEAD' state, the output will be empty. However, in that case we already get + // the ref from `.git/HEAD`, so we can safely skip this part. + if !current_branch.is_empty() { + let git_current_branch_ref = git_dir.join("refs").join("heads").join(current_branch); + if git_current_branch_ref.exists() { + println!( + "cargo:rerun-if-changed={}", + git_current_branch_ref.display() + ); + } } - Some(format!( - "-dev-{}", - &head_commit_hash[..GIT_HASH_DEV_SUFFIX_LEN] - )) + + // Since the product version depends on if the build is done on the commit with the + // corresponding release tag or not, we must track creation of/changes to said tag + let git_release_tag_ref = git_dir.join("refs").join("tags").join(release_tag); + if git_release_tag_ref.exists() { + println!("cargo:rerun-if-changed={}", git_release_tag_ref.display()); + }; + Some(()) } /// Returns the commit hash for the commit that `git_ref` is pointing to. From 7f7b95ba115a2edc8e3c66639ce9babecbce483c Mon Sep 17 00:00:00 2001 From: Sebastian Holmin Date: Fri, 3 May 2024 10:51:45 +0200 Subject: [PATCH 3/5] Add comments explaining the files we are tracking Also add a note on the `.git/packed-refs` file and why we don't need to track it. --- mullvad-version/build.rs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/mullvad-version/build.rs b/mullvad-version/build.rs index fc7eb9edbe70..5d890448699f 100644 --- a/mullvad-version/build.rs +++ b/mullvad-version/build.rs @@ -101,14 +101,17 @@ fn get_dev_suffix(release_tag: &str) -> Option { fn rerun_if_git_ref_changed(release_tag: &str) -> std::io::Result<()> { let git_dir = Path::new("..").join(".git"); - // If we build our output on information about HEAD we need to re-run if HEAD moves + // The `.git/HEAD` file contains the position of the current head. If in 'detached HEAD' state, + // this will be the ref of the current commit. If on a branch it will just point to it, e.g. + // `ref: refs/heads/main`. Tracking changes to this file will tell us if we change branch, or + // modify the current detached HEAD state (e.g. committing or rebasing). let head_path = git_dir.join("HEAD"); if head_path.exists() { println!("cargo:rerun-if-changed={}", head_path.display()); } - // If we build our output on information about the ref of the current branch, we need to re-run - // on changes to it + // The above check will not cause a rebuild when modifying commits on a currently checked out + // branch. To catch this, we need to track the `.git/refs/heads/$current_branch` file. let output = Command::new("git") .arg("branch") .arg("--show-current") @@ -135,7 +138,15 @@ fn rerun_if_git_ref_changed(release_tag: &str) -> std::io::Result<()> { if git_release_tag_ref.exists() { println!("cargo:rerun-if-changed={}", git_release_tag_ref.display()); }; - Some(()) + + // NOTE: As the repository has gotten quite large, you may find the contents of the + // `.git/refs/heads` and `.git/refs/tags` empty. This happens because `git pack-refs` compresses + // and moves the information into the `.git/packed-refs` file to save storage. We do not have to + // track this file, however, as any changes to the current branch, 'detached HEAD' state + // or tags will update the corresponding `.git/refs` file we are tracking, even if it had + // previously been pruned. + + Ok(()) } /// Returns the commit hash for the commit that `git_ref` is pointing to. From 8ef7e68411362b0685b6af4e1d371ec143f131d1 Mon Sep 17 00:00:00 2001 From: Sebastian Holmin Date: Mon, 13 May 2024 10:38:41 +0200 Subject: [PATCH 4/5] Clarify side-effect in docstring --- mullvad-version/build.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/mullvad-version/build.rs b/mullvad-version/build.rs index 5d890448699f..1714d54d9f72 100644 --- a/mullvad-version/build.rs +++ b/mullvad-version/build.rs @@ -44,8 +44,9 @@ fn main() { .unwrap(); } -/// Returns the Mullvad product version from the corresponding metadata files, -/// depending on target platform. +/// Computes the Mullvad product version using the latest release on the given platform and the git +/// hash pointed to by `HEAD`. Also triggers a rebuild of this crate when the information becomes +/// outdated. fn get_product_version(target: Target) -> String { let version_file_path = match target { Target::Android => ANDROID_VERSION_FILE_PATH, From 479ecf796629b146b04c73906c98b911f9b1ba27 Mon Sep 17 00:00:00 2001 From: Sebastian Holmin Date: Mon, 13 May 2024 14:01:28 +0200 Subject: [PATCH 5/5] Refactor and add comments --- mullvad-version/build.rs | 63 ++++++++++++++++++++-------------------- 1 file changed, 31 insertions(+), 32 deletions(-) diff --git a/mullvad-version/build.rs b/mullvad-version/build.rs index 1714d54d9f72..d5f3859e98ed 100644 --- a/mullvad-version/build.rs +++ b/mullvad-version/build.rs @@ -53,53 +53,53 @@ fn get_product_version(target: Target) -> String { Target::Desktop => DESKTOP_VERSION_FILE_PATH, }; println!("cargo:rerun-if-changed={version_file_path}"); - let product_version = fs::read_to_string(version_file_path) + + let release_version = fs::read_to_string(version_file_path) .unwrap_or_else(|_| panic!("Failed to read {version_file_path}")) .trim() .to_owned(); // Compute the expected tag name for the release named `product_version` let release_tag = match target { - Target::Android => format!("android/{product_version}"), - Target::Desktop => product_version.to_owned(), + Target::Android => format!("android/{release_version}"), + Target::Desktop => release_version.to_owned(), }; - // Rerun this build script on changes to the git ref that affects the build version. - // NOTE: This must be kept up to date with the behavior of `git_rev_parse_commit_hash`. - rerun_if_git_ref_changed(&release_tag) - .expect("Failed to set 'cargo:rerun-if-changed' on git ref changes"); - - if let Some(dev_suffix) = get_dev_suffix(&product_version) { - format!("{product_version}{dev_suffix}") - } else { - product_version - } + format!("{release_version}{}", get_suffix(&release_tag)) } -/// Returns the development suffix for the current build. A build has a development -/// suffix if the build is not done on a git tag named `product_version`. -/// This also returns `None` if the `git` command can't run, or the code does -/// not live in a git repository. -fn get_dev_suffix(release_tag: &str) -> Option { - // Get the git commit hashes for the latest release and current HEAD - // Return `None` if unable to find the hash for HEAD. - let head_commit_hash = git_rev_parse_commit_hash("HEAD")?; +/// Returns the suffix for the current build. If the build is done on a git tag named +/// `product_version` or a git repository cannot be found, the suffix is empty. Otherwise, +/// `-dev-$hash` is appended to the release version. +fn get_suffix(release_tag: &str) -> String { + if !valid_git_repo() { + return String::new(); + }; + // Rerun this build script on changes to the git ref that affects the build version. + // NOTE: This must be kept up to date with the behavior of `git_rev_parse_commit_hash`. + rerun_if_git_ref_changed(release_tag); + let head_commit_hash = + git_rev_parse_commit_hash("HEAD").expect("Failed to run `git rev-parse HEAD^{{commit}}`"); let product_version_commit_hash = git_rev_parse_commit_hash(release_tag); // If we are currently building the release tag, there is no dev suffix if Some(&head_commit_hash) == product_version_commit_hash.as_ref() { - return None; + String::new() + } else { + format!("-dev-{}", &head_commit_hash[..GIT_HASH_DEV_SUFFIX_LEN]) } - Some(format!( - "-dev-{}", - &head_commit_hash[..GIT_HASH_DEV_SUFFIX_LEN] - )) +} + +fn valid_git_repo() -> bool { + matches!(Command::new("git").arg("status").status(), Ok(status) if status.success()) } /// Trigger rebuild of `mullvad-version` on changing branch (`.git/HEAD`), on changes to the ref of /// the current branch (`.git/refs/heads/$current_branch`) and on changes to the ref of the current /// release tag (`.git/refs/tags/$current_release_tag`). -fn rerun_if_git_ref_changed(release_tag: &str) -> std::io::Result<()> { +/// +/// Returns an error if not in a git repository, or the git binary is not in `PATH`. +fn rerun_if_git_ref_changed(release_tag: &str) { let git_dir = Path::new("..").join(".git"); // The `.git/HEAD` file contains the position of the current head. If in 'detached HEAD' state, @@ -116,7 +116,8 @@ fn rerun_if_git_ref_changed(release_tag: &str) -> std::io::Result<()> { let output = Command::new("git") .arg("branch") .arg("--show-current") - .output()?; + .output() + .expect("Failed to execute `git branch --show-current`"); let current_branch = String::from_utf8(output.stdout).unwrap(); let current_branch = current_branch.trim(); @@ -146,19 +147,17 @@ fn rerun_if_git_ref_changed(release_tag: &str) -> std::io::Result<()> { // track this file, however, as any changes to the current branch, 'detached HEAD' state // or tags will update the corresponding `.git/refs` file we are tracking, even if it had // previously been pruned. - - Ok(()) } /// Returns the commit hash for the commit that `git_ref` is pointing to. /// -/// Returns `None` if executing the `git rev-parse` command fails for some reason. +/// Returns `None` if the git reference cannot be found. fn git_rev_parse_commit_hash(git_ref: &str) -> Option { let output = Command::new("git") .arg("rev-parse") .arg(format!("{git_ref}^{{commit}}")) .output() - .ok()?; + .expect("Failed to run `git rev-parse`"); if !output.status.success() { return None; }