go1.16: remove #168480

ajs124 · 2022-04-13T12:22:23Z

Description of changes

go1.16 reached EOL with the release of go1.18

Things done

ajs124 · 2022-04-13T13:01:20Z

Result of nixpkgs-review pr 168480 run on x86_64-linux 1

3 packages marked as broken and skipped:

goofys
gvisor
utahfs

17 packages built:

godu
ipfs
ipfs-migrator
ipfs-migrator-all-fs-repo-migrations
mycorrhiza
ncdns
obfs4
onionshare
onionshare-gui
python310Packages.ipfshttpclient
python310Packages.ipwhl
python39Packages.ipfshttpclient
python39Packages.ipwhl
sampler
textql
vouch-proxy
yajsv

zowoq · 2022-04-13T13:07:19Z

@ofborg test ipfs
@ofborg build godu ipfs mycorrhiza ncdns obfs4 sampler textql vouch-proxy yajsv

endocrimes

I see a lot of packages here that have go references removed without corresponding package bumps - have you verified that the software still works with newer go versions? (beyond just building?)

endocrimes · 2022-04-13T13:06:20Z

pkgs/applications/misc/sampler/default.nix

@@ -11,7 +11,15 @@ buildGoModule rec {
    sha256 = "1lanighxhnn28dfzils7i55zgxbw2abd6y723mq7x9wg1aa2bd0z";
  };

-  vendorSha256 = "04nywhkil5xkipcibrp6vi63rfcvqgv7yxbxmmrhqys2cdxfvazv";
+  patches = [


we should wait for a release or use an untagged commit - randomly pulling in patches sucks for everyone.

There isn't one, yet. We can wait, but then we'll need to keep this EOL go release around.

endocrimes · 2022-04-13T13:07:31Z

pkgs/applications/virtualization/gvisor/default.nix

@@ -120,5 +120,7 @@ in buildBazelPackage rec {
    license = licenses.asl20;
    maintainers = with maintainers; [ andrew-d ];
    platforms = [ "x86_64-linux" ];
+    # the version we have right now does not compile with go 1.17
+    broken = true;


latest builds with go1.17 - lets bump before landing this PR. https://github.com/google/gvisor/blob/master/go.mod#L3

Can you bump it? I think I tried and it was non-trivial. IMO this is the maintainer's (@andrew-d) job not ours.

The release is ~10 months out of date, with a software that releases very often, looking at https://github.com/google/gvisor/tags.

endocrimes · 2022-04-13T13:09:01Z

pkgs/development/tools/textql/default.nix

  };

+  patches = [
+    (fetchpatch {
+      url = "https://github.com/dinedal/textql/commit/a0d7038c8c30671dfd618f47322814ab492c11a1.patch";


This is a commit from a fork - we should reference it via the fork to avoid confusion and to be explicit.

we should reference it via the fork

Revert this change. If the fork is deleted the patch will disappear. Please add a comment linking the PR the commit came from.

It'll be in the binary cache either way, so I don't really care, tbh.

The same diff can be gotten many ways, so how can we find a consensus here? I assume nixpkgs doesn't have a policy for this, despite there being dozens if not hundreds of occurrences of this throughout the codebase, as always?

Using a source that won't disappear instead of one that may disappear seems link a better idea.

I've seen lots of reviews asking for a comment referencing the PR that a commit is sourced from, I haven't seen anyone ask that we actually use the PR authors fork to source the commits.

ajs124 · 2022-04-13T17:16:20Z

iirc I've tested ones with tests, e.g. ncdns

Luflosi · 2022-04-15T15:18:50Z

This breaks ipfs-migrator-all-fs-repo-migrations. See #161547, where I tested this and documented it in the PR description and commit message. Feel free to test this again as described in the commit message. Also see ipfs/fs-repo-migrations#156, which I just created.

Please also run the IPFS NixOS test. Go 1.17 support seems to still be in progress: ipfs/kubo#8815.

ajs124 · 2022-04-15T15:57:18Z

[…]
(finished: must succeed: ipfs --api /unix/run/ipfs.sock cat /ipfs/QmfNnecR7tB17fa4tjY9V2dDVwsanVnJPws3HoEM9W2wjC | grep fnord2, in 0.17 seconds)
machine: must succeed: test -e /mnt/ipfs/config
(finished: must succeed: test -e /mnt/ipfs/config, in 0.02 seconds)
machine: must succeed: test ! -e /var/lib/ipfs/
(finished: must succeed: test ! -e /var/lib/ipfs/, in 0.02 seconds)
(finished: run the VM test script, in 42.02 seconds)
test script finished in 42.09s
cleanup
kill machine (pid 8)
machine # qemu-kvm: terminating on signal 15 from pid 6 (/nix/store/bvzpppawf2naqv3qhxqv66jxaq3iaxyj-python3-3.9.11/bin/python3.9)
(finished: cleanup, in 0.02 seconds)
kill vlan (pid 7)
/nix/store/ciff928nb15cnj17jhlhbk5klnpgb08r-vm-test-run-ipfs

so ipfs is fine, but ipfs-migrator-all-fs-repo-migrations is apparently still broken.

Luflosi · 2022-04-26T12:12:11Z

See ipfs/kubo#8815 for why the currently existing migrations probably won't be updated to support a newer Go version. This means, we either need to keep Go 1.16 or remove the incompatible migrations. In the second case I could write a stub to replace them, which prints some text, explaining the situation.

ajs124 · 2022-05-05T10:27:28Z

In the second case I could write a stub to replace them, which prints some text, explaining the situation.

@Luflosi can you go ahead and do that? go1.16 is EOL and I'd very much like to see it removed before branch-off.

dasJ · 2022-05-21T17:06:32Z

I'd like to remind the people here that branch-off is scheduled for tomorrow and depending on how good the staging builds progress, we will branch off rather soon.

pkgs/top-level/all-packages.nix

This reverts commit cee6ccf.

mausch · 2022-06-04T14:44:20Z

pkgs/tools/filesystems/goofys/default.nix

@@ -30,7 +30,8 @@ buildGoModule {
    description = "A high-performance, POSIX-ish Amazon S3 file system written in Go.";
    license = [ lib.licenses.mit ];
    maintainers = [ lib.maintainers.adisbladis ];
-    broken = stdenv.isDarwin; # needs to update gopsutil to at least v3.21.3 to include https://github.com/shirou/gopsutil/pull/1042
+    # does not build with go 1.17
+    broken = true;


ping @adisbladis , I don't think you've been notified?

Thanks for the ping. Fixed in #176255.

ofborg bot requested review from uvNikita, kalbasit, snglth, andrew-d and vrthra April 13, 2022 12:45

ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Apr 13, 2022

ajs124 force-pushed the drop/go116 branch from 44aead9 to 7de12fe Compare April 13, 2022 12:55

github-actions bot added the 6.topic: golang label Apr 13, 2022

ajs124 marked this pull request as ready for review April 13, 2022 13:01

ajs124 requested review from Mic92 and zowoq as code owners April 13, 2022 13:01

ofborg bot added 8.has: clean-up 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 and removed 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Apr 13, 2022

endocrimes suggested changes Apr 13, 2022

View reviewed changes

ajs124 force-pushed the drop/go116 branch from 7de12fe to bec28f6 Compare April 13, 2022 17:15

Luflosi mentioned this pull request Apr 15, 2022

Go 1.16 is EOL ipfs/fs-repo-migrations#156

Open

ajs124 force-pushed the drop/go116 branch from bec28f6 to 9fc2ed0 Compare May 4, 2022 23:05

dasJ reviewed May 21, 2022

View reviewed changes

pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved

mweinelt mentioned this pull request May 22, 2022

Upgrade to go 1.18 by default for 22.05 #168534

Closed

ajs124 added 13 commits May 22, 2022 13:47

Revert "ncdns: pin to go 1.16"

1909454

This reverts commit cee6ccf.

gomobile: don't pin to go 1.16

0ebd558

ipfs: don't pin to go1.16

8da4426

ipfs-migrator-all-fs-repo-migrations: don't pin to go1.16

8d2436d

godu: don't pin to go1.16

5c46ce1

textql: 2.0.3 -> unstable-2021-07-06

ff95455

mycorrhiza: 1.8.2 -> 1.9.0

fa88746

vouch-proxy: disable checkPhase

6a93220

obfs4: 0.0.11 -> 0.0.12

07e0094

sampler: fix build with go 1.17

ceebedb

yajsv: fix build with newer go

d070705

goofys: mark as broken

caa2b2e

utahfs: mark as broken

0ec34d4

Mic92 force-pushed the drop/go116 branch 2 times, most recently from 51cda9a to 8a51267 Compare May 22, 2022 13:58

ajs124 added 3 commits May 22, 2022 15:22

gvisor: mark as broken

235dd72

nomad_1_1: drop

dab5668

go_1_16: remove

b7a56be

Mic92 force-pushed the drop/go116 branch from 8a51267 to b7a56be Compare May 22, 2022 15:22

mob: build against newer go version

3217850

Mic92 merged commit 3f39d66 into master May 22, 2022

Mic92 deleted the drop/go116 branch May 22, 2022 16:05

mausch reviewed Jun 4, 2022

View reviewed changes

Luflosi mentioned this pull request Jan 6, 2023

kubo-migrator: add migration from 12 to 13 #209304

Merged

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

go1.16: remove #168480

go1.16: remove #168480

ajs124 commented Apr 13, 2022

ajs124 commented Apr 13, 2022

zowoq commented Apr 13, 2022 •

edited

Loading

endocrimes left a comment

endocrimes Apr 13, 2022

ajs124 Apr 13, 2022

endocrimes Apr 13, 2022

ajs124 Apr 13, 2022

endocrimes Apr 13, 2022

ajs124 Apr 13, 2022

zowoq Apr 13, 2022

ajs124 Apr 13, 2022

zowoq Apr 13, 2022

ajs124 commented Apr 13, 2022

Luflosi commented Apr 15, 2022 •

edited

Loading

ajs124 commented Apr 15, 2022

Luflosi commented Apr 26, 2022

ajs124 commented May 5, 2022

dasJ commented May 21, 2022

mausch Jun 4, 2022

adisbladis Jun 4, 2022

go1.16: remove #168480

go1.16: remove #168480

Conversation

ajs124 commented Apr 13, 2022

Description of changes

Things done

ajs124 commented Apr 13, 2022

zowoq commented Apr 13, 2022 • edited Loading

endocrimes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajs124 commented Apr 13, 2022

Luflosi commented Apr 15, 2022 • edited Loading

ajs124 commented Apr 15, 2022

Luflosi commented Apr 26, 2022

ajs124 commented May 5, 2022

dasJ commented May 21, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zowoq commented Apr 13, 2022 •

edited

Loading

Luflosi commented Apr 15, 2022 •

edited

Loading