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

Handle GO15VENDOREXPERIMENT properly across Go versions #148

Closed
wants to merge 1 commit into from

Conversation

2opremio
Copy link
Contributor

Fixes #147

@ugorji
Copy link
Owner

ugorji commented Mar 14, 2016

I don't agree with this fix.

For one, it only works for releases. go version for development looks like:

go version devel +8a2d6e9 Mon Mar 14 14:53:29 2016 +0000 linux/amd64

This doesn't have a go1.N in there anywhere. This PR only works fine for releases.

The proper solution is to use build tags. This way, the job of determining what code to use is the purview of the build tool, and not our code deciphering it.

I have a change for that which I will submit soon.

@2opremio
Copy link
Contributor Author

Won't build tags lead to inconsistent results if gen.go is built with one
version of Go and codecgen (i.e go run) is invoked with a different Go
version?

On Monday, March 14, 2016, Ugorji Nwoke <[email protected]
javascript:_e(%7B%7D,'cvml','[email protected]');> wrote:

I don't agree with this fix.

For one, it only works for releases. go version for development looks like:

go version devel +8a2d6e9 Mon Mar 14 14:53:29 2016 +0000 linux/amd64

This doesn't have a go1.N in there anywhere. This PR only works fine for
releases.

The proper solution is to use build tags. This way, the job of determining
what code to use is the purview of the build tool, and not our code
deciphering it.

I have a change for that which I will submit soon.


Reply to this email directly or view it on GitHub
#148 (comment).

@ugorji
Copy link
Owner

ugorji commented Mar 14, 2016

I took some time to think about this earlier.

codecgen is run by taking the .go input files and the GO SDK, and generates new go files. It is not a build tool - it is a runtime generation tool. The installed Go SDK that it uses determines how it handles vendored things, etc.

There's consequently no separate point where gen.go is built. That Go SDK will select appropriately based on its version.

@ugorji
Copy link
Owner

ugorji commented Mar 14, 2016

9072926 is the fix for #147

@2opremio
Copy link
Contributor Author

fair enough

On Monday, March 14, 2016, Ugorji Nwoke [email protected] wrote:

I took some time to think about this earlier.

codecgen is run by taking the .go input files and the GO SDK, and
generates new go files. It is not a build tool - it is a runtime generation
tool. The installed Go SDK that it uses determines how it handles vendored
things, etc.

There's consequently no separate point where gen.go is built. That Go SDK
will select appropriately based on its version.


Reply to this email directly or view it on GitHub
#148 (comment).

@2opremio 2opremio closed this Mar 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants