-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
buildGoModule: remove modSha256 #93513
Conversation
Yes. Actually I think should be not merge before the 20.09 branch off. So people that migrate from 20.03 to 20.09 have the time to update their packages - They would get one release time to migrate everything which is the smallest time unit we can give them. |
Why? |
This was not present in 20.03. If users upgrade to 20.09 all their packages needs an update. If you have a infrastructure large enough, you may not upgrade all installations at once. The branch off is soonish anyway so these 200 lines should not bother us. |
Yes, it is a breaking change that is documented. People will need to set |
Ok. Than we can merge it already. |
People that needs to support both can just have both checksums defined in the package. |
@@ -257,8 +255,5 @@ let | |||
}); | |||
in if disabled then | |||
throw "${package.name} not supported for go ${go.meta.branch}" | |||
else if modSha256 != null then | |||
lib.warn "modSha256 is deprecated and will be removed in the next release (20.09), use vendorSha256 instead" ( |
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.
Can you convert this in a throw statement that provides instruction to use vendorSha256
instead?
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.
Changed to throw.
With the change to throw the above won't be true anymore. I think it'll be useful for people to support both. Can we throw only if modSha256 is defined but vendorSha256 is not? |
I'd rather we go back to the original which just outright removed IMHO the error that |
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 you are going to merge it before the 20.09 branch-off, please update this bit:
nixpkgs/nixos/doc/manual/release-notes/rl-2009.xml
Lines 205 to 207 in c581528
attribute to pin fetched version data. <literal>buildGoModule</literal> | |
still accepts <literal>modSha256</literal> with a warning, but support will | |
be removed in the next release. |
Oh, I didn't realise it said that. It's been in the 20.09 release notes for a couple of months now, is it okay |
This is dead code but I'll wait until we're closer to 20.09 before merging.