tailscale: init at 0.97-0 [backport 19.09]

[danderson]

Motivation for this change

Tailscale is a new package+nixos module, merged recently in master. I would like to backport it to 19.09, selfishly so that I can install Tailscale on my NixOS servers.

It's not clear if this is okay or not. I backported the pppd module I created into 19.09 late last year, and back then the reasoning was: it's a "leaf" config, so it only risks breaking something if you explicitly decide to enable it. Therefore the risk was low to other 19.09 users.

I think the same reasoning applies here: the tailscale package is a "leaf" package that creates binaries (not libs), and the nixos module does nothing unless you explicitly opt-in. So, this should be completely safe to backport.

The only policy I could find was in the NixOS manual, where it says RMs decide what bugs & features should be backported. So, @disassembler and @lheckemann, I think this is your decision.

There are 4 commits in this PR: 3 cherrypicks, and 1 19.09-only change.

1. Cherrypick c5733e7 to fix buildGo113Module (otherwise it can't build anything)
2. Cherrypick 6e055c9 (tailscale pkg+module init)
3. Cherrypick 1e59307 (pkg version bump to fix a severe bug)
4. Change the pkg to use buildGo113Module instead of buildGoModule, because 19.09 defaults to Go 1.12 and Tailscale requires >=1.13.

Of note: even though buildGo113Module exists in 19.09, this would be the 1st and only package that uses it in 19.09 (that's why it was broken without anyone noticing).

So, um, there. That's all the information on this change. Hopefully that helps you decide if this is acceptable to backport or not. I would like to document some general guidelines for this for future maintainers (because nobody was sure if this is okay or not), so I anxiously await your feedback :)

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.

[danderson]

Ouch, mass rebuild. That's because of c5733e7 , triggering the rebuild of anything that's go+modules. It should be a no-op for anything on Go 1.12 (except hash mutations, of course), but that's a much bigger change than I bargained for when I sent this PR.

tailscale can't work without that backport, so I think that makes me lean towards rejecting this PR, and just waiting for tailscale in 20.03. That's a big rebuild for 19.09 late in its life.

But you're welcome to talk me out of it ;)

[KamilaBorowska]

This looks reasonable. The rebuild is unfortunate, but seems harmless as GOSUMDB doesn't exist in Go 1.12^[1].

References

1. Go 1.13 Release Notes, Modules Environment Variables

[Profpatsch]

Same comment as in #82827 (review) (I think you can even squash the version update commit), then LGTM

[danderson]

Done.

[worldofpeace]

Approving as I did on IRC.

[danderson]

Updated to use mostly cherrypicks from master. The one change that's not a cherrypick is specific to 19.09: 609a3da makes the derivation use Go 1.13 explicitly, as it's required by tailscale and not the default on 19.09.

The cherry-pick of 0e1cf19 still causes a rebuild of all Go code. The rebuild is a no-op because Go 1.12 doesn't know about the sumdb environment variable that got added, but that change alters hashes and so, yay rebuild.

[worldofpeace]

@GrahamcOfBorg build antibody

[worldofpeace]

testing it's no op change there.

[danderson]

Minor diff in the antibody binary:

--- /home/dave/antibody.before	2020-03-23 15:06:44.702861189 -0700
+++ /home/dave/antibody.after	2020-03-23 15:06:37.322806354 -0700
@@ -40,12 +40,12 @@
 00000270  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
 *
 00000f40  00 00 00 00 00 00 00 00  00 00 00 00 04 00 00 00  |................|
-00000f50  53 00 00 00 04 00 00 00  47 6f 00 00 70 32 4c 53  |S.......Go..p2LS|
-00000f60  4c 38 46 56 75 56 6e 4c  6a 6d 73 6d 63 6c 77 6b  |L8FVuVnLjmsmclwk|
+00000f50  53 00 00 00 04 00 00 00  47 6f 00 00 36 69 4e 33  |S.......Go..6iN3|
+00000f60  79 4a 43 35 38 33 5a 34  7a 51 32 77 71 77 37 76  |yJC583Z4zQ2wqw7v|
 00000f70  2f 6e 31 79 63 72 73 4b  66 32 5f 68 34 63 6a 5f  |/n1ycrsKf2_h4cj_|
 00000f80  56 6a 30 6d 74 2f 75 63  4f 53 42 37 77 66 71 76  |Vj0mt/ucOSB7wfqv|
-00000f90  38 57 42 67 6e 70 43 55  53 4f 2f 38 75 61 6b 5a  |8WBgnpCUSO/8uakZ|
-00000fa0  6c 4d 75 65 57 54 50 6d  4b 47 4e 62 2d 74 48 00  |lMueWTPmKGNb-tH.|
+00000f90  38 57 42 67 6e 70 43 55  53 4f 2f 78 50 70 57 52  |8WBgnpCUSO/xPpWR|
+00000fa0  7a 35 51 6b 34 37 6a 64  36 44 77 45 5f 61 53 00  |z5Qk47jd6DwE_aS.|
 00000fb0  2f 6e 69 78 2f 73 74 6f  72 65 2f 77 78 31 76 6b  |/nix/store/wx1vk|
 00000fc0  37 35 62 70 64 72 36 35  67 36 78 77 78 62 6a 34  |75bpdr65g6xwxbj4|
 00000fd0  72 77 30 70 6b 30 34 76  35 6a 33 2d 67 6c 69 62  |rw0pk04v5j3-glib|

Looks pretty minor, but can't identify the diff more precisely without further disasm.

[danderson]

Readelf says all the changed bytes are in the .note.go.buildid ELF section, which is Go's own deterministic build ID that hashes its inputs. I suspect it's taking in the env as a hash input, which would explain why it changed and nothing else - but digging further.

Either way, the actual code+data of the binary is unchanged.

[worldofpeace]

@danderson I was wondering this exactly if that somehow will change build reproducibility if we changed the configure environment. And when you mentioned something about hashes changing I naively thought "well hopefully not modSha256", but reading the code changes I believe that sort of change would not be possible.

[danderson]

I'm looking through the code of Go 1.12 that generates the build ID. It cares about a lot of factors, but afaict the environment variables is not one of them...

Clearly one of the inputs to the Go build ID is changing, but I can't figure out which one. I've sent a distress call on twitter (https://twitter.com/dave_universetf/status/1242222407810572288) because I have no idea where to go from here to identify the cause. Will keep looking.

On the bright side though: this change is a no-op in practice, there is no change in the generated machine code. I still want to get a definitive answer on why the build ID changed though.

[danderson]

Oh actually, does buildGoModule use a deterministic rootdir path when building? The build path is hashed into the build ID, so if the derivation hash changes (which it did, because buildGoModule's derivation hash changed, because of the env var change), then it would hash in a different build path into the build ID.

That would explain the "build ID only" diff: produced exactly the same code, but built from a different path?

@worldofpeace I'm too new to Nix to be sure of my reasoning here. Does this seem plausible, or would you expect buildGoModule to eradicate that harmless non-determinism? (harmless because it only happens if the derivation hash changes, but the produced binaries are identical)

[worldofpeace]

Oh actually, does buildGoModule use a deterministic rootdir path when building? The build path is hashed into the build ID, so if the derivation hash changes (which it did, because buildGoModule's derivation hash changed, because of the env var change), then it would hash in a different build path into the build ID.

That would explain the "build ID only" diff: produced exactly the same code, but built from a different path?

@worldofpeace I'm too new to Nix to be sure of my reasoning here. Does this seem plausible, or would you expect buildGoModule to eradicate that harmless non-determinism? (harmless because it only happens if the derivation hash changes, but the produced binaries are identical)

@kalbasit Authored buildGoModule, so he would know. This is the buildPhase

nixpkgs/pkgs/development/go-modules/generic/default.nix

Line 122 in 0509686

buildPhase = args.buildPhase or ''

[worldofpeace]

I do think that could be a plausible thing.

[danderson]

I just dived into the Go compiler to try and figure out what goes into build ID, and failed. There's um, a lot of code that goes into computing build ID, and it's intertwined with the build caching logic. I'm going to need an adult in the Go world to help me further (fortunately I might have access to one...).

[Profpatsch]

Are you afraid that some packages might break because of the binary metadata change? Sounds like a reasonably small chance to me.

Can code actually use / branch on that metadata?

[worldofpeace]

Are you afraid that some packages might break because of the binary metadata change? Sounds like a reasonably small chance to me.

Can code actually use / branch on that metadata?

I personally don't see a problem here, I thought this was just a curiosity investigation. This will all rebuild anyways because the deriver was changed.

[danderson]

Technically, Go code can read the build ID, and could branch on it. I can't think of any reasonable justification for doing that though.

This is mostly a curiosity investigation, yes. Unfortunately it's stalled because all my Go team experts are overloaded due to *waves generally at everything*, so I'm not going to pursue this further right now. I've made a note in my nix contrib todo that I want to investigate making buildGoModule more strictly deterministic, even in the face of changing build roots (which is the current suspect cause).

But if you're okay with the rebuild resulting in different build IDs being burned into the binaries, I'm good with merging this as-is.