Skip to content

Commit 414c1eb

Browse files
committed
Auto merge of #6940 - alexcrichton:readonly-compat, r=ehuss
Re-enable compatibility with readonly CARGO_HOME Previously Cargo would attempt to work as much as possible with a previously filled out CARGO_HOME, even if it was mounted as read-only. In #6880 this was regressed as a few global locks and files were always attempted to be opened in writable mode. This commit fixes these issues by correcting two locations: * First the global package cache lock has error handling to allow acquiring the lock in read-only mode inaddition to read/write mode. If the read/write mode failed due to an error that looks like a readonly filesystem then we assume everything in the package cache is readonly and we switch to just acquiring any lock, this time a shared readonly one. We in theory aren't actually doing any synchronization at that point since it's all readonly anyway. * Next when unpacking package we're careful to issue a `stat` call before opening a file in writable mode. This way our preexisting guard to return early if a package is unpacked will succeed before we open anything in writable mode. Closes #6928
2 parents fd3d06b + 5d9383e commit 414c1eb

File tree

4 files changed

+117
-12
lines changed

4 files changed

+117
-12
lines changed

src/cargo/sources/registry/mod.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -449,15 +449,16 @@ impl<'cfg> RegistrySource<'cfg> {
449449
let path = dst.join(PACKAGE_SOURCE_LOCK);
450450
let path = self.config.assert_package_cache_locked(&path);
451451
let unpack_dir = path.parent().unwrap();
452+
if let Ok(meta) = path.metadata() {
453+
if meta.len() > 0 {
454+
return Ok(unpack_dir.to_path_buf());
455+
}
456+
}
452457
let mut ok = OpenOptions::new()
453458
.create(true)
454459
.read(true)
455460
.write(true)
456461
.open(&path)?;
457-
let meta = ok.metadata()?;
458-
if meta.len() > 0 {
459-
return Ok(unpack_dir.to_path_buf());
460-
}
461462

462463
let gz = GzDecoder::new(tarball);
463464
let mut tar = Archive::new(gz);

src/cargo/util/config.rs

Lines changed: 57 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use std::env;
66
use std::fmt;
77
use std::fs::{self, File};
88
use std::io::prelude::*;
9-
use std::io::SeekFrom;
9+
use std::io::{self, SeekFrom};
1010
use std::mem;
1111
use std::path::{Path, PathBuf};
1212
use std::str::FromStr;
@@ -860,21 +860,71 @@ impl Config {
860860
return ret;
861861
}
862862

863+
/// Acquires an exclusive lock on the global "package cache"
864+
///
865+
/// This lock is global per-process and can be acquired recursively. An RAII
866+
/// structure is returned to release the lock, and if this process
867+
/// abnormally terminates the lock is also released.
863868
pub fn acquire_package_cache_lock<'a>(&'a self) -> CargoResult<PackageCacheLock<'a>> {
864869
let mut slot = self.package_cache_lock.borrow_mut();
865870
match *slot {
871+
// We've already acquired the lock in this process, so simply bump
872+
// the count and continue.
866873
Some((_, ref mut cnt)) => {
867874
*cnt += 1;
868875
}
869876
None => {
870-
let lock = self
871-
.home_path
872-
.open_rw(".package-cache", self, "package cache lock")
873-
.chain_err(|| "failed to acquire package cache lock")?;
874-
*slot = Some((lock, 1));
877+
let path = ".package-cache";
878+
let desc = "package cache lock";
879+
880+
// First, attempt to open an exclusive lock which is in general
881+
// the purpose of this lock!
882+
//
883+
// If that fails because of a readonly filesystem, though, then
884+
// we don't want to fail because it's a readonly filesystem. In
885+
// some situations Cargo is prepared to have a readonly
886+
// filesystem yet still work since it's all been pre-downloaded
887+
// and/or pre-unpacked. In these situations we want to keep
888+
// Cargo running if possible, so if it's a readonly filesystem
889+
// switch to a shared lock which should hopefully succeed so we
890+
// can continue.
891+
//
892+
// Note that the package cache lock protects files in the same
893+
// directory, so if it's a readonly filesystem we assume that
894+
// the entire package cache is readonly, so we're just acquiring
895+
// something to prove it works, we're not actually doing any
896+
// synchronization at that point.
897+
match self.home_path.open_rw(path, self, desc) {
898+
Ok(lock) => *slot = Some((lock, 1)),
899+
Err(e) => {
900+
if maybe_readonly(&e) {
901+
if let Ok(lock) = self.home_path.open_ro(path, self, desc) {
902+
*slot = Some((lock, 1));
903+
return Ok(PackageCacheLock(self));
904+
}
905+
}
906+
907+
Err(e).chain_err(|| "failed to acquire package cache lock")?;
908+
}
909+
}
875910
}
876911
}
877-
Ok(PackageCacheLock(self))
912+
return Ok(PackageCacheLock(self));
913+
914+
fn maybe_readonly(err: &failure::Error) -> bool {
915+
err.iter_chain().any(|err| {
916+
if let Some(io) = err.downcast_ref::<io::Error>() {
917+
if io.kind() == io::ErrorKind::PermissionDenied {
918+
return true;
919+
}
920+
921+
#[cfg(unix)]
922+
return io.raw_os_error() == Some(libc::EROFS);
923+
}
924+
925+
false
926+
})
927+
}
878928
}
879929

