cmd/go: improve handling of symlinked module root for `go mod (tidy)` #45609

thaJeztah · 2021-04-17T15:09:03Z

What version of Go are you using (`go version`)?

$ go version
go version go1.16.3 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (`go env`)?

go env Output

$ go env
(not relevant)

What did you do?

For easy navigation between projects, I have a "jump" directory on my machine,
in which I keep symlinks to frequently used projects. go mod tidy ignores
symlinks (and warns about ignoring them), which is generally ok, but in situations
where the whole module is detected to be in a "symlinked" path, it can be more
disruptive.

To reproduce this situation (you can run this inside a container to not clutter
up your host);

have a Go project on your machine

git clone https://github.com/moby/spdystream /go/src/github.com/moby/spdystream

Create a "projects" directory with a symlink to the project

mkdir -p /projects \
  && ln -s /go/src/github.com/moby/spdystream/ /projects/spdystream

Navigate to the project using the symlink
```
cd /projects/spdystream
```
Notice that pwd shows the path of the symlink
```
pwd
/projects/spdystream
```
(This is expected, and could be avoided by using cd -P instead of cd)

Run go mod tidy (usually, this would be after updating go.mod to update some dependencies)

go mod tidy
warning: ignoring symlink /projects/spdystream
go: warning: "all" matched no packages

Find out that all dependencies were removed from go.mod, and go.sum tobe emptied

git diff

diff --git a/go.mod b/go.mod
index d9b9ad5..ce733f8 100644
--- a/go.mod
+++ b/go.mod
@@ -1,5 +1,3 @@
 module github.com/moby/spdystream

 go 1.13
-
-require github.com/gorilla/websocket v1.4.2
diff --git a/go.sum b/go.sum
index 85efffd..e69de29 100644
--- a/go.sum
+++ b/go.sum
@@ -1,2 +0,0 @@
-github.com/gorilla/websocket v1.4.2 h1:+/TMaTYc4QFitKJxsQ7Yye35DkWvkdLcvGKqM+x0Ufc=
-github.com/gorilla/websocket v1.4.2/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=

While it's generally ok to ignore symlinks within the project/module (there must be reasons for that), it's somewhat disruptive in this situation; especially in situations where you just edited go.mod, and then see your changes being wiped.

What did you expect to see?

What I expected to see for step 5., is go mod (tidy) to either:

follow the symlink to /go/src/github.com/moby/spdystream/ and resolve modules from inside that context
or go mod tidy to produce an error instead of a warning, and terminate before modifying go.mod and go.sum

What did you see instead?

All dependencies removed from go.mod, and go.sum emptied

The text was updated successfully, but these errors were encountered:

toothrot · 2021-04-20T21:42:04Z

/cc @bcmills @jayconrod @matloob

It might be worth adding a friendlier error message to this scenario, and not emptying out the go.mod and go.sum files.

thaJeztah · 2021-04-20T22:06:25Z

Either option would be fine with me; out of curiosity (I tried to find the back story on "ignoring the symlinks"); what problems could occur if (in this specific scenario) the symlink would not be ignored?

Also happy to open a pull request if there's consensus on the approach.

bcmills · 2021-04-21T03:35:35Z

This is probably a fencepost error somewhere in the go command. We certainly shouldn't ignore the module root if the entire module is a symlink.

FWIW, one reason we ignore symlinks in general is because if you commit a change in a repository containing a symlink, that symlink can't be reliably reproduced in the module cache. (Not all filesystems support symlinks, and not all symlinks are repo-relative.)

thaJeztah · 2021-04-21T11:40:42Z

FWIW, one reason we ignore symlinks in general is because if you commit a change in a repository containing a symlink, that symlink can't be reliably reproduced in the module cache. (Not all filesystems support symlinks, and not all symlinks are repo-relative.)

Ah, yes, that makes sense (symlinks can be a bit of a PITA)

So from the above;

