gomod2nix icon indicating copy to clipboard operation
gomod2nix copied to clipboard

fix: build with go 1.23 by tracking modules.txt

Open obreitwi opened this issue 1 year ago • 1 comments

Fixes: #117

Since go 1.23, vendor/modules.txt is required.

Hence, this is a first attempt to track modules.txt via gomod2nix.

Since, unfortunately, module.txt employs a bespoke custom format in which varying numbers of # hold meaning I deemed it more stable to not reverse engineer it.

Instead, we generate it when running gomod2nix by executing go mod vendor and tracking its content in gomod2nix.toml.

Not the most elegant solution, but it works and enables us to use gomod2nix with go v1.23.

Any comments appreached…

obreitwi avatar Sep 01 '24 21:09 obreitwi

I checked why the unit test for the cli-args fails and the situation is somewhat complicated.

  1. go build -mod=vendor is now checking that imports are in vendor/modules.txt.
  2. The import that is missing in the build ( github.com/yuin/goldmark ) is in the go.mod of golang.org/x/tools but not in the vendor/modules.txt we generate.
  3. It's not in our vendor/modules.txt because the test only builds a subpackage that doesn't use it. Go can detect this because we create our own temporary module and only reference the subpackage there. The resulting go.mod is
module gomod2nix/dummy/package

go 1.23.1

require golang.org/x/tools v0.1.11

require (
        golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 // indirect
        golang.org/x/sys v0.0.0-20211019181941-9d821ace8654 // indirect
)

and so the resulting vendor/modules.txt is

# golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4
## explicit; go 1.17
golang.org/x/mod/semver
# golang.org/x/sys v0.0.0-20211019181941-9d821ace8654
## explicit; go 1.17
golang.org/x/sys/execabs
# golang.org/x/tools v0.1.11
## explicit; go 1.17
golang.org/x/tools/cmd/stringer
golang.org/x/tools/go/gcexportdata
golang.org/x/tools/go/internal/gcimporter
golang.org/x/tools/go/internal/packagesdriver
golang.org/x/tools/go/packages
golang.org/x/tools/internal/event
golang.org/x/tools/internal/event/core
golang.org/x/tools/internal/event/keys
golang.org/x/tools/internal/event/label
golang.org/x/tools/internal/gocommand
golang.org/x/tools/internal/packagesinternal
golang.org/x/tools/internal/typeparams
golang.org/x/tools/internal/typesinternal

This is absolutely correct. If you run go build -mod=vendor golang.org/x/tools/cmd/stringer on that, it works.

  1. So why does go then later claim that it's missing? That's because we are not building our temporary go module but from the source of golang.org/x/tools which does include the dependency in its go.mod.

So, how to fix this? I think ultimately we would need to bring the environment that generates the gomod2nix.toml and the environment that consumes it closer together which is tedious :(

hannesg avatar Sep 22 '24 09:09 hannesg

I tested this branch, and it seems it broke the --dir flag. Whatever is put in the --dir flag will be repeated in front of the downloaded vendor-gomod2nix, so inside the dir, you will have $DIR/$DIR/vendor-gomod2nix. Then you get a panic when it tries to read the modules.txt file which tries to read in the correct location.

panic: error generating pkgs: open $DIR/vendor-gomod2nix/modules.txt: no such file or directory

terlar avatar Nov 11 '24 13:11 terlar

have been using this branch for a while, seems working fine 👍

yihuang avatar Dec 07 '24 08:12 yihuang

@obreitwi I think I fixed the nix build which should help the CI pass here if you merge my PR into your branch that this PR is based on.

  1. https://github.com/obreitwi/gomod2nix/pull/1

ghthor avatar Dec 07 '24 23:12 ghthor

For more readable diffs it would be nice if the generated vendor modules txt was using a multiline """ string instead of a single line string with \n.

terlar avatar Dec 08 '24 10:12 terlar

For more readable diffs it would be nice if the generated vendor modules txt was using a multiline """ string instead of a single line string with \n.

While I also prefer it, this would require a change of toml encoder since encoding multi-line strings is still an open issue.

obreitwi avatar Dec 09 '24 17:12 obreitwi

Oh, I throught it was already supported when looking at this comment: https://github.com/toml-lang/toml/issues/163#issuecomment-656007706

terlar avatar Dec 09 '24 20:12 terlar

For more readable diffs it would be nice if the generated vendor modules txt was using a multiline """ string instead of a single line string with \n.

While I also prefer it, this would require a change of toml encoder since encoding multi-line strings is still an open issue.

~~can we split it by \n and encode it as a list.~~ it's also encoded in the same line.

yihuang avatar Dec 10 '24 00:12 yihuang

it's also encoded in the same line.

Can confirm (didn't see your edit beforehand). I think in the end it comes down to switching to another toml encoder (or contributing the functionality upstream).

obreitwi avatar Dec 10 '24 22:12 obreitwi

There are a number of issues and PRs turning up related to this. Given the length of time the toml package has had a fix for the multi line pending in it's large rewrite branch, it seems like the toml package will not be released anytime soon. Switching to another toml package might be better done in another PR first, but would be easier if using at least a go version that is in stable nixpkgs, e.g. go_1_22.

What needs to be done to get this merged?

splack avatar Jan 01 '25 17:01 splack

What needs to be done to get this merged?

@splack I think we just need to fix the failing tests.

Looking at them, it's complaints about missing modules.txt, trying to figure out exactly how these tests are working and why that's a failure.

ghthor avatar Jan 06 '25 16:01 ghthor

I think the error is coming from this temporary module generation[1] that it utilized in the tests[2]. But not exactly sure as of yet what we need to change about that, but I'm looking into it this week.

  1. https://github.com/ghthor/gomod2nix/blob/fix/go_mod_vendor_go_1_23/internal/generate/temp.go
  2. https://github.com/ghthor/gomod2nix/blob/fix/go_mod_vendor_go_1_23/tests/cli-args/script#L4

ghthor avatar Jan 06 '25 17:01 ghthor

Hey, I was facing the same problem with https://github.com/katexochen/gobuild.nix. There is a patch on the Go toolchain in nixpkgs that introduced GO_NO_VENDOR_CHECKS env variable, which is used by gomod2nix, too. That patch wasn't updated when the additional checks in v1.23 were added. I've created a PR to also disable the import check, and I think it will fix your issue: https://github.com/NixOS/nixpkgs/pull/372367

katexochen avatar Jan 09 '25 13:01 katexochen

Great, that sounds like the way to go! :+1:

obreitwi avatar Jan 10 '25 13:01 obreitwi

Great, that sounds like the way to go! 👍

@obreitwi as I'm not used to gomod2nix, could you verify this solves the issue? Then I can merge the nixpkgs PR.

katexochen avatar Jan 10 '25 13:01 katexochen

@obreitwi as I'm not used to gomod2nix, could you verify this solves the issue? Then I can merge the nixpkgs PR.

Can confirm that applying https://github.com/NixOS/nixpkgs/pull/372367 solves our build issues with go 1.23 :+1:

obreitwi avatar Jan 11 '25 00:01 obreitwi

https://github.com/NixOS/nixpkgs/pull/372367 finally landed in nixpkgs-unstable, i.e. this PR is obsolete.

Feel free to reopen if still needed.

obreitwi avatar Feb 10 '25 16:02 obreitwi

I don't think this PR is obsolete, and that it should be re-opened. Otherwise anyone using non-nix golang - for whatever reason - is not going to be able to use gomod2nix to build their application. This includes the case of wanting to use official golang binaries.

maru-ava avatar Feb 20 '25 13:02 maru-ava

It seems sad to give up on this PR, because it looks like the closest anyone has gotten to upgrading gomod2nix.

I guess I don't understand why more people don't want newer go. go1.17 is very old now, and there have been a lot of important improvements. Do I miss understand the importance of upgrading?

randomizedcoder avatar Mar 23 '25 17:03 randomizedcoder

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

https://discourse.nixos.org/t/go-on-nix-upgrading-gomod2nix/62072/1

nixos-discourse avatar Mar 23 '25 18:03 nixos-discourse

It seems sad to give up on this PR, because it looks like the closest anyone has gotten to upgrading gomod2nix.

I guess I don't understand why more people don't want newer go. go1.17 is very old now, and there have been a lot of important improvements. Do I miss understand the importance of upgrading?

you can use latest golang in nix with GO_NO_VENDOR_CHECKS=1

yihuang avatar Mar 24 '25 00:03 yihuang

I switched templ away from gomod2nix in January because of this issue (https://github.com/a-h/templ/commit/2cc28840cf59927f7e3d0862014921c9e364b11d) but I decided to give it another shot today.

I had to add gomod2nix in to the inputs. To avoid unresolved issues https://github.com/nix-community/gomod2nix/issues/181 and https://github.com/nix-community/gomod2nix/issues/172, I had to tell gomod2nix to use a later version of nixpkgs. I'm using nixpkgs github:NixOS/nixpkgs/nixos-24.11 which has resolved to commit cdd2ef009676ac92b715ff26630164bb88fec4e0.

    gomod2nix = {
      url = "github:nix-community/gomod2nix/8f3534eb8f6c5c3fce799376dc3b91bae6b11884";
      inputs.nixpkgs.follows = "nixpkgs";
    };

Then, add gomod2nix as an argument to the function that creates outputs:

outputs = { self, nixpkgs, nixpkgs-unstable, gomod2nix, gitignore, version, xc }:

Next, I replaced my stdlib buildGo123Module command, with a call to gomod2nix's build command, removed the vendorHash argument, and replaced it with the gomod2nix modules argument, pointing at the gomod2nix.toml file created by running gomod2nix in the local directory.

-          templ = pkgs.buildGo123Module {
+          templ = gomod2nix.legacyPackages.${system}.buildGoApplication {
             name = "templ";
             subPackages = [ "cmd/templ" ];
             src = gitignore.lib.gitignoreSource ./.;
-            vendorHash = "sha256-JVOsjBn1LV8p6HHelfAO1Qcqi/tPg1S3xBffo+0aplE=";
             CGO_ENABLED = 0;
             flags = [
               "-trimpath"
               "-w"
               "-extldflags -static"
             ];
+            modules = ./gomod2nix.toml;
           };
         });

After this, the program built without any extra setting of environment variables or other workarounds.

I could verify it with:

nix build && go version -m ./result/bin/templ

The output clearly shows the use of go1.23.6, as expected:

./result/bin/templ: go1.23.6
        path    github.com/a-h/templ/cmd/templ
        mod     github.com/a-h/templ    (devel)
        build   -buildmode=exe
        build   -compiler=gc
        build   -trimpath=true
        build   CGO_ENABLED=0
        build   GOARCH=arm64
        build   GOOS=darwin
        build   GOARM64=v8.0

a-h avatar Mar 24 '25 08:03 a-h