cmd/go: go generate should be more resilient to source changes #36068

rogpeppe · 2019-12-10T14:14:28Z

go version devel +3c0fbeea7d Tue Nov 5 05:22:07 2019 +0000 linux/amd64

When go generate runs, it's quite likely to add and remove source files.
This can result in errors which are inappropriate.

Here's a testscript command script that demonstrates the issue. It's a pared-down version of some real code:

go generate ./...
cp names.new names
go generate ./...

-- names --
a
b
c
-- names.new --
a
b
d
-- gen.go --
// +build ignore

package main

import (
	"io/ioutil"
	"log"
	"os"
	"path/filepath"
	"strings"
)

func main() {
	os.RemoveAll("generated")
	testData, err := ioutil.ReadFile("names")
	if err != nil {
		log.Fatal(err)
	}
	for _, name := range strings.Fields(string(testData)) {
		if err := os.MkdirAll(filepath.Join("generated", name), 0777); err != nil {
			log.Fatal(err)
		}
		err := ioutil.WriteFile(filepath.Join("generated", name, "f.go"), []byte(`
// Code generated for this test. DO NOT EDIT.

package `+name+`

func F() {}
`), 0666)
		if err != nil {
			log.Fatal(err)
		}
	}
}
-- go.mod --
module m

go 1.14
-- main.go --
package main

//go:generate go run gen.go

func main() {
}

When I run it, I see this:

% testscript goissue.txtar
> go generate ./...
> cp names.new names
> go generate ./...
[stderr]
generate: open $WORK/generated/c/f.go: no such file or directory

[exit status 1]
FAIL: /tmp/testscript089422577/0/script.txt:3: unexpected go command failure
error running tst.txtar in /tmp/testscript089422577/0

That is, the second time that go generate runs, it fails.
It seems that go generate is evaluating all the files when it runs, and not being resilient when they change (in this case a file changed name).

Perhaps it should be silent in cases like this.

There may also be other cases where go generate could be quieter about errors (for example when there's a package name mismatch in files in a package, which can be caused when a package is renamed and go generate is called again to regenerate the new files).

One possible approach might be for go generate to ignore any errors on files which contain the standard "DO NOT EDIT" comment.

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

toothrot · 2019-12-10T19:05:24Z

/cc @bcmills @jayconrod

bcmills · 2019-12-10T20:06:33Z

@rogpeppe, it seems very odd for go generate . to produce source files for a different package (./generated/c). (I would expect that to be pretty common for testadata directories, but testdata directories are ignored for patterns like ./....)

I'm assuming that you ran into this with a more realistic example somewhere. Can you give some more detail about the concrete case?

rogpeppe · 2019-12-10T23:57:38Z

In this particular case, I was writing tests for a code generator. The generator generates code in a package, and the go generate command iterates over a bunch of test data, creating a directory, running the generator in it and generating some test code to test the generated code.

The advantage of doing this is that I can check in the resulting test code, which serves as unit tests for the imported package used by the generated code. It also means that I can easily obtain coverage info for the code covered by the test cases.

There's nothing about go generate that says that it should only generate code in the same directory, so I thought this could work ok, but it turns out a bit awkwardly, hence this issue.

bcmills · 2019-12-11T15:54:23Z

The generator generates code in a package, and the go generate command iterates over a bunch of test data, creating a directory, running the generator in it and generating some test code to test the generated code.

So the go generate command is being run for a newly-generated package?

It still seems like the solution here belongs on the caller side: if go generate foo might delete the package foo/generated/c entirely, then the user should not run go generate on foo and foo/generated/c simultaneously.

OTOH, if the package foo/generated/c is just testdata for foo, then it should probably be named foo/testdata/c instead, and then the ./... pattern won't match it at all.

rogpeppe · 2019-12-11T16:20:55Z

It still seems like the solution here belongs on the caller side: if go generate foo might delete the package foo/generated/c entirely, then the user should not run go generate on foo and foo/generated/c simultaneously.

I don't really see how this is so different from running go generate on a package where some of the files in the package are generated and might appear or disappear. This is just doing the generation at a package level rather than a file level.

OTOH, if the package foo/generated/c is just testdata for foo, then it should probably be named foo/testdata/c instead, and then the ./... pattern won't match it at all.

If I did that, I couldn't run the generated tests by running go test ./..., which is one of the main reasons I'm doing this this way.

tie · 2021-04-20T00:45:07Z

A smaller testscript that reproduces the error:

go generate ./...

-- go.mod --
module genclean

go 1.16

-- a.go --
package genclean

//go:generate rm b.go

-- b.go --
package genclean

Output

testscript goissue.txtar

> go generate ./...
[stderr]
generate: open $WORK/b.go: no such file or directory

[exit status 1]
FAIL: /tmp/testscript083100219/goissue.txtar/script.txt:1: unexpected go command failure
error running goissue.txtar in /tmp/testscript083100219/goissue.txtar

This change allows go generate to process packages where files may disappear during code generation. See also golang#36422. Fixes golang#36068

gopherbot · 2021-04-20T01:45:54Z

Change https://golang.org/cl/311531 mentions this issue: cmd/go: allow generate to skip missing files

nightlyone · 2021-04-20T11:33:48Z

Another solution is to put opposite build tags on the generator and the generated file. So the generated file will be excluded from building the generator.

tie · 2021-04-21T16:05:45Z

@nightlyone not sure how that’d help. E.g. gotd/td recently stumbled upon this issue because it puts types from Telegram’s TL schema in a separate files when generating tg package. So removing a type from schema breaks code gen since it needs to remove the file too (uh, and currently go generate doesn’t like it). An ugly workaround would be to place go:generate directive at the end of the package files list (perhaps zgen.go).

This change allows go generate to process packages where files may disappear during code generation. See also golang#36422. Updates golang#36068

dmitryax · 2022-05-18T17:12:00Z

Just wanted to mention that we are running into the same issue in https://github.com/open-telemetry/opentelemetry-collector-contrib.

The use case is to generate a file in an internal sub-package ./internal/metadata/metrics.go.

It works fine for creating new files and replacing them. But if we want to change the filename from the generator, e.g. remove ./internal/metadata/metrics.go and create ./internal/metadata/metrics_v2.go, the generator program works fine - files are replaced, but go generate ./... fails at the same time with

generate: open /Users/danoshin/Projects/opentelemetry-collector-contrib/receiver/hostmetricsreceiver/internal/scraper/memoryscraper/internal/metadata/generated_metrics_v2.go: no such file or directory

toothrot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 10, 2019

toothrot added this to the Backlog milestone Dec 10, 2019

bcmills added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Dec 10, 2019

bcmills removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Dec 11, 2019

rogpeppe mentioned this issue Jan 7, 2020

cmd/go: generate shouldn't require a complete valid package #36422

Closed

tie added a commit to tie/go that referenced this issue Apr 20, 2021

cmd/go: allow generate to skip missing files

d13404b

This change allows go generate to process packages where files may disappear during code generation. See also golang#36422. Fixes golang#36068

tie mentioned this issue Apr 20, 2021

cmd/go: allow generate to skip missing files #45642

Open

tie added a commit to tie/go that referenced this issue Apr 21, 2021

cmd/go: allow generate to skip missing files

4ec27c1

This change allows go generate to process packages where files may disappear during code generation. See also golang#36422. Updates golang#36068

dmitryax mentioned this issue May 18, 2022

[cmd/mdatagen] Work around a go-generate issue blocking make generate open-telemetry/opentelemetry-collector-contrib#10133

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd/go: go generate should be more resilient to source changes #36068

cmd/go: go generate should be more resilient to source changes #36068

rogpeppe commented Dec 10, 2019 •

edited

toothrot commented Dec 10, 2019

bcmills commented Dec 10, 2019 •

edited

rogpeppe commented Dec 10, 2019 •

edited

bcmills commented Dec 11, 2019

rogpeppe commented Dec 11, 2019 •

edited

tie commented Apr 20, 2021 •

edited

gopherbot commented Apr 20, 2021

nightlyone commented Apr 20, 2021

tie commented Apr 21, 2021

dmitryax commented May 18, 2022

cmd/go: go generate should be more resilient to source changes #36068

cmd/go: go generate should be more resilient to source changes #36068

Comments

rogpeppe commented Dec 10, 2019 • edited

toothrot commented Dec 10, 2019

bcmills commented Dec 10, 2019 • edited

rogpeppe commented Dec 10, 2019 • edited

bcmills commented Dec 11, 2019

rogpeppe commented Dec 11, 2019 • edited

tie commented Apr 20, 2021 • edited

gopherbot commented Apr 20, 2021

nightlyone commented Apr 20, 2021

tie commented Apr 21, 2021

dmitryax commented May 18, 2022

rogpeppe commented Dec 10, 2019 •

edited

bcmills commented Dec 10, 2019 •

edited

rogpeppe commented Dec 10, 2019 •

edited

rogpeppe commented Dec 11, 2019 •

edited

tie commented Apr 20, 2021 •

edited