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
Add regenGoMod option #87855
Add regenGoMod option #87855
Conversation
This comment has been minimized.
This comment has been minimized.
@mweinelt - you need to update vendorSha256 :) |
Spot on! I tip my hat to you, sir! |
All builds successfully on Darwin as well (except for firectl, which was unsupported anyway). |
Result of 235 packages built:- _3mux - act - aerc - age - amass - antibody - archiver - argo - argocd - atlantis - aws-vault - awsweeper - azure-storage-azcopy - bat-extras.prettybat - bazel-gazelle - bazelisk - berglas - bettercap - blockbook - browserpass - buildah - caddy - caddy2 - cassowary - certigo - cheat - chezmoi - circleci-cli - clair - clash - clipman - cloudflared - cni-plugins - conftest - consul - coredns - corerad - cri-o - cri-tools - croc - ctop - cue - curlie - dbmate - dive - dnsproxy - do-agent - docker-machine-kvm2 - docui - documize-community - dolt - drone - drone-cli - echoip - editorconfig-checker - eksctl - elvish - exercism - firectl - fluxctl - fly - flyctl - frp - fscrypt-experimental - fzf - fzf-zsh - geoipupdate - ghq (gitAndTools.ghq) - gitAndTools.gh - gitAndTools.git-bug - gitAndTools.git-subtrac - gitAndTools.lab - lefthook (gitAndTools.lefthook) - gjo - glow - gmailctl - go-ethereum - go-jsonnet - go-license-detector - go-protobuf - go-tools - go2nix - gobetween - gobuster - godef - gofumpt - gogetdoc - golangci-lint - gomatrix - gomodifytags - gomuks - gopass - gopkgs - gopls - gortr - gotestsum - gotify-cli - gotify-server - gotools - gotop - grpcui - gvisor-containerd-shim - hasmail - hasura-cli - hcloud - helmfile - helmsman - hetzner-kube - hey - hugo - hydroxide - iamy - imgproxy - influxdb - ipfs - ipfs-cluster - ipfs-migrator - joker - jump - jx - k9s - kcli - kepubify - kind - kube3d - kubeprompt - kubernetes-helm - kubeseal - kubeval - kustomize - lego - lf - libgen-cli - linkerd - lnd - mage - magnetico - matterbridge - mautrix-whatsapp - minify - minikube - minio - minio-client - mkcert - mod - mpd-mpris - mtail - mutagen - mynewt-newt - nebula - neo-cowsay - netdata - nfpm - nix-prefetch-docker - node-problem-detector - obfs4 - onionshare - onionshare-gui - packet-cli - packr - pdfcpu - pet - pg_flame - pgcenter - pgmetrics - pistol - pixiecore - podman - podman-compose - podman-unwrapped - powerline-go - prometheus-dnsmasq-exporter - prometheus-mikrotik-exporter - prometheus-varnish-exporter - proto-contrib - protoc-gen-doc - protolock - prototool - prow - syncthing-gtk (python27Packages.syncthing-gtk) - qbec - qsyncthingtray - reftools - renderizer - reviewdog - richgo - run - saml2aws - sampler - sensu-go-agent - sensu-go-backend - sensu-go-cli - shadowfox - shfmt - shiori - skopeo - sops - sourcehut.buildsrht - sourcehut.gitsrht - sourcehut.hgsrht - styx - syncthing - syncthing-cli - syncthing-discovery - syncthing-relay - tailscale - telegraf - tendermint - terminal-parrot - termshark - terracognita - terraform-full - terraform-providers.elasticsearch - terraform-providers.lxd - terraform-providers.vpsadmin - terraform_0_11-full - terragrunt - tflint - thanos - tinygo - todoist - traefik - up - v2ray - vimPlugins.YouCompleteMe - vimPlugins.vim-go - wal-g - websocketd - wtf - xmloscopy - ycmd - yggdrasil - yq-go - zoxide - zsh-history |
@@ -20,6 +20,8 @@ | |||
, vendorSha256 ? null | |||
# Whether to delete the vendor folder supplied with the source. | |||
, deleteVendor ? false | |||
# Lots of packages don't have a good go.mod file, this regenerates it and uses it for downloading & building. | |||
, regenGoMod ? false |
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.
regenGoMod
needs to be documented in doc/languages-frameworks/go.xml
.
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.
Is there any harm in running this by default?
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.
It might obfuscate the fact, that the go.mod
should be reported to upstream. On the other hand this put‘s an additional burden on to nixpkgs packagers. So I opt for making it the default.
What I don‘t know is if that might trigger vendor hashes to change on modules that use the module. Someone with more insight into go could perhaps tell, if it is possible to have two different valid go.mod
files.
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.
If this is a potential source for non-reproducibility we should instead of regenerating it just log a diff file that users should include as the patch. Otherwise we are just running danger of breaking hash sums.
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.
Someone with more insight into go could perhaps tell, if it is possible to have two different valid go.mod files.
I think, in general, as long as the contents of vendor contain what the code needs to build, and what's marked as explicit in vendor/modulex.txt matches what's marked as explicit in go.mod - I believe there are an infinite number of potential valid go.mod files for a given set of go code (since you can always add a go module).
What I don‘t know is if that might trigger vendor hashes to change on modules that use the module
I think there are two interpretations of this question - would we need to regenerate all the hashes? (yes). Would this cause some sort of cascading effect on go code that imports other go code? (no - we don't have go code import other nix packages).
Is there any harm in running this by default?
It sounds safe, but there is actually a massive problem. Not all go code can run go mod tidy. I think given that only 5 packages need this fix and the rest build either way, I'd prefer to not spend a lot of time updating all the hashes and flipping the go around.
If this is a potential source for non-reproducibility we should instead of regenerating it just log a diff file that users should include as the patch.
This is not a source of non reproducibility - go mod tidy should always produce the same output and is deterministic based on the source code.
doc/languages-frameworks/go.xml
I really wish nix used autogenerated documentation from source (or maybe markdown). Either way, updated.
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 don't understand how it can be reproducible when it only read the source code. What happens if a referenced package is updated after we added the checksum? Would it not than jump to a newer version and therefore break the old checksum?
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.
Yeah - if go mod tidy is adding packages, this will cause regeneration to break. I think patches is probably the better solution since it's sticky. Hopefully the conformance of the go ecosystem will improve and we'll stop seeing horribly broken go.mod files.
@Mic92 - Should be updated :) |
@@ -47,6 +49,7 @@ let | |||
removeExpr = refs: ''remove-references-to ${lib.concatMapStrings (ref: " -t ${ref}") refs}''; | |||
|
|||
deleteFlag = if deleteVendor then "true" else "false"; | |||
regenFlag = if regenGoMod then "true" else "false"; |
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 use boolToString regenGoMod
instead of writing this manually.
Someone poked me on IRC about why this wasn't reproducible, so I'm appending a little more information. Consider two cases currently example.com/foo is set to V1.0 if we run regenGoMod then the go.mod file with include if example.com/foo is updated to v1.1 and someone builds the same package, they'll get |
Motivation for this change
A number of go modules need
go mod tidy
run. Due to this a number of derivations are carrying patches. This removes the patches and just teaches nix how to do this.Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)