1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2024-11-25 15:29:32 -05:00

fix: create HTTP cache lazily (#5795)

This commit is contained in:
Bartek Iwańczuk 2020-05-24 19:20:40 +02:00 committed by GitHub
parent 4ca0d6e2d3
commit e934df5f7d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 35 additions and 16 deletions

View file

@ -40,7 +40,7 @@ impl DenoDir {
root, root,
gen_cache: DiskCache::new(&gen_path), gen_cache: DiskCache::new(&gen_path),
}; };
deno_dir.gen_cache.ensure_location()?; deno_dir.gen_cache.ensure_dir_exists(&gen_path)?;
Ok(deno_dir) Ok(deno_dir)
} }

View file

@ -31,14 +31,14 @@ impl DiskCache {
} }
/// Ensures the location of the cache. /// Ensures the location of the cache.
pub fn ensure_location(&self) -> io::Result<()> { pub fn ensure_dir_exists(&self, path: &Path) -> io::Result<()> {
if self.location.is_dir() { if path.is_dir() {
return Ok(()); return Ok(());
} }
fs::create_dir_all(&self.location).map_err(|e| { fs::create_dir_all(&path).map_err(|e| {
io::Error::new(e.kind(), format!( io::Error::new(e.kind(), format!(
"Could not create TypeScript compiler cache location: {:?}\nCheck the permission of the directory.", "Could not create TypeScript compiler cache location: {:?}\nCheck the permission of the directory.",
self.location path
)) ))
}) })
} }
@ -129,8 +129,7 @@ impl DiskCache {
pub fn set(&self, filename: &Path, data: &[u8]) -> std::io::Result<()> { pub fn set(&self, filename: &Path, data: &[u8]) -> std::io::Result<()> {
let path = self.location.join(filename); let path = self.location.join(filename);
match path.parent() { match path.parent() {
Some(ref parent) => fs::create_dir_all(parent) Some(ref parent) => self.ensure_dir_exists(parent),
.map_err(|e| with_io_context(&e, format!("{:#?}", &path))),
None => Ok(()), None => Ok(()),
}?; }?;
deno_fs::write_file(&path, data, 0o666) deno_fs::write_file(&path, data, 0o666)
@ -154,7 +153,9 @@ mod tests {
let mut cache_path = cache_location.path().to_owned(); let mut cache_path = cache_location.path().to_owned();
cache_path.push("foo"); cache_path.push("foo");
let cache = DiskCache::new(&cache_path); let cache = DiskCache::new(&cache_path);
cache.ensure_location().expect("Testing expect:"); cache
.ensure_dir_exists(&cache.location)
.expect("Testing expect:");
assert!(cache_path.is_dir()); assert!(cache_path.is_dir());
} }
@ -166,7 +167,9 @@ mod tests {
cache_location.push("foo"); cache_location.push("foo");
assert_eq!(cache_location.is_dir(), false); assert_eq!(cache_location.is_dir(), false);
let cache = DiskCache::new(&cache_location); let cache = DiskCache::new(&cache_location);
cache.ensure_location().expect("Testing expect:"); cache
.ensure_dir_exists(&cache.location)
.expect("Testing expect:");
assert_eq!(cache_location.is_dir(), true); assert_eq!(cache_location.is_dir(), true);
} }

View file

@ -51,7 +51,6 @@ impl GlobalState {
let dir = deno_dir::DenoDir::new(custom_root)?; let dir = deno_dir::DenoDir::new(custom_root)?;
let deps_cache_location = dir.root.join("deps"); let deps_cache_location = dir.root.join("deps");
let http_cache = http_cache::HttpCache::new(&deps_cache_location); let http_cache = http_cache::HttpCache::new(&deps_cache_location);
http_cache.ensure_location()?;
let file_fetcher = SourceFileFetcher::new( let file_fetcher = SourceFileFetcher::new(
http_cache, http_cache,

View file

@ -113,16 +113,16 @@ impl HttpCache {
} }
/// Ensures the location of the cache. /// Ensures the location of the cache.
pub fn ensure_location(&self) -> io::Result<()> { fn ensure_dir_exists(&self, path: &Path) -> io::Result<()> {
if self.location.is_dir() { if path.is_dir() {
return Ok(()); return Ok(());
} }
fs::create_dir_all(&self.location).map_err(|e| { fs::create_dir_all(&path).map_err(|e| {
io::Error::new( io::Error::new(
e.kind(), e.kind(),
format!( format!(
"Could not create remote modules cache location: {:?}\nCheck the permission of the directory.", "Could not create remote modules cache location: {:?}\nCheck the permission of the directory.",
self.location path
), ),
) )
}) })
@ -163,7 +163,7 @@ impl HttpCache {
let parent_filename = cache_filename let parent_filename = cache_filename
.parent() .parent()
.expect("Cache filename should have a parent dir"); .expect("Cache filename should have a parent dir");
fs::create_dir_all(parent_filename)?; self.ensure_dir_exists(parent_filename)?;
// Cache content // Cache content
deno_fs::write_file(&cache_filename, content, 0o666)?; deno_fs::write_file(&cache_filename, content, 0o666)?;
@ -187,8 +187,25 @@ mod tests {
let dir = TempDir::new().unwrap(); let dir = TempDir::new().unwrap();
let mut cache_path = dir.path().to_owned(); let mut cache_path = dir.path().to_owned();
cache_path.push("foobar"); cache_path.push("foobar");
// HttpCache should be created lazily on first use:
// when zipping up a local project with no external dependencies
// "$DENO_DIR/deps" is empty. When unzipping such project
// "$DENO_DIR/deps" might not get restored and in situation
// when directory is owned by root we might not be able
// to create that directory. However if it's not needed it
// doesn't make sense to return error in such specific scenarios.
// For more details check issue:
// https://github.com/denoland/deno/issues/5688
let cache = HttpCache::new(&cache_path); let cache = HttpCache::new(&cache_path);
assert!(cache.ensure_location().is_ok()); assert!(!cache.location.exists());
cache
.set(
&Url::parse("http://example.com/foo/bar.js").unwrap(),
HeadersMap::new(),
b"hello world",
)
.expect("Failed to add to cache");
assert!(cache.ensure_dir_exists(&cache.location).is_ok());
assert!(cache_path.is_dir()); assert!(cache_path.is_dir());
} }