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

Update desugar_jdk_libs #11821

Closed
wants to merge 2 commits into from
Closed

Update desugar_jdk_libs #11821

wants to merge 2 commits into from

Conversation

menny
Copy link
Contributor

@menny menny commented Jul 22, 2020

@menny menny force-pushed the patch-1 branch 2 times, most recently from eacf242 to cb4c8bf Compare July 24, 2020 14:17
@jin jin requested a review from kevin1e100 July 27, 2020 03:06
@jin jin added the team-Android Issues for Android team label Jul 27, 2020
Copy link
Contributor

@kevin1e100 kevin1e100 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most likely tools/android/desugar.sh needs to be updated to account for changes/additions. Please have @donaldchai review this change as well.

@donaldchai
Copy link
Contributor

This is new to me, but based on a964125 it seems like you need to add any new classes to desugar.sh? google/desugar_jdk_libs@ab1edba desugars a new class not already listed.

@menny
Copy link
Contributor Author

menny commented Jul 30, 2020

oh.. this is a hard one to complete. I took a look at what Android Studio source code is specifying but it seems pretty much what you have right now here at HEAD.

@menny
Copy link
Contributor Author

menny commented Aug 3, 2020

I compared the commit currently in bazel to HEAD at the desugar repo and looking at the retarget details, and it seems that there aren't any new things that need to be added explicitly to the shell script.

@aiuto
Copy link
Contributor

aiuto commented Jan 7, 2021

ping.
Also, if you wait about a week, I'll have completed an overhaul to how we specify deps like this, so the PR will be entirely different, but much smaller (look at //distdir_deps.bzl)

@menny
Copy link
Contributor Author

menny commented Jan 7, 2021

@aiuto I don't mind waiting, but since this is a pretty small PR anyway I think it's okay as is. Your call.

@aiuto
Copy link
Contributor

aiuto commented Jan 7, 2021

I'm on triage duty this week, so I'll merge it. But first the tests have to pass.

@aiuto
Copy link
Contributor

aiuto commented Jan 12, 2021

See #12811. That does this in the new style. The tests should work now too, since I updated the mirror.

@bazel-io bazel-io closed this in f431b0c Jan 13, 2021
@aiuto
Copy link
Contributor

aiuto commented Jan 13, 2021

Sorry this took so long.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes team-Android Issues for Android team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants