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

[native_assets_cli] iOS/MacOS privacy manifest #878

Open
dcharkes opened this issue Jan 4, 2024 · 5 comments
Open

[native_assets_cli] iOS/MacOS privacy manifest #878

dcharkes opened this issue Jan 4, 2024 · 5 comments
Labels
P2 A bug or feature request we're likely to work on package:native_assets_cli

Comments

@dcharkes
Copy link
Collaborator

dcharkes commented Jan 4, 2024

When using something like the camera on iOS/MacOS, the dylib needs to be bundled with a privacy manifest about why the permission is needed.

We need some way to communicate a privacy manifest from build.dart to Flutter.

This could possibly be a new asset type.

We need to check if we need a privacy manifest per dylib or just per Dart package.

@dcharkes dcharkes added the P1 A high priority bug; for example, a single project is unusable or has many test failures label Jun 18, 2024
@dcharkes
Copy link
Collaborator Author

dcharkes commented Jul 11, 2024

Discussing with @mosuem we have multiple options that we can pursue with adding new asset types that are connected to existing asset types:

API options

1. NativeCodeAsset.privacyManifest Uri? field

Introduce a NativeCodeAsset.privacyManifest Uri? field

output.addAssets([
  NativeCodeAsset(
    package: 'native_add',
    name: 'native_add',
    linkMode: DynamicLoadingBundled(),
    os: OS.current,
    architecture: Architecture.current,
    file: builtDylibUri,
    privacyManifestFile:
      (OS.current == OS.macOS || OS.current == OS.iOS) ?
      config.packageRoot.resolve('assets/PrivacyInfo.xcprivacy') :
      null,
  ),
]);
  • Pro: concise
  • Con: doesn't communicate that privacy manifests are not supported on various embedders. (We have BuildConfig.supportedAssetTypes, but it's not used this way.)
  • Con: pollutes the NativeCodeAsset API for other OSes that don't have privacy manifests.

2. PrivacyManifestAsset with dylib field

Introduce a PrivacyManifestAsset which implements Asset and has a field dylib which points to a NativeCodeAsset.

(Note the class should not implement/extend DataAsset because the bytes should not be bundled in Dart/Flutter apps and accessible with data assets at runtime.)

output.addAssets([
  NativeCodeAsset(
    package: 'native_add',
    name: 'native_add',
    linkMode: DynamicLoadingBundled(),
    os: OS.current,
    architecture: Architecture.current,
    file: builtDylibUri,
  ),
  if (OS.current == OS.macOS || OS.current == OS.iOS)
    PrivacyManifestAsset(
      // Asset IDs are mandatory, and required to be unique at the moment.
      package: 'native_add',
      name: 'native_add/PrivacyInfo.xcprivacy',
      // Together with `package` defines the asset ID that this privacy manifest belongs to.
      nativeCodeAssetName: 'native_add',
      file: config.packageRoot.resolve('assets/PrivacyInfo.xcprivacy'),
    ),
]);
  • pro: completely separate from NativeCodeAsset, no API pollution.
  • pro: whether it's supported can be expressed with BuildConfig.supportedAssetTypes.
  • con: Embedders need to first scan all assets for bundling for PrivacyManifest and NativeCodeAssets to connect the ones that belong together.
  • con: verbose in the API because the PrivacyManifest needs to repeat the NativeCodeAsset asset name/id.

3. Asset.metadata

Introduce Asset.metadata List<Asset> field. (Name to be discussed)
This avoids the need for a PrivacyManifestAsset.dylib field as the PrivacyManifestAsset can be nested in the dylib

output.addAssets([
  NativeCodeAsset(
    package: 'native_add',
    name: 'native_add',
    linkMode: DynamicLoadingBundled(),
    os: OS.current,
    architecture: Architecture.current,
    file: builtDylibUri,
    metadata: [
      if (OS.current == OS.macOS || OS.current == OS.iOS)
        PrivacyManifestAsset(
          // Asset IDs are mandatory, and required to be unique at the moment.
          package: 'native_add',
          name: 'native_add/PrivacyInfo.xcprivacy',
          file: config.packageRoot.resolve('assets/native_add/PrivacyInfo.xcprivacy'),
        ),
    ],
  ),
]);
  • Pro: connection between dylib and privacy manifest is more obvious.
  • Pro: can still be controlled by BuildConfig.supportedAssetTypes.
  • Con: Asset.children is quite generic, it might look like arbitrary children are supported in arbitrary asset types.
  • Con: more complexity in the API and concepts.

@mosuem I think I'm leaning towards option 2 now as well.

Class location

We could opt to not specify this asset type in package:hook/package:native_assets_cli. It would enable us to battle-test the extensibility of the protocol.

However, I would also be weary of introducing a new package for every new asset type. It would create churn specifying all the imports for users. (We can't be putting it in package:hook_flutter because once we start supporting it in Dart or another embedder it would have to be moved, and moving classes between packages is a real pain, I tried it once.)

@dcharkes
Copy link
Collaborator Author

dcharkes commented Jul 11, 2024

cc @HosseinYousefi as well for discussion on how to deal with "assets-that-belong-to-other-assets" or "assets-that-augment-other-assets". ProguardRuleAssets augment JarAssets later on.

@mosuem
Copy link
Member

mosuem commented Jul 11, 2024

Option2, but in it's own package.

I would try to keep things simple, and have a new asset type for the PrivacyManifestAsset defined in package:privacy_manifest_asset, which also exposes helper methods for creating the asset. Then all that has to be done is implement support for PrivacyManifestAsset in Flutter.

class PrivacyManifestAsset implements Asset{
  final Sting nativeCodeAssetId;
  ...
}

Asset buildPrivacyManifest(NativeCodeAsset library, PrivacyManifestOptions options) { ... }

It would create churn specifying all the imports for users.

What do you mean?

@HosseinYousefi
Copy link
Member

HosseinYousefi commented Jul 11, 2024

In the most general form, the thing we're building could be a graph, not a tree. So using children-like API will not scale. So I like option 2 as well.

and moving classes between packages is a real pain

What if hook_flutter has a dependency on the privacy_manifest_asset but just exports it. The user will transitively depend on privacy_manifest_asset and won't have to import another thing. Later on if the same thing is available on dart standalone, hook itself can export the same class. Would this work?

@dcharkes dcharkes added P2 A bug or feature request we're likely to work on and removed P1 A high priority bug; for example, a single project is unusable or has many test failures labels Jul 15, 2024
@dcharkes
Copy link
Collaborator Author

Note from discussion with @mkustermann

  • Privacy manifests belong to a NativeCodeAssets. So that leans towards option 1. This not only important for embedders consuming assets, but also for if you're writing a link hook helper package that links native code static libs together.

Deprioritizing because the list of required reason APIs is quite small.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 A bug or feature request we're likely to work on package:native_assets_cli
Projects
None yet
Development

No branches or pull requests

3 participants