"easy" / "quick fix" is to: "if isSymlink(path) && isModuleRoot(path)" then error and abort (before modifying)
"nicer" fix (but needs to be checked for possible side-effects; "if isSymlink(path) && isModuleRoot(path)" then "proceed as usual

bcmills · 2021-04-21T13:21:30Z

Exactly. I think we should go for the “nicer” fix — unlike for nested packages, I don't see a compelling reason to disallow symlinks at the module root.

thaJeztah · 2021-04-21T14:15:44Z

Yup; nicer fixes are always good.

Having a quick look at the link I included in my report; warning is coming from here;

go/src/cmd/go/internal/modload/search.go

Lines 88 to 95 in 954879d

    
           if !fi.IsDir() { 
        
           	if fi.Mode()&fs.ModeSymlink != 0 && want { 
        
           		if target, err := fsys.Stat(path); err == nil && target.IsDir() { 
        
           			fmt.Fprintf(os.Stderr, "warning: ignoring symlink %s\n", path) 
        
           		} 
        
           	} 
        
           	return nil 
        
           }

I see a ModRoot() function used;

go/src/cmd/go/internal/modload/search.go

Line 135 in 954879d

walkPkgs(ModRoot(), targetPrefix, pruneGoMod|pruneVendor)

Which (20 second glance) looks to be the path to the module root;

go/src/cmd/go/internal/modload/init.go

Lines 299 to 301 in 4d56576

    
           // ModRoot returns the root of the main module. 
        
           // It calls base.Fatalf if there is no main module. 
        
           func ModRoot() string {

Looks like it's using a variable to keep that, so repeatedly using it inside the walkFn should (at a glance) likely be ok.

thockin · 2021-10-24T01:02:36Z

Would love to see this fixed - I just wasted half an hour on it :(

ebauman · 2023-03-05T19:25:34Z

Just ran into this today when trying to do exactly what issue OP is doing - a "workspaces" directory with multiple symlinked projects I'm integrating.

Would love to see this fixed too :)

bcmills · 2023-03-06T18:06:53Z

@ebauman, it is possible that this is fixed by https://go.dev/cl/463179. Could you try using a cmd/go built at head (or using golang.org/dl/gotip) and see if it still reproduces?

JetSetIlly · 2023-03-07T08:46:22Z

I would love to see this fixed too. I knew about the problem but because I encounter it so infrequently I spent a good 10 minutes this morning refreshing my memory. If nothing else I believe that a better error message is warranted.

JetSetIlly · 2023-03-07T18:58:38Z

@ebauman, it is possible that this is fixed by https://go.dev/cl/463179. Could you try using a cmd/go built at head (or using golang.org/dl/gotip) and see if it still reproduces?

I've just tested and this fix does appear to work.

bcmills · 2023-03-08T13:14:46Z

Thanks! I'm going to mark this as tentatively fixed for Go 1.21, but please do let me know if you run into this failure mode at Go 1.21 or higher.

thaJeztah · 2023-03-08T15:49:22Z

Thanks all! Sorry, been too busy to catch up on all of these (or to open a PR), but understanding from the above, it's fixed (happy!)

toothrot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 20, 2021

toothrot added this to the Backlog milestone Apr 20, 2021

zrhoffman mentioned this issue Aug 17, 2021

Update Go version to 1.17 apache/trafficcontrol#6111

Merged

4 tasks

bcmills added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Mar 6, 2023

seankhliao mentioned this issue Mar 7, 2023

"go mod tidy" does not work with UNIX soft links #58911

Closed

bcmills closed this as completed Mar 8, 2023

bcmills added GoCommand cmd/go and removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Mar 8, 2023

bcmills modified the milestones: Backlog, Go1.21 Mar 8, 2023

bcmills self-assigned this Mar 8, 2023

golang locked and limited conversation to collaborators Mar 7, 2024

gopherbot added the FrozenDueToAge label Mar 7, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd/go: improve handling of symlinked module root for `go mod (tidy)` #45609

cmd/go: improve handling of symlinked module root for `go mod (tidy)` #45609

thaJeztah commented Apr 17, 2021 •

edited

toothrot commented Apr 20, 2021

thaJeztah commented Apr 20, 2021

bcmills commented Apr 21, 2021

thaJeztah commented Apr 21, 2021

bcmills commented Apr 21, 2021

thaJeztah commented Apr 21, 2021

thockin commented Oct 24, 2021

ebauman commented Mar 5, 2023

bcmills commented Mar 6, 2023

JetSetIlly commented Mar 7, 2023

JetSetIlly commented Mar 7, 2023

bcmills commented Mar 8, 2023

thaJeztah commented Mar 8, 2023

cmd/go: improve handling of symlinked module root for go mod (tidy) #45609

cmd/go: improve handling of symlinked module root for go mod (tidy) #45609

Comments

thaJeztah commented Apr 17, 2021 • edited

What version of Go are you using (go version)?

Does this issue reproduce with the latest release?

What operating system and processor architecture are you using (go env)?

What did you do?

What did you expect to see?

What did you see instead?

toothrot commented Apr 20, 2021

thaJeztah commented Apr 20, 2021

bcmills commented Apr 21, 2021

thaJeztah commented Apr 21, 2021

bcmills commented Apr 21, 2021

thaJeztah commented Apr 21, 2021

thockin commented Oct 24, 2021

ebauman commented Mar 5, 2023

bcmills commented Mar 6, 2023

JetSetIlly commented Mar 7, 2023

JetSetIlly commented Mar 7, 2023

bcmills commented Mar 8, 2023

thaJeztah commented Mar 8, 2023

cmd/go: improve handling of symlinked module root for `go mod (tidy)` #45609

cmd/go: improve handling of symlinked module root for `go mod (tidy)` #45609

thaJeztah commented Apr 17, 2021 •

edited

What version of Go are you using (`go version`)?

What operating system and processor architecture are you using (`go env`)?