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.

Sign up for GitHub

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

Jump to bottom

cmd/go: Using go modules in a child cmd directory with local path in replace directive needs valid version #38972

Closed
montanaflynn opened this issue May 9, 2020 · 7 comments
Closed

cmd/go: Using go modules in a child cmd directory with local path in replace directive needs valid version #38972

montanaflynn opened this issue May 9, 2020 · 7 comments
Labels
FrozenDueToAge modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone
Backlog

Comments

@montanaflynn
Copy link

montanaflynn commented May 9, 2020 •
edited
Loading

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

$ go version
go version go1.14.2 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
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/montanaflynn/Library/Caches/go-build"
GOENV="/Users/montanaflynn/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GONOPROXY="github.com/mathpix/api/util"
GONOSUMDB="github.com/mathpix/api/util"
GOOS="darwin"
GOPATH="/Users/montanaflynn/Development/go"
GOPRIVATE="github.com/mathpix/api/util"
GOPROXY="direct"
GOROOT="/usr/local/Cellar/go/1.14.2_1/libexec"
GOSUMDB="off"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.14.2_1/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/montanaflynn/Development/go/src/github.com/montanaflynn/notify-cli/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/j1/q0f3l2dj5lj89gr14gzfnw080000gn/T/go-build774174740=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Created a new project with a ./cmd/notify/ directory to allow users to use as a library or install a cli program easily using go get github.com/montanaflynn/notify-cli/cmd/notify.

The ./cmd/notify/go.mod is set to use the local package path using replace directive along with require set to version v0.0.0 as mentioned in the wiki replace section:

In Go 1.11, for direct dependencies a require directive is needed even when doing a replace. For example, if foo is a direct dependency, you cannot do replace foo => ../foo without a corresponding require for foo. If you are not sure what version to use in the require directive, you can often use v0.0.0 such as require foo v0.0.0. This was addressed in Go 1.12 with #26241.

require github.com/montanaflynn/notify-cli v0.0.0
replace github.com/montanaflynn/notify-cli => ../../

What did you expect to see?

I expected go get github.com/montanaflynn/notify-cli/cmd/notify to work.

What did you see instead?

go: downloading github.com/montanaflynn/notify-cli/cmd/notify v0.0.0-20200508151944-ea02fb5189fc
go: github.com/montanaflynn/notify-cli/cmd/notify upgrade => v0.0.0-20200508151944-ea02fb5189fc
go get: github.com/montanaflynn/notify-cli/cmd/notify@v0.0.0-20200508151944-ea02fb5189fc requires
	github.com/montanaflynn/notify-cli@v0.0.0: reading github.com/montanaflynn/notify-cli/go.mod at revision v0.0.0: unknown revision v0.0.0

If I change the require directive to a valid version it works:

require github.com/montanaflynn/notify-cli v0.0.6

Even though it's said to require v0.0.6 it does successfully use the local path to install the latest version. I've had a lot of confusion surrounding the migration to go modules and I think this is a common pattern a lot of Go developers are using so I think a best practice example in the wiki would be helpful. Also I wonder why the require is needed at all or if there's a better way to signal that the it's a local dependency.
@seankhliao
Copy link
Member

separate modules are independent, there is no guarantee the parent directories exist

that said the wiki also recommends against multi module repositories for most use cases

@toothrot toothrot changed the title Using go modules in a child cmd directory with local path in replace directive needs valid version cmd/go: Using go modules in a child cmd directory with local path in replace directive needs valid version May 11, 2020
@toothrot toothrot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 11, 2020
@toothrot toothrot added this to the Backlog milestone May 11, 2020
@toothrot
Copy link
Contributor

I'm not certain there is a bug here. If you want the module at github.com/montanaflynn/notify-cli/cmd/notify to always be the same version as github.com/montanaflynn/notify-cli, it may be better not to have a second module at cmd/notify.

/cc @bcmills, who may be able to explain this better.
See also #27542 and #27056

@bcmills
Copy link
Contributor

bcmills commented May 11, 2020

I agree with @seankhliao and @toothrot's assessment. For your use-case, it is likely preferable to keep the command within the same module as the library.

If you want to maintain a separate module and allow it to be fetched with go get, then you need to maintain accurate version requirements for that module. Separate modules are stored separately within the module cache, so without a version requirement the go command has no way to determine which version of github.com/montanaflynn/notify-cli actually corresponds to ../../.

@bcmills bcmills added modules WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels May 11, 2020
@bcmills
Copy link
Contributor

bcmills commented May 11, 2020

@montanaflynn, could you explain why you wanted to split cmd/notify into a separate module to begin with? As far as I can tell, the best we can do here is likely to better document the reasons not to do that...

@jakesylvestre
Copy link

@bcmills one use case is dependency management. We have something of a standard library in go internally that has a ton of depdencies and it's a bit of a pain to have to install them all for every repo in our ci workflow. We would like to version them in lcok step though for sanity's sake

@montanaflynn
Copy link
Author

@seankhliao @toothrot @bcmills thanks, I removed the second cmd module and my problem went away. I was under the impression that in order to use the library I needed to create a module with it as a dependency.

@bcmills
Copy link
Contributor

bcmills commented May 12, 2020

Thanks for following up. Since you have a viable alternative and the go command seems to be working as designed, I'm going to close out this issue, but we'll continue to keep an eye out for related usability and documentation issues.

@bcmills bcmills closed this as completed May 12, 2020
@golang golang locked and limited conversation to collaborators May 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
6 participants
@toothrot @montanaflynn @bcmills @jakesylvestre @gopherbot @seankhliao