From e46e185e4d007daa1a73f3e99e051d9ad039f5a6 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 19 Jul 2019 07:56:33 -0700 Subject: [PATCH] Don't fail if we can't acquire readonly lock This commit updates support from #6940 to not only gracefully handle situations where the lock can be acquired in readonly but not read/write mode but also handle situations where even a readonly lock can't be acquired. If a readonly lock can't be acquired (and the read/write failed) then we likely can't touch anything in the directory, so there's no value gained from locking anyway. Closes #7147 --- src/cargo/util/config.rs | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/src/cargo/util/config.rs b/src/cargo/util/config.rs index 54ca624c805..4ae67cb36bb 100644 --- a/src/cargo/util/config.rs +++ b/src/cargo/util/config.rs @@ -83,7 +83,7 @@ pub struct Config { updated_sources: LazyCell>>, /// Lock, if held, of the global package cache along with the number of /// acquisitions so far. - package_cache_lock: RefCell>, + package_cache_lock: RefCell, usize)>>, } impl Config { @@ -887,28 +887,26 @@ impl Config { // First, attempt to open an exclusive lock which is in general // the purpose of this lock! // - // If that fails because of a readonly filesystem, though, then - // we don't want to fail because it's a readonly filesystem. In - // some situations Cargo is prepared to have a readonly - // filesystem yet still work since it's all been pre-downloaded - // and/or pre-unpacked. In these situations we want to keep - // Cargo running if possible, so if it's a readonly filesystem - // switch to a shared lock which should hopefully succeed so we - // can continue. + // If that fails because of a readonly filesystem or a + // permission error, though, then we don't really want to fail + // just because of this. All files that this lock protects are + // in subfolders, so they're assumed by Cargo to also be + // readonly or have invalid permissions for us to write to. If + // that's the case, then we don't really need to grab a lock in + // the first place here. // - // Note that the package cache lock protects files in the same - // directory, so if it's a readonly filesystem we assume that - // the entire package cache is readonly, so we're just acquiring - // something to prove it works, we're not actually doing any - // synchronization at that point. + // Despite this we attempt to grab a readonly lock. This means + // that if our read-only folder is shared read-write with + // someone else on the system we should synchronize with them, + // but if we can't even do that then we did our best and we just + // keep on chugging elsewhere. match self.home_path.open_rw(path, self, desc) { - Ok(lock) => *slot = Some((lock, 1)), + Ok(lock) => *slot = Some((Some(lock), 1)), Err(e) => { if maybe_readonly(&e) { - if let Ok(lock) = self.home_path.open_ro(path, self, desc) { - *slot = Some((lock, 1)); - return Ok(PackageCacheLock(self)); - } + let lock = self.home_path.open_ro(path, self, desc).ok(); + *slot = Some((lock, 1)); + return Ok(PackageCacheLock(self)); } Err(e).chain_err(|| "failed to acquire package cache lock")?;