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

Migrate androidx.window to 1.1.0 stable #52474

Closed
wants to merge 1 commit into from

Conversation

kchyn3
Copy link

@kchyn3 kchyn3 commented Apr 30, 2024

Updates the androidx.window dependency from 1.1.0-beta04 to 1.1.0-stable.

Note that since androidx.window is a maven dependency (and not a CIPD dependency), step 7 and 8 of the android_embedding_bundle/README.md were skipped.

Issue: 129307
Test: et build -c android_debug_arm64

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • test-exempt I added new tests to check the change I am making or feature I am adding, or the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note that since androidx.window is a maven dependency (and not a
CIPD dependency), step 7 and 8 of the android_embedding_bundle/README.md
were skipped.

Bug: 314377438
Test: et build -c android_debug_arm64
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@reidbaker
Copy link
Contributor

@kchyn3 it looks like the android tests are all failing.

@chinmaygarde
Copy link
Member

@kchyn3 Any updates on the failing tests?

@gmackall
Copy link
Member

gmackall commented May 23, 2024

FYI this will be covered in #53001

@kchyn3
Copy link
Author

kchyn3 commented May 24, 2024

I think the tests are failing because I don't have access to update CIPD deps in steps 7-9 here, last time when I checked the logs looks like it couldn't find the new dependencies.

https://github.com/flutter/engine/blob/main/tools/cipd/android_embedding_bundle/README.md#steps

Thanks for the heads up, I'll abandon this change.

@kchyn3 kchyn3 closed this May 24, 2024
@reidbaker
Copy link
Contributor

@kchyn3 I thought we found out that you did not need to update cipd to modify androidx.window since it was downloaded from maven.

@reidbaker reidbaker reopened this May 28, 2024
@reidbaker reidbaker closed this May 28, 2024
@kchyn3
Copy link
Author

kchyn3 commented May 28, 2024

That's what we initially thought. But I chatted with Jonah a bit on discord and it looks like the presubmit runs couldn't find the new artifacts so we are assuming that the command does in fact need to be run 🤷

@gmackall
Copy link
Member

gmackall commented May 28, 2024

I would guess that the confusion here is that the dependencies in files.json do get downloaded from maven, but that file is just an input to the tool at https://github.com/flutter/engine/tree/main/tools/cipd/android_embedding_bundle which is used to make the bundle of dependencies that end up in src/third_party/android_embedding_dependencies

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