Skip to content

regenerate gradle lockfiles & roll pub packages #145564

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.

Sign up for GitHub

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

Jump to bottom
Closed
christopherfujino wants to merge 3 commits into from
Closed

regenerate gradle lockfiles & roll pub packages #145564

christopherfujino wants to merge 3 commits into from

Conversation

@christopherfujino
Copy link
Contributor

@christopherfujino christopherfujino commented Mar 21, 2024

@christopherfujino christopherfujino changed the title regenerate gradle lockfiles regenerate gradle lockfiles & roll pub packages Mar 21, 2024
@github-actions github-actions bot added a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels. f: integration_test The flutter/packages/integration_test plugin labels Mar 21, 2024
@christopherfujino
Copy link
Contributor Author

christopherfujino commented Mar 21, 2024

This previously landed in #145509 but was reverted in #145550

flutter-pub-roller-bot and others added 2 commits March 21, 2024 13:50
@flutter-pub-roller-bot @christopherfujino
Roll pub packages (flutter#145509)
cf3ac99 
This PR was generated by `flutter update-packages --force-upgrade`.
@christopherfujino
regenerate gradle lockfiles
3bb813f
@christopherfujino christopherfujino force-pushed the roll-pub-packages-and-fix-regenerate-gradle-lockfiles branch from 8945eeb to 3bb813f Compare March 21, 2024 20:52
christopherfujino
christopherfujino commented Mar 21, 2024
View reviewed changes
dev/benchmarks/test_apps/stocks/lib/i18n/stock_strings.dart Outdated
/// Applications need to include `StockStrings.delegate()` in their app's
/// localizationDelegates list, and the locales they support in the app's
/// supportedLocales list. For example:
/// `localizationDelegates` list, and the locales they support in the app's
Copy link
Contributor Author

@christopherfujino christopherfujino Mar 21, 2024

Choose a reason for hiding this comment

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

@gmackall Does the regenerate gradle lockfiles script change dartdocs?

Copy link
Contributor Author

@christopherfujino christopherfujino Mar 21, 2024

Choose a reason for hiding this comment

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

Hmm, these diffs also broke analysis:

Analyzing flutter...                                            

   info • The expression has no effect and can be removed • dev/benchmarks/test_apps/stocks/lib/i18n/stock_strings.dart:69:83 • noop_primitive_operations
   info • Convert 'locale' to a super parameter • dev/benchmarks/test_apps/stocks/lib/i18n/stock_strings_en.dart:12:3 • use_super_parameters
   info • Convert 'locale' to a super parameter • dev/benchmarks/test_apps/stocks/lib/i18n/stock_strings_es.dart:12:3 • use_super_parameters

Copy link
Contributor Author

@christopherfujino christopherfujino Mar 21, 2024

Choose a reason for hiding this comment

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

This happened before: https://github.com/flutter/flutter/pull/137190/files#r1371888460

It's unclear to me how that script would touch dart files, though...

Copy link
Contributor Author

@christopherfujino christopherfujino Mar 21, 2024

Choose a reason for hiding this comment

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

Oh, we're calling flutter pub get, which WILL do localization code generation

Copy link
Contributor Author

@christopherfujino christopherfujino Mar 21, 2024

Choose a reason for hiding this comment

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

I guess I should fix analysis and check it in

@gmackall
Copy link
Member

gmackall commented Mar 21, 2024 •
edited
Loading

The generate_gradle_lockfiles.dart script is poorly named, as it also re-generates all the gradle files. That is going to make one of the tests here fail deprecated_gradle_settings_test.

I think the answer isn't to change the test but to add a flag to the script to allow running it in a mode where we don't re-generate the gradle files themselves (I don't see a reason why the pub roller bot should need to re generate the gradle files themselves for each roll). I'll put up a pr to add that mode

@gmackall gmackall mentioned this pull request Mar 21, 2024
Merged
9 tasks
auto-submit bot pushed a commit that referenced this pull request Mar 21, 2024
@gmackall
Add a --no-gradle-generation mode to the `generate_gradle_lockfiles…
3b390c5 
….dart` script (#145568)

The script currently overwrites existing `settings.gradle`, `build.gradle`, and `gradle-wrapper.properties` files in the directories it processes. This mode makes it not do that, and just generate the lockfiles themselves.

Related to #145564 (comment)
@github-actions github-actions bot removed a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels. f: integration_test The flutter/packages/integration_test plugin labels Mar 22, 2024
gmackall
gmackall reviewed Mar 22, 2024
View reviewed changes
Copy link
Member

@gmackall gmackall left a comment

Choose a reason for hiding this comment

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

Looks like the gradle_deprecated_settings test is still failing. I think the issue here is that generate gradle files -> generate lockfiles -> revert grade file generation is not exactly the same as just generate lockfiles.

I was able to reproduce the error with those steps, but it built successfully if I instead started on a new branch and just did flutter update-packages --force-upgrade followed by running the lockfile generation script with the --no-gradle-generation flag

@christopherfujino
Copy link
Contributor Author

christopherfujino commented Mar 22, 2024

Looks like the gradle_deprecated_settings test is still failing. I think the issue here is that generate gradle files -> generate lockfiles -> revert grade file generation is not exactly the same as just generate lockfiles.

I was able to reproduce the error with those steps, but it built successfully if I instead started on a new branch and just did flutter update-packages --force-upgrade followed by running the lockfile generation script with the --no-gradle-generation flag

Thanks

@christopherfujino christopherfujino mentioned this pull request Mar 25, 2024
Merged
@christopherfujino
Copy link
Contributor Author

christopherfujino commented Mar 26, 2024

Superceded by: #145727

auto-submit bot pushed a commit that referenced this pull request Mar 26, 2024
@christopherfujino
Roll pub packages and regenerate gradle lockfiles (#145727)
385bec0 
Example failing devicelab build https://ci.chromium.org/ui/p/flutter/builders/prod/Linux_pixel_7pro%20platform_views_scroll_perf__timeline_summary/2980/overview

Fixes #145561

Supercedes #145564
@gmackall gmackall mentioned this pull request May 30, 2024
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gmackall gmackall gmackall left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Make generate_gradle_lockfiles.dart script check exit code of flutter build apk --config-only

3 participants

@christopherfujino @gmackall @flutter-pub-roller-bot