Skip to content

Commit

Permalink
Auto merge of #9131 - ehuss:fix-vendor-permissions, r=alexcrichton
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bors committed Feb 4, 2021
2 parents aaaf296 + 33f648a commit 34170fc
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 22 deletions.
75 changes: 57 additions & 18 deletions crates/cargo-test-support/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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() {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
}

Expand All @@ -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
}

Expand Down Expand Up @@ -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>) {
Expand Down Expand Up @@ -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()));
}
Expand Down
20 changes: 16 additions & 4 deletions src/cargo/ops/vendor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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
Expand Down
33 changes: 33 additions & 0 deletions tests/testsuite/vendor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

0 comments on commit 34170fc

Please sign in to comment.