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

x/mobile/cmd/gomobile: bind fails for cloud.google.com/go/trace #24882

Open
matti777 opened this issue Apr 16, 2018 · 5 comments

Comments

@matti777
Copy link

commented Apr 16, 2018

What version of Go are you using (go version)?

1.10

Does this issue reproduce with the latest release?

Not quite sure. I tried re-installing gomobile but still getting:

[matti@babylon trace (master)] % gomobile version
gomobile version unknown: binary is out of date, re-install it

I followed this for (re)installing the gomobile cmd:

$ go get golang.org/x/mobile/cmd/gomobile
$ gomobile init

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/matti/Library/Caches/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/matti/go"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.10/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.10/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/jw/jrh0fgfn42ndjty5zpbsv8p00000gn/T/go-build504316821=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I was attempting to build (bind) iOS framework out of the Google Stackdriver Cloud Trace Go library cloud.google.com/go/trace. Reproduce by

go get -u cloud.google.com/go/trace
cd ~/go/src/cloud.google.com/go/trace
gomobile bind -target ios -o /tmp/Cloudtrace.framework

What did you expect to see?

The /tmp/Cloudtrace.framework to be built.

What did you see instead?

The build failed to:

gomobile: darwin-arm: go build -tags ios -buildmode=c-archive -o /var/folders/jw/jrh0fgfn42ndjty5zpbsv8p00000gn/T/gomobile-work-736257513/trace-arm.a gobind failed: exit status 2
# gobind
/var/folders/jw/jrh0fgfn42ndjty5zpbsv8p00000gn/T/gomobile-work-736257513/src/gobind/go_tracemain.go:53: cannot use (*proxytrace_SamplingPolicy)(_param_p0_ref) (type *proxytrace_SamplingPolicy) as type "cloud.google.com/go/trace".SamplingPolicy in assignment:
	*proxytrace_SamplingPolicy does not implement "cloud.google.com/go/trace".SamplingPolicy (missing Sample method)

Building some other Google modules (eg. storage) works fine. I checked the type limitations for gomobile bind and what this page https://godoc.org/golang.org/x/mobile/cmd/gobind lists does not seem to list anything that would prevent the following from being built:

type SamplingPolicy interface {
        // Sample returns a Decision.
        // If Trace is false in the returned Decision, then the Decision should be
        // the zero value.
        Sample(p Parameters) Decision
}

// Parameters contains the values passed to a SamplingPolicy's Sample method.
type Parameters struct {
        HasTraceHeader bool // whether the incoming request has a valid X-Cloud-Trace-Context header.
}

// Decision is the value returned by a call to a SamplingPolicy's Sample method.
type Decision struct {
        Trace  bool    // Whether to trace the request.
        Sample bool    // Whether the trace is included in the random sample.
        Policy string  // Name of the sampling policy.
        Weight float64 // Sample weight to be used in statistical calculations.
}

I am guessing *proxytrace_SamplingPolicy is something the gomobile bind is creating in its process, there is nothing like that in the trace sources.

@ALTree ALTree changed the title gomobile bind fails for cloud.google.com/go/trace x/mobile/cmd/gomobile: bind fails for cloud.google.com/go/trace Apr 16, 2018

@gopherbot gopherbot added this to the Unreleased milestone Apr 16, 2018

@gopherbot gopherbot added the mobile label Apr 16, 2018

@matti777

This comment has been minimized.

Copy link
Author

commented Apr 16, 2018

If there is something to be done manually to make it build, even if it would require a fork/temporary change to the trace module, I'd be interested to know in order to make it build once - this is for a PoC product and any (working) quick hack would do.

@matti777

This comment has been minimized.

Copy link
Author

commented Apr 16, 2018

Oh, after realizing I can run use -work switch to make it leave the intermediate temp files on the disk, I dug into it and found go_tracemain.go:

// skipped method SamplingPolicy.Sample with unsupported parameter or return types
type proxytrace_SamplingPolicy _seq.Ref

Given the bind's list of supported types:

- Signed integer and floating point types.

- String and boolean types.

- Byte slice types. Note that byte slices are passed by reference,
  and support mutation.

- Any function type all of whose parameters and results have
  supported types. Functions must return either no results,
  one result, or two results where the type of the second is
  the built-in 'error' type.

- Any interface type, all of whose exported methods have
  supported function types.

- Any struct type, all of whose exported methods have
  supported function types and all of whose exported fields
  have supported types.

I dont understand which rule the method Sample() is breaking?

To go around gomobile bind's type limitations, would it be a proper pattern to write a more bind-friendly facade package that would wrap these non-bind:able libraries? Meanwhile type support for bind gets more thorough, that is..

@mier85

This comment has been minimized.

Copy link

commented Oct 4, 2018

@matti777 did you ever find out why you couldn't build it? i have a similar issue

@matti777

This comment has been minimized.

Copy link
Author

commented Oct 5, 2018

I did not. It was a PoC project and did not have time to dig into gomobile's code. Solved my issue by writing a custom facade library which then provided an API which did not produce gomobile bind errors, which I think was (in my case) actually better than using the trace api directly.

bemasc added a commit to bemasc/gobind-fail that referenced this issue May 29, 2019
@bemasc

This comment has been minimized.

Copy link
Contributor

commented May 29, 2019

This is biting me too, so I made a minimal repro case: https://github.com/bemasc/gobind-fail. I hope this helps the maintainers to understand the problem.

I think this has something to do with a failure to follow interface embedding across package boundaries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.