Skip to content

Commit

Permalink
Auto merge of #1141 - RalfJung:permissions, r=Diggsey
Browse files Browse the repository at this point in the history 
set_file_perms: if the file is already executable, keep it executable

This was the smallest change I could come up with to fix <rust-lang/rust#36488>: Maintain the X bit when fixing up permissions.

However, the code is still somewhat inconsistent in that it does apply different rules when applied to a file vs. when applied to a directory -- the code recursively walking the directory still removes the X bits from all files, just like it previously ignored the "bin" directory. If you want, I can refactor this some more to be consistent here.

Fixes: #1140
  • Loading branch information
@bors
bors committed Jun 7, 2017
2 parents 6019b30 + d1d694b commit 6cac413
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 63 deletions.
31 changes: 13 additions & 18 deletions src/rustup-dist/src/component/package.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,10 +123,16 @@ impl Package for DirectoryPackage {
// but due to rust-lang/rust#25479 they don't.
#[cfg(unix)]
fn set_file_perms(dest_path: &Path, src_path: &Path) -> Result<()> {
use std::fs;
use std::fs::{self, Metadata};
use std::os::unix::fs::PermissionsExt;
use walkdir::WalkDir;

// Compute whether this entry needs the X bit
fn needs_x(meta: &Metadata) -> bool {
meta.is_dir() || // Directories need it
meta.permissions().mode() & 0o700 == 0o700 // If it is rwx for the user, it gets the X bit
}

// By convention, anything in the bin/ directory of the package is a binary
let is_bin = if let Some(p) = src_path.parent() {
p.ends_with("bin")
Expand All @@ -141,25 +147,14 @@ fn set_file_perms(dest_path: &Path, src_path: &Path) -> Result<()> {
for entry in WalkDir::new(dest_path) {
let entry = try!(entry.chain_err(|| ErrorKind::ComponentDirPermissionsFailed));
let meta = try!(entry.metadata().chain_err(|| ErrorKind::ComponentDirPermissionsFailed));
if meta.is_dir() {
let mut perm = meta.permissions();
perm.set_mode(0o755);
try!(fs::set_permissions(entry.path(), perm).chain_err(|| ErrorKind::ComponentFilePermissionsFailed));
} else {
let mut perm = meta.permissions();
perm.set_mode(0o644);
try!(fs::set_permissions(entry.path(), perm).chain_err(|| ErrorKind::ComponentFilePermissionsFailed));
}
let mut perm = meta.permissions();
perm.set_mode(if needs_x(&meta) { 0o755 } else { 0o644 });
try!(fs::set_permissions(entry.path(), perm).chain_err(|| ErrorKind::ComponentFilePermissionsFailed));
}
} else if is_bin {
let mut perm = try!(fs::metadata(dest_path).chain_err(|| ErrorKind::ComponentFilePermissionsFailed))
.permissions();
perm.set_mode(0o755);
try!(fs::set_permissions(dest_path, perm).chain_err(|| ErrorKind::ComponentFilePermissionsFailed));
} else {
let mut perm = try!(fs::metadata(dest_path).chain_err(|| ErrorKind::ComponentFilePermissionsFailed))
.permissions();
perm.set_mode(0o644);
let meta = try!(fs::metadata(dest_path).chain_err(|| ErrorKind::ComponentFilePermissionsFailed));
let mut perm = meta.permissions();
perm.set_mode(if is_bin || needs_x(&meta) { 0o755 } else { 0o644 });
try!(fs::set_permissions(dest_path, perm).chain_err(|| ErrorKind::ComponentFilePermissionsFailed));
}

Expand Down
10 changes: 5 additions & 5 deletions src/rustup-dist/tests/dist.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ pub fn create_mock_channel(channel: &str, date: &str,
MockCommand::File("bin/rustc".to_string()),
],
vec![
("bin/rustc".to_string(), contents.clone())
("bin/rustc".to_string(), contents.clone(), false)
],
),
]
Expand Down Expand Up @@ -152,7 +152,7 @@ pub fn create_mock_channel(channel: &str, date: &str,
MockCommand::File("lib/libstd.rlib".to_string()),
],
vec![
("lib/libstd.rlib".to_string(), contents.clone())
("lib/libstd.rlib".to_string(), contents.clone(), false)
],
),
]
Expand All @@ -170,7 +170,7 @@ pub fn create_mock_channel(channel: &str, date: &str,
MockCommand::File("lib/i686-apple-darwin/libstd.rlib".to_string()),
],
vec![
("lib/i686-apple-darwin/libstd.rlib".to_string(), contents.clone())
("lib/i686-apple-darwin/libstd.rlib".to_string(), contents.clone(), false)
],
),
]
Expand All @@ -188,7 +188,7 @@ pub fn create_mock_channel(channel: &str, date: &str,
MockCommand::File("lib/i686-unknown-linux-gnu/libstd.rlib".to_string()),
],
vec![
("lib/i686-unknown-linux-gnu/libstd.rlib".to_string(), contents.clone())
("lib/i686-unknown-linux-gnu/libstd.rlib".to_string(), contents.clone(), false)
],
),
]
Expand All @@ -215,7 +215,7 @@ pub fn create_mock_channel(channel: &str, date: &str,
MockCommand::File("bin/bonus".to_string()),
],
vec![
("bin/bonus".to_string(), contents.clone())
("bin/bonus".to_string(), contents.clone(), false)
],
),
]
Expand Down
57 changes: 32 additions & 25 deletions src/rustup-dist/tests/install.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ fn mock_smoke_test() {
vec![MockCommand::File("bin/foo".to_string()),
MockCommand::File("lib/bar".to_string()),
MockCommand::Dir("doc/stuff".to_string())],
vec![("bin/foo".to_string(), "foo".into()),
("lib/bar".to_string(), "bar".into()),
("doc/stuff/doc1".to_string(), "".into()),
("doc/stuff/doc2".to_string(), "".into())]),
vec![("bin/foo".to_string(), "foo".into(), false),
("lib/bar".to_string(), "bar".into(), false),
("doc/stuff/doc1".to_string(), "".into(), false),
("doc/stuff/doc2".to_string(), "".into(), false)]),
("mycomponent2".to_string(),
vec![MockCommand::File("bin/quux".to_string())],
vec![("bin/quux".to_string(), "quux".into())]
vec![("bin/quux".to_string(), "quux".into(), false)]
)]
};

