Skip to content

Conversation

@christopherfujino
Copy link
Contributor

@christopherfujino christopherfujino commented Nov 3, 2023

Per discord discussion, building an AAR out of a plugin project has not worked for years, so let's just disable the functionality. Context: #137564

@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Nov 3, 2023
@christopherfujino christopherfujino force-pushed the disable-flutter-build-aar-plugins branch from 1f2f769 to 918a62b Compare November 7, 2023 00:27
@christopherfujino christopherfujino marked this pull request as ready for review November 7, 2023 00:32
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 we have a test that tests that you can do this, so that test will probably need to be disabled

00:57 +611: test/commands.shard/permeable/build_aar_test.dart: flag parsing parses flags                                                                                                               
Error: AARs can only be built from modules.

@christopherfujino
Copy link
Contributor Author

Looks like we have a test that tests that you can do this, so that test will probably need to be disabled

00:57 +611: test/commands.shard/permeable/build_aar_test.dart: flag parsing parses flags                                                                                                               
Error: AARs can only be built from modules.

Thanks, let me investigate further what the test is actually testing :)

@christopherfujino
Copy link
Contributor Author

Looks like we have a test that tests that you can do this, so that test will probably need to be disabled

00:57 +611: test/commands.shard/permeable/build_aar_test.dart: flag parsing parses flags                                                                                                               
Error: AARs can only be built from modules.

Thanks, let me investigate further what the test is actually testing :)

Ahh, it was just verifying that if we build aar from a plugin, we send the right analytics event.

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 these two tests are also failing

testUsingContext('parses flags', () async {

Presumably because they aren't passing in --template=module to createProject - though its odd that they were passing, because at a first glance they also don't seem to be requesting a plugin template either?

@christopherfujino
Copy link
Contributor Author

Looks like these two tests are also failing

testUsingContext('parses flags', () async {

Presumably because they aren't passing in --template=module to createProject - though its odd that they were passing, because at a first glance they also don't seem to be requesting a plugin template either?

might be defaulting to an app template?

@gmackall
Copy link
Member

gmackall commented Nov 7, 2023

Looks like these two tests are also failing

testUsingContext('parses flags', () async {

Presumably because they aren't passing in --template=module to createProject - though its odd that they were passing, because at a first glance they also don't seem to be requesting a plugin template either?

might be defaulting to an app template?

But if they were defaulting to an app template, wouldn't they be failing before this change anyways?

@christopherfujino
Copy link
Contributor Author

Looks like these two tests are also failing

testUsingContext('parses flags', () async {

Presumably because they aren't passing in --template=module to createProject - though its odd that they were passing, because at a first glance they also don't seem to be requesting a plugin template either?

might be defaulting to an app template?

But if they were defaulting to an app template, wouldn't they be failing before this change anyways?

the tool exit that is failing is a new one I added

@christopherfujino
Copy link
Contributor Author

@gmackall i think this is (finally!) ready for review :)

@christopherfujino
Copy link
Contributor Author

Looks like these two tests are also failing

testUsingContext('parses flags', () async {

Presumably because they aren't passing in --template=module to createProject - though its odd that they were passing, because at a first glance they also don't seem to be requesting a plugin template either?

might be defaulting to an app template?

But if they were defaulting to an app template, wouldn't they be failing before this change anyways?

these tests are using a FakeAndroidBuilder, so they're not really running a gradle build.

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.

LGTM

@christopherfujino christopherfujino added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 8, 2023
@gmackall
Copy link
Member

gmackall commented Nov 8, 2023

Clicking update branch to hope it gets google testing unstuck

@auto-submit auto-submit bot merged commit a408c6d into flutter:master Nov 8, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 9, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 9, 2023
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Nov 9, 2023
flutter/flutter@4b4a1fe...f662150

2023-11-09 [email protected] Roll Packages from 94c7623 to b69f54e (1 revision) (flutter/flutter#138149)
2023-11-09 [email protected] prevent tool crash when `IntelliJValidatorOnMac` encounters an installation with a missing `CFBundleIdentifier` (flutter/flutter#138095)
2023-11-09 [email protected] Roll Flutter Engine from 233bd6263c62 to 8b490a9f1650 (1 revision) (flutter/flutter#138118)
2023-11-09 [email protected] Roll pub packages (flutter/flutter#138114)
2023-11-09 [email protected] Roll Flutter Engine from b3af5d64d3e6 to 233bd6263c62 (8 revisions) (flutter/flutter#138116)
2023-11-08 [email protected] Use specific version of mac_toolchain (flutter/flutter#138115)
2023-11-08 [email protected] [flutter_tools] disable flutter build AAR for plugins (flutter/flutter#137878)
2023-11-08 [email protected] Roll Flutter Engine from 21f055f7d8d0 to b3af5d64d3e6 (1 revision) (flutter/flutter#138113)
2023-11-08 [email protected] Roll Flutter Engine from 5306213d9d19 to 21f055f7d8d0 (3 revisions) (flutter/flutter#138111)
2023-11-08 [email protected] Remove physicalGeometry (flutter/flutter#138103)
2023-11-08 [email protected] [Android] Fix `FlutterTestRunner.java` deprecations (flutter/flutter#138093)
2023-11-08 [email protected] Remove fuchsia mac version (flutter/flutter#138101)
2023-11-08 [email protected] Roll Flutter Engine from 6dbcf8f13132 to 5306213d9d19 (4 revisions) (flutter/flutter#138108)
2023-11-08 [email protected] Marks Mac_android hot_mode_dev_cycle__benchmark to be flaky (flutter/flutter#138073)
2023-11-08 [email protected] Document additional cases  (flutter/flutter#137957)
2023-11-08 [email protected] Roll Flutter Engine from b0310da3254d to 6dbcf8f13132 (2 revisions) (flutter/flutter#138100)
2023-11-08 [email protected] Add support for color and color blendmode in FadeInImage (flutter/flutter#137681)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@christopherfujino christopherfujino deleted the disable-flutter-build-aar-plugins branch November 9, 2023 20:57
eliasyishak pushed a commit to eliasyishak/flutter that referenced this pull request Jan 5, 2024
Per discord discussion, building an AAR out of a plugin project has not worked for years, so let's just disable the functionality. Context: flutter#137564
eliasyishak added a commit to eliasyishak/flutter that referenced this pull request Jan 5, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants