-
Notifications
You must be signed in to change notification settings - Fork 545
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement read-only local cache #2048
Changes from 8 commits
081c4f2
e910ff4
843448a
beac2ec
1bf9f61
609b405
8c54c2f
805e4ef
b065f20
c765ec1
6364e35
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -340,42 +340,51 @@ where | |
let mut updated = false; | ||
let hit = preprocessor_cache_entry | ||
.lookup_result_digest(preprocessor_cache_mode_config, &mut updated); | ||
|
||
let mut update_failed = false; | ||
if updated { | ||
// Time macros have been found, we need to update | ||
// the preprocessor cache entry. See [`PreprocessorCacheEntry::result_matches`]. | ||
debug!( | ||
"Preprocessor cache updated because of time macros: {preprocessor_key}" | ||
); | ||
storage.put_preprocessor_cache_entry( | ||
|
||
if let Err(e) = storage.put_preprocessor_cache_entry( | ||
preprocessor_key, | ||
preprocessor_cache_entry, | ||
)?; | ||
) { | ||
debug!("Failed to update preprocessor cache: {}", e); | ||
update_failed = true; | ||
} | ||
Comment on lines
-349
to
+358
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm unsure about this behavior. It's based on how Realistically, errors shouldn't be used here, but it seems to be how it's handled with GCS read-only, so I did it the same way. I thought about changing it, but I'd like to hear the input about someone more knowledgeable about sccache's internals first. |
||
} | ||
if let Some(key) = hit { | ||
debug!("Preprocessor cache hit: {preprocessor_key}"); | ||
// A compiler binary may be a symlink to another and | ||
// so has the same digest, but that means | ||
// the toolchain will not contain the correct path | ||
// to invoke the compiler! Add the compiler | ||
// executable path to try and prevent this | ||
let weak_toolchain_key = | ||
format!("{}-{}", executable.to_string_lossy(), executable_digest); | ||
return Ok(HashResult { | ||
key, | ||
compilation: Box::new(CCompilation { | ||
parsed_args: parsed_args.to_owned(), | ||
#[cfg(feature = "dist-client")] | ||
// TODO or is it never relevant since dist? | ||
preprocessed_input: vec![], | ||
executable: executable.to_owned(), | ||
compiler: compiler.to_owned(), | ||
cwd: cwd.to_owned(), | ||
env_vars: env_vars.to_owned(), | ||
}), | ||
weak_toolchain_key, | ||
}); | ||
} else { | ||
debug!("Preprocessor cache miss: {preprocessor_key}"); | ||
|
||
if !update_failed { | ||
if let Some(key) = hit { | ||
debug!("Preprocessor cache hit: {preprocessor_key}"); | ||
// A compiler binary may be a symlink to another and | ||
// so has the same digest, but that means | ||
// the toolchain will not contain the correct path | ||
// to invoke the compiler! Add the compiler | ||
// executable path to try and prevent this | ||
let weak_toolchain_key = | ||
format!("{}-{}", executable.to_string_lossy(), executable_digest); | ||
return Ok(HashResult { | ||
key, | ||
compilation: Box::new(CCompilation { | ||
parsed_args: parsed_args.to_owned(), | ||
#[cfg(feature = "dist-client")] | ||
// TODO or is it never relevant since dist? | ||
preprocessed_input: vec![], | ||
executable: executable.to_owned(), | ||
compiler: compiler.to_owned(), | ||
cwd: cwd.to_owned(), | ||
env_vars: env_vars.to_owned(), | ||
}), | ||
weak_toolchain_key, | ||
}); | ||
} else { | ||
debug!("Preprocessor cache miss: {preprocessor_key}"); | ||
} | ||
} | ||
} | ||
} | ||
|
@@ -491,8 +500,12 @@ where | |
.collect(); | ||
files.sort_unstable_by(|a, b| a.1.cmp(&b.1)); | ||
preprocessor_cache_entry.add_result(start_of_compilation, &key, files); | ||
storage | ||
.put_preprocessor_cache_entry(&preprocessor_key, preprocessor_cache_entry)?; | ||
|
||
if let Err(e) = storage | ||
.put_preprocessor_cache_entry(&preprocessor_key, preprocessor_cache_entry) | ||
{ | ||
debug!("Failed to update preprocessor cache: {}", e); | ||
} | ||
Comment on lines
-494
to
+508
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above. |
||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you please add a test to trigger this error ? thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sylvestre Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you please check for the error message string too ?
thanks