Expand All @@ -56,11 +56,11 @@ fn package_contains() {
let mock = MockInstallerBuilder {
components: vec![("mycomponent".to_string(),
vec![MockCommand::File("bin/foo".to_string())],
vec![("bin/foo".to_string(), "foo".into())],
vec![("bin/foo".to_string(), "foo".into(), false)],
),
("mycomponent2".to_string(),
vec![MockCommand::File("bin/bar".to_string())],
vec![("bin/bar".to_string(), "bar".into())]
vec![("bin/bar".to_string(), "bar".into(), false)]
)]
};

Expand All @@ -78,7 +78,7 @@ fn package_bad_version() {
let mock = MockInstallerBuilder {
components: vec![("mycomponent".to_string(),
vec![MockCommand::File("bin/foo".to_string())],
vec![("bin/foo".to_string(), "foo".into())])]
vec![("bin/foo".to_string(), "foo".into(), false)])]
};

mock.build(tempdir.path());
Expand All @@ -98,10 +98,10 @@ fn basic_install() {
vec![MockCommand::File("bin/foo".to_string()),
MockCommand::File("lib/bar".to_string()),
MockCommand::Dir("doc/stuff".to_string())],
vec![("bin/foo".to_string(), "foo".into()),
("lib/bar".to_string(), "bar".into()),
("doc/stuff/doc1".to_string(), "".into()),
("doc/stuff/doc2".to_string(), "".into())])]
vec![("bin/foo".to_string(), "foo".into(), false),
("lib/bar".to_string(), "bar".into(), false),
("doc/stuff/doc1".to_string(), "".into(), false),
("doc/stuff/doc2".to_string(), "".into(), false)])]
};

mock.build(pkgdir.path());
Expand Down Expand Up @@ -136,10 +136,10 @@ fn multiple_component_install() {
let mock = MockInstallerBuilder {
components: vec![("mycomponent".to_string(),
vec![MockCommand::File("bin/foo".to_string())],
vec![("bin/foo".to_string(), "foo".into())]),
vec![("bin/foo".to_string(), "foo".into(), false)]),
("mycomponent2".to_string(),
vec![MockCommand::File("lib/bar".to_string())],
vec![("lib/bar".to_string(), "bar".into())])]
vec![("lib/bar".to_string(), "bar".into(), false)])]
};

mock.build(pkgdir.path());
Expand Down Expand Up @@ -176,13 +176,13 @@ fn uninstall() {
vec![MockCommand::File("bin/foo".to_string()),
MockCommand::File("lib/bar".to_string()),
MockCommand::Dir("doc/stuff".to_string())],
vec![("bin/foo".to_string(), "foo".into()),
("lib/bar".to_string(), "bar".into()),
("doc/stuff/doc1".to_string(), "".into()),
("doc/stuff/doc2".to_string(), "".into())]),
vec![("bin/foo".to_string(), "foo".into(), false),
("lib/bar".to_string(), "bar".into(), false),
("doc/stuff/doc1".to_string(), "".into(), false),
("doc/stuff/doc2".to_string(), "".into(), false)]),
("mycomponent2".to_string(),
vec![MockCommand::File("lib/quux".to_string())],
vec![("lib/quux".to_string(), "quux".into())])]
vec![("lib/quux".to_string(), "quux".into(), false)])]
};

mock.build(pkgdir.path());
Expand Down Expand Up @@ -234,7 +234,7 @@ fn component_bad_version() {
let mock = MockInstallerBuilder {
components: vec![("mycomponent".to_string(),
vec![MockCommand::File("bin/foo".to_string())],
vec![("bin/foo".to_string(), "foo".into())])]
vec![("bin/foo".to_string(), "foo".into(), false)])]
};

mock.build(pkgdir.path());
Expand Down Expand Up @@ -276,11 +276,14 @@ fn unix_permissions() {
components: vec![("mycomponent".to_string(),
vec![MockCommand::File("bin/foo".to_string()),
MockCommand::File("lib/bar".to_string()),
MockCommand::File("lib/foobar".to_string()),
MockCommand::Dir("doc/stuff".to_string())],
vec![("bin/foo".to_string(), "foo".into()),
("lib/bar".to_string(), "bar".into()),
("doc/stuff/doc1".to_string(), "".into()),
("doc/stuff/morestuff/doc2".to_string(), "".into())])]
vec![("bin/foo".to_string(), "foo".into(), false),
("lib/bar".to_string(), "bar".into(), false),
("lib/foobar".to_string(), "foobar".into(), true),
("doc/stuff/doc1".to_string(), "".into(), false),
("doc/stuff/morestuff/doc2".to_string(), "".into(), false),
("doc/stuff/morestuff/tool".to_string(), "".into(), true)])]
};

mock.build(pkgdir.path());
Expand All @@ -304,6 +307,8 @@ fn unix_permissions() {
assert_eq!(m, 0o755);
let m = fs::metadata(instdir.path().join("lib/bar")).unwrap().permissions().mode();
assert_eq!(m, 0o644);
let m = fs::metadata(instdir.path().join("lib/foobar")).unwrap().permissions().mode();
assert_eq!(m, 0o755);
let m = fs::metadata(instdir.path().join("doc/stuff/")).unwrap().permissions().mode();
assert_eq!(m, 0o755);
let m = fs::metadata(instdir.path().join("doc/stuff/doc1")).unwrap().permissions().mode();
Expand All @@ -312,6 +317,8 @@ fn unix_permissions() {
assert_eq!(m, 0o755);
let m = fs::metadata(instdir.path().join("doc/stuff/morestuff/doc2")).unwrap().permissions().mode();
assert_eq!(m, 0o644);
let m = fs::metadata(instdir.path().join("doc/stuff/morestuff/tool")).unwrap().permissions().mode();
assert_eq!(m, 0o755);
}

// Installing to a prefix that doesn't exist creates it automatically
Expand All @@ -322,7 +329,7 @@ fn install_to_prefix_that_does_not_exist() {
let mock = MockInstallerBuilder {
components: vec![("mycomponent".to_string(),
vec![MockCommand::File("bin/foo".to_string())],
vec![("bin/foo".to_string(), "foo".into())])]
vec![("bin/foo".to_string(), "foo".into(), false)])]
};

