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: android resource identifiers are hard-coded without relevant api info #13016

Closed
dskinner opened this issue Oct 21, 2015 · 7 comments

Comments

@dskinner
Copy link
Member

The current minimum target of gomobile for android is api-9. The gomobile cmd has hard coded resource identifiers to avoid requiring the android.jar from the sdk for that platform.

Resource identifiers that can not be extracted from an api-9 android.jar, gomobile's minimum target, should be excluded. Since resource identifiers are hard-coded but contain no information wrt the api level the identifier was introduced, this may lead to invalid manifests containing resource identifiers for an api level higher than the declared target api of a developers manifest. I'm unaware of the defined behavior in such a case, but this would normally lead to a warning or error in a java app.

It's unclear if this problem has been made aware. I think an additional comment in binary_xml.go is suitable, but also a test in binary_xml_test.go that actually inspects api-9 android.jar to assure a developer has not added an identifier that should not be supported by gomobile's minimum target.

But, excluding resource identifiers higher than gomobile's minimum target is the bare minimum. Does the gomobile cmd need to intentionally exclude other identifiers that simply don't make sense for a pure go app?

For example, while deliberating for an api to allow programmatically setting an app full-screen, there's little reason to not allow a developer to declare the full screen resource theme in their android manifest file. At the same time, there's little reason to add identifiers for other various themes that can be found in api-9's android.jar. What other cases may be excluded?

@crawshaw

@dskinner
Copy link
Member Author

@crawshaw @hyangah
It was mentioned on https://go-review.googlesource.com/#/c/16159/ that generating resource ids from R.java might be worth looking into. I'm actually not sure if R.java was meant literally (as I don't know where that'd be available in sdk, is it?) but each platform folder does contain an android.jar that has most everything that would be required, including the android/R.class file.

Writing a bare-bones class parser specific to cmd gomobile's needs would be ok I think, so that's one option.

Another option is to use aapt which is capable of printing out resource information from android.jar. See aapt l -a android.jar | grep resource for example. I consider this an option since aapt is already required to run tests but some details might have to be inferred. This would be simpler to parse but prone to issues if aapt decides to change it's output.

With that said, I just recalled an xml autocomplete plugin I had written a long a while back for android layouts. This was possible due resources in a platforms data/res folder in the sdk. I believe this contains all the information required for auto-generating identifiers and it's well structured xml.

For example:
The values/attrs_manifest.xml file contains all possible attributes available in a manifest. The theme attribute is listed as <attr name="theme" format="reference" />. The format attr tells us how to serialize when writing out. By convention, this also informs us that valid references are located in res/values/themes.xml which contains, for example, <style name="Theme.NoTitleBar.Fullscreen">. We can now reference values/public.xml for the id, <public type="style" name="Theme.NoTitleBar.Fullscreen" id="0x01030007" />

As another short example, in values/attrs_manifest.xml is <attr name="screenOrientation">.... This node has children of type <enum/>, e.g. <enum name="landscape" value="0" />.

The point is these xml resources can be used to not only auto-generate resource identifiers, but also determine during generation what valid values are. This would allow gomobile cmd to identify bad values during compile instead of finding out you have an invalid manifest when attempting to install the apk.

If there's interest in this approach, I can work on a CL.

@dskinner dskinner changed the title x/mobile/cmd/gomobile: android resource identifiers are hard-coded without relevant api info proposal: x/mobile/cmd/gomobile android resource identifiers are hard-coded without relevant api info Oct 23, 2015
@hyangah
Copy link
Contributor

hyangah commented Oct 23, 2015

I meant taking advantage of android.jar, not R.java (I don't know where I can find it either).
I am fine with either approach if it's about generating the code (hardcoded values).

I am not so sure about making the 'gomobile build' command depend on SDK yet. I think @crawshaw designed the gomobile tool to work even without android SDK present in the system.

@dskinner
Copy link
Member Author

To be clear, I was not suggesting gomobile build should depend on the sdk; rather, the sdk would be required to generate code used by gomobile build. The generated source would be required to build cmd gomobile and would be included in the repository.

The platform xml in the sdk appears, at least to me, to provide the most information about how everything connects. As far as I'm aware, the conventions and format used in these xml documents hasn't changed in 4 years (if ever).

Parsing files in the android.jar is another option but I didn't immediately see a method to retrieve additional connectivity for validating a manifest during gomobile build.

@rsc rsc added this to the Proposal milestone Oct 24, 2015
@rsc rsc changed the title proposal: x/mobile/cmd/gomobile android resource identifiers are hard-coded without relevant api info proposal: x/mobile/cmd/gomobile: android resource identifiers are hard-coded without relevant api info Oct 24, 2015
@rsc rsc added the Proposal label Oct 24, 2015
@dskinner
Copy link
Member Author

a minor update on this while working on #13036, I have not yet confirmed but pretty sure android.jar does contain all relevant info required for resolving resource identifiers. This would be stored in the binary resource table format.

If the binary xml implementation can, to a high degree, be proven to be correct, it may be much simpler to simply support the binary resource table format as well instead of creating and maintaining an independent resource resolver.

@rsc
Copy link
Contributor

rsc commented Oct 30, 2015

Is the title wrong? Are you really proposing to hard-code android resource identifiers without relevant api info? The title should say what you are proposing to do, not describe a bug. Thanks.

@dskinner
Copy link
Member Author

@rsc It was incorrect for me to rename this as a proposal and I should have filed that separately. I'll remove "proposal" from title and file a proper proposal that's meant to address this and #13036 which are interdependent.

@dskinner dskinner changed the title proposal: x/mobile/cmd/gomobile: android resource identifiers are hard-coded without relevant api info x/mobile/cmd/gomobile: android resource identifiers are hard-coded without relevant api info Oct 30, 2015
@dskinner
Copy link
Member Author

dskinner commented Mar 5, 2016

this has been addressed by #13109

@dskinner dskinner closed this as completed Mar 5, 2016
@golang golang locked and limited conversation to collaborators Mar 13, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants