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

Support for go modules (automatically infer module path)? #30

Open
dmke opened this issue Jan 25, 2021 · 7 comments
Open

Support for go modules (automatically infer module path)? #30

dmke opened this issue Jan 25, 2021 · 7 comments
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@dmke
Copy link

dmke commented Jan 25, 2021

Hi,

at $company, we have a bunch of Go projects named gitlab.$company.com/$project.

All projects use Go modules, i.e. there's a go.mod containing module gitlab.$company.com/$project.

$ head -1 go.mod
module gitlab.$company.com/$project

However, gci doesn't seem to make use of this information, compare the output of the following gci runs (with and without -local flag):

$ gci -d -local gitlab.$company.com/$project queue/client.go
[no output]
$ gci -d queue/client.go
--- queue/client.go.orig        2021-01-25 11:25:02.582529319 +0100
+++ queue/client.go     2021-01-25 11:25:02.582529319 +0100
@@ -11,7 +11,6 @@
 
        ws "nhooyr.io/websocket"
        "nhooyr.io/websocket/wsjson"
-
        "gitlab.$company.com/$project/turn"
 )

The client.go contains this import block:

import (
	"context"
	"encoding/json"
	"errors"
	"fmt"
	"log"
	"sync"
	"time"

	ws "nhooyr.io/websocket"
	"nhooyr.io/websocket/wsjson"

	"gitlab.$company.com/$project/turn"
)

Would this be a valid feature request? I'd like gci to lookup and use the module path when invoked without -local.

@daixiang0
Copy link
Owner

It is why -local exists, automatically check to infer module path works for some cases, while if repos use custom domain rather than Github, it would do harm.

@dmke
Copy link
Author

dmke commented Jan 26, 2021

automatically check to infer module path works for some cases

Hmm... Looking at the code (ProcessFileprocessFilenewPkggetPkgType in gci.go), I don't see how that would work. Only if I provide a -local flag, getPkgType will return local:

gci/pkg/gci/gci.go

Lines 159 to 171 in 7eb50f3

func getPkgType(line, localFlag string) int {
pkgName := strings.Trim(line, "\"\\`")
if localFlag != "" && strings.HasPrefix(pkgName, localFlag) {
return local
}
if isStandardPackage(pkgName) {
return standard
}
return remote
}

For example, with this directory structure:

./
+-- go.mod               (module foo)
+-- internal
|   +-- service
|       +-- service.go   (package service)
+-- main.go              (package main, imports foo/internal/service)

there should be no difference between gci -local foo ./ and gci ./. Do you know what I mean?

For this to work, gci would need to find a go.mod, extract the module path from its module directive, and wire the module path down to getPkgType. I'd be OK if this only works if -local is not present on the command line.

I'll try to come up with a better example/PR later when I'm back home.

dmke added a commit to dmke/gci that referenced this issue Jan 26, 2021
This is just a proof of concept for daixiang0#30.
dmke added a commit to dmke/gci that referenced this issue Jan 26, 2021
This is just a proof of concept for daixiang0#30.

Signed-off-by: Dominik Menke <dom@digineo.de>
@dmke
Copy link
Author

dmke commented Jan 26, 2021

I have started an example in #31. Some notes:

  • The implementation of type moduleResolver might be incomplete, I haven't written any tests (yet, I'll add some tomorrow)
    • I'm not sure if this works properly on Windows
    • it also doesn't deal with symlinks
  • The important bit is the integration in gci.processFile, where modCache.Lookup happens only if set.LocalFlag is empty.
  • ClearModCache is virtually superfluous in main.go, but I've kept it there for completeness.

dmke added a commit to dmke/gci that referenced this issue Jan 26, 2021
This is just a proof of concept for daixiang0#30.

Signed-off-by: Dominik Menke <dom@digineo.de>
dmke added a commit to dmke/gci that referenced this issue Jan 26, 2021
This is just a proof of concept for daixiang0#30.

Signed-off-by: Dominik Menke <dom@digineo.de>
@invidian
Copy link

I also think this is confusing that gci does not find local module name automatically.

@daixiang0 daixiang0 added enhancement New feature or request help wanted Extra attention is needed good first issue Good for newcomers labels May 30, 2022
dmke added a commit to dmke/gci that referenced this issue May 30, 2022
This started as a proof-of-concept for daixiang0#30.

Signed-off-by: Dominik Menke <dom@digineo.de>
dmke added a commit to dmke/gci that referenced this issue May 30, 2022
This started as a proof-of-concept for daixiang0#30.

Signed-off-by: Dominik Menke <dom@digineo.de>
@ymohl-cl
Copy link

Hello, i have the same need :)

This issue still in progress ? Or always planned ?

@dmke
Copy link
Author

dmke commented Jul 10, 2023

I find myself hard-pressed to continue the work started in #31, feel free to take over :)

@palsivertsen
Copy link

There are some packages that might help resolving the module name:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

5 participants