fix(nix): snapshot builds can now hash files as well #5894
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the Nix integration to always use real file hashing (via nix-hash) instead of the previous zero-only hasher, consolidates two constructors into one, and updates all usages and tests accordingly.
- Consolidate
NewBuildandNewPublishinto a singleNewfunction that always usesrealHasher. - Remove the
alwaysZeroHasherandzeroHashfrom production code and adjust tests to match the new behavior. - Update all pipeline, publish, and healthcheck references to call
nix.New()and revise tests innix_test.go.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/healthcheck/healthcheck.go | Replaced nix.NewPublish() with nix.New() |
| internal/pipeline/pipeline.go | Replaced nix.NewBuild() with nix.New() |
| internal/pipe/publish/publish.go | Replaced nix.NewPublish() with nix.New() |
| internal/pipe/nix/nix.go | Removed zero-only hasher, consolidated constructors into New |
| internal/pipe/nix/nix_test.go | Updated tests to use New(), removed zero-hasher tests |
Comments suppressed due to low confidence (1)
internal/pipe/nix/nix.go:53
- The comment refers only to the publish phase, but
Newnow handles both build and publish. Consider updating it to something like// New returns a Nix hashing pipe used in both build and publish phases.
// New returns a pipe to be used in the publish phase.
Comment on lines
681
to
685
| type unavailableHasher struct{} | ||
|
|
||
| func (m unavailableHasher) Hash(string) (string, error) { | ||
| return "", errors.New("unavailable hasher") | ||
| } | ||
| func (m unavailableHasher) Available() bool { return false } | ||
| func (m unavailableHasher) Hash(string) (string, error) { return "", errors.New("unavailable hasher") } | ||
| func (m unavailableHasher) Available() bool { return false } | ||
|
|
There was a problem hiding this comment.
The unavailableHasher type is defined but never used in any test. You can remove this dead code to keep the test file focused.
Copilot uses AI. Check for mistakes.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5894 +/- ##
==========================================
- Coverage 82.68% 82.68% -0.01%
==========================================
Files 165 165
Lines 16520 16516 -4
==========================================
- Hits 13660 13656 -4
Misses 2268 2268
Partials 592 592 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Historically, we used to use
nix-prefetch-url, so it could only calculate the hashes after the artifacts were pushed, so when using--snapshot, hashes would always be0000....Rather recently, we've changed it to use
nix-hashinstead, so we could also have changed it so it uses real hashes all the time.This PR does that.
Discussion: https://github.com/orgs/goreleaser/discussions/5893