mock.build(pkgdir.path());
Expand Down
18 changes: 9 additions & 9 deletions src/rustup-mock/src/clitools.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,7 @@ fn build_mock_std_installer(trip: &str) -> MockInstallerBuilder {
components: vec![
(format!("rust-std-{}", trip.clone()),
vec![MockCommand::File(format!("lib/rustlib/{}/libstd.rlib", trip))],
vec![(format!("lib/rustlib/{}/libstd.rlib", trip), "".into())])
vec![(format!("lib/rustlib/{}/libstd.rlib", trip), "".into(), false)])
]
}
}
Expand All @@ -524,8 +524,8 @@ fn build_mock_cross_std_installer(target: &str, date: &str) -> MockInstallerBuil
(format!("rust-std-{}", target.clone()),
vec![MockCommand::File(format!("lib/rustlib/{}/lib/libstd.rlib", target)),
MockCommand::File(format!("lib/rustlib/{}/lib/{}", target, date))],
vec![(format!("lib/rustlib/{}/lib/libstd.rlib", target), "".into()),
(format!("lib/rustlib/{}/lib/{}", target, date), "".into())])
vec![(format!("lib/rustlib/{}/lib/libstd.rlib", target), "".into(), false),
(format!("lib/rustlib/{}/lib/{}", target, date), "".into(), false)])
]
}
}
Expand All @@ -546,7 +546,7 @@ fn build_mock_rustc_installer(target: &str, version: &str, version_hash_: &str)
components: vec![
("rustc".to_string(),
vec![MockCommand::File(rustc.clone())],
vec![(rustc, mock_bin("rustc", version, &version_hash))])
vec![(rustc, mock_bin("rustc", version, &version_hash), false)])
]
}
}
Expand All @@ -557,7 +557,7 @@ fn build_mock_cargo_installer(version: &str, version_hash: &str) -> MockInstalle
components: vec![
("cargo".to_string(),
vec![MockCommand::File(cargo.clone())],
vec![(cargo, mock_bin("cargo", version, version_hash))])
vec![(cargo, mock_bin("cargo", version, version_hash), false)])
]
}
}
Expand All @@ -568,7 +568,7 @@ fn build_mock_rls_installer(version: &str, version_hash: &str) -> MockInstallerB
components: vec![
("rls".to_string(),
vec![MockCommand::File(cargo.clone())],
vec![(cargo, mock_bin("rls", version, version_hash))])
vec![(cargo, mock_bin("rls", version, version_hash), false)])
]
}
}
Expand All @@ -578,7 +578,7 @@ fn build_mock_rust_doc_installer() -> MockInstallerBuilder {
components: vec![
("rust-docs".to_string(),
vec![MockCommand::File("share/doc/rust/html/index.html".to_string())],
vec![("share/doc/rust/html/index.html".to_string(), "".into())])
vec![("share/doc/rust/html/index.html".to_string(), "".into(), false)])
]
}
}
Expand All @@ -588,7 +588,7 @@ fn build_mock_rust_analysis_installer(trip: &str) -> MockInstallerBuilder {
components: vec![
(format!("rust-analysis-{}", trip),
vec![MockCommand::File(format!("lib/rustlib/{}/analysis/libfoo.json", trip))],
vec![(format!("lib/rustlib/{}/analysis/libfoo.json", trip), "".into())])
vec![(format!("lib/rustlib/{}/analysis/libfoo.json", trip), "".into(), false)])
]
}
}
Expand All @@ -598,7 +598,7 @@ fn build_mock_rust_src_installer() -> MockInstallerBuilder {
components: vec![
("rust-src".to_string(),
vec![MockCommand::File("lib/rustlib/src/rust-src/foo.rs".to_string())],
vec![("lib/rustlib/src/rust-src/foo.rs".to_string(), "".into())])
vec![("lib/rustlib/src/rust-src/foo.rs".to_string(), "".into(), false)])
]
}
}
Expand Down
22 changes: 16 additions & 6 deletions src/rustup-mock/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ pub struct MockInstallerBuilder {
}

// A component name, the installation commands for installing files
// (either "file:" or "dir:") and the file paths and contents.
pub type MockComponent = (String, Vec<MockCommand>, Vec<(String, Vec<u8>)>);
// (either "file:" or "dir:") and the file paths, contents and X bit.
pub type MockComponent = (String, Vec<MockCommand>, Vec<(String, Vec<u8>, bool)>);

#[derive(Clone)]
pub enum MockCommand {
Expand Down Expand Up @@ -67,13 +67,23 @@ impl MockInstallerBuilder {
}

// Create the component files
for &(ref f_path, ref content) in files {
let dir_path = component_dir.join(f_path);
let dir_path = dir_path.parent().unwrap();
for &(ref f_path, ref content, executable) in files {
let fname = component_dir.join(f_path);
let dir_path = fname.parent().unwrap().to_owned();
fs::create_dir_all(dir_path).unwrap();
let ref mut f = File::create(component_dir.join(f_path)).unwrap();
let ref mut f = File::create(&fname).unwrap();

f.write_all(&content).unwrap();
drop(f);
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
if executable {
let mut perm = fs::metadata(&fname).unwrap().permissions();
perm.set_mode(0o755);
fs::set_permissions(&fname, perm).unwrap();
}
}
}
}

Expand Down

0 comments on commit 6cac413

Please sign in to comment.