You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This follows a suggestion from @flowrean in #18 to use xgo for cross-compiling the gocsv binary for distribution.
In general this works well and should allow the sql subcommand (which depends on the go-sqlite3 package) to work on other platforms and architectures. It also provides more platforms and architectures than the whitelisted set previously provided.
I have only tested the Mac binary (darwin and amd64) on my local machine and the Linux binary (linux and amd64) on the alpine:3.12.0 and ubuntu:12.04 Docker images.
On my Mac, everything works with the xgo compiled version of gocsv.
With Linux, all subcommands now work on the ubuntu image, including sql which previously did not work. However, there is a regression in that the xgo-built gocsv does not work at all on alpine, whereas previously all subcommands except the sql subcommand worked.
The issue I ran into with alpine was (after docker cp-ing a CSV and the cross-compiled gocsv binary into an interactive terminal in the alpine container):
/ # ./gocsv view tmp.csv
/bin/sh: ./gocsv: not found
This may be a duplicate of karalabe/xgo#183 (i.e. xgo may not support alpine). Regardless, I'm not too worried about it. I doubt people will be using gocsv in a container based on alpine, and if they are they can compile it themselves IMO.
I have not tested the Windows binaries, which would be helpful to determine if this can close the issue #18.
Here are the zip files that it now creates via xgo, which I'm fine with publishing in the release(s):
Hi @aotimme, this looks really nice!
I have tested on Windows and the SQL subcommand works well there. (There was another unrelated issue I discovered doing this, see #34 for a pull request that should solve it.)
Two remarks:
Docker is now required for building. Not a problem for me.
The ZIP files have slightly different names, e.g. gocsv-darwin-amd64.zip is now called gocsv-darwin-10.6-amd64.zip. I don't know if this is an issue, but if we continue, the macOS install script should be updated (as well as the filenames in the installation part of the README for macOS and Windows).
Good point re Docker. I just updated DEV.md to make a note of that (and needing to go get github.com/karalabe/xgo). I'm okay with it as well, especially because you can still build a binary for yourself without Docker via make which uses the usual go build.
Also good catch re the installation script. I'm fine with the name change (gocsv-darwin-amd64.zip to gocsv-darwin-10.6-amd64.zip). Now that we are using xgo to cross-compile, I'd like to delegate the naming scheme for the cross-compiled binaries to xgo.
I'll merge with the small changes now and then re-do the build and upload the full set of ZIP files.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This follows a suggestion from @flowrean in #18 to use
xgofor cross-compiling thegocsvbinary for distribution.In general this works well and should allow the
sqlsubcommand (which depends on the go-sqlite3 package) to work on other platforms and architectures. It also provides more platforms and architectures than the whitelisted set previously provided.I have only tested the Mac binary (
darwinandamd64) on my local machine and the Linux binary (linuxandamd64) on thealpine:3.12.0andubuntu:12.04Docker images.On my Mac, everything works with the
xgocompiled version ofgocsv.With Linux, all subcommands now work on the
ubuntuimage, includingsqlwhich previously did not work. However, there is a regression in that thexgo-builtgocsvdoes not work at all onalpine, whereas previously all subcommands except thesqlsubcommand worked.The issue I ran into with
alpinewas (afterdocker cp-ing a CSV and the cross-compiledgocsvbinary into an interactive terminal in thealpinecontainer):This may be a duplicate of karalabe/xgo#183 (i.e.
xgomay not supportalpine). Regardless, I'm not too worried about it. I doubt people will be usinggocsvin a container based onalpine, and if they are they can compile it themselves IMO.I have not tested the Windows binaries, which would be helpful to determine if this can close the issue #18.
Here are the zip files that it now creates via
xgo, which I'm fine with publishing in the release(s):