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

golint package doesn't build #167717

Closed
jficz opened this issue Apr 7, 2022 · 7 comments · Fixed by #167912
Closed

golint package doesn't build #167717

jficz opened this issue Apr 7, 2022 · 7 comments · Fixed by #167912
Labels

Comments

@jficz
Copy link
Contributor

jficz commented Apr 7, 2022

Describe the bug

The golint package from nixos-unstable doesn't build

Steps To Reproduce

Steps to reproduce the behavior:

  1. nix-env -iA nixos.golint for instance
% nix-env -iA nixos.golint
installing 'lint-20201208-83fdc39'
this derivation will be built:
  /nix/store/dsz5l71bnidizizf1isqrs8xcnz4fcwv-lint-20201208-83fdc39.drv
building '/nix/store/dsz5l71bnidizizf1isqrs8xcnz4fcwv-lint-20201208-83fdc39.drv'...
unpacking sources
unpacking source archive /nix/store/3w2lkbbqj11364g84y72pgy6svcp7dzg-lint-83fdc39
source root is lint-83fdc39
patching sources
configuring
building
Building subPackage .
golang.org/x/tools/go/ast/astutil
golang.org/x/tools/go/internal/gcimporter
golang.org/x/tools/go/gcexportdata
golang.org/x/lint
Building subPackage ./golint
golang.org/x/lint/golint
running tests
=== RUN   TestAll
--- PASS: TestAll (0.14s)
=== RUN   TestLine
--- PASS: TestLine (0.00s)
=== RUN   TestLintName
--- PASS: TestLintName (0.00s)
=== RUN   TestExportedType
--- PASS: TestExportedType (0.00s)
=== RUN   TestIsGenerated
--- PASS: TestIsGenerated (0.00s)
PASS
ok      golang.org/x/lint       0.149s
found packages pkg (4.go) and foo (blank-import-lib.go) in /build/lint-83fdc39/testdata
testdata/errorf-custom.go:9:2: cannot find package "." in:
        /build/lint-83fdc39/vendor/github.com/pkg/errors
error: builder for '/nix/store/dsz5l71bnidizizf1isqrs8xcnz4fcwv-lint-20201208-83fdc39.drv' failed with exit code 1;
       last 10 log lines:
       > --- PASS: TestLintName (0.00s)
       > === RUN   TestExportedType
       > --- PASS: TestExportedType (0.00s)
       > === RUN   TestIsGenerated
       > --- PASS: TestIsGenerated (0.00s)
       > PASS
       > ok   golang.org/x/lint       0.149s
       > found packages pkg (4.go) and foo (blank-import-lib.go) in /build/lint-83fdc39/testdata
       > testdata/errorf-custom.go:9:2: cannot find package "." in:
       >    /build/lint-83fdc39/vendor/github.com/pkg/errors
       For full logs, run 'nix log /nix/store/dsz5l71bnidizizf1isqrs8xcnz4fcwv-lint-20201208-83fdc39.drv'.

Expected behavior

package builds

Additional context

Happens also when installed as a dependency of some other packages via Home Manager.

Notify maintainers

@jhillyerd
@tomberek

Metadata

 - system: `"x86_64-linux"`
 - host os: `Linux 5.17.1, NixOS, 22.05 (Quokka), 22.05pre367572.b6966d911da`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.7.0`
 - channels(miky): `"home-manager, nixos"`
 - channels(root): `"nixos, nixos-hardware"`
 - nixpkgs: `/nix/var/nix/profiles/per-user/root/channels/nixos`
@jficz jficz added the 0.kind: bug Something is broken label Apr 7, 2022
@NobbZ
Copy link
Contributor

NobbZ commented Apr 8, 2022

Perhaps you should just drop golint?

NOTE: Golint is deprecated and frozen. There's no drop-in replacement for it, but tools such as Staticcheck and go vet should be used instead.

Anyway, I started to bisect which commit introduced the issue.

@jficz
Copy link
Contributor Author

jficz commented Apr 8, 2022

I don't use it directly, it's been installed as a dependency of vim-go plugin and possibly others so it's not an immediate issue. It may be better to reach out to maintainers of the depending packages or upstream apps.

Is there a preferred way of deprecation notice or should I just reach out with an issue as I did here?

@NobbZ
Copy link
Contributor

NobbZ commented Apr 8, 2022

Bisection took a while, as it had to compile GCC and systemd from scratch a couple of times…

8b5a794:

8b5a7940b0cd1f6f232933099f12f383c3b3c32a is the first bad commit
commit 8b5a7940b0cd1f6f232933099f12f383c3b3c32a
Author: Manuel Mendez <mmendez534@gmail.com>
Date:   Fri Feb 25 17:06:28 2022 -0500

    go: Bunch of fixes when using excludedPackages and other bits
    
    Few things going on in this commit:
    
    Do not print "Building subPakage $pkg" message if actually going to skip the
    package. This was confusing to me when I was trying to figure out how to set
    excludedPackages and seeing the "Building subpackage $pkg" messages for
    packages I wanted to skip. Turns out this messages was being printed before
    checking if we actually wanted to build the package and not necessarily that my
    excludedPackages was wrong.
    
    Make go-packages look a little bit more like go-modules, by adding testdata to
    the default list of excluded packages.
    
    This commit also does some setup outside the buildGoDir function so that we
    avoid checking `excludedPackages` for every package and cut down the number
    of grep calls by half since we always want at least one grep for the default
    excludedPackages, might as well just add to the patterns being checked.
    
    Finally, adds documentation for usage of excludedPackages and subPackages. I
    had to read the implementation to figure out how to correctly use these
    function arguments since there was no documentation and different uses in the
    code base. So this commit documents usage of the arguments.

 doc/languages-frameworks/go.section.md           |  6 +++++-
 pkgs/development/go-modules/generic/default.nix  | 12 ++++++++++--
 pkgs/development/go-packages/generic/default.nix | 13 +++++++++++--
 3 files changed, 26 insertions(+), 5 deletions(-)
bisect found first bad commit

My assumption is, that it is related to the fact that the exclude mechanism has been changed here.

@mmlb any idea how to fix this?

@mmlb
Copy link
Contributor

mmlb commented Apr 8, 2022

I see what the issue is. buildGoPackage previously did not exclude testdata, while buildGoModule did. I added it to buildGoPackage as testdata as its ignored by the go tool too, from go help test:

The go tool will ignore a directory named "testdata", making it available to hold ancillary data needed by the tests.

I can revert that bit, but would like a moment to see if I can fix it better instead.

@mmlb
Copy link
Contributor

mmlb commented Apr 8, 2022

Nevermind ^, golint is built with buildGoModule. Still looking.

@mmlb
Copy link
Contributor

mmlb commented Apr 8, 2022

Ok figured it out, in that buggy commit I moved the exclusion logic out of getGoDirs and did the exclusion in the for loop, but I only applied it for building and not tests. Fix incoming.

mmlb added a commit to mmlb/NixOS-nixpkgs that referenced this issue Apr 8, 2022
The exclusion logic was moved out of getGoDirs but only buildPhase was updated
causing checkPhase to possibly fail. This happened in golint as it has go
files in testdata that are meant as testdata files and not go packages to
test which caused the checkPhase to fail.

Fixes NixOS#167717
@mmlb
Copy link
Contributor

mmlb commented Apr 8, 2022

#167912 has the fix. Verified that golint is fixed.

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

Successfully merging a pull request may close this issue.

4 participants