Auto merge of #9131 - ehuss:fix-vendor-permissions, r=alexcrichton

Fix permission issue with `cargo vendor`.

I think there was an unintended regression in #8937 where the vendored output does not retain the original permissions.

Fixes #9127.

Author: bors

crates/cargo-test-support/src/registry.rs

@@ -317,8 +317,7 @@ pub struct Package {
     name: String,
     vers: String,
     deps: Vec<Dependency>,
-    files: Vec<(String, String)>,
-    extra_files: Vec<(String, String)>,
+    files: Vec<PackageFile>,
     yanked: bool,
     features: HashMap<String, Vec<String>>,
     local: bool,
@@ -342,6 +341,20 @@ pub struct Dependency {
     optional: bool,
 }
 
+/// A file to be created in a package.
+struct PackageFile {
+    path: String,
+    contents: String,
+    /// The Unix mode for the file. Note that when extracted on Windows, this
+    /// is mostly ignored since it doesn't have the same style of permissions.
+    mode: u32,
+    /// If `true`, the file is created in the root of the tarfile, used for
+    /// testing invalid packages.
+    extra: bool,
+}
+
+const DEFAULT_MODE: u32 = 0o644;
+
 /// Initializes the on-disk registry and sets up the config so that crates.io
 /// is replaced with the one on disk.
 pub fn init() {
@@ -379,7 +392,6 @@ impl Package {
             vers: vers.to_string(),
             deps: Vec::new(),
             files: Vec::new(),
-            extra_files: Vec::new(),
             yanked: false,
             features: HashMap::new(),
             local: false,
@@ -416,7 +428,17 @@ impl Package {
 
     /// Adds a file to the package.
     pub fn file(&mut self, name: &str, contents: &str) -> &mut Package {
-        self.files.push((name.to_string(), contents.to_string()));
+        self.file_with_mode(name, DEFAULT_MODE, contents)
+    }
+
+    /// Adds a file with a specific Unix mode.
+    pub fn file_with_mode(&mut self, path: &str, mode: u32, contents: &str) -> &mut Package {
+        self.files.push(PackageFile {
+            path: path.to_string(),
+            contents: contents.to_string(),
+            mode,
+            extra: false,
+        });
         self
     }
 
@@ -425,9 +447,13 @@ impl Package {
     /// Normal files are automatically placed within a directory named
     /// `$PACKAGE-$VERSION`. This allows you to override that behavior,
     /// typically for testing invalid behavior.
-    pub fn extra_file(&mut self, name: &str, contents: &str) -> &mut Package {
-        self.extra_files
-            .push((name.to_string(), contents.to_string()));
+    pub fn extra_file(&mut self, path: &str, contents: &str) -> &mut Package {
+        self.files.push(PackageFile {
+            path: path.to_string(),
+            contents: contents.to_string(),
+            mode: DEFAULT_MODE,
+            extra: true,
+        });
         self
     }
 
@@ -639,19 +665,30 @@ impl Package {
         let f = t!(File::create(&dst));
         let mut a = Builder::new(GzEncoder::new(f, Compression::default()));
 
-        if !self.files.iter().any(|(name, _)| name == "Cargo.toml") {
+        if !self
+            .files
+            .iter()
+            .any(|PackageFile { path, .. }| path == "Cargo.toml")
+        {
             self.append_manifest(&mut a);
         }
         if self.files.is_empty() {
-            self.append(&mut a, "src/lib.rs", "");
+            self.append(&mut a, "src/lib.rs", DEFAULT_MODE, "");
         } else {
-            for &(ref name, ref contents) in self.files.iter() {
-                self.append(&mut a, name, contents);
+            for PackageFile {
+                path,
+                contents,
+                mode,
+                extra,
+            } in &self.files
+            {
+                if *extra {
+                    self.append_raw(&mut a, path, *mode, contents);
+                } else {
+                    self.append(&mut a, path, *mode, contents);
+                }
             }
         }
-        for &(ref name, ref contents) in self.extra_files.iter() {
-            self.append_extra(&mut a, name, contents);
-        }
     }
 
     fn append_manifest<W: Write>(&self, ar: &mut Builder<W>) {
@@ -704,21 +741,23 @@ impl Package {
             manifest.push_str("[lib]\nproc-macro = true\n");
         }
 
-        self.append(ar, "Cargo.toml", &manifest);
+        self.append(ar, "Cargo.toml", DEFAULT_MODE, &manifest);
     }
 
-    fn append<W: Write>(&self, ar: &mut Builder<W>, file: &str, contents: &str) {
-        self.append_extra(
+    fn append<W: Write>(&self, ar: &mut Builder<W>, file: &str, mode: u32, contents: &str) {
+        self.append_raw(
             ar,
             &format!("{}-{}/{}", self.name, self.vers, file),
+            mode,
             contents,
         );
     }
 
-    fn append_extra<W: Write>(&self, ar: &mut Builder<W>, path: &str, contents: &str) {
+    fn append_raw<W: Write>(&self, ar: &mut Builder<W>, path: &str, mode: u32, contents: &str) {
         let mut header = Header::new_ustar();
         header.set_size(contents.len() as u64);
         t!(header.set_path(path));
+        header.set_mode(mode);
         header.set_cksum();
         t!(ar.append(&header, contents.as_bytes()));
     }

src/cargo/ops/vendor.rs

@@ -8,8 +8,7 @@ use anyhow::bail;
 use serde::Serialize;
 use std::collections::HashSet;
 use std::collections::{BTreeMap, BTreeSet, HashMap};
-use std::fs;
-use std::fs::File;
+use std::fs::{self, File, OpenOptions};
 use std::io::{Read, Write};
 use std::path::{Path, PathBuf};
 
@@ -346,8 +345,21 @@ fn cp_sources(
 
 fn copy_and_checksum(src_path: &Path, dst_path: &Path, buf: &mut [u8]) -> CargoResult<String> {
     let mut src = File::open(src_path).chain_err(|| format!("failed to open {:?}", src_path))?;
-    let mut dst =
-        File::create(dst_path).chain_err(|| format!("failed to create {:?}", dst_path))?;
+    let mut dst_opts = OpenOptions::new();
+    dst_opts.write(true).create(true).truncate(true);
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::{MetadataExt, OpenOptionsExt};
+        let src_metadata = src
+            .metadata()
+            .chain_err(|| format!("failed to stat {:?}", src_path))?;
+        dst_opts.mode(src_metadata.mode());
+    }
+    let mut dst = dst_opts
+        .open(dst_path)
+        .chain_err(|| format!("failed to create {:?}", dst_path))?;
+    // Not going to bother setting mode on pre-existing files, since there
+    // shouldn't be any under normal conditions.
     let mut cksum = Sha256::new();
     loop {
         let n = src

tests/testsuite/vendor.rs

@@ -676,3 +676,36 @@ fn git_crlf_preservation() {
     let output = p.read_file("vendor/a/src/lib.rs");
     assert_eq!(input, output);
 }
+
+#[cargo_test]
+#[cfg(unix)]
+fn vendor_preserves_permissions() {
+    use std::os::unix::fs::MetadataExt;
+
+    Package::new("bar", "1.0.0")
+        .file_with_mode("example.sh", 0o755, "#!/bin/sh")
+        .file("src/lib.rs", "")
+        .publish();
+
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+                [package]
+                name = "foo"
+                version = "0.1.0"
+
+                [dependencies]
+                bar = "1.0"
+            "#,
+        )
+        .file("src/lib.rs", "")
+        .build();
+
+    p.cargo("vendor --respect-source-config").run();
+
+    let metadata = fs::metadata(p.root().join("vendor/bar/src/lib.rs")).unwrap();
+    assert_eq!(metadata.mode() & 0o777, 0o644);
+    let metadata = fs::metadata(p.root().join("vendor/bar/example.sh")).unwrap();
+    assert_eq!(metadata.mode() & 0o777, 0o755);
+}