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

Jump to bottom

x/mobile: iOS bind clobbers existing build tags #18523

Closed
adam-p opened this issue Jan 5, 2017 · 1 comment
Closed

x/mobile: iOS bind clobbers existing build tags #18523

adam-p opened this issue Jan 5, 2017 · 1 comment
Labels
FrozenDueToAge
Milestone
Unreleased

Comments

@adam-p
Copy link
Contributor

adam-p commented Jan 5, 2017 •
edited

gomobile bind for iOS adds a -tags ios argument that clobbers any user-supplied -tags.

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

go1.7.4

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

darwin/amd64

What did you do?

Reproduction scenario:

First, modify the bind iOS example code (golang.org/x/mobile/example/bind/hello) to require the presence of a build tag. Like so:

diff --git a/example/bind/hello/hello.go b/example/bind/hello/hello.go
index 2d98ff9..441030b 100644
--- a/example/bind/hello/hello.go
+++ b/example/bind/hello/hello.go
@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.

+// +build aaa
+
 // Package hello is a trivial package for gomobile bind example.
 package hello

Attempting to gomobile bind now without the build tag fails as expected.

$ gomobile bind -v -x -target=ios golang.org/x/mobile/example/bind/hello

gomobile: package "golang.org/x/mobile/example/bind/hello": no buildable Go source files in GOPATH/src/golang.org/x/mobile/example/bind/hello

However, providing the tag still fails, although at a later point in the build:

$ gomobile bind -tags aaa -v -x -target=ios golang.org/x/mobile/example/bind/hello

...lots of building output...
/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS10.2.sdk -miphoneos-version-min=6.1 -arch armv7 CGO_ENABLED=1 GOPATH=$WORK/gen:$GOPATH go build -pkgdir=$GOMOBILE/pkg_darwin_arm -tags aaa -v -x -buildmode=c-archive -tags=ios -o $WORK/hello-arm.a $WORK/src/iosbin/main.go
WORK=/var/folders/m5/9r6jj7g57z5gkl_7441qzk440000gn/T/go-build842214217
/var/folders/m5/9r6jj7g57z5gkl_7441qzk440000gn/T/gomobile-work-594580178/src/gomobile_bind/go_hellomain.go:18:2: no buildable Go source files in GOPATH/src/golang.org/x/mobile/example/bind/hello
rm -r -f "$WORK"
gomobile: darwin-armv7: go build -pkgdir=GOPATH/pkg/gomobile/pkg_darwin_arm -tags aaa -v -x -buildmode=c-archive -tags=ios -o /var/folders/m5/9r6jj7g57z5gkl_7441qzk440000gn/T/gomobile-work-594580178/hello-arm.a /var/folders/m5/9r6jj7g57z5gkl_7441qzk440000gn/T/gomobile-work-594580178/src/iosbin/main.go failed: exit status 1

Notice the problem: There are two -tags arguments. The first is the one we supplied, and the second -- which clobbers the first -- is added by the iOS bind code.

The fix for the problem is this:

diff --git a/cmd/gomobile/bind_iosapp.go b/cmd/gomobile/bind_iosapp.go
index 4224b06..3d3214d 100644
--- a/cmd/gomobile/bind_iosapp.go
+++ b/cmd/gomobile/bind_iosapp.go
@@ -197,7 +197,8 @@ var iosModuleMapTmpl = template.Must(template.New("iosmmap").Parse(`framework mo
 func goIOSBindArchive(name, path string, env, fileBases []string) (string, error) {
        arch := getenv(env, "GOARCH")
        archive := filepath.Join(tmpdir, name+"-"+arch+".a")
-       err := goBuild(path, env, "-buildmode=c-archive", "-tags=ios", "-o", archive)
+       ctx.BuildTags = append(ctx.BuildTags, "ios")
+       err := goBuild(path, env, "-buildmode=c-archive", "-o", archive)
        if err != nil {
                return "", err
        }

NOTE: This is imperfect. It results in the ios tag get added multiple times for a large build. But that still functions correctly, whereas the current code does not. A proper fix should probably check for the presence of the ios tag before re-adding it.

After rebuilding gomobile, we get this:

$ gomobile bind -tags aaa -v -x -target=ios golang.org/x/mobile/example/bind/hello

...lots of output...
gomobile: darwin-armv7: go build -pkgdir=GOPATH/pkg/gomobile/pkg_darwin_arm -tags="aaa,ios" -v -x -buildmode=c-archive -o /var/folders/m5/9r6jj7g57z5gkl_7441qzk440000gn/T/gomobile-work-441301049/hello-arm.a /var/folders/m5/9r6jj7g57z5gkl_7441qzk440000gn/T/gomobile-work-441301049/src/iosbin/main.go failed: exit status 1

You can see that the -tags argument is better, but incorrect. This is due to issue #18515. If we apply the fix for that, we get:

$ gomobile bind -tags aaa -v -x -target=ios golang.org/x/mobile/example/bind/hello

...lots of output
GOPATH=$WORK/gen:$GOPATH go build -pkgdir=$GOMOBILE/pkg_darwin_arm -tags aaa ios -v -x -buildmode=c-archive -o $WORK/hello-arm.a $WORK/src/iosbin/main.go
...lots of output...
...success!

Note that the -tags argument is correct and works as expected. (However, later on in the output you'll see -tags aaa ios ios ios and so on.)

Impact

We discovered this the hard way and had to hack around it by patching gomobile before building it. This can be seen here.
@bradfitz bradfitz added this to the Unreleased milestone Jan 5, 2017
@gopherbot
Copy link

CL https://golang.org/cl/34955 mentions this issue.

@adam-p adam-p mentioned this issue Jan 17, 2017
Merged
@golang golang locked and limited conversation to collaborators Jan 12, 2018
imWildCat pushed a commit to imWildCat/go-mobile that referenced this issue Apr 10, 2021
cmd: fix handling of (multiple) tags
e16b318 
The gomobile tool mishandled build tags in two ways, first by
ignoring tags for iOS, second by passing multiple tags along to
the go tool incorrectly. This CL fixes both.

Fixes golang/go#18523
Fixes golang/go#18515

Change-Id: I28a49c1e23670adb085617d9f5fb5cd5e22a4b65
Reviewed-on: https://go-review.googlesource.com/34955
Reviewed-by: David Crawshaw <crawshaw@golang.org>
imWildCat pushed a commit to imWildCat/go-mobile that referenced this issue Apr 11, 2021
cmd: fix handling of (multiple) tags
5767bb7 
The gomobile tool mishandled build tags in two ways, first by
ignoring tags for iOS, second by passing multiple tags along to
the go tool incorrectly. This CL fixes both.

Fixes golang/go#18523
Fixes golang/go#18515

Change-Id: I28a49c1e23670adb085617d9f5fb5cd5e22a4b65
Reviewed-on: https://go-review.googlesource.com/34955
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@bradfitz @adam-p @eliasnaur @gopherbot