ndk-build: Add support for <queries> element
#259
Conversation
Adds support for <queries> element added to API level 30: https://developer.android.com/guide/topics/manifest/queries-element Note: <provider> elements contained with <queries> only require an android:authorities attribute however aapt does not seem recognize this and throw errors about missing android:name attribuites. Maybe ndk-build needs to update to use aapt2?
Applying cargo fmt to code.
ndk-build/src/manifest.rs
Outdated
| @@ -29,6 +29,9 @@ pub struct AndroidManifest { | |||
| #[serde(default)] | |||
| pub uses_permission: Vec<Permission>, | |||
|
|
|||
| #[serde(default)] | |||
| pub queries: Vec<Query>, | |||
There was a problem hiding this comment.
Can this element be specified multiple times?
There was a problem hiding this comment.
I assume not but i can't find any definitive claification in the docs, I'll change it to only one optional.
There was a problem hiding this comment.
I thought I had removed this comment again, it seems like this is possible.
Since you have a use case for <queries> and have presumably tested this PR, could you perhaps add this element twice (with "bogus" values) and see if aapt complains?
There was a problem hiding this comment.
@korejan Did you manage to test this, seeing as the current revision of this PR uses Option<Queries> instead of Vec<Queries>?
There was a problem hiding this comment.
If you mean test the change then yes, I'm using these features for my own project.
There was a problem hiding this comment.
Have you tested it with multiple Queries elements, specifically?
There was a problem hiding this comment.
No, it changed to an Option type, not the way around and that it is what I'm using.
There was a problem hiding this comment.
The request is to test that while it is a Vec, instead of assuming my question above implies that it does not, nor being confirmed by the lack of definitive documentation on the case.
There was a problem hiding this comment.
It initially started as a Vec and changed to an Option because I can't get any clarification on if more than 1 is correct, so when I say I tested the change I mean, the current change.
I'm getting a feed-up with all these constant request to change minor things, I've never had this experinice with PRs I made to other projects. I have more changes I'd like to submit a PR for but I can't because it depends on these changes as well. I'm not working on this project, I'm working on project that depends on this project, I don't have time to spend on investigating various things on it. If you don't want this PR then I'll just leave it in my fork.
There was a problem hiding this comment.
That's a bit harsh isn't it? I'm asking for you to undo and test a simple change, which you seem to have made on my request while I was really just asking because I have no idea either. This should be a trivial thing to test on your end.
I've never had this experinice with PRs I made to other projects
Maybe their level of quality assurance just isn't as high? I'm also mainly maintaining and contributing to this project in my free time, if I "just leave it" there's unlikely to be someone else with more time on their hands to review and approve your PR. And I don't want to merge in something that requires maintainer cleanup afterwards, nor incomplete/incorrect docs for end users. The only obvious solution is handling it in PR review.
|
Don't forget to document all the new elements in the |
Co-authored-by: Marijn Suijten <[email protected]>
Applying requested changes: * Fixes comment typos and better wording * Adds new elements to cargo-apk README * Minor type renaming * Changes manifest's `queries` member from a vec to singular option.
…oid-ndk-rs into add-queries-element
Applying suggested changes to documentation, adding some missing new elements.
MarijnS95
changed the title
<queries> element
MarijnS95
changed the title
<queries> element<queries> element
Adds support for
<queries>element added to API level 30: https://developer.android.com/guide/topics/manifest/queries-elementNote:
<provider>elements contained with<queries>only require an android:authorities attribute however aapt does not seem to understand this and throws errors about missing android:name attribuites so I have added an extra android:name attribute as a workaround. Maybe ndk-build needs to update to use aapt2?Also note there are suppose to be restrictions on what child elements and attributes for those are usuable for
<intent>elements within an<queries>element but this change only reueses the same structs used by<intent-filter>.