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

cmd/go: newlines inconsistently preserved in go.mod rewriting #33779

Closed
zx2c4 opened this issue Aug 22, 2019 · 11 comments
Closed

cmd/go: newlines inconsistently preserved in go.mod rewriting #33779

zx2c4 opened this issue Aug 22, 2019 · 11 comments

Comments

@zx2c4
Copy link
Contributor

@zx2c4 zx2c4 commented Aug 22, 2019

I'm seeing this in 1.13beta1.

Phase 1: Make a go.mod file:

$ cat > go.mod <<_EOF
module golang.zx2c4.com/wireguard/windows

require (
        golang.zx2c4.com/wireguard master

        golang.org/x/crypto latest
        golang.org/x/net latest
        golang.org/x/sys latest
        golang.org/x/text latest

        github.com/lxn/walk latest
        github.com/lxn/win latest
)

replace (
        github.com/lxn/walk => golang.zx2c4.com/wireguard/windows pkg/walk
        github.com/lxn/win => golang.zx2c4.com/wireguard/windows pkg/walk-win
)

Phase 2: Resolve everything in it using go get:

$ GOPROXY=direct go get
go: finding golang.zx2c4.com/wireguard master
go: finding golang.org/x/crypto latest
go: finding golang.org/x/net latest
go: finding golang.org/x/sys latest
go: finding github.com/lxn/walk latest
go: finding github.com/lxn/win latest
go: finding golang.zx2c4.com/wireguard/windows pkg/walk
go: finding golang.zx2c4.com/wireguard/windows pkg/walk-win
$ cat go.mod
module golang.zx2c4.com/wireguard/windows

require (

        github.com/lxn/walk v0.0.0-20190619151032-86d8802c197a
        github.com/lxn/win v0.0.0-20190716185335-d1d36f0e4f48

        golang.org/x/crypto v0.0.0-20190820162420-60c769a6c586
        golang.org/x/net v0.0.0-20190813141303-74dc4d7220e7
        golang.org/x/sys v0.0.0-20190813064441-fde4db37ae7a
        golang.org/x/text v0.3.2
        golang.zx2c4.com/wireguard v0.0.20190806-0.20190822065259-3cedc22d7b49
)

replace (
        github.com/lxn/walk => golang.zx2c4.com/wireguard/windows v0.0.0-20190805140616-31a1b114e4f4
        github.com/lxn/win => golang.zx2c4.com/wireguard/windows v0.0.0-20190716185335-d1d36f0e4f48
)

go 1.13

Phase 3: Build it:

$ go build
...
$ cat go.mod
module golang.zx2c4.com/wireguard/windows

require (
        github.com/lxn/walk v0.0.0-20190619151032-86d8802c197a
        github.com/lxn/win v0.0.0-20190716185335-d1d36f0e4f48

        golang.org/x/crypto v0.0.0-20190820162420-60c769a6c586
        golang.org/x/net v0.0.0-20190813141303-74dc4d7220e7
        golang.org/x/sys v0.0.0-20190813064441-fde4db37ae7a
        golang.org/x/text v0.3.2
        golang.zx2c4.com/wireguard v0.0.20190806-0.20190822065259-3cedc22d7b49
)

replace (
        github.com/lxn/walk => golang.zx2c4.com/wireguard/windows v0.0.0-20190805140616-31a1b114e4f4
        github.com/lxn/win => golang.zx2c4.com/wireguard/windows v0.0.0-20190716185335-d1d36f0e4f48
)

go 1.13

Notice how between Phase 2 and Phase 3, a new line after require ( is added and then removed?

Phase 2:

require (

        github.com/lxn/walk v0.0.0-20190619151032-86d8802c197a

Phase 3:

require (
        github.com/lxn/walk v0.0.0-20190619151032-86d8802c197a
@bcmills

This comment has been minimized.

Copy link
Member

@bcmills bcmills commented Aug 22, 2019

Resolve everything in it using go get:

Note that go get is equivalent to go get .: it fetches and builds the package in the current directory, not everything in the go.mod file. (Still shouldn't be adding spurious newlines, though.)

@bcmills bcmills changed the title go.mod: newline added after `go get` but removed after `go build` cmd/go: newline added after `go get` but removed after `go build` Aug 22, 2019
@bcmills bcmills added this to the Go1.14 milestone Aug 22, 2019
@bcmills bcmills added the help wanted label Aug 22, 2019
@karuppiah7890

This comment has been minimized.

Copy link

@karuppiah7890 karuppiah7890 commented Aug 24, 2019

I'm willing to fix this issue. I'll check it out and ping here if I need help 😄

@karuppiah7890

This comment has been minimized.

Copy link

@karuppiah7890 karuppiah7890 commented Aug 25, 2019

@zx2c4 I'm not able to reproduce the issue. I get an error instead. I'm not using the beta version though, I'm using the latest master branch commit 66ff373

$ go version
go version devel +66ff373911 Sat Aug 24 01:11:56 2019 +0000 darwin/amd64

This is the go.mod file

$ cat go.mod
module golang.zx2c4.com/wireguard/windows

require (
        golang.zx2c4.com/wireguard master

        golang.org/x/crypto latest
        golang.org/x/net latest
        golang.org/x/sys latest
        golang.org/x/text latest

        github.com/lxn/walk latest
        github.com/lxn/win latest
)

replace (
        github.com/lxn/walk => golang.zx2c4.com/wireguard/windows pkg/walk
        github.com/lxn/win => golang.zx2c4.com/wireguard/windows pkg/walk-win
)

on executing:

$ go get -v
go: finding golang.zx2c4.com/wireguard master
go: finding golang.org/x/crypto latest
go: finding golang.org/x/net latest
go: finding golang.org/x/sys latest
go: finding github.com/lxn/walk latest
go: finding github.com/lxn/win latest
go: finding golang.zx2c4.com/wireguard/windows pkg/walk
go: finding golang.zx2c4.com/wireguard/windows pkg/walk-win
go: errors parsing go.mod:
/Users/karuppiahoss/oss/github.com/karuppiah7890/go-issues/33779/go.mod:16: replace golang.zx2c4.com/wireguard/wind
ows: version "pkg/walk" invalid: version "pkg/walk" invalid: disallowed version string
/Users/karuppiahoss/oss/github.com/karuppiah7890/go-issues/33779/go.mod:17: replace golang.zx2c4.com/wireguard/wind
ows: version "pkg/walk-win" invalid: version "pkg/walk-win" invalid: disallowed version string
@zx2c4

This comment has been minimized.

Copy link
Contributor Author

@zx2c4 zx2c4 commented Aug 25, 2019

export GOPROXY=direct
@karuppiah7890

This comment has been minimized.

Copy link

@karuppiah7890 karuppiah7890 commented Aug 26, 2019

Thanks @zx2c4 😄 I'm able to reproduce the issue!

@karuppiah7890

This comment has been minimized.

Copy link

@karuppiah7890 karuppiah7890 commented Sep 29, 2019

I haven't proceeded much with this issue. Me and my friend @aswinkarthik worked on this. We tried to debug it, but couldn't do much. If someone would help us on how to debug golang code using a debugger , it would be great. We use Intellij IDEA and VSCode too. We'll try more from our side too.

The one we noticed was, the new line comes due to the rearranging of the new line that was put by the user. We didn't know where to get rid of it and how it came as manually putting println and compiling and running didn't help much with debugging

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Oct 8, 2019

Change https://golang.org/cl/199917 mentions this issue: cmd/go/internal/modfile: do not print a newline for empty comment lines

@zx2c4

This comment has been minimized.

Copy link
Contributor Author

@zx2c4 zx2c4 commented Oct 8, 2019

Actually what's happening here is that the module list is being sorted, but the empty line above them in the original file is considered to be a "comment" line, which has an association with the non-comment line below it, and this association is preserved. It probably doesn't make sense to persist blank comment lines during sorting. I'll fix this up.

@zx2c4 zx2c4 changed the title cmd/go: newline added after `go get` but removed after `go build` cmd/go: newlines inconsistently preserved in go.mod rewriting Oct 8, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@gopherbot gopherbot closed this in 4c7a8d6 Oct 9, 2019
@zx2c4

This comment has been minimized.

Copy link
Contributor Author

@zx2c4 zx2c4 commented Oct 9, 2019

@gopherbot please backport this to 1.13, because it's a regression from the 1. 12, and it causes dirty files in version control and disturbs other automated systems that don't appreciate stable file contents flopping around.

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Oct 9, 2019

Backport issue(s) opened: #34800 (for 1.13).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Oct 9, 2019

Change https://golang.org/cl/200118 mentions this issue: cmd/go/internal/modfile: remove preceding empty lines when setting require

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.