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

Remove unused flag `--target-platform` from `flutter run` #34369

Merged
merged 2 commits into from Jun 13, 2019

Conversation

@blasten
Copy link
Contributor

blasten commented Jun 13, 2019

Description

The target platform is inferred from the device or simulator, so the --target-platform doesn't change anything.

Tests

I added the following tests:

n/a

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require Flutter developers to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (Please read Handling breaking changes). Replace this with a link to the e-mail where you asked for input on this proposed change.
  • No, this is not a breaking change.
@googlebot googlebot added the cla: yes label Jun 13, 2019
@blasten blasten requested review from dnfield and jason-simmons Jun 13, 2019
@blasten blasten force-pushed the blasten:remove_run_target_platform branch from 373bed9 to be78242 Jun 13, 2019
@dnfield

This comment has been minimized.

Copy link
Member

dnfield commented Jun 13, 2019

Does this mean flutter run no longer supports running a 32 bit APK on a 64 bit device?

I guess that's supposed to not work soon anyway, right? Or at least be strongly discouraged? And someone could just build the APK and manually install it.

Copy link
Member

dnfield left a comment

LGTM

@Piinks Piinks added the tool label Jun 13, 2019
@blasten

This comment has been minimized.

Copy link
Contributor Author

blasten commented Jun 13, 2019

Correct. If they need to build an APK that doesn't match the device arch, they can always build it using build apk

@blasten blasten merged commit 2488c38 into flutter:master Jun 13, 2019
37 checks passed
37 checks passed
WIP Ready for review
Details
add2app-macos Task Summary
Details
add2app-macos
Details
analyze Task Summary
Details
analyze
Details
build_tests-linux Task Summary
Details
build_tests-linux
Details
build_tests-macos Task Summary
Details
build_tests-macos
Details
build_tests-windows Task Summary
Details
build_tests-windows
Details
cla/google All necessary CLAs are signed
deploy_gallery Task Summary
Details
deploy_gallery
Details
deploy_gallery-macos Task Summary
Details
deploy_gallery-macos
Details
docs Task Summary
Details
docs
Details
flutter-build
Details
integration_tests-linux Task Summary
Details
integration_tests-linux
Details
integration_tests-windows Task Summary
Details
integration_tests-windows
Details
tests-linux Task Summary
Details
tests-linux
Details
tests-macos Task Summary
Details
tests-macos
Details
tests-windows Task Summary
Details
tests-windows
Details
tool_tests-linux Task Summary
Details
tool_tests-linux
Details
tool_tests-macos Task Summary
Details
tool_tests-macos
Details
tool_tests-windows Task Summary
Details
tool_tests-windows
Details
web_tests-linux Task Summary
Details
web_tests-linux
Details
@blasten blasten deleted the blasten:remove_run_target_platform branch Jun 13, 2019
Kiku-Reise added a commit to Kiku-Reise/flutter that referenced this pull request Jun 14, 2019
tango5614 added a commit to tango5614/flutter that referenced this pull request Jun 14, 2019
* master: (24 commits)
  [flutter_tool,fuchsia] Update the install flow for packaging migration. (flutter#34447)
  SliverFillRemaining flag for different use cases (flutter#33627)
  SizedBox documentation (flutter#34424)
  Change API doc link to api.dart.dev (flutter#34388)
  2589785 Roll src/third_party/skia 87e885038893..c3252a04b377 (3 commits) (flutter/engine#9327) (flutter#34484)
  ace5d59 Fix rawTypes errors in Android embedding classes (flutter/engine#9326) (flutter#34481)
  bf0def6 Roll src/third_party/skia 4c4945a25248..87e885038893 (1 commits) (flutter/engine#9325) (flutter#34471)
  Roll engine f1d821d..6f5347c (13 commits) (flutter#34466)
  Allow "from" hero state to survive hero animation in a push transition (flutter#32842)
  Roll pub dependencies (flutter#33677)
  Skip flaky test on Windows (flutter#34464)
  Allow flaky tests to pass or fail and mark web tests as flaky (flutter#34456)
  Dont depend on web SDK unless running tests on chrome (flutter#34457)
  Fix semantics_tester (flutter#34368)
  Include raw value in Diagnostics json for basic types (flutter#34417)
  Refactor Gradle plugin (flutter#34353)
  Allow web tests to fail in cirrus config (flutter#34436)
  skip bottom_sheet (flutter#34430)
  Remove unused flag `--target-platform` from `flutter run` (flutter#34369)
  Extract DiagnosticsNode serializer from WidgetInspector (flutter#34012)
  ...
@ymback

This comment has been minimized.

Copy link

ymback commented Jun 21, 2019

So I have a situation, flutter cannot add libflutter.so to armeabi-v7a automatically when using studio without --target-platform=android-arm because it declared that my Phone is x64, but some libraries don't supply arm64 so files, so without --target-platform=android-arm, arm64 only have libflutter.so, app crash.
So is there a solution to fix my situation.

@jonahwilliams

This comment has been minimized.

Copy link
Contributor

jonahwilliams commented Jun 28, 2019

@blasten

This comment has been minimized.

Copy link
Contributor Author

blasten commented Jun 28, 2019

@ymback Just to confirm, are you using 3P libraries that don’t provide an arm64 equivalent?

@ymback

This comment has been minimized.

Copy link

ymback commented Jun 28, 2019

Yes, 3P libraries only provice armeabi-v7a.
I finally found this commit 8627ff4#diff-e2bd341219c8e758eb91659fe88dbeb5R463, in gradle.dart file line 463, it sets the target-platform to android-arm64 forcely, ignore my argument --target-platform android-arm after I readd the option target-platform which you removed and recreate flutter_tools.snapshot file.
And I tried to force set -Ptarget-platform=android-arm in gradle.dart, after recreate the flutter_tools.snapshot file, everything works fine, no crash.

@ymback

This comment has been minimized.

Copy link

ymback commented Jun 28, 2019

I think the gradle.dart file should check if it's from flutter run, and use different plan to generate target-platform.

@jason-simmons

This comment has been minimized.

Copy link
Contributor

jason-simmons commented Jun 28, 2019

If you need to build a 32-bit binary and run it on a 64-bit device, then try using the flutter run --use-application-binary flag to specify an APK built for a 32-bit target:

flutter build apk --debug --target-platform=android-arm
flutter run --use-application-binary=build/app/outputs/apk/debug/app-debug.apk
@ymback

This comment has been minimized.

Copy link

ymback commented Jul 1, 2019

If you need to build a 32-bit binary and run it on a 64-bit device, then try using the flutter run --use-application-binary flag to specify an APK built for a 32-bit target:

flutter build apk --debug --target-platform=android-arm
flutter run --use-application-binary=build/app/outputs/apk/debug/app-debug.apk

Thouth I don't think it's a good solution cause I have to run in command first, but it's useful enough, thank you, I'll do like this.

@zhihesoft

This comment has been minimized.

Copy link

zhihesoft commented Jul 10, 2019

remove --target-platform flag makes debug in vscode very inconvenient while with 32-bit 3P lib.

@shingohu

This comment has been minimized.

Copy link

shingohu commented Jul 16, 2019

the same situation

So I have a situation, flutter cannot add libflutter.so to armeabi-v7a automatically when using studio without --target-platform=android-arm because it declared that my Phone is x64, but some libraries don't supply arm64 so files, so without --target-platform=android-arm, arm64 only have libflutter.so, app crash.
So is there a solution to fix my situation.

@buaashuai

This comment has been minimized.

Copy link

buaashuai commented Jul 27, 2019

1、edit the flutter/packages/flutter_tools/lib/src/android/gradle.dart file and set -Ptarget-platform=android-arm
2、delete flutter/bin/cache/flutter_tools.snapshot

Then your Android studio can run armeabi-v7a apk on the 64-bit developing device.

@CarGuo

This comment has been minimized.

Copy link

CarGuo commented Jul 30, 2019

remove --target-platform flag makes debug very inconvenient while with 32-bit 3P lib.

@hurshi

This comment has been minimized.

Copy link

hurshi commented Aug 3, 2019

Here is a solution for debug mode: puts the libflutter.so into jniLibs:
image
and you can happy flutter run, or click "Run" in AndroidStudio

@chenenyu

This comment has been minimized.

Copy link

chenenyu commented Sep 16, 2019

Plz restore this flag.

@liugangnhm

This comment was marked as off-topic.

Copy link

liugangnhm commented Oct 22, 2019

most useless pr

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

Successfully merging this pull request may close these issues.

None yet

You can’t perform that action at this time.