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

Update "dirty" file only when GitIsDirty changes #184

Merged
merged 2 commits into from
Aug 5, 2022

Conversation

AmoreCadenza
Copy link
Contributor

This change fixes what looks like a bug in the current implementation of the _GitRoot target: namely, that the target always appends a 0 or 1, on a new line, to the _GitIsDirtyFile every time said target runs, causing all other targets that depend on it as an input to always rerun as well. Ultimately, this breaks incremental builds of projects that rely on the GitInfo package.

With this change, the _GitIsDirtyFile behaves as its logic appears to have been intended to: instead of always appending the current value of $(GitIsDirty) to the file, the current value of ($GitIsDirty) will replace the file's contents if and only if the value differs from what's already in the file. Otherwise, the file remains untouched and all other targets that use it as an input will function as expected in incremental builds.

This fixes or at least significantly mitigates the problems described in #183.

This change fixes what looks like a bug in the current implementation of the `_GitRoot` target: namely, that the target always appends a `0` or `1`, on a new line, to the `_GitIsDirtyFile` every time said target runs, causing all other targets that depend on it as an input to always rerun as well. Ultimately, this breaks incremental builds of projects that rely on the `GitInfo` package.

With this change, the `_GitIsDirtyFile` behaves as its logic appears to have been intended to: instead of always appending the current value of `$(GitIsDirty)` to the file, the current value of `($GitIsDirty)` will replace the file's contents if and only if the value differs from what's already in the file. Otherwise, the file remains untouched and all other targets that use it as an input will function as expected in incremental builds.

This fixes or at least significantly mitigates the problems described in devlooped#183.
@Sergio0694
Copy link

Hey @devlooped, are there any concerns on this to get it merged? We also had to temporarily disable this package reference to reduce the build time, as this issue was breaking the incremental build and causing projects to be rebuilt unnecessarily. Would be great to have an updated version of the library with this fix, assuming the changes in here look right to you 😊

@kzu kzu enabled auto-merge (rebase) August 5, 2022 06:25
@kzu kzu merged commit ae5427b into devlooped:main Aug 5, 2022
@kzu kzu mentioned this pull request Feb 6, 2023
@devlooped devlooped locked and limited conversation to collaborators Sep 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants