Merge pull request #1590 from EliahKagan/run-ci/check-clean

Have CI check if we need to commit regenerated archives

Sebastian Thiel committed 2 years ago

Commit 4f921402 parents c485a2b 4150af42 files · +31 −22Browse files

Unified · Split

2 files

.github/workflows/ci.yml +4 −0
tests/tools/src/lib.rs +27 −22

         with:
           tool: nextest
       - name: "Test (nextest)"
+        env:
+          GIX_TEST_CREATE_ARCHIVES_EVEN_ON_CI: 1
         run: cargo nextest run --all --no-fail-fast
       - name: Doctest
         run: cargo test --doc
+      - name: Check that tracked archives are up to date
+        run: git diff --exit-code  # If this fails, the fix is usually to commit a regenerated archive.
   test-32bit:
     runs-on: ubuntu-latest

tests/tools/src/lib.rs+27−22

 use std::{
     collections::BTreeMap,
+    env,
     ffi::OsString,
     io::Read,
     path::{Path, PathBuf},
 /// Set the current working dir to `new_cwd` and return a type that returns to the previous working dir on drop.
 pub fn set_current_dir(new_cwd: impl AsRef<Path>) -> std::io::Result<AutoRevertToPreviousCWD> {
-    let cwd = std::env::current_dir()?;
+    let cwd = env::current_dir()?;
-    std::env::set_current_dir(new_cwd)?;
+    env::set_current_dir(new_cwd)?;
     Ok(AutoRevertToPreviousCWD(cwd))
 }
 impl Drop for AutoRevertToPreviousCWD {
     fn drop(&mut self) {
-        std::env::set_current_dir(&self.0).unwrap();
+        env::set_current_dir(&self.0).unwrap();
     }
 }
                 crc_digest.update(&std::fs::read(&script_path).unwrap_or_else(|err| {
                     panic!(
                         "file {script_path:?} in CWD {:?} could not be read: {err}",
-                        std::env::current_dir().expect("valid cwd"),
+                        env::current_dir().expect("valid cwd"),
                     )
                 }));
                 for arg in &args {
                         script_location.display()
                     );
                 }
-                let script_absolute_path = std::env::current_dir()?.join(script_path);
+                let script_absolute_path = env::current_dir()?.join(script_path);
                 let mut cmd = std::process::Command::new(&script_absolute_path);
                 let output = match configure_command(&mut cmd, &args, &script_result_directory).output() {
                     Ok(out) => out,
                     output.stdout.as_bstr(),
                     output.stderr.as_bstr()
                 );
-                create_archive_if_not_on_ci(
+                create_archive_if_we_should(
                     &script_result_directory,
                     &archive_file_path,
                     script_identity_for_archive,
     args: &[String],
     script_result_directory: &Path,
 ) -> &'a mut std::process::Command {
-    let mut msys_for_git_bash_on_windows = std::env::var_os("MSYS").unwrap_or_default();
+    let mut msys_for_git_bash_on_windows = env::var_os("MSYS").unwrap_or_default();
     msys_for_git_bash_on_windows.push(" winsymlinks:nativestrict");
     cmd.args(args)
         .stdout(std::process::Stdio::piped())
     std::fs::write(failure_marker, []).ok();
 }
+fn should_skip_all_archive_creation() -> bool {
+    // On Windows, we fail to remove the meta_dir and can't do anything about it, which means tests will see more
+    // in the directory than they should which makes them fail. It's probably a bad idea to generate archives on Windows
+    // anyway. Either Unix is portable OR no archive is created anywhere. This also means that Windows users can't create
+    // archives, but that's not a deal-breaker.
+    cfg!(windows) || (is_ci::cached() && env::var_os("GIX_TEST_CREATE_ARCHIVES_EVEN_ON_CI").is_none())
+}
 fn is_lfs_pointer_file(path: &Path) -> bool {
     const PREFIX: &[u8] = b"version https://gix-lfs";
     let mut buf = [0_u8; PREFIX.len()];
         .map_or(false, |_| buf.starts_with(PREFIX))
 }
-/// The `script_identity` will be baked into the soon to be created `archive` as it identitifies the script
+/// The `script_identity` will be baked into the soon to be created `archive` as it identifies the script
 /// that created the contents of `source_dir`.
-fn create_archive_if_not_on_ci(source_dir: &Path, archive: &Path, script_identity: u32) -> std::io::Result<()> {
+fn create_archive_if_we_should(source_dir: &Path, archive: &Path, script_identity: u32) -> std::io::Result<()> {
-    // on windows, we fail to remove the meta_dir and can't do anything about it, which means tests will see more
+    if should_skip_all_archive_creation() || is_excluded(archive) {
-    // in the directory than they should which makes them fail. It's probably a bad idea to generate archives on windows
-    // anyway. Either unix is portable OR no archive is created anywhere. This also means that windows users can't create
-    // archives, but that's not a deal-breaker.
-    if cfg!(windows) || is_ci::cached() || is_excluded(archive) {
         return Ok(());
     }
     if is_lfs_pointer_file(archive) {
     let mut lut = EXCLUDE_LUT.lock();
     lut.as_mut()
         .and_then(|cache| {
-            let archive = std::env::current_dir().ok()?.join(archive);
+            let archive = env::current_dir().ok()?.join(archive);
             let relative_path = archive.strip_prefix(cache.base()).ok()?;
             cache
                 .at_path(
         let mut buf = Vec::new();
         #[cfg_attr(feature = "xz", allow(unused_mut))]
         let mut input_archive = std::fs::File::open(archive)?;
-        if std::env::var_os("GIX_TEST_IGNORE_ARCHIVES").is_some() {
+        if env::var_os("GIX_TEST_IGNORE_ARCHIVES").is_some() {
             return Err(std::io::Error::new(
                 std::io::ErrorKind::Other,
                 format!(
     /// Set `var` to `value`.
     pub fn set(mut self, var: &'a str, value: impl Into<String>) -> Self {
-        let prev = std::env::var_os(var);
+        let prev = env::var_os(var);
-        std::env::set_var(var, value.into());
+        env::set_var(var, value.into());
         self.altered_vars.push((var, prev));
         self
     }
     /// Unset `var`.
     pub fn unset(mut self, var: &'a str) -> Self {
-        let prev = std::env::var_os(var);
+        let prev = env::var_os(var);
-        std::env::remove_var(var);
+        env::remove_var(var);
         self.altered_vars.push((var, prev));
         self
     }
     fn drop(&mut self) {
         for (var, prev_value) in self.altered_vars.iter().rev() {
             match prev_value {
-                Some(value) => std::env::set_var(var, value),
+                Some(value) => env::set_var(var, value),
-                None => std::env::remove_var(var),
+                None => env::remove_var(var),
             }
         }
     }

Merge pull request #1590 from EliahKagan/run-ci/check-clean

Keyboard shortcuts

General

Issues and pull requests

Issues, pull requests and Inbox

Repository code