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

go_1_14: init at 1.14 and switch to it #81071

Merged
merged 2 commits into from Feb 27, 2020
Merged

go_1_14: init at 1.14 and switch to it #81071

merged 2 commits into from Feb 27, 2020

Conversation

Frostman
Copy link
Member

@Frostman Frostman commented Feb 26, 2020

Motivation for this change

https://golang.org/doc/go1.14

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

5 package failed to build - same set as in minor go version bump #80506 (related to git2go or not related to go)
act blsd gitAndTools.grv gitaly python38Packages.aria2p

736 package built


This change is Reviewable

@Frostman Frostman changed the title go_1_14: init at 1.14 and switch to it (WIP) go_1_14: init at 1.14 and switch to it Feb 26, 2020
@Frostman
Copy link
Member Author

@GrahamcOfBorg build yq-go hugo restic terraform

@kalbasit
Copy link
Member

@GrahamcOfBorg build yq-go hugo restic terraform jx pet

@rvolosatovs
Copy link
Member

Got this when applying the patch, which may or may not be relevant:

$ curl -sL https://github.com/NixOS/nixpkgs/pull/81071.patch | git am
Applying: go_1_14: init at 1.14 and switch to it
/home/rvolosatovs/src/github.com/NixOS/nixpkgs/.git/worktrees/nixos-unstable/rebase-apply/patch:275: space before tab in indent.
 		})
/home/rvolosatovs/src/github.com/NixOS/nixpkgs/.git/worktrees/nixos-unstable/rebase-apply/patch:276: space before tab in indent.
 	}
/home/rvolosatovs/src/github.com/NixOS/nixpkgs/.git/worktrees/nixos-unstable/rebase-apply/patch:277: trailing whitespace.
 
/home/rvolosatovs/src/github.com/NixOS/nixpkgs/.git/worktrees/nixos-unstable/rebase-apply/patch:301: space before tab in indent.
 	// sync tests
/home/rvolosatovs/src/github.com/NixOS/nixpkgs/.git/worktrees/nixos-unstable/rebase-apply/patch:302: space before tab in indent.
 	if goos != "js" { // js doesn't support -cpu=10
warning: squelched 13 whitespace errors
warning: 18 lines add whitespace errors.

@Frostman
Copy link
Member Author

@rvolosatovs I think it's okay. The patch applies successfully anyway. There are whitespaces in the patch because of the way how it's formatted (it has two chars at the beginning of the line "booked" for +- chars and on the lines where only addition happens you'll have + and whitespace) in combination with tabs usage in Go.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nixos-20-03-feature-freeze/5655/37

Copy link
Contributor

@c00w c00w left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks awesome :) - It looks like nothing is getting overridden to go1.13 - do we need to still keep it around?

@c00w
Copy link
Contributor

c00w commented Feb 27, 2020

Also you motivated me to go check some of the go 1.12 overrides, so I opened #81159 to get rid of some of them.

@Frostman
Copy link
Member Author

@c00w I've been thinking about going through 1.12 first to get rid of it and then consider getting rid of 1.13 as well. Generally, it probably makes sense to keep the last two versions of the compiler, but not sure about it.

@FRidh FRidh changed the base branch from master to staging February 27, 2020 09:17
@FRidh FRidh merged commit 1b7c71d into NixOS:staging Feb 27, 2020
@Frostman Frostman deleted the go-1.14 branch February 27, 2020 09:23
@Mic92
Copy link
Member

Mic92 commented Feb 27, 2020

In general we should only keep go versions around that are also supported upstream with security updates.

@c00w
Copy link
Contributor

c00w commented Feb 27, 2020

Per https://golang.org/doc/devel/release.html#policy - only the last 2 releases are supported (i.e. go 1.12 is no longer supported). I think before go1.13 landed we only had 1 version of go. go 1.12 was kept around because a number of packages didn't work with go 1.13. I don't think there is a lot of benefit to keeping a go version around other than the latest one if we don't need it to build a given package.

@Frostman
Copy link
Member Author

@Mic92 @c00w yeah, makes sense. I'll look at the rest of the packages that are currently not yet builds with 1.14.

@Mic92
Copy link
Member

Mic92 commented Feb 27, 2020

@Frostman if there are fairly unmaintained it might also an option to remove them from nixpkgs depending on the package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants