Skip to content
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

cache-policy = configuration is often insufficient and maybe buggy #4

Open
calebzulawski opened this issue Jan 12, 2023 · 7 comments
Open

Comments

@calebzulawski
Copy link
Collaborator

I originally suspected git was resetting modification times (and std:fs:track assumes it monotonically increases), but it looks like git actually always updates the modification time of files it changes when switching branches.

Also, considering it's always link errors (e.g. missing symbols) I'm guessing the relationship with source files and object files is somehow broken when switching branches.

@afranchuk
Copy link
Collaborator

std:fs:track doesn't assume increasing mod times, it just checks for differences. But as you've found that is likely not the culprit (not to mention if it were we would have likely found the issue sooner).

@calebzulawski
Copy link
Collaborator Author

This line makes me think the hash is only calculated if the file is newer, but not older:

https://github.com/CACI-International/ergo/blob/7ca0725ee95147bd3000e05626b6e7c90b4e9021/ergo_std/src/fs.rs#L743

@afranchuk
Copy link
Collaborator

Ah so it does, that should probably be changed. The modification time is only used as an optimization.

@calebzulawski
Copy link
Collaborator Author

Opened CACI-International/ergo#6 to change that. Also, I realized this is probably due to cache-policy--I'm going to try using cache-policy = content and see how that goes.

@calebzulawski calebzulawski changed the title Changing branches in git can cause link errors in incremental builds cache-policy = configuration is often insufficient and maybe buggy Jan 16, 2023
@calebzulawski
Copy link
Collaborator Author

calebzulawski commented Jan 16, 2023

@pdurbano reports that incremental builds sometimes (often) don't pick up source files changes and produce outdated binaries. Some source files (cpp files, not headers) always pick up changes, while others don't (depending on the module). Using cache-policy = content fixes the issue.

Unless there's extreme overhead (I didn't measure anything substantial), I think the default should be changed to content.

@afranchuk
Copy link
Collaborator

The inherent overhead is disk space. There's no policy to remove old versions. Otherwise, cache-policy = content is fine (that's what it's there for)!

@afranchuk
Copy link
Collaborator

I have some unit tests for change tracking, I should expand them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants