unconvert icon indicating copy to clipboard operation
unconvert copied to clipboard

Support cross-builds and cgo

Open karalabe opened this issue 8 years ago • 9 comments

The type of syscall.Stdin differs on Linux (int) and Windows (syscall.Handle). As such, any calling code needs to always cast syscall.Stdin to an integer to use it. But this gets flagged by unconvert on Linux.

My current workaround is to do int(os.Stdin.Fd()) instead of syscall.Stdin. Would be nice to do without the hack.

karalabe avatar Nov 10 '17 15:11 karalabe

To confirm, are you using unconvert's -all flag?

mdempsky avatar Nov 10 '17 16:11 mdempsky

I'm using unconvert via the gometalinter using --enable=unconvert. Not sure exactly how the meta linter invokes unconvert.

karalabe avatar Nov 10 '17 16:11 karalabe

According to https://github.com/alecthomas/gometalinter/blob/0262fb20957a4c2d3bb7c834a6a125ae3884a2c6/linters.go#L373, it looks like gometalinter invokes unconvert without -all or any other flags.

dmitshur avatar Nov 10 '17 16:11 dmitshur

I see. Unconvert should support your case with the -all flag. If not, that's definitely a bug.

I'm not familiar with gometalinter though. I don't know how to have it run unconvert with -all, or if that's even possible without changing the source.

mdempsky avatar Nov 10 '17 16:11 mdempsky

I did try running locally with the --all option on my repo, but it dies on some CGO packages:

$ unconvert --all ./cmd/puppeth/
/work/src/github.com/ethereum/go-ethereum/crypto/signature_cgo.go:27:2: could not import github.com/ethereum/go-ethereum/crypto/secp256k1 (invalid package name: "")
2017/11/10 18:34:47 couldn't load packages due to errors: github.com/ethereum/go-ethereum/crypto

karalabe avatar Nov 10 '17 16:11 karalabe

You can customize the command run by gometalinter using a config file:

{
  "Linters": {
    "unconvert": {"Command": "unconvert --all"}
  }
}

I tried --all on another repo, and I hit this problems with CGO as well.

snapshots/btrfs/btrfs.go:212:12: SubvolCreate not declared by package btrfs

However:

  • this function is declared in the btrfs package
  • there are no build tags on the file
  • it doesn't fail without --all

The only thing different about this package seems to be the import "C"

dnephin avatar Jan 08 '18 22:01 dnephin

The problem here is that when cross-compiling, the Go build system disables cgo support by default. When cgo is disabled, files that include import "C" are automatically omitted from the build, just as if they contained an unsatisfied //+build predicate or _${GOOS/GOARCH} file name suffix.

You can setup CGO_ENABLED=1 to force cgo to be available in cross-compiles, but then you'll also have to set CC_FOR_${GOOS}_${GOARCH} and CXX_FOR_${GOOS}_${GOARCH} for every build target that uses cgo.

It's probably unlikely that you have Plan 9, Solaris, AIX, etc. cross compilers handy, however. (And assuming your code even compiles on them.)

--

What I'm thinking is the best solution here is to provide more control over the set of environment variables that unconvert runs in -all mode. For example, maybe you don't even really care about every OS/arch, and you've setup a C cross-compiler for Windows and Linux x86. I think it would be reasonable to support a config file like:

[
  // Host build; cgo support implied.
  ["GOOS=linux", "GOARCH=amd64"],

  // Cross-compile builds with explicit cgo support via cross-compilers.
  ["GOOS=linux", "GOARCH=386", "CGO_ENABLED=1", "CC_FOR_TARGET=gcc -m32"],
  ["GOOS=windows", "GOARCH=amd64", "CGO_ENABLED=1", "CC_FOR_TARGET=mingw32-gcc"],

  // Go-only cross builds.
  ["GOOS=darwin", "GOARCH=amd64"],
  ["GOOS=openbsd", "GOARCH=arm"],
]

mdempsky avatar Jan 11 '19 01:01 mdempsky

I added support at master for a -configs flag. Please take a look and let me know if it's helpful.

Open questions:

  • Is JSON the best configuration format for this?
  • Is listing out the config on the command-line okay, or do folks want to store it in a file? (Even as is, you can always do something like -configs="$(cat unconvert.json)".)
  • Should I allow setting build flags (e.g., -tags) on a per-config basis?

mdempsky avatar Jan 11 '19 01:01 mdempsky

I'm using the -configs option to limit cross-compilation to the combinations we support. Thank you.

chipaca avatar Aug 25 '21 08:08 chipaca