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: mod init outside of GOPATH silently fails #31255

Closed
wants to merge 5 commits into from

Conversation

ikkerens
Copy link
Contributor

@ikkerens ikkerens commented Apr 4, 2019

Running go mod init outside of GOPATH with GO111MODULE=off
silently fails. This behavior was undocumented.

This CL makes go mod fail with the error:

go: modules disabled by GO111MODULE=off; see 'go help modules'
Comparing with already erroring GO111MODULE= conditions:

  • With GO111MODULE=auto, inside GOPATH:
    go modules disabled inside GOPATH/src by GO111MODULE=auto; see 'go help modules'
  • With GO111MODULE=auto outside of GOPATH:
    go: cannot determine module path for source directory /path/to/dir (outside GOPATH, no import comments)

Fixes #31342

@googlebot googlebot added the cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change. label Apr 4, 2019
@gopherbot
Copy link

This PR (HEAD: 47a35bd) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/170697 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link

Message from Gobot Gobot:

Patch Set 1:

Congratulations on opening your first change. Thank you for your contribution!

Next steps:
Within the next week or so, a maintainer will review your change and provide
feedback. See https://golang.org/doc/contribute.html#review for more info and
tips to get your patch through code review.

Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.

During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11, it means that this CL will be reviewed as part of the next development
cycle. See https://golang.org/s/release for more details.


Please don’t reply on this GitHub thread. Visit golang.org/cl/170697.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Emmanuel Odeke:

Patch Set 1:

(1 comment)

Thank you for this change Rens and welcome to the Go project!

Great find! I have made suggestions for the commit message
but also here are 2 more to perhaps enhance your change:

  1. Please file a bug on the Go issue tracker https://github.com/golang/go/issues,
    for posterity purposes but also for a discussion and perhaps get context as to
    why it might be like this.

The issue could perhaps be:

cmd/go: mod init outside of GOPATH silently fails

as the title and the content to explain what you explained in the previous commit message
such as:

Running go mod init outside of GOPATH silently fails and this behavior isn't
documented. However, these scenarios fail with an error message when go mod:

  • With GO111MODULE=auto, inside GOPATH:
    go modules disabled inside GOPATH/src by GO111MODULE=auto; see 'go help modules'

  • With GO111MODULE=auto outside of GOPATH:
    go: cannot determine module path for source directory /path/to/dir (outside GOPATH, no import comments)

and once we have the issue number, in the commit message update at the bottom, where
you see:
Fixes #NNN
please replace NNN with the issue number from Github.

  1. Let's add this test in cmd/go/testdata/script/mod_off_init.txt:

    env GO111MODULE=off

    This script tests that running go mod init with

    GO111MODULE=off when outside of GOPATH will fatal

    with an error message.

    ! go mod init
    stderr 'go: modules disabled by GO111MODULE=off; see ''go help modules'''

and that should ensure that we never regress.

Thank you and I look forward to reviewing your updates!


Please don’t reply on this GitHub thread. Visit golang.org/cl/170697.
After addressing review feedback, remember to publish your drafts!

@ikkerens ikkerens changed the title cmd/go/internal/modload: add error message for go mod init when disabled cmd/go: mod init outside of GOPATH silently fails Apr 8, 2019
@gopherbot
Copy link

This PR (HEAD: 226e846) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/170697 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link

Message from Rens Rikkerink:

Patch Set 6:

(1 comment)

Patch Set 1:

(1 comment)

Thank you for this change Rens and welcome to the Go project!

Great find! I have made suggestions for the commit message
but also here are 2 more to perhaps enhance your change:

  1. Please file a bug on the Go issue tracker https://github.com/golang/go/issues,
    for posterity purposes but also for a discussion and perhaps get context as to
    why it might be like this.

The issue could perhaps be:

cmd/go: mod init outside of GOPATH silently fails

as the title and the content to explain what you explained in the previous commit message
such as:

Running go mod init outside of GOPATH silently fails and this behavior isn't
documented. However, these scenarios fail with an error message when go mod:

  • With GO111MODULE=auto, inside GOPATH:
    go modules disabled inside GOPATH/src by GO111MODULE=auto; see 'go help modules'

  • With GO111MODULE=auto outside of GOPATH:
    go: cannot determine module path for source directory /path/to/dir (outside GOPATH, no import comments)

and once we have the issue number, in the commit message update at the bottom, where
you see:
Fixes #NNN
please replace NNN with the issue number from Github.

  1. Let's add this test in cmd/go/testdata/script/mod_off_init.txt:

    env GO111MODULE=off

    This script tests that running go mod init with

    GO111MODULE=off when outside of GOPATH will fatal

    with an error message.

    ! go mod init
    stderr 'go: modules disabled by GO111MODULE=off; see ''go help modules'''

and that should ensure that we never regress.

Thank you and I look forward to reviewing your updates!

I've applied the requested changes, with an additional replacement on the actual implementation of the fix, as it appears there was already a helper method to achieve this, it was just not invoked in this specific case.


Please don’t reply on this GitHub thread. Visit golang.org/cl/170697.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Emmanuel Odeke:

Patch Set 6: Run-TryBot+1 Code-Review+2

LGTM, thank you Rens! I've blessed this CL but I'll wait for say 2 days for Bryan, Russ and Ian to chime in otherwise the fix looks correct to me.


Please don’t reply on this GitHub thread. Visit golang.org/cl/170697.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Gobot Gobot:

Patch Set 6:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=b581577d


Please don’t reply on this GitHub thread. Visit golang.org/cl/170697.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Gobot Gobot:

Patch Set 6:

Build is still in progress...
This change failed on freebsd-amd64-12_0:
See https://storage.googleapis.com/go-build-log/b581577d/freebsd-amd64-12_0_848bbbd0.log

Other builds still in progress; subsequent failure notices suppressed until final report. Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.


Please don’t reply on this GitHub thread. Visit golang.org/cl/170697.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Gobot Gobot:

Patch Set 6: TryBot-Result-1

9 of 19 TryBots failed:
Failed on freebsd-amd64-12_0: https://storage.googleapis.com/go-build-log/b581577d/freebsd-amd64-12_0_848bbbd0.log
Failed on openbsd-amd64-64: https://storage.googleapis.com/go-build-log/b581577d/openbsd-amd64-64_9b94014c.log
Failed on linux-amd64: https://storage.googleapis.com/go-build-log/b581577d/linux-amd64_0e8947eb.log
Failed on linux-386: https://storage.googleapis.com/go-build-log/b581577d/linux-386_1b329302.log
Failed on js-wasm: https://storage.googleapis.com/go-build-log/b581577d/js-wasm_28cd2e99.log
Failed on windows-amd64-2016: https://storage.googleapis.com/go-build-log/b581577d/windows-amd64-2016_14a7468e.log
Failed on nacl-amd64p32: https://storage.googleapis.com/go-build-log/b581577d/nacl-amd64p32_81f8ab13.log
Failed on nacl-386: https://storage.googleapis.com/go-build-log/b581577d/nacl-386_f2c63db1.log
Failed on windows-386-2008: https://storage.googleapis.com/go-build-log/b581577d/windows-386-2008_e4cb2fb0.log

Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.


Please don’t reply on this GitHub thread. Visit golang.org/cl/170697.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Bryan C. Mills:

Patch Set 6: Code-Review-2

The test failures here are real. Both go get and go help invoke modload.Init() even if module mode is disabled.

A fix specific to go mod init probably belongs in cmd/go/internal/modcmd.


Please don’t reply on this GitHub thread. Visit golang.org/cl/170697.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

This PR (HEAD: ada1aba) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/170697 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link

Message from Rens Rikkerink:

Patch Set 6:

I've moved the check from modinit to modcmd, even though it feels a bit off to me as most of the env-var checks happen in the modinit package, and the die function has not been exported.

Is hardcopying the error message from the modinit package a desirable method of applying a fix like this?


Please don’t reply on this GitHub thread. Visit golang.org/cl/170697.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Emmanuel Odeke:

Patch Set 7: -Code-Review

Oh I see, I shall rescind my +2 and defer to Bryan and other's reviews. Sorry for the noise.


Please don’t reply on this GitHub thread. Visit golang.org/cl/170697.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Emmanuel Odeke:

Patch Set 7: Run-TryBot+1


Please don’t reply on this GitHub thread. Visit golang.org/cl/170697.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Gobot Gobot:

Patch Set 7:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=cfee6747


Please don’t reply on this GitHub thread. Visit golang.org/cl/170697.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Gobot Gobot:

Patch Set 7:

Build is still in progress...
This change failed on freebsd-amd64-12_0:
See https://storage.googleapis.com/go-build-log/cfee6747/freebsd-amd64-12_0_02ab16b7.log

Other builds still in progress; subsequent failure notices suppressed until final report. Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.


Please don’t reply on this GitHub thread. Visit golang.org/cl/170697.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Gobot Gobot:

Patch Set 7: TryBot-Result-1

6 of 19 TryBots failed:
Failed on freebsd-amd64-12_0: https://storage.googleapis.com/go-build-log/cfee6747/freebsd-amd64-12_0_02ab16b7.log
Failed on linux-386: https://storage.googleapis.com/go-build-log/cfee6747/linux-386_4f795ad3.log
Failed on openbsd-amd64-64: https://storage.googleapis.com/go-build-log/cfee6747/openbsd-amd64-64_823e0638.log
Failed on linux-amd64: https://storage.googleapis.com/go-build-log/cfee6747/linux-amd64_e940300b.log
Failed on windows-amd64-2016: https://storage.googleapis.com/go-build-log/cfee6747/windows-amd64-2016_f149b777.log
Failed on windows-386-2008: https://storage.googleapis.com/go-build-log/cfee6747/windows-386-2008_c3e6be1b.log

Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.


Please don’t reply on this GitHub thread. Visit golang.org/cl/170697.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

This PR (HEAD: b384474) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/170697 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link

Message from Rens Rikkerink:

Patch Set 7:

Apologies for this latest attempt, hadn't updated the test to reflect the change according with modcmd yet.


Please don’t reply on this GitHub thread. Visit golang.org/cl/170697.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Rens Rikkerink:

Patch Set 8:

Could anybody do me a favour and run trybot? I can't do it myself ^^'


Please don’t reply on this GitHub thread. Visit golang.org/cl/170697.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Emmanuel Odeke:

Patch Set 8: Run-TryBot+1

Patch Set 8:

Could anybody do me a favour and run trybot? I can't do it myself ^^'

Done


Please don’t reply on this GitHub thread. Visit golang.org/cl/170697.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Gobot Gobot:

Patch Set 8:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=8cb49943


Please don’t reply on this GitHub thread. Visit golang.org/cl/170697.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Gobot Gobot:

Patch Set 8: TryBot-Result+1

TryBots are happy.


Please don’t reply on this GitHub thread. Visit golang.org/cl/170697.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Bryan C. Mills:

Patch Set 9: Patch Set 8 was rebased


Please don’t reply on this GitHub thread. Visit golang.org/cl/170697.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Bryan C. Mills:

Patch Set 9: Run-TryBot+1 Code-Review+2


Please don’t reply on this GitHub thread. Visit golang.org/cl/170697.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Gobot Gobot:

Patch Set 9:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=7d51264b


Please don’t reply on this GitHub thread. Visit golang.org/cl/170697.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Gobot Gobot:

Patch Set 9: TryBot-Result+1

TryBots are happy.


Please don’t reply on this GitHub thread. Visit golang.org/cl/170697.
After addressing review feedback, remember to publish your drafts!

gopherbot pushed a commit that referenced this pull request Apr 18, 2019
Running `go mod init` outside of GOPATH with `GO111MODULE=off`
silently fails. This behavior was undocumented.

This CL makes go mod fail with the error:

   go: modules disabled by GO111MODULE=off; see 'go help modules'
Comparing with already erroring GO111MODULE=<value> conditions:

* With GO111MODULE=auto, inside GOPATH:
    go modules disabled inside GOPATH/src by GO111MODULE=auto; see 'go help modules'
* With GO111MODULE=auto outside of GOPATH:
    go: cannot determine module path for source directory /path/to/dir (outside GOPATH, no import comments)

Fixes #31342

Change-Id: I749787d2a8640913c4ac263072d051314d76e778
GitHub-Last-Rev: b384474
GitHub-Pull-Request: #31255
Reviewed-on: https://go-review.googlesource.com/c/go/+/170697
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@gopherbot
Copy link

This PR is being closed because golang.org/cl/170697 has been merged.

@gopherbot gopherbot closed this Apr 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cmd/go: mod init outside of GOPATH silently fails
3 participants