-
Notifications
You must be signed in to change notification settings - Fork 2k
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
[bug] azure xml 1.2.0 depends on javax.xml.stream which is not available on android making core and identity not usable on android #44078
Comments
@alzimmermsft can you work with @baywet to unblock MSGraph? |
Thanks for reporting this @baywet! I'll work on a possible solution for this issue, but I don't have a good setup for Android verification. Would it be possible for you to help with verifying the solution I create? |
I believe part of your fix should include adding some CI to ensure there are no other regressions like these in the future. The way we're doing this is through an android project which runs android api level validation (see the plugins) Let me know if you have any additional comments or questions. |
Without changing code I was able to get |
Thanks for the suggestion, I've logged this issue, we'll revert once the changes are implemented. |
Also, after performing further investigation into this, I found that the Microsoft Graph's build had suppressed an Android build warning about I'm going to leave this issue pending with the statement that <dependency>
<groupId>javax.xml.stream</groupId>
<artifactId>stax-api</artifactId>
<version>1.0</version>
</dependency> or
Is added into the application to import the missing class files. |
Thanks for the additional information. |
Hi folks. The |
@alzimmermsft while looking into that further, I realized that the dependency you suggested was deprecated in favor of this one (source) @JonathanGiles while I understand your team is not looking to expand their support burden with android applications, I think this issue outlines a broader gap: If android is not supported, this should probably be mentioned somewhere. Both to set customers expectations, and so other teams depending on this library can provide a rationale as to why they don't support it themselves. If android is indeed supported, having CI to prevent regressions like this one would go a long way to provide a reliable experience to anyone who takes a dependency on libraries here. Or at the very least, a list of "additional things that are needed for android" is maintained somewhere for reference. It seems abnormal to me that as a consumer of a library, I'd need to add additional dependencies for my direct dependencies, keeping the list and versions in sync would probably be a huge pain. Other libraries are usually self sustaining (they bring everything they need) and/or provide variants (e.g. google guava with the -android variants). As for the size argument here, I'm not sure optimizing for performance is relevant when functionality is broken. Especially when you consider that xml is a dependency of core, which is pretty much a dependency of everything here as far as I can tell. In a server/desktop context, we're talking about a 127kb jar, in a mobile application, this can seem like a lot, until you remember the android SDK does trimming of the application binaries anyway. |
Hi,
Your friends from Microsoft Graph here 👋
Our CI recently started failing with azure identity 1.55.0 providing the following error. Version 1.54.1 was working perfectly fine.
This is because azure identity 1.55.0 updates its dependency on xml from 1.1.0 to 1.2.0
This version has a dependency on javax.xml.stream which is not available on Android.
This dependency was introduced by @alzimmermsft with PR #43282
This represents a pretty big regression in my views, and that change should either be amended not to depend on this package, or reverted, or modularized as a variant so it's not necessary to have it as a direct dependency.
Here is the failing PR, which also gives you access to our entire repository configuration.
CC @JonathanGiles the author of the original xml implementation issue.
The text was updated successfully, but these errors were encountered: