Fix a couple of build caching issues - #76
Conversation
- Avoid overwriting src/protobuf.rs if it has not changed - Avoid unconditionally copying some libpg_query files to outdir Both of these inadvertently break build caching.
| @@ -28,9 +32,15 @@ | |||
| build_path.join("vendor"), | |||
| ]; | |||
|
|
|||
| let copy_options = CopyOptions { overwrite: true, ..CopyOptions::default() }; | |||
| let hash_file = out_dir.join(".source_hash"); | |||
| let current_hash = hash_source_paths(&source_paths); | |||
| let cached_hash = std::fs::read_to_string(&hash_file).unwrap_or_default(); | |||
There was a problem hiding this comment.
I don't love this approach, but to fix the build caching issues, we need some way to avoid overwriting the files if they have not changed. Although, I'm not sure why it's necessary: these files are just going to outdir. I'm not sure why Cargo cares that they're newer than expected.
| if path.is_file() { | ||
| if let Ok(contents) = std::fs::read(path) { | ||
| hasher.write(path.to_string_lossy().as_bytes()); | ||
| hasher.write(&contents); |
There was a problem hiding this comment.
How slow is this? If it's slow it may be acceptable to just hash the file name + last modified timestamp
There was a problem hiding this comment.
I considered that, but that could cause issues with switching branches. The whole hashing step takes 20-25ms here, so I don't think we need to worry about it.
| let out_protobuf_path = out_dir.join("protobuf"); | ||
| let target = env::var("TARGET").unwrap(); | ||
|
|
||
| println!("cargo:rerun-if-changed=libpg_query"); |
There was a problem hiding this comment.
It's possible this could include the submodule's git files, so any git operation inside the submodule might invalidate the cache. It doesn't have to be part of this PR, but it might make sense to make this call use source_paths as an input (calling it once for each path).
There was a problem hiding this comment.
In practice, most git operations would also result in changes to the actual input files, no? No objections to doing that, but I think the changes here should greatly improve caching in the common case.
2 participants
Both of these inadvertently break build caching.
Note on AI use: Claude tracked down the problem and drafted the fix with some guidance.