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
conftest: 0.7.0 -> 0.12.0 #68438
conftest: 0.7.0 -> 0.12.0 #68438
Conversation
@GrahamcOfBorg build conftest |
Maybe depending on a patch based on PR URL was not the best idea - |
bfaefae
to
e60d7d6
Compare
e60d7d6
to
186e575
Compare
@GrahamcOfBorg build conftest |
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.
LGTM thanks!
@GrahamcOfBorg build conftest |
patches = [ | ||
# Version 0.12.0 does not build with go 1.13. See https://github.com/instrumenta/conftest/pull/85. | ||
# TODO: Remove once https://github.com/instrumenta/conftest/pull/85 is merged and lands in a release. | ||
./go-1.13-deps.patch |
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.
I assume this patch differs from, so you cannot use i.e. fetchpatch
? https://github.com/instrumenta/conftest/commit/e8494d819585452e501728a364e31d38100136de.patch
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.
Yes, initially it was the same, but @jpreese asked for more changes, including dependency update. IMO this is too much to add onto the 0.12.0
and can potentially change behavior - the proposed patch is the bare minimum to make the 0.12.0
version build on go 1.13
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.
You can see that the proposed patch keeps dependency hashes intact (apart from xerrors
, since the version dependent on is incompatible with go 1.13)
I get a different checksum for the go modules: |
186e575
to
2a7c0f2
Compare
@GrahamcOfBorg build conftest |
@Mic92 Mildly related: I sometime have issues where my local machine comes up with different hashes from e.g. Travis. Any idea what might cause that? I have a somewhat elaborate set up with various overlays and such, so it's a bit tricky for me to try to track down the root cause. Any hints would be greatly appreciated. |
@yurrriq I have not seen this with golang in particular. One recurring issue I saw, is that macOS used a case-insensitive filesystem (HFS+). If you have derivation produced by programs like go/cargo it could also depend on the program's version - which would be a bug of our fetcher in that case. |
Yeah, it's not specific to golang for me either, I don't think. I'm on NixOS, by the way. Here's an example with renderizer: https://travis-ci.org/yurrriq/nur-packages/builds/572301956 |
Motivation for this change
Upstream update
Blocks #68135
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)Notify maintainers
cc @yurrriq