Skip to content

Commit

Permalink
Copy resources.json for linking (#1177)
Browse files Browse the repository at this point in the history
The `resources.json` should be copied to the link outputs, so that it can be inspected, cached, and used as a dependency to determine if a link hook needs to rerun.

This adds it as a regular dependency for now - we probably rather want to have some custom logic later, which doesn't rerun the link hook if only things like line numbers change.

---

<details>
  <summary>Contribution guidelines:</summary><br>

- See our [contributor guide](https://github.com/dart-lang/.github/blob/main/CONTRIBUTING.md) for general expectations for PRs.
- Larger or significant changes should be discussed in an issue before creating a PR.
- Contributions to our repos should follow the [Dart style guide](https://dart.dev/guides/language/effective-dart) and use `dart format`.
- Most changes should add an entry to the changelog and may need to [rev the pubspec package version](https://github.com/dart-lang/sdk/wiki/External-Package-Maintenance#making-a-change).
- Changes to packages require [corresponding tests](https://github.com/dart-lang/.github/blob/main/CONTRIBUTING.md#Testing).

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.
</details>
  • Loading branch information
mosuem authored May 29, 2024
1 parent 7749425 commit 1549dc8
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 17 deletions.
1 change: 1 addition & 0 deletions pkgs/native_assets_builder/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
Link hooks are not ordered.
- Fix test.
- Bump `package:native_assets_cli` to 0.6.0.
- Copy `resources.json` to the build directory.

## 0.6.0

Expand Down
17 changes: 11 additions & 6 deletions pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -211,15 +211,23 @@ class NativeAssetsBuildRunner {
supportedAssetTypes: supportedAssetTypes,
hook: hook,
);
final outDirUri =
packageLayout.dartToolNativeAssetsBuilder.resolve('$buildDirName/out/');
final buildDirUri =
packageLayout.dartToolNativeAssetsBuilder.resolve('$buildDirName/');
final outDirUri = buildDirUri.resolve('out/');
final outDir = Directory.fromUri(outDirUri);
if (!await outDir.exists()) {
// TODO(https://dartbug.com/50565): Purge old or unused folders.
await outDir.create(recursive: true);
}

if (hook == Hook.link) {
File? resourcesFile;
if (resourceIdentifiers != null) {
resourcesFile = File.fromUri(buildDirUri.resolve('resources.json'));
await resourcesFile.create();
await File.fromUri(resourceIdentifiers).copy(resourcesFile.path);
}

return LinkConfigImpl(
outputDirectory: outDirUri,
packageName: package.name,
Expand All @@ -230,7 +238,7 @@ class NativeAssetsBuildRunner {
targetIOSSdk: targetIOSSdk,
cCompiler: cCompilerConfig,
targetAndroidNdkApi: targetAndroidNdkApi,
resourceIdentifierUri: resourceIdentifiers,
resourceIdentifierUri: resourcesFile?.uri,
assets: buildResult!.assetsForLinking[package.name] ?? [],
supportedAssetTypes: supportedAssetTypes,
linkModePreference: linkModePreference,
Expand Down Expand Up @@ -388,9 +396,6 @@ class NativeAssetsBuildRunner {
Uri? resources,
) async {
final outDir = config.outputDirectory;
if (!await Directory.fromUri(outDir).exists()) {
await Directory.fromUri(outDir).create(recursive: true);
}

final hookOutput = HookOutputImpl.readFromFile(file: config.outputFile);
if (hookOutput != null) {
Expand Down
2 changes: 2 additions & 0 deletions pkgs/native_assets_builder/test/build_runner/helpers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ Future<LinkResult> link(
List<String>? capturedLogs,
PackageLayout? packageLayout,
required BuildResult buildResult,
Uri? resourceIdentifiers,
}) async =>
await runWithLog(capturedLogs, () async {
final result = await NativeAssetsBuildRunner(
Expand All @@ -87,6 +88,7 @@ Future<LinkResult> link(
includeParentEnvironment: includeParentEnvironment,
packageLayout: packageLayout,
buildResult: buildResult,
resourceIdentifiers: resourceIdentifiers,
);

if (result.success) {
Expand Down
56 changes: 56 additions & 0 deletions pkgs/native_assets_builder/test/build_runner/resources_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'dart:io';

import 'package:test/test.dart';

import '../helpers.dart';
import 'helpers.dart';

const Timeout longTimeout = Timeout(Duration(minutes: 5));

void main() async {
test(
'simple_link linking',
timeout: longTimeout,
() async {
await inTempDir((tempUri) async {
await copyTestProjects(targetUri: tempUri);
final packageUri = tempUri.resolve('simple_link/');

final resourcesUri = tempUri.resolve('treeshaking_info.json');
await File.fromUri(resourcesUri).create();

// First, run `pub get`, we need pub to resolve our dependencies.
await runPubGet(
workingDirectory: packageUri,
logger: logger,
);

final buildResult = await build(
packageUri,
logger,
dartExecutable,
);

Iterable<String> buildFiles() => Directory.fromUri(
packageUri.resolve('.dart_tool/native_assets_builder/'))
.listSync(recursive: true)
.map((file) => file.path);

expect(buildFiles(), isNot(anyElement(endsWith('resources.json'))));

await link(
packageUri,
logger,
dartExecutable,
buildResult: buildResult,
resourceIdentifiers: resourcesUri,
);
expect(buildFiles(), anyElement(endsWith('resources.json')));
});
},
);
}
1 change: 1 addition & 0 deletions pkgs/native_assets_cli/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
## 0.6.1-wip

- Introduce `Builder` and `Linker` interface.
- Copy `resources.json` to the build directory.

## 0.6.0

Expand Down
20 changes: 9 additions & 11 deletions pkgs/native_assets_cli/lib/src/model/link_config.dart
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ class LinkConfigImpl extends HookConfigImpl implements LinkConfig {

// TODO: Placeholder for the resources.json file URL. We don't want to change
// native_assets_builder when implementing the parsing.
final Uri? _resourceIdentifierUri;
final Uri? resourceIdentifierUri;

LinkConfigImpl({
required this.assets,
Uri? resourceIdentifierUri,
this.resourceIdentifierUri,
required super.outputDirectory,
required super.packageName,
required super.packageRoot,
Expand All @@ -36,25 +36,23 @@ class LinkConfigImpl extends HookConfigImpl implements LinkConfig {
required super.targetOS,
required super.linkModePreference,
super.dryRun,
}) : _resourceIdentifierUri = resourceIdentifierUri,
super(
}) : super(
hook: Hook.link,
version: version ?? HookConfigImpl.latestVersion,
supportedAssetTypes: supportedAssetTypes ?? [NativeCodeAsset.type],
);

LinkConfigImpl.dryRun({
required this.assets,
Uri? resourceIdentifierUri,
this.resourceIdentifierUri,
required super.outputDirectory,
required super.packageName,
required super.packageRoot,
Version? version,
Iterable<String>? supportedAssetTypes,
required super.linkModePreference,
required super.targetOS,
}) : _resourceIdentifierUri = resourceIdentifierUri,
super.dryRun(
}) : super.dryRun(
hook: Hook.link,
version: version ?? HookConfigImpl.latestVersion,
supportedAssetTypes: supportedAssetTypes ?? [NativeCodeAsset.type],
Expand All @@ -72,8 +70,8 @@ class LinkConfigImpl extends HookConfigImpl implements LinkConfig {
@override
Map<String, Object> toJson() => {
...hookToJson(),
if (_resourceIdentifierUri != null)
resourceIdentifierKey: _resourceIdentifierUri.toFilePath(),
if (resourceIdentifierUri != null)
resourceIdentifierKey: resourceIdentifierUri!.toFilePath(),
assetsKey: AssetImpl.listToJson(assets, version),
}.sortOnKey();

Expand Down Expand Up @@ -129,7 +127,7 @@ class LinkConfigImpl extends HookConfigImpl implements LinkConfig {
if (other is! LinkConfigImpl) {
return false;
}
if (other._resourceIdentifierUri != _resourceIdentifierUri) {
if (other.resourceIdentifierUri != resourceIdentifierUri) {
return false;
}
if (!const DeepCollectionEquality().equals(other.assets, assets)) {
Expand All @@ -141,7 +139,7 @@ class LinkConfigImpl extends HookConfigImpl implements LinkConfig {
@override
int get hashCode => Object.hashAll([
super.hashCode,
_resourceIdentifierUri,
resourceIdentifierUri,
const DeepCollectionEquality().hash(assets),
]);

Expand Down

0 comments on commit 1549dc8

Please sign in to comment.