Exclude target/doc from backups as well

Author: jstasiak

src/cargo/core/compiler/layout.rs

@@ -102,9 +102,7 @@ use crate::core::compiler::CompileTarget;
 use crate::core::Workspace;
 use crate::util::paths;
 use crate::util::{CargoResult, FileLock};
-use std::fs;
 use std::path::{Path, PathBuf};
-use tempfile::Builder as TempFileBuilder;
 
 /// Contains the paths of all target output locations.
 ///
@@ -148,43 +146,12 @@ impl Layout {
         if let Some(target) = target {
             root.push(target.short_name());
         }
-        // We need root to exist before we do the tempdir/rename dance inside it.
-        if !root.as_path_unlocked().exists() {
-            root.create_dir()?;
-        }
-        let dest_base = dest;
         let dest = root.join(dest);
         // If the root directory doesn't already exist go ahead and create it
         // here. Use this opportunity to exclude it from backups as well if the
         // system supports it since this is a freshly created folder.
         //
-        // We do this in two steps (first create a temporary directory and exlucde
-        // it from backups, then rename it to the desired name. If we created the
-        // directory directly where it should be and then excluded it from backups
-        // we would risk a situation where cargo is interrupted right after the directory
-        // creation but before the exclusion the the directory would remain non-excluded from
-        // backups because we only perform exclusion right after we created the directory
-        // ourselves.
-        if !dest.as_path_unlocked().exists() {
-            // We need the tempdir created in root instead of $TMP, because only then we can be
-            // easily sure that rename() will succeed (the new name needs to be on the same mount
-            // point as the old one).
-            let tempdir = TempFileBuilder::new()
-                .prefix(dest_base)
-                .tempdir_in(root.as_path_unlocked())?;
-            exclude_from_backups(&tempdir.path());
-            // Previously std::fs::create_dir_all() (through paths::create_dir_all()) was used
-            // here to create the directory directly and fs::create_dir_all() explicitly treats
-            // the directory being created concurrently by another thread or process as success,
-            // hence the check below to follow the existing behavior. If we get an error at
-            // rename() and suddently the directory (which didn't exist a moment earlier) exists
-            // we can infer from it it's another cargo process doing work.
-            if let Err(e) = fs::rename(tempdir.path(), dest.as_path_unlocked()) {
-                if !dest.as_path_unlocked().exists() {
-                    return Err(anyhow::Error::from(e));
-                }
-            }
-        }
+        paths::create_dir_all_excluded_from_backups_atomic(dest.as_path_unlocked())?;
 
         // For now we don't do any more finer-grained locking on the artifact
         // directory, so just lock the entire thing for the duration of this
@@ -250,48 +217,3 @@ impl Layout {
         &self.build
     }
 }
-
-/// Marks the directory as excluded from archives/backups.
-///
-/// This is recommended to prevent derived/temporary files from bloating backups. There are two
-/// mechanisms used to achieve this right now:
-///
-/// * A dedicated resource property excluding from Time Machine backups on macOS
-/// * CACHEDIR.TAG files supported by various tools in a platform-independent way
-fn exclude_from_backups(path: &Path) {
-    exclude_from_time_machine(path);
-    let _ = std::fs::write(
-        path.join("CACHEDIR.TAG"),
-        "Signature: 8a477f597d28d172789f06886806bc55
-# This file is a cache directory tag created by cargo.
-# For information about cache directory tags see https://bford.info/cachedir/",
-    );
-    // Similarly to exclude_from_time_machine() we ignore errors here as it's an optional feature.
-}
-
-#[cfg(not(target_os = "macos"))]
-fn exclude_from_time_machine(_: &Path) {}
-
-#[cfg(target_os = "macos")]
-/// Marks files or directories as excluded from Time Machine on macOS
-fn exclude_from_time_machine(path: &Path) {
-    use core_foundation::base::TCFType;
-    use core_foundation::{number, string, url};
-    use std::ptr;
-
-    // For compatibility with 10.7 a string is used instead of global kCFURLIsExcludedFromBackupKey
-    let is_excluded_key: Result<string::CFString, _> = "NSURLIsExcludedFromBackupKey".parse();
-    let path = url::CFURL::from_path(path, false);
-    if let (Some(path), Ok(is_excluded_key)) = (path, is_excluded_key) {
-        unsafe {
-            url::CFURLSetResourcePropertyForKey(
-                path.as_concrete_TypeRef(),
-                is_excluded_key.as_concrete_TypeRef(),
-                number::kCFBooleanTrue as *const _,
-                ptr::null_mut(),
-            );
-        }
-    }
-    // Errors are ignored, since it's an optional feature and failure
-    // doesn't prevent Cargo from working
-}

src/cargo/core/compiler/mod.rs

@@ -557,7 +557,7 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Work> {
     // Create the documentation directory ahead of time as rustdoc currently has
     // a bug where concurrent invocations will race to create this directory if
     // it doesn't already exist.
-    paths::create_dir_all(&doc_dir)?;
+    paths::create_dir_all_excluded_from_backups_atomic(&doc_dir)?;
 
     rustdoc.arg("-o").arg(doc_dir);
 

src/cargo/util/paths.rs

@@ -7,6 +7,7 @@ use std::iter;
 use std::path::{Component, Path, PathBuf};
 
 use filetime::FileTime;
+use tempfile::Builder as TempFileBuilder;
 
 use crate::util::errors::{CargoResult, CargoResultExt};
 
@@ -457,3 +458,91 @@ pub fn strip_prefix_canonical<P: AsRef<Path>>(
     let canon_base = safe_canonicalize(base.as_ref());
     canon_path.strip_prefix(canon_base).map(|p| p.to_path_buf())
 }
+
+/// Creates an excluded from cache directory atomically with its parents as needed.
+///
+/// The atomicity only covers creating the leaf directory and exclusion from cache. Any missing
+/// parent directories will not be created in an atomic manner.
+///
+/// This function is idempotent and in addition to that it won't exclude ``p`` from cache if it
+/// already exists.
+pub fn create_dir_all_excluded_from_backups_atomic(p: impl AsRef<Path>) -> CargoResult<()> {
+    let path = p.as_ref();
+    if path.is_dir() {
+        return Ok(());
+    }
+
+    let parent = path.parent().unwrap();
+    let base = path.file_name().unwrap();
+    create_dir_all(parent)?;
+    // We do this in two steps (first create a temporary directory and exlucde
+    // it from backups, then rename it to the desired name. If we created the
+    // directory directly where it should be and then excluded it from backups
+    // we would risk a situation where cargo is interrupted right after the directory
+    // creation but before the exclusion the the directory would remain non-excluded from
+    // backups because we only perform exclusion right after we created the directory
+    // ourselves.
+    //
+    // We need the tempdir created in parent instead of $TMP, because only then we can be
+    // easily sure that rename() will succeed (the new name needs to be on the same mount
+    // point as the old one).
+    let tempdir = TempFileBuilder::new().prefix(base).tempdir_in(parent)?;
+    exclude_from_backups(&tempdir.path());
+    // Previously std::fs::create_dir_all() (through paths::create_dir_all()) was used
+    // here to create the directory directly and fs::create_dir_all() explicitly treats
+    // the directory being created concurrently by another thread or process as success,
+    // hence the check below to follow the existing behavior. If we get an error at
+    // rename() and suddently the directory (which didn't exist a moment earlier) exists
+    // we can infer from it it's another cargo process doing work.
+    if let Err(e) = fs::rename(tempdir.path(), path) {
+        if !path.exists() {
+            return Err(anyhow::Error::from(e));
+        }
+    }
+    Ok(())
+}
+
+/// Marks the directory as excluded from archives/backups.
+///
+/// This is recommended to prevent derived/temporary files from bloating backups. There are two
+/// mechanisms used to achieve this right now:
+///
+/// * A dedicated resource property excluding from Time Machine backups on macOS
+/// * CACHEDIR.TAG files supported by various tools in a platform-independent way
+fn exclude_from_backups(path: &Path) {
+    exclude_from_time_machine(path);
+    let _ = std::fs::write(
+        path.join("CACHEDIR.TAG"),
+        "Signature: 8a477f597d28d172789f06886806bc55
+# This file is a cache directory tag created by cargo.
+# For information about cache directory tags see https://bford.info/cachedir/",
+    );
+    // Similarly to exclude_from_time_machine() we ignore errors here as it's an optional feature.
+}
+
+#[cfg(not(target_os = "macos"))]
+fn exclude_from_time_machine(_: &Path) {}
+
+#[cfg(target_os = "macos")]
+/// Marks files or directories as excluded from Time Machine on macOS
+fn exclude_from_time_machine(path: &Path) {
+    use core_foundation::base::TCFType;
+    use core_foundation::{number, string, url};
+    use std::ptr;
+
+    // For compatibility with 10.7 a string is used instead of global kCFURLIsExcludedFromBackupKey
+    let is_excluded_key: Result<string::CFString, _> = "NSURLIsExcludedFromBackupKey".parse();
+    let path = url::CFURL::from_path(path, false);
+    if let (Some(path), Ok(is_excluded_key)) = (path, is_excluded_key) {
+        unsafe {
+            url::CFURLSetResourcePropertyForKey(
+                path.as_concrete_TypeRef(),
+                is_excluded_key.as_concrete_TypeRef(),
+                number::kCFBooleanTrue as *const _,
+                ptr::null_mut(),
+            );
+        }
+    }
+    // Errors are ignored, since it's an optional feature and failure
+    // doesn't prevent Cargo from working
+}

tests/testsuite/doc.rs

@@ -1551,3 +1551,67 @@ fn crate_versions_flag_is_overridden() {
         .run();
     asserts(output_documentation());
 }
+
+#[cargo_test]
+fn target_directory_is_excluded_from_backups() {
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+            [package]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+            build = "build.rs"
+        "#,
+        )
+        .file("build.rs", "fn main() {}")
+        .file("src/lib.rs", "pub fn foo() {}")
+        .build();
+
+    p.cargo("doc")
+        .with_stderr(
+            "\
+[..] foo v0.0.1 ([CWD])
+[..] foo v0.0.1 ([CWD])
+[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
+",
+        )
+        .run();
+    let cachedir_tag = p.root().join("target/doc/CACHEDIR.TAG");
+    assert!(cachedir_tag.is_file());
+    assert!(fs::read_to_string(&cachedir_tag)
+        .unwrap()
+        .starts_with("Signature: 8a477f597d28d172789f06886806bc55"));
+}
+
+#[cargo_test]
+fn target_directory_is_not_excluded_from_backups_if_it_already_exists() {
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+            [package]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+            build = "build.rs"
+        "#,
+        )
+        .file("build.rs", "fn main() {}")
+        .file("src/lib.rs", "pub fn foo() {}")
+        .build();
+    fs::create_dir_all(p.root().join("target/doc")).unwrap();
+
+    p.cargo("doc")
+        .with_stderr(
+            "\
+[..] foo v0.0.1 ([CWD])
+[..] foo v0.0.1 ([CWD])
+[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
+",
+        )
+        .run();
+    let cachedir_tag = p.root().join("target/doc/CACHEDIR.TAG");
+    assert!(!&cachedir_tag.is_file());
+}