880930
pub fn release_package_cache_lock(&self) {}

tests/testsuite/registry.rs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use std::fs::{self, File};
22
use std::io::prelude::*;
3+
use std::path::Path;
34

45
use crate::support::cargo_process;
56
use crate::support::git;
@@ -1979,3 +1980,48 @@ fn ignore_invalid_json_lines() {
19791980

19801981
p.cargo("build").run();
19811982
}
1983+
1984+
#[test]
1985+
fn readonly_registry_still_works() {
1986+
Package::new("foo", "0.1.0").publish();
1987+
1988+
let p = project()
1989+
.file(
1990+
"Cargo.toml",
1991+
r#"
1992+
[project]
1993+
name = "a"
1994+
version = "0.5.0"
1995+
authors = []
1996+
1997+
[dependencies]
1998+
foo = '0.1.0'
1999+
"#,
2000+
)
2001+
.file("src/lib.rs", "")
2002+
.build();
2003+
2004+
p.cargo("generate-lockfile").run();
2005+
p.cargo("fetch --locked").run();
2006+
chmod_readonly(&paths::home());
2007+
p.cargo("build").run();
2008+
2009+
fn chmod_readonly(path: &Path) {
2010+
for entry in t!(path.read_dir()) {
2011+
let entry = t!(entry);
2012+
let path = entry.path();
2013+
if t!(entry.file_type()).is_dir() {
2014+
chmod_readonly(&path);
2015+
} else {
2016+
set_readonly(&path);
2017+
}
2018+
}
2019+
set_readonly(path);
2020+
}
2021+
2022+
fn set_readonly(path: &Path) {
2023+
let mut perms = t!(path.metadata()).permissions();
2024+
perms.set_readonly(true);
2025+
t!(fs::set_permissions(path, perms));
2026+
}
2027+
}

tests/testsuite/support/paths.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,10 +158,18 @@ where
158158
{
159159
match f(path) {
160160
Ok(()) => {}
161-
Err(ref e) if cfg!(windows) && e.kind() == ErrorKind::PermissionDenied => {
161+
Err(ref e) if e.kind() == ErrorKind::PermissionDenied => {
162162
let mut p = t!(path.metadata()).permissions();
163163
p.set_readonly(false);
164164
t!(fs::set_permissions(path, p));
165+
166+
// Unix also requires the parent to not be readonly for example when
167+
// removing files
168+
let parent = path.parent().unwrap();
169+
let mut p = t!(parent.metadata()).permissions();
170+
p.set_readonly(false);
171+
t!(fs::set_permissions(parent, p));
172+
165173
f(path).unwrap_or_else(|e| {
166174
panic!("failed to {} {}: {}", desc, path.display(), e);
167175
})

0 commit comments

Comments
 (0)