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: reject 'go generate' on packages in unreplaced module dependencies? #29751

Open
myitcv opened this issue Jan 15, 2019 · 5 comments
Open

cmd/go: reject 'go generate' on packages in unreplaced module dependencies? #29751

myitcv opened this issue Jan 15, 2019 · 5 comments
Labels
GoCommand cmd/go help wanted modules NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Backlog

Comments

@myitcv
Copy link
Member

myitcv commented Jan 15, 2019

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

$ go version
go version go1.12beta2 linux/amd64

Does this issue reproduce with the latest release?

Testing with the beta pre the 1.12 release.

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

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/myitcv/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/myitcv/gostuff"
GOPROXY=""
GORACE=""
GOROOT="/home/myitcv/gos"
GOTMPDIR=""
GOTOOLDIR="/home/myitcv/gos/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build968143131=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Ran testscript on the following:

go generate fruit.com/fruit

-- go.mod --
module mod

require fruit.com v1.0.0

-- main.go --
package main

func main() {
}

-- .gomodproxy/fruit.com_v1.0.0/.mod --
module fruit.com

-- .gomodproxy/fruit.com_v1.0.0/.info --
{"Version":"v1.0.0","Time":"2018-10-22T18:45:39Z"}

-- .gomodproxy/fruit.com_v1.0.0/go.mod --
module fruit.com

-- .gomodproxy/fruit.com_v1.0.0/fruit/fruit.go --
package fruit

//go:generate bash -c "echo 'package fruit' > something.go"

What did you expect to see?

A failed run.

What did you see instead?

> go generate fruit.com/fruit
[stderr]
go: finding fruit.com v1.0.0
go: downloading fruit.com v1.0.0
go: extracting fruit.com v1.0.0
go: not generating in packages in dependency modules

PASS

It feels like this should be an error instead of a noisy success. Reason being, I've specified the pattern of a non-main module package(s) on the command line; based on the current implementation, that will never succeed (even if there is a replace directive).

cc @bcmills.
@bcmills
Copy link
Contributor

bcmills commented Apr 12, 2019

CC @jayconrod

Personally, I'd prefer that we allow go generate to refer to other modules with filesystem replace directives: that way, for example, you could test the impact of changes to a code-generator on a few important modules that use that generator.

@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 12, 2019
@bcmills bcmills added this to the Go1.14 milestone Apr 12, 2019
@bcmills bcmills changed the title cmd/go: attempt to generate in dependency modules should fail cmd/go: reject 'go generate' on packages in unreplaced module dependencies? Apr 12, 2019
@myitcv
Copy link
Member Author

myitcv commented Apr 13, 2019

@bcmills

that way, for example, you could test the impact of changes to a code-generator on a few important modules that use that generator.

Not averse to your suggestion, but just to note (at least from my experience) the requirement is the other way around, i.e. the module with the go:generate directive requires the generator. As such the module with the directive is still the main module. Testing of a quick fix would also generally be from the perspective of the directive-containing module, with/without a replace directive.

Unless you're talking about temporarily adding the reverse requirement for testing purposes?

@bcmills
Copy link
Contributor

bcmills commented Apr 13, 2019

Unless you're talking about temporarily adding the reverse requirement for testing purposes?

Yes.

@bcmills bcmills mentioned this issue Sep 3, 2019
Closed
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@rsc
Copy link
Contributor

rsc commented Jan 14, 2021

I thought 'go generate' in dependencies was already disabled, same as 'go fmt' and 'go fix'. If not, it should be.

@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label Jan 14, 2021
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 14, 2021
@bcmills bcmills mentioned this issue Oct 7, 2022
Closed
@maksadbek
Copy link
Contributor

maksadbek commented Mar 1, 2023 •
edited
Loading

Hey @bcmills I want to work on this. What should be the required behaviour in this case ? Should the go command just return 1 exit code ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
GoCommand cmd/go help wanted modules NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
5 participants
@rsc @maksadbek @myitcv @bcmills @gopherbot