-
Notifications
You must be signed in to change notification settings - Fork 373
fix: save glob-root for correct rebuilds #4839
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
fix: save glob-root for correct rebuilds #4839
Conversation
| /// E.g we are building from `package.build.source.path = "../"` we want | ||
| /// to resolve globs from that location | ||
| #[serde(default, skip_serializing_if = "Option::is_none")] | ||
| pub glob_root: Option<PathBuf>, |
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.
This is the new field.
| alias = "source_code_root", | ||
| alias = "input_root" | ||
| )] | ||
| pub glob_root: Option<PathBuf>, |
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.
This as well.
| return Ok(Some(metadata)); | ||
| }; | ||
|
|
||
| let Some(cached_root) = metadata.glob_root.as_ref() else { |
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.
Here we regenerate if its not found.
| .await | ||
| .map_err_with(SourceBuildCacheStatusError::SourceCheckout)?; | ||
|
|
||
| let Some(glob_root) = source_info.glob_root.as_ref().cloned() else { |
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.
Same here.
I assumed that we would only bump this if we were to make a breaking change in the |
|
@tdejager and I further discussed the version bump of the metadata cache, and we agreed that we should indeed bump it and then make the |
|
Now that I'm thinking about it, I feel this is not the correct approach, because we should have the data available, we also have it in the discovery code, so I need to close this for now. I might need: #4446 actually first? @remimimimimi lets discuss. |
This should fix: #4837
We now save the
glob-root, so where need to start globbing for the file for changes. We save this in the metadata cache, this does invalidate the current entries though, we could also opt to version it to v-2. Wdyt?The test-suite change is here: prefix-dev/pixi-build-testsuite#91