[go: nahoru, domu]

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

Document the fact that desugar_jdk_libs is required for media3 to work on old API levels #1312

Closed
MGaetan89 opened this issue Apr 24, 2024 · 10 comments
Assignees
Labels

Comments

@MGaetan89
Copy link

Since 889f435, Media3 uses the desugar_jdk_libs library to use newer Java APIs on older API levels.
For example in Format:

|| (builder.labels.stream().anyMatch(l -> l.value.equals(builder.label))));

This means that applications/libraries using Media3 also need to integrate desugar_jdk_libs. However, this is not documented in the Java 8 support section of the Readme:
https://github.com/androidx/media/tree/release?tab=readme-ov-file#2-turn-on-java-8-support

Would it be possible to add a note about it?

@MGaetan89
Copy link
Author

By digging a bit more into this, here are all the newer API usages I could find in production code:

Since there aren't that many usages, maybe removing desugar_jdk_libs from production could also be a solution?

@tonihei
Copy link
Collaborator
tonihei commented May 2, 2024

Thanks for highlighting this issue!

We added the additional Java8 desugaring before releasing 1.3.0. Our assumption was that this automatically propagates to apps similar to other dependencies. This was unfortunately not the case because of a tooling gap (see https://issuetracker.google.com/203113147).

The tooling gap is now fixed and upgrading our AGP version ensures this requirement automatically propagates to apps depending on our libraries. However, it just triggers a build error if the desugaring is not activated in the app. This means it's not fully automatic and still requires every app developer to add these flags manually to their gradle files.

Given this additional effort and the small number of current usage sites within our code, we decided not to depend on the enhanced desugaring for now to avoid this additional app config burden. Only once other core AndroidX libraries require this as well, we probably reintroduce it for our libraries too in the future.

@MGaetan89
Copy link
Author

Given this additional effort and the small number of current usage sites within our code, we decided not to depend on the enhanced desugaring for now to avoid this additional app config burden. Only once other core AndroidX libraries require this as well, we probably reintroduce it for our libraries too in the future.

Will it be worked on internally, or are you open for contributions for this?

@tonihei
Copy link
Collaborator
tonihei commented May 7, 2024

Thanks for considering a contribution! We already have the internal code change in review though and it should appear here soon.

1 similar comment
@tonihei
Copy link
Collaborator
tonihei commented May 7, 2024

Thanks for considering a contribution! We already have the internal code change in review though and it should appear here soon.

copybara-service bot pushed a commit that referenced this issue May 7, 2024
The newer versions include a bugfix that automatically highlights
when our project requires enhanced Java 8 desugaring.

Issue: #1312
PiperOrigin-RevId: 631373018
copybara-service bot pushed a commit that referenced this issue May 7, 2024
This avoids that apps have to depend on this additional config

Issue: #1312
PiperOrigin-RevId: 631447767
@MGaetan89
Copy link
Author

Thanks @tonihei, I see the changes now.
Do you know if they'll land in 1.4.0-alpha02 (or any followup release)? Or will there be a 1.3.2 release?

@tonihei
Copy link
Collaborator
tonihei commented May 8, 2024

Most likely only in 1.4.0 pre-releases. So far we are not planning to do a 1.3.2.

(Closing the issue now as the original problem has been fixed/avoided)

@cadalt
Copy link
cadalt commented May 20, 2024

any advice/knowledge on what api level this affects back to? i see in the linked issue api 22. what's the latest api level that is affected?

@icbaker
Copy link
Collaborator
icbaker commented May 20, 2024

6ac60c6 removed the problematic usages. It looks like they were mostly introduced in API 24 (e.g. Collection.stream() and LongConsumer), so I think any API level before that is affected.

@cadalt
Copy link
cadalt commented May 20, 2024

@icbaker thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants