Skip to content

Support directory mtime to fix tests with recent buildbox-casd#2001

Merged
juergbi merged 2 commits intomasterfrom
juerg/directory-mtime
Mar 25, 2025
Merged

Support directory mtime to fix tests with recent buildbox-casd#2001
juergbi merged 2 commits intomasterfrom
juerg/directory-mtime

Conversation

@juergbi
Copy link
Contributor

@juergbi juergbi commented Mar 14, 2025

This fixes test failures with recent versions of buildbox-casd, see commit 85f0578f ("common: Support capturing directory mtime and mode").

juergbi added 2 commits March 14, 2025 14:55
This fixes test failures with recent versions of buildbox-casd, see
commit 85f0578f ("common: Support capturing directory mtime and mode").
@gtristan
Copy link
Contributor

This changes the artifact content correct ?

Building a given artifact before this fix and after this fix, for the same cache key, will give you a different cas digest correct ?

Or, does this avoid buildstream automatically consuming this new behaviour from buildbox-casd ? So users who have not yet applied this patch but happen to have a newer buildbox-casd, will be producing different artifacts by default ?

@juergbi
Copy link
Contributor Author

juergbi commented Mar 25, 2025

mtime capture is enabled only for elements with a workspace. Normal builds are neither affected by this change nor the buildbox-casd change.

For workspace builds, the buildbox-casd change will result in a different CAS digest as directory mtimes are now captured as well. However, CAS digests of workspace builds are not deterministic anyway due to the file timestamps.

What this change does is to apply the directory mtimes captured by recent casd to directories in bst artifact checkout. So the only noticeable difference should be when you checkout artifacts of workspace builds. It will restore the directory timestamps from the build, and I can't think of any negative side-effects of this in the context of workspace builds.

And it fixes a test to work with old and new versions of buildbox-casd.

Copy link
Contributor

@gtristan gtristan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM !

@juergbi juergbi merged commit 922b150 into master Mar 25, 2025
17 checks passed
@juergbi juergbi deleted the juerg/directory-mtime branch March 25, 2025 13:46
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

Successfully merging this pull request may close these issues.

2 participants