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: Remove gomobile dependency on gobind binary #34708

Open
anjmao opened this issue Oct 5, 2019 · 13 comments
Open

x/mobile: Remove gomobile dependency on gobind binary #34708

anjmao opened this issue Oct 5, 2019 · 13 comments
Labels
mobile Android, iOS, and x/mobile NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@anjmao
Copy link

anjmao commented Oct 5, 2019

Currently gomobile bind internally depends on gobind command. This is not user friendly and seems unnecessary too complex as user need to build both binaries. I propose to remove cmd/gobind so there is only one binary. As there is difference what gomobile bind and gobind does we can pass extra flag to gomobile bind -gen_only to only generate bindings without compiling to final project.

If cmd/gobind is still needed (not sure why) we can extra all reusable logic to separate package so it can be used directly in both cmd/gomobile and cmd/gobind.

If this make sense I can work on it.

/cc @hajimehoshi @eliasnaur

@gopherbot gopherbot added this to the Unreleased milestone Oct 5, 2019
@gopherbot gopherbot added the mobile Android, iOS, and x/mobile label Oct 5, 2019
@hajimehoshi
Copy link
Member

hajimehoshi commented Oct 7, 2019

CC @hyangah

I would like to keep separate binaries. gobind generates a source code and gomobile generates an application or a shared library, so they are different.

In the context of Go modules, gobind is obvious how to fix for Go module, while gomobile is not.

@anjmao
Copy link
Author

anjmao commented Oct 7, 2019

Commands go build, go test, go tool pprof etc. are all doing different things but are under one binary. One cmd for gomobile would be more clear as you should not split into separate binaries if you have different output but all commands are related to it's general purpose to compile, link, generate code for mobile native apps.

@hajimehoshi
Copy link
Member

Commands go build, go test, go tool pprof etc. are all doing different things but are under one binary.

Sorry if I'm wrong, but some of the Go tools you mentioned are separated binaries.

https://golang.org/src/cmd/

@anjmao
Copy link
Author

anjmao commented Oct 7, 2019

Here is example https://github.com/golang/go/blob/master/src/cmd/go/main.go#L43 how different tools are used with go command. In the end they can be both as separate binaries and reused for main binary command.

@katiehockman katiehockman added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Oct 7, 2019
@rsc
Copy link
Contributor

rsc commented Oct 9, 2019

/cc @hyangah @eliasnaur

@rsc
Copy link
Contributor

rsc commented Oct 30, 2019

ping @hyangah and @eliasnaur again. Any thoughts?

@rsc
Copy link
Contributor

rsc commented Oct 30, 2019

/cc @steeve

@eliasnaur
Copy link
Contributor

SGTM.

@steeve
Copy link
Contributor

steeve commented Oct 30, 2019

SGTM, as long as the functionality is kept (we use it in https://github.com/znly/rules_gomobile).
It makes it for the slightly more complex build [1], but nothing major.

  1. https://github.com/znly/rules_gomobile/tree/master/third_party/org_golang_x_mobile

@hyangah
Copy link
Contributor

hyangah commented Oct 30, 2019

SGTM eventually, but not sure who will work on and who will maintain.

IIRC, gomobile originally invoked the bind library. But as it became more complex and major users of go mobile binding move out of gomobile and the development focus was moved more on gobind itself, gomobile was changed to simply invoke gobind for easier code maintenance.

@eliasnaur
Copy link
Contributor

code maintenance

I assume this proposal is about merging the binaries, not the code. The bind code stays in a separate package.

@hajimehoshi
Copy link
Member

I assume this proposal is about merging the binaries, not the code. The bind code stays in a separate package.

ping @hyangah

@hyangah
Copy link
Contributor

hyangah commented Nov 20, 2019

cmd/gobind was originally capturing a bigger concept - providing a general binding facility beyond android/ios. (python, java for general use, etc.). cmd/gomobile assumes a particular code layout and a specific build tool setup (go, ndk, xcode) and provides an easy-to-access tool for beginners. Thus, I feel this proposal is a bit reversed (like merging compile code logic to go, etc).

  • What made it difficult to make the majority of the gobind logic a library and make the gomobile simply call the library instead of invoking the cmd/gobind binary, and the cmd/gobind binary simply wraps the library?

Once we can make the library (probably internal) and succeed to make cmd/gobind minimal, I think we will be in a better position to make this decision.

  • gobind and gomobile bind flags look different. can someone layout more detailed plans on merge those flags and avoid confusion?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mobile Android, iOS, and x/mobile NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

8 participants