V2 remove go generate from the build process

[abitrolly]

What type of PR is this?

• cleanup

What this PR does / why we need it:

Simplifies build process.

Which issue(s) this PR fixes:

Fixes #1801

Testing

Run make. Remove generated files. Run again. See they are regenerated and identical.

zz_generated.flags.go
zz_generated.flags_test.go

Release Notes

(REQUIRED)

[dearchap]

@meatballhat Can you review as this is more in your area ?

[meatballhat]

@abitrolly sorry for my big delay! I had thought that using the built-in Go tooling for this generation hook was preferable 🤔 Can you help me understand why you think this approach is better?

[abitrolly]

@meatballhat go generate is calling make, and there seems to be no benefit in going another level of indirection when it is possible to just call make.

[dolmen]

I would prefer to get rid of the Makefile towards a build that uses only //go:generate

[abitrolly]

@dolmen what is the point in go generate if go build doesn't invoke it automatically? To build the project, you still need some script/Makefile that will call go generate before go build.

[abitrolly]

Resolved conflict and rebased.

It would be nice to change -- generating Go source files -- comment to mention what and why is generated.

[dolmen]

The principle with "go generate" is that it is run by the developer and its result is committed in the repo so when you run "go build" on any platform you don't need to run "go generate" again.

[abitrolly]

The principle with "go generate" is that it is run by the developer and its result is committed in the repo so when you run "go build" on any platform you don't need to run "go generate" again.

I haven't found this principle in go generate help. go generate is way to run preprocessor in go when there is no other executors. We use build.go and make as build executors. Chaining a third one here complicates the build system, and makes it harder for people to support it. It took me several days to trace problems with generated flags. Then I decided to send PR that removes extra step to simplify build chain. See the #1801 for diagram.

[dolmen]

From Rob Pike himself on the Go blog (emphasis mine):

Go generate does nothing that couldn’t be done with Make or some other build mechanism, but it comes with the go tool—no extra installation required—and fits nicely into the Go ecosystem. Just keep in mind that it is for package authors, not clients, if only for the reason that the program it invokes might not be available on the target machine. Also, if the containing package is intended for import by go get, once the file is generated (and tested!) it must be checked into the source code repository to be available to clients.

I understand that you want the code generation to be simplified, but I disagree with the choice of switching to make instead of using just the Go toolchain.

I'll propose something else. But I'm concerned with the dual maintenance of v2 and v3.

[abitrolly]

@dolmen I could agree with the argument if it was go generate as a replacement for make, but we are already using make as a primary tool for v2, and go generate doesn't add any value to it - only complexity.

[meatballhat]

I'm still feeling ➕ 0️⃣ on this one @abitrolly 🤷🏼 It's true that we have a Makefile, but it's pretty slim, and I want to make sure the project doesn't slip into requiring make to build a usable library. I tend to agree with @dolmen and I would also prefer to move towards getting rid of make instead of getting rid of the use of go generate. </2cents> ❤️

[julian7]

I'm personally against having a makefile in a go project, except if it's really necessary.

[abitrolly]

@julian7 would you be able to come up with PR to remove Makefile?

[abitrolly]

@meatballhat it is not about Makefile vs go generate. It is about reducing build chain complexity, so that maintainers who still need to run and patch v2 to update their codebase, spend less time than me on debugging how it works (especially when it doesn't). I believe it took me few days to draw this diagram from #1801.

graph LR
make --> build.go --> gogen[go generate] --> cli.go --> urfave-cli-genflags/make --> urfave-cli-genflags --> goimports

Loading

[dearchap]

@abitrolly I think I understand the intent of the original design i.e to use go tooling as much as possible. I would suggest to drop this PR. We should document the build process somewhere though

[abitrolly]

@dearchap I wouldn't mind pure Go code, but this Go code calls Make, which calls Go program through shell. If there is a way to use Go program directly, maybe as a library, that would make it use Go.