Skip to content

Commit e5d926d

Browse files
dcharkesCommit Queue
authored andcommitted
[native assets] Use workspace pubspec for user-defines
Native assets are cached for the whole pub workspace. So to avoid invalidating caches for running hooks from different root packages, only read user-defines from the workspace `pubspec.yaml`. Bug: dart-lang/native#39 Bug: dart-lang/native#2187 Change-Id: I3aeb91455a418004d3e28c231dc1f5d002c15739 Cq-Include-Trybots: luci.dart.try:pkg-linux-debug-try,pkg-linux-release-arm64-try,pkg-linux-release-try,pkg-mac-release-arm64-try,pkg-win-release-arm64-try,pkg-mac-release-try,pkg-win-release-try Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/422101 Auto-Submit: Daco Harkes <[email protected]> Reviewed-by: Hossein Yousefi <[email protected]> Commit-Queue: Daco Harkes <[email protected]>
1 parent 423479c commit e5d926d

File tree

9 files changed

+120
-88
lines changed

9 files changed

+120
-88
lines changed

pkg/dartdev/lib/src/commands/build.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,8 @@ class BuildCommand extends DartdevCommand {
135135
final runPackageName = await DartNativeAssetsBuilder.findRootPackageName(
136136
sourceUri,
137137
);
138-
final pubspecUri = await DartNativeAssetsBuilder.findPubspec(sourceUri);
138+
final pubspecUri =
139+
await DartNativeAssetsBuilder.findWorkspacePubspec(packageConfig);
139140
final Map? pubspec;
140141
if (pubspecUri == null) {
141142
pubspec = null;

pkg/dartdev/lib/src/commands/compile.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -630,7 +630,7 @@ Remove debugging information from the output and save it separately to the speci
630630
);
631631
if (runPackageName != null) {
632632
final pubspecUri =
633-
await DartNativeAssetsBuilder.findPubspec(Directory.current.uri);
633+
await DartNativeAssetsBuilder.findWorkspacePubspec(packageConfig);
634634
final Map? pubspec;
635635
if (pubspecUri == null) {
636636
pubspec = null;

pkg/dartdev/lib/src/commands/run.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,7 @@ class RunCommand extends DartdevCommand {
393393
);
394394
if (runPackageName != null) {
395395
final pubspecUri =
396-
await DartNativeAssetsBuilder.findPubspec(Directory.current.uri);
396+
await DartNativeAssetsBuilder.findWorkspacePubspec(packageConfig);
397397
final Map? pubspec;
398398
if (pubspecUri == null) {
399399
pubspec = null;

pkg/dartdev/lib/src/commands/test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ Run "${runner!.executableName} help" to see global options.''');
5757
);
5858
if (runPackageName != null) {
5959
final pubspecUri =
60-
await DartNativeAssetsBuilder.findPubspec(Directory.current.uri);
60+
await DartNativeAssetsBuilder.findWorkspacePubspec(packageConfig);
6161
final Map? pubspec;
6262
if (pubspecUri == null) {
6363
pubspec = null;

pkg/dartdev/lib/src/native_assets.dart

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,17 @@ class DartNativeAssetsBuilder {
287287
}
288288
}
289289

290+
static Future<Uri?> findWorkspacePubspec(Uri? workspacePackageConfig) async {
291+
if (workspacePackageConfig == null) {
292+
return null;
293+
}
294+
final candidate = workspacePackageConfig.resolve('../pubspec.yaml');
295+
if (File.fromUri(candidate).existsSync()) {
296+
return candidate;
297+
}
298+
return null;
299+
}
300+
290301
/// Tries to find the package name that [uri] is in.
291302
///
292303
/// Returns `null` if package cannnot be determined.

pkg/dartdev/test/native_assets/build_test.dart

Lines changed: 29 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -328,34 +328,37 @@ void main(List<String> args) {
328328
},
329329
);
330330

331-
test(
332-
'dart build with user defines',
333-
timeout: longTimeout,
334-
() async {
335-
await nativeAssetsTest('user_defines', (packageUri) async {
336-
await runDart(
337-
arguments: [
338-
'--enable-experiment=native-assets',
339-
'build',
340-
'bin/user_defines.dart',
341-
],
342-
workingDirectory: packageUri,
343-
logger: logger,
344-
);
331+
for (final usePubWorkspace in [true, false]) {
332+
test(
333+
'dart build with user defines',
334+
timeout: longTimeout,
335+
() async {
336+
await nativeAssetsTest('user_defines', usePubWorkspace: usePubWorkspace,
337+
(packageUri) async {
338+
await runDart(
339+
arguments: [
340+
'--enable-experiment=native-assets',
341+
'build',
342+
'bin/user_defines.dart',
343+
],
344+
workingDirectory: packageUri,
345+
logger: logger,
346+
);
345347

346-
final outputDirectory =
347-
Directory.fromUri(packageUri.resolve('bin/user_defines'));
348-
expect(outputDirectory.existsSync(), true);
348+
final outputDirectory =
349+
Directory.fromUri(packageUri.resolve('bin/user_defines'));
350+
expect(outputDirectory.existsSync(), true);
349351

350-
final proccessResult = await runProcess(
351-
executable: outputDirectory.uri.resolve('user_defines.exe'),
352-
logger: logger,
353-
throwOnUnexpectedExitCode: true,
354-
);
355-
expect(proccessResult.stdout, contains('Hello world!'));
356-
});
357-
},
358-
);
352+
final proccessResult = await runProcess(
353+
executable: outputDirectory.uri.resolve('user_defines.exe'),
354+
logger: logger,
355+
throwOnUnexpectedExitCode: true,
356+
);
357+
expect(proccessResult.stdout, contains('Hello world!'));
358+
});
359+
},
360+
);
361+
}
359362
}
360363

361364
Future<void> _withTempDir(Future<void> Function(Uri tempUri) fun) async {

pkg/dartdev/test/native_assets/helpers.dart

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -99,33 +99,41 @@ Future<void> copyTestProjects(Uri copyTargetUri, Logger logger,
9999
await sourceFile.copy(targetUri.toFilePath());
100100
}
101101
final dependencyOverrides = {
102-
'native_assets_cli': {
103-
'path': sdkRoot
104-
.resolve('third_party/pkg/native/pkgs/native_assets_cli/')
105-
.toFilePath(),
106-
},
107-
'native_toolchain_c': {
108-
'path': sdkRoot
109-
.resolve('third_party/pkg/native/pkgs/native_toolchain_c/')
110-
.toFilePath(),
111-
},
112-
'meta': {
113-
'path': sdkRoot.resolve('pkg/meta/').toFilePath(),
114-
},
115-
'record_use': {
116-
'path': sdkRoot.resolve('pkg/record_use/').toFilePath(),
117-
},
102+
'native_assets_cli': {
103+
'path': sdkRoot
104+
.resolve('third_party/pkg/native/pkgs/native_assets_cli/')
105+
.toFilePath(),
106+
},
107+
'native_toolchain_c': {
108+
'path': sdkRoot
109+
.resolve('third_party/pkg/native/pkgs/native_toolchain_c/')
110+
.toFilePath(),
111+
},
112+
'meta': {
113+
'path': sdkRoot.resolve('pkg/meta/').toFilePath(),
114+
},
115+
'record_use': {
116+
'path': sdkRoot.resolve('pkg/record_use/').toFilePath(),
117+
},
118118
};
119+
final userDefinesWorkspace = {};
119120
for (final pubspecPath in pubspecPaths) {
120121
final sourceFile = File.fromUri(testProjectsUri.resolveUri(pubspecPath));
121122
final targetUri = copyTargetUri.resolveUri(pubspecPath);
122123
final sourceString = await sourceFile.readAsString();
123124
final pubspec = YamlEditor(sourceString);
125+
final pubspecRead = loadYamlNode(sourceString) as Map;
124126
if (!usePubWorkspace) {
125-
if ((loadYamlNode(sourceString) as Map)['resolution'] != null) {
127+
if (pubspecRead['resolution'] != null) {
126128
pubspec.remove(['resolution']);
127129
}
128130
pubspec.update(['dependency_overrides'], dependencyOverrides);
131+
} else {
132+
final userDefines = pubspecRead['hooks']?['user_defines'];
133+
if (userDefines is Map) {
134+
pubspec.remove(['hooks', 'user_defines']);
135+
userDefinesWorkspace.addAll(userDefines);
136+
}
129137
}
130138
final modifiedString = pubspec.toString();
131139
await File.fromUri(targetUri).writeAsString(modifiedString);
@@ -141,6 +149,9 @@ Future<void> copyTestProjects(Uri copyTargetUri, Logger logger,
141149
pubspec.toFilePath().replaceAll('pubspec.yaml', ''),
142150
],
143151
'dependency_overrides': dependencyOverrides,
152+
'hooks': {
153+
'user_defines': userDefinesWorkspace,
154+
}
144155
});
145156
final pubspecUri = copyTargetUri.resolve('pubspec.yaml');
146157
await File.fromUri(pubspecUri).writeAsString(workspacePubspec.toString());

pkg/dartdev/test/native_assets/run_test.dart

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -193,22 +193,25 @@ Couldn't resolve native function 'multiply' in 'package:drop_dylib_link/dylib_mu
193193
});
194194
});
195195

196-
test(
197-
'dart run with user defines',
198-
timeout: longTimeout,
199-
() async {
200-
await nativeAssetsTest('user_defines', (packageUri) async {
201-
final result = await runDart(
202-
arguments: [
203-
'--enable-experiment=native-assets',
204-
'run',
205-
'bin/user_defines.dart',
206-
],
207-
workingDirectory: packageUri,
208-
logger: logger,
209-
);
210-
expect(result.stdout, contains('Hello world!'));
211-
});
212-
},
213-
);
196+
for (final usePubWorkspace in [true, false]) {
197+
test(
198+
'dart run with user defines',
199+
timeout: longTimeout,
200+
() async {
201+
await nativeAssetsTest('user_defines', usePubWorkspace: usePubWorkspace,
202+
(packageUri) async {
203+
final result = await runDart(
204+
arguments: [
205+
'--enable-experiment=native-assets',
206+
'run',
207+
'bin/user_defines.dart',
208+
],
209+
workingDirectory: packageUri,
210+
logger: logger,
211+
);
212+
expect(result.stdout, contains('Hello world!'));
213+
});
214+
},
215+
);
216+
}
214217
}

pkg/dartdev/test/native_assets/test_test.dart

Lines changed: 26 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -125,28 +125,31 @@ void main(List<String> args) async {
125125
});
126126
});
127127

128-
test(
129-
'dart test with user defines',
130-
timeout: longTimeout,
131-
() async {
132-
await nativeAssetsTest('user_defines', (packageUri) async {
133-
final result = await runDart(
134-
arguments: [
135-
'--enable-experiment=native-assets',
136-
'test',
137-
],
138-
workingDirectory: packageUri,
139-
logger: logger,
140-
);
141-
expect(
142-
result.stdout,
143-
stringContainsInOrder(
144-
[
145-
'All tests passed!',
128+
for (final usePubWorkspace in [true, false]) {
129+
test(
130+
'dart test with user defines',
131+
timeout: longTimeout,
132+
() async {
133+
await nativeAssetsTest('user_defines', usePubWorkspace: usePubWorkspace,
134+
(packageUri) async {
135+
final result = await runDart(
136+
arguments: [
137+
'--enable-experiment=native-assets',
138+
'test',
146139
],
147-
),
148-
);
149-
});
150-
},
151-
);
140+
workingDirectory: packageUri,
141+
logger: logger,
142+
);
143+
expect(
144+
result.stdout,
145+
stringContainsInOrder(
146+
[
147+
'All tests passed!',
148+
],
149+
),
150+
);
151+
});
152+
},
153+
);
154+
}
152155
}

0 commit comments

Comments
 (0)