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

Comments

@anjmao
Copy link

@anjmao 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 label Oct 5, 2019
@hajimehoshi

This comment has been minimized.

Copy link
Contributor

@hajimehoshi 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

This comment has been minimized.

Copy link
Author

@anjmao 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

This comment has been minimized.

Copy link
Contributor

@hajimehoshi hajimehoshi commented Oct 7, 2019

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

This comment has been minimized.

Copy link
Author

@anjmao 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.

@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Oct 9, 2019

@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Oct 30, 2019

ping @hyangah and @eliasnaur again. Any thoughts?

@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Oct 30, 2019

/cc @steeve

@eliasnaur

This comment has been minimized.

Copy link
Contributor

@eliasnaur eliasnaur commented Oct 30, 2019

SGTM.

@steeve

This comment has been minimized.

Copy link
Contributor

@steeve 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

This comment has been minimized.

Copy link
Contributor

@hyangah 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

This comment has been minimized.

Copy link
Contributor

@eliasnaur eliasnaur commented Oct 30, 2019

code maintenance

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

@hajimehoshi

This comment has been minimized.

Copy link
Contributor

@hajimehoshi hajimehoshi commented Nov 20, 2019

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

ping @hyangah

@hyangah

This comment has been minimized.

Copy link
Contributor

@hyangah 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
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
8 participants
You can’t perform that action at this time.