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

Idea: use go run in generate line #160

Closed
zombiezen opened this issue Apr 29, 2019 · 8 comments · Fixed by #268
Closed

Idea: use go run in generate line #160

zombiezen opened this issue Apr 29, 2019 · 8 comments · Fixed by #268
Labels
enhancement New feature or request
Milestone

Comments

@zombiezen
Copy link
Collaborator

zombiezen commented Apr 29, 2019

An idea suggested by @steebchen is to use //go:generate go run github.com/google/wire/cmd/wire as the automatically inserted generate line for Wire output. This would have the advantage of always using a version of the code generator that's the same as the library being used, at the cost of perhaps some running time.

EDIT (Hit send too early) I don't think we want to make this change until Go Modules is the default, thus I'm marking this issue blocked.

@zombiezen zombiezen added enhancement New feature or request blocked Blocked on a different issue labels Apr 29, 2019
@vangent vangent added this to the Unplanned milestone Sep 10, 2019
ernado added a commit to ernado/wire that referenced this issue Jan 28, 2020
Leaving this feature as opt-in for backward compatibility.

Relates: google#160, google#106
ernado added a commit to gortc/wire that referenced this issue Feb 19, 2020
@ernado
Copy link
Contributor

ernado commented Feb 21, 2020

This probably should be blocked until golang/go#33468 is resolved.

Also, we can detect case when binary is started with go run, so there will be no backward compatibility issues at all. That is, if wired binary was started via go run we emit go:generate go run github.com/google/wire/cmd/wire, otherwise we fallback to current go:generate wired.

@steebchen
Copy link
Contributor

Go modules is pretty much the default now, so that part should not be blocking anymore.

@steebchen
Copy link
Contributor

I want to raise this again – go modules is the way to go by now.

@zombiezen zombiezen removed the blocked Blocked on a different issue label Dec 3, 2020
@zombiezen
Copy link
Collaborator Author

With Go 1.16 having GO111MODULE=on by default, I agree. Happy to accept a PR to fix.

steebchen added a commit to steebchen/wire that referenced this issue Dec 4, 2020
steebchen added a commit to steebchen/wire that referenced this issue Dec 4, 2020
Change the go:generate command to use the full go run ... command
to ensure the version specified in the current go module is used
instead of the global binary.

fix google#160
@steebchen
Copy link
Contributor

Sounds good, I created #268

@odsod
Copy link

odsod commented Dec 13, 2020

@zombiezen This is something we've been looking for too, to simplify our go generate invocations (we currently pre-build the correct version of the wire CLI and add it to $PATH before invoking go generate.

When this gets merged to master, we'd be happy to test this out and provide feedback before the next stable release.

@ernado
Copy link
Contributor

ernado commented Dec 13, 2020

Note that go run will be slower that direct call of wire because go run does not cache linked binary.

Also just simply changing from wire to go run can break some users, I've implemented backward compatible version in #239

@steebchen
Copy link
Contributor

Note that go run will be slower that direct call of wire because go run does not cache linked binary.

That's negligible:

❯ time go run github.com/google/wire/cmd/wire
wire: api: wrote /Users/luca/projects/ws-api/wire_gen.go
go run github.com/google/wire/cmd/wire  5.27s user 1.60s system 274% cpu 2.503 total

❯ time wire
wire: api: wrote /Users/luca/projects/ws-api/wire_gen.go
wire  5.26s user 1.05s system 236% cpu 2.673 total

Also just simply changing from wire to go run can break some users, I've implemented backward compatible version in #239

Good point. However, I think either way the new default should be to use go generate, so that new users can make immediate use of it. Users who explicitly want the old behaviour should not upgrade, or set the option explicitly should they update.

zombiezen pushed a commit that referenced this issue Jan 26, 2021
Change the go:generate command to use the full go run ... command
to ensure the version specified in the current go module is used
instead of the global binary.

Fixes #160
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants