Panic in gta for ImportPath "."

[kdvy]

Hi,

Thanks for creating this project!

Unfortunately with the recent changes to optimise gta processing speed, we've come across a panic whilst running gta on one of our repos. I'm afraid I'm not allowed to share the repo.

The error output is:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x606721]

goroutine 1 [running]:
github.com/digitalocean/gta.(*GTA).ChangedPackages(0xc0000c3310, 0x3, 0x4, 0xc0000c3310)
        /root/gta/gta.go:187 +0x201
main.main()
        /root/gta/cmd/gta/main.go:80 +0x6ef
r

I have tracked the issue down to:

• Change in commit: https://github.com/digitalocean/gta/commit/d5dac295d3625c15402e57fbd35f32fc0b0f7c2b
• Actual error is: no buildable Go source files in .
• The error is not trapped through any error detection and eventually panics in https://github.com/digitalocean/gta/blob/master/gta.go#L193 when a call to pkg.ImportPath is made but pkg is nil.
• Specifically:

  if _, ok := p.forward[importPath]; !ok {
      // TODO(bc): importPath is probably _not_ the right thing to use for Dir. So what _is_?
      return nil, &build.NoGoError{Dir: importPath}
  }

• Path . is not in the p.forward list

I'm afraid I'm not sure why there is . in the p.forward map.

Any help would be gratefully received!

[bhcleek]

@kdvy I also noticed this and have a fix locally that I'm testing now.

Can you tell me what the build tags and prefixes are that you're using when you encounter this? Also, which file paths have been modified on the branch that you're running gta against?

[bhcleek]

@kdvy can you give https://github.com/digitalocean/gta/pull/20 a try? I should either give you a useful error or resolve the problem for you depending on the flags you're setting.

[kdvy]

Hi, Thanks for responding so quickly. #20 has fixed the issue with the panic but there is still the underlying issue of no buildable Go source files in . which did not exist before #18 was merged.

I did some digging and it appears that inside *GTA.markedPackages() when the abs path contains a package that is not in the list of included packages in the cli arguments then it results in changed[.] = false on line 287 being called which adds an importPath of "." which is not found in the p.forward map.

I believe this happens for us because we have multiple go.mods in our monorepo and I run gta from each of them in turn.

The above issue of "no buildable Go source files in ." appeared after the #18 merge, so I think this is a bug ? but could it be that we're using the gta tool in some unintended way ?

[bhcleek]

As is, gta won't be able to work effectively in that situation in module aware mode, because it has to rely on golang.org/x/tools/go/packages, which in turn relies on go list, which is required to be run from within a module. There may be some way around this; I'll dig some more after #20 is merged, but I'll need some information from you to better understand the problem you're seeing:

1. What does go env report when run from within gta's working directory?
2. What is your GO111MODULE value?
3. Does the issue you're seeing occur in GOPATH mode, or only in module-aware mode?
4. Are you using module aware mode or gopath mode?
5. I understand that prior to #18 that you didn't see panics or errors. How confident are you about the correctness of the results you see with gta prior to #18?
6. Can you provide a way to duplicate this without exposing your private code? A simple repository would be helpful.

[bhcleek]

@kdvy I realized that there may be a way for you to work around this problem

There's a -changed-files option. If you were to run git diff --no-renames --name-only $BASE... where $BASE is the branch you are going to merge into, partition the results into files on the module boundaries, then you could run gta once for each module and files for each's module's changes.

[kdvy]

Thanks for looking into this, as it's end of day here in the UK - I'll try to spend some time on this on Monday. Really appreciate your help on this!

[kdvy]

Thanks @bhcleek , your suggestion of using changed-files works. Unfortunately this also means that gta doesn't quite meet our use case. Thanks again for the project though!

[bhcleek]

@kdvy Can you tell me more about your use case?

I'll try to spend some time to find a solution that doesn't require using -changed-files soon.

[kdvy]

Sure, it's a git repo monorepo where the go services each have their own go.mod but where shared libraries are stored separately with their own go.mod

E.g.

root/
  cmd/
    service-a/
      go.mod - imports pkg/library-a
    service-b/
      go.mod - imports pkg/library-a
  pkg/
     library-a/
       go.mod

My use case then is if library-a is changed by service-a then service-b is identified as depending on pkg/library-a as well and would be built. In our repo the shared libraries are versioned in go.mod as requiring v0.0.0 so hopefully always using the latest commit.

[bhcleek]

Yeah, gta expects that dependencies are vendored, even in module aware mode. I have plans to remove the requirement to use the vendor directory later this year.

However, in your case using v0.0.0 will work when you're on the default branch (e.g. main or master, depending on how your repository is configured), but on a branch you'll also need replace directives to use changes in pkg from within its dependents.

I'll leave this issue open, because the use case you present will be important to think about when we remove the requirement to use vendoring.

[kdvy]

Sorry, I should have said that we currently also use the replace directives in the services to point to the shared packages.

I'll keep an eye on this project in case you solve the vendoring soon!

Thanks again for the help.