paypal / gatt

Gatt is a Go package for building Bluetooth Low Energy peripherals
BSD 3-Clause "New" or "Revised" License
1.13k stars 284 forks source link

added x86 support for Linux #21

Closed acmacalister closed 9 years ago

roylee17 commented 9 years ago

Commit looks good to me, but it would only build with go1.4+ (which is still relative new at this time)

 #include "textflag.h"

I think I'll merge it, with some note in the README or commit message. So users having issues can either update their Go, or revert this single commit as a workaround.

roylee17 commented 9 years ago

Hold on, it breaks ARM build as well. And backup a little bit, I can build a 386 binary without this commit. So, is it trying to fix some runtime error, or ?

acmacalister commented 9 years ago

ah, yes I forgot to include that you had to be on go 1.4, nice catch. In what ways is it breaking the ARM build? I don't have an ARM device to test with, so I can't check. Backing up a bit, the reason I did this PR, as when I tried to build for Linux i386 with what is currently in master I got an error like this:

austin:edison ACherry$ GOARCH=386 GOOS=linux go build
# github.com/paypal/gatt/linux/internal/socket
../../paypal/gatt/linux/internal/socket/socket.go:91: undefined: syscall.SYS_BIND
../../paypal/gatt/linux/internal/socket/socket.go:99: undefined: syscall.SYS_SETSOCKOPT

Looking at http://golang.org/pkg/syscall/, I noticed that SYS_BIND was 49 and SYS_SETSOCKOPT was 54, both which exist on 64bit Linux. Looking at the i386 table in the Linux source:

https://github.com/torvalds/linux/blob/master/arch/x86/syscalls/syscall_32.tbl

49 and 54, do not map to those calls. Instead it uses another layer of indirection through socketcall (syscall 102 in i386) as noted in the Go syscall source here:

https://github.com/golang/go/blob/master/src/syscall/syscall_linux_386.go#L177

The changes in this PR, use the same strategy as the sys/unix package does. That being said, if I went down this trail and got way off track, I would be happy to be shown the way back. :smile: Also, if there is anything in ARM build that I need to fix, let me know and would be happy to do so.

roylee17 commented 9 years ago

Cool, that's a decent effort and analysis.

But I still can't reproduce the 386 build fail with either go1.4 or go1.4.2. Does the "HOST" have to be 386 to reproduce it? Which go version are you using precisely?

My build machine:

$ uname -a Linux localhost 3.14.25 #36 SMP Mon Nov 24 09:06:07 PST 2014 x86_64 Intel(R) Core(TM) i7-4960HQ CPU @ 2.60GHz GenuineIntel GNU/Linux

Current tip commit of master branch HEAD is now at df506cf Merge pull request #14 from wolfeidau/improved_error_handling

$ go version go version go1.4.2 linux/amd64 $ GOARCH=386 GOOS=linux go build sample.go && file sample sample: ELF 32-bit LSB executable, Intel 80386, version 1 (SYSV), statically linked, not stripped

p.s. ARM build can be fixed by renaming the _amd64.go to something else,(_common.go maybe?) as you already had a build flag in it.

// +build !386

roylee17 commented 9 years ago

Okay, I got it. It has to be the "tip" of go to reproduce this. :-) I see @davecheney was actively working on it, is it possible that you found a regression, and a opportunity to contribute upstream Go? I'm not sure, I'll try pick up more context tomorrow. Thanks

roylee17 commented 9 years ago

Somehow, I got the same 386 build error with go1.4/go1.4.2 now... So, please ignore my previous comment. Thanks for the work!

BTW, I'm bring this change to the "next" branch, would you be able to test it on 386?

acmacalister commented 9 years ago

@roylee17 I would be happy to. I also run OS X as my primary machine, so I can test that as well, if you need.

roylee17 commented 9 years ago

@acmacalister that will be great!