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

element.py: make BST_STRICT_REBUILD part of the cache key #1416

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Cynical-Optimist
Copy link

See original merge request on GitLab
In GitLab by [Gitlab user @abderrahimk] on May 4, 2020, 11:13

Description

Add the value of BST_STRICT_REBUILD to the cache key. This prevents confusing situations like #1270.

This merge request, when approved, will close: #1270


Otherwise, this could give rise to confusing situations like #1270
@Cynical-Optimist
Copy link
Author

In GitLab by [Gitlab user @juergbi] on May 4, 2020, 11:38

Commented on src/buildstream/element.py line 2051

It might make sense to add this dict key only when BST_STRICT_REBUILD is set to True. This would reduce unnecessary rebuilds for current users of BuildStream master.

@Cynical-Optimist
Copy link
Author

In GitLab by [Gitlab user @abderrahimk] on May 4, 2020, 11:41

Commented on src/buildstream/element.py line 2051

That's true, but I noticed that nothing in the current dict is set conditionally. I'm fine with it either way.

@Cynical-Optimist
Copy link
Author

In GitLab by [Gitlab user @tristanvb] on May 4, 2020, 12:20

Do we have this same issue with the strict attribute of dependency declarations ?

@Cynical-Optimist
Copy link
Author

In GitLab by [Gitlab user @juergbi] on May 4, 2020, 14:20

Yes. This makes we wonder whether it would be clearer to include the weak cache key in the calculation of the strong cache key, preventing this whole issue class. It would mean that the weak cache key and the strong cache key would never match but I don't think this would cause any issues. And it would obviously invalidate all strong cache keys in BuildStream master.

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