Commit Graph

3 Commits

Author SHA1 Message Date
Martin Kustermann
abcba1fa7e
Reland "[flutter_tools] Cleanup of native asset related code (removes around 50% of the native asset related code) (#155430)" (#155745)
Changes to original CL: The code that issues an error on unsupported
operating system in the dry-run case was missing a case for iOS and
Android

Original CL description

  tl;dr Removes 50% (>1650 locs) of native asset related code in
  `packages/flutter_tools`

Before this PR the invocation of dart build/link/dry-run was implemented
  per OS. This lead to very large code duplication of almost identical,
  but slightly different code. It also led to similarly duplicated test
  code.

  Almost the entire dart build/link/dry-run implementation is identical
  across OSes. There's small variations:

- configuration of the build (e.g. android/macos/ios version, ios sdk,
...)
  - determining target locations & copying the final shared libraries

This PR unifies the implementation by reducing the code to basically two
  main functions:

    * `runFlutterSpecificDartBuild` which is responsible for
      - obtain flutter configuration
      - perform dart build (& link)
      - determine target location & install binaries

  * `runFlutterSpecificDartDryRunOnPlatforms` which is responsible for a
  similar (but not same):
      - obtain flutter configuration
      - perform dart dry run
      - determine target location

  these two functions will call out to helpers for the OS specific
  functionality:

* `_assetTargetLocationsForOS` for determining the location of the code
  assets

* `_copyNativeCodeAssetsForOS` for copying the code assets (and possibly
  overriting the install name, etc)

=> Since we get rid of the code duplication across OSes and have only a
  single code path for the build/link/dry-run, we can also remove the
  duplicated tests that were pretty much identical across OSes.

  We also harden the building code by adding asserts, e.g.

    * the dry fun functionality should never be used by `flutter test`

* the `build/native_assets/<os>/native_assets.yaml` should only be used
       by `flutter test` and the dry-run of `flutter run`

  => We change the tests to also comply with these invariants (so the
  tests are not testing things that cannot happen in reality)

  We also rename `{,Flutter}NativeAssetsBuildRunner` to disambiguate it
  from the `package:native_asset_builder`'s `NativeAssetsBuildRunner`.
2024-09-26 12:21:29 +02:00
auto-submit[bot]
4dfa688ec4
Reverts "[flutter_tools] Cleanup of native asset related code (removes around 50% of the native asset related code) (#155430)" (#155713)
Reverts: flutter/flutter#155430
Initiated by: eyebrowsoffire
Reason for reverting: Postsubmit failures closing the tree. See the following examples:

https://ci.chromium.org/ui/p/flutter/builders/prod/Mac_ios%20native_assets_ios/5738/overview
https://ci.chromium.org/ui/p/flutter/builders/prod/Mac_arm64_mokey%20native_assets_android/583/overview
https://ci.chromium.org/ui/p/flutter/builders/prod/Linux_pixel_7pro%20native_assets_android/4075/overview
https://ci.chromium.org/u
Original PR Author: mkustermann

Reviewed By: {bkonyi, dcharkes}

This change reverts the following previous change:
tl;dr Removes 50% (>1650 locs) of native asset related code in `packages/flutter_tools`

Before this PR the invocation of dart build/link/dry-run was implemented per OS. This lead to very large code duplication of almost identical, but sligthly different code. It also led to similarly duplicated test code.

Almost the entire dart build/link/dry-run implementation is identical across OSes. There's small variations:

  - configuration of the build (e.g. android/macos/ios version, ios sdk, ...)
  - determining target locations & copying the final shared libraries

This PR unifies the implementation by reducing the code to basically two main functions:

  * `runFlutterSpecificDartBuild` which is responsible for
    - obtain flutter configuration
    - perform dart build (& link)
    - determine target location & install binaries

  * `runFlutterSpecificDartDryRunOnPlatforms` which is responsible for a similar (but not same):
    - obtain flutter configuration
    - perform dart dry run
    - determine target location

these two functions will call out to helpers for the OS specific functionality:

  * `_assetTargetLocationsForOS` for determining the location of the code assets

  * `_copyNativeCodeAssetsForOS` for copying the code assets (and possibly overriting the install name, etc)

=> Since we get rid of the code duplication across OSes and have only a single code path for the build/link/dry-run, we can also remove the duplicated tests that were pretty much identical across OSes.

We also harden the building code by adding asserts, e.g.

  * the dry fun functionality should never be used by `flutter test`

  * the `build/native_assets/<os>/native_assets.yaml` should only be used by `flutter test` and the dry-run of `flutter run`

=> We change the tests to also comply with these invariants (so the tests are not testing things that cannot happen in reality)

We also rename `{,Flutter}NativeAssetsBuildRunner` to disambiguate it from the `package:native_asset_builder`'s `NativeAssetsBuildRunner`.

We also reorganize the main code to make it readable from top-down and make members private where they can be.
2024-09-25 21:46:22 +00:00
Martin Kustermann
621e7ef951
[flutter_tools] Cleanup of native asset related code (removes around 50% of the native asset related code) (#155430)
tl;dr Removes 50% (>1650 locs) of native asset related code in
`packages/flutter_tools`

Before this PR the invocation of dart build/link/dry-run was implemented
per OS. This lead to very large code duplication of almost identical,
but slightly different code. It also led to similarly duplicated test
code.

Almost the entire dart build/link/dry-run implementation is identical
across OSes. There's small variations:

- configuration of the build (e.g. android/macos/ios version, ios sdk, ...)
- determining target locations & copying the final shared libraries

This PR unifies the implementation by reducing the code to basically two
main functions:

  * `runFlutterSpecificDartBuild` which is responsible for
    - obtain flutter configuration
    - perform dart build (& link)
    - determine target location & install binaries

* `runFlutterSpecificDartDryRunOnPlatforms` which is responsible for a
similar (but not same):
    - obtain flutter configuration
    - perform dart dry run
    - determine target location

these two functions will call out to helpers for the OS specific
functionality:

* `_assetTargetLocationsForOS` for determining the location of the code
assets

* `_copyNativeCodeAssetsForOS` for copying the code assets (and possibly
overriting the install name, etc)

=> Since we get rid of the code duplication across OSes and have only a
single code path for the build/link/dry-run, we can also remove the
duplicated tests that were pretty much identical across OSes.

We also harden the building code by adding asserts, e.g.

  * the dry fun functionality should never be used by `flutter test`

  * the `build/native_assets/<os>/native_assets.yaml` should only be used
     by `flutter test` and the dry-run of `flutter run`

=> We change the tests to also comply with these invariants (so the
tests are not testing things that cannot happen in reality)

We also rename `{,Flutter}NativeAssetsBuildRunner` to disambiguate it
from the `package:native_asset_builder`'s `NativeAssetsBuildRunner`.
2024-09-25 22:50:43 +02:00