search

ugorji / go

idiomatic codec and rpc lib for msgpack, cbor, json, etc. msgpack.org[Go]
MIT License
1.86k stars 295 forks source link

Handle GO15VENDOREXPERIMENT properly across Go versions #148

Closed 2opremio closed 8 years ago

2opremio commented 8 years ago

Fixes #147

ugorji commented 8 years ago

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 commented 8 years ago

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 <notifications@github.com javascript:_e(%7B%7D,'cvml','notifications@github.com');> 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 https://github.com/ugorji/go/pull/148#issuecomment-196398421.

ugorji commented 8 years ago

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 commented 8 years ago

907292679fff3dbc38f9bb2361f47953ed7aea75 is the fix for #147

2opremio commented 8 years ago

fair enough

On Monday, March 14, 2016, Ugorji Nwoke notifications@github.com 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 https://github.com/ugorji/go/pull/148#issuecomment-196418404.