Skip to content

Commit 8a994cb

Browse files
sigurdmsigurdm
authored
Give error if global activated package depends on hooks (#4612)
1 parent f6457fd commit 8a994cb

File tree

7 files changed

+220
-34
lines changed

7 files changed

+220
-34
lines changed

‎lib/src/command/deps.dart‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -417,7 +417,12 @@ class DepsCommand extends PubCommand {
417417
],
418418
];
419419
return nonDevDependencies
420-
.expand(graph.transitiveDependencies)
420+
.expand(
421+
(p) => graph.transitiveDependencies(
422+
p,
423+
followDevDependenciesFromRoot: false,
424+
),
425+
)
421426
.map((package) => package.name)
422427
.toSet();
423428
}

‎lib/src/command/upgrade.dart‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,8 +121,12 @@ class UpgradeCommand extends PubCommand {
121121
final graph = await entrypoint.packageGraph;
122122
return argResults.rest
123123
.expand(
124-
(package) =>
125-
graph.transitiveDependencies(package).map((p) => p.name),
124+
(package) => graph
125+
.transitiveDependencies(
126+
package,
127+
followDevDependenciesFromRoot: true,
128+
)
129+
.map((p) => p.name),
126130
)
127131
.toSet()
128132
.toList();

‎lib/src/global_packages.dart‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,23 @@ class GlobalPackages {
174174
);
175175
}
176176

177+
void _testForHooks(Package package, String activatedPackageName) {
178+
final prelude =
179+
(package.name == activatedPackageName)
180+
? 'Package $activatedPackageName uses hooks.'
181+
: 'The dependency of $activatedPackageName, '
182+
'${package.name} uses hooks.';
183+
if (fileExists(p.join(package.dir, 'hooks', 'build.dart'))) {
184+
fail('''
185+
$prelude
186+
187+
You currently cannot `global activate` packages relying on hooks.
188+
189+
Follow progress in https://github.com/dart-lang/sdk/issues/60889.
190+
''');
191+
}
192+
}
193+
177194
/// Makes the local package at [path] globally active.
178195
///
179196
/// [executables] is the names of the executables that should have binstubs.
@@ -194,6 +211,10 @@ class GlobalPackages {
194211
await entrypoint.acquireDependencies(SolveType.get);
195212
final activatedPackage = entrypoint.workPackage;
196213
final name = activatedPackage.name;
214+
for (final package in (await entrypoint.packageGraph)
215+
.transitiveDependencies(name, followDevDependenciesFromRoot: false)) {
216+
_testForHooks(package, name);
217+
}
197218
_describeActive(name, cache);
198219

199220
// Write a lockfile that points to the local package.
@@ -260,10 +281,27 @@ class GlobalPackages {
260281
}
261282
rethrow;
262283
}
284+
263285
// We want the entrypoint to be rooted at 'dep' not the dummy-package.
264286
result.packages.removeWhere((id) => id.name == 'pub global activate');
265287

266288
final lockFile = await result.downloadCachedPackages(cache);
289+
290+
// Because we know that the dummy package never is a workspace we can
291+
// iterate all packages.
292+
for (final package in result.packages) {
293+
_testForHooks(
294+
// TODO(sigurdm): refactor PackageGraph to make it possible to query
295+
// without loading the entrypoint.
296+
Package(
297+
result.pubspecs[package.name]!,
298+
cache.getDirectory(package),
299+
[],
300+
),
301+
name,
302+
);
303+
}
304+
267305
final sameVersions =
268306
originalLockFile != null && originalLockFile.samePackageIds(lockFile);
269307

‎lib/src/package_graph.dart‎

Lines changed: 18 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,11 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5-
import 'package:graphs/graphs.dart';
6-
75
import 'entrypoint.dart';
86
import 'package.dart';
97
import 'solver.dart';
108
import 'source/cached.dart';
119
import 'source/sdk.dart';
12-
import 'utils.dart';
1310

1411
/// A holistic view of the entire transitive dependency graph for an entrypoint.
1512
class PackageGraph {
@@ -23,9 +20,6 @@ class PackageGraph {
2320
/// relevant in the current context.
2421
final Map<String, Package> packages;
2522

26-
/// A map of transitive dependencies for each package.
27-
Map<String, Set<Package>>? _transitiveDependencies;
28-
2923
PackageGraph(this.entrypoint, this.packages);
3024

3125
/// Creates a package graph using the data from [result].
@@ -55,28 +49,25 @@ class PackageGraph {
5549
/// For the entrypoint this returns all packages in [packages], which includes
5650
/// dev and override. For any other package, it ignores dev and override
5751
/// dependencies.
58-
Set<Package> transitiveDependencies(String package) {
59-
if (package == entrypoint.workspaceRoot.name) {
60-
return packages.values.toSet();
61-
}
52+
Set<Package> transitiveDependencies(
53+
String package, {
54+
required bool followDevDependenciesFromRoot,
55+
}) {
56+
final result = <Package>{};
6257

63-
if (_transitiveDependencies == null) {
64-
final graph = mapMap<String, Package, String, Iterable<String>>(
65-
packages,
66-
value: (_, package) => package.dependencies.keys,
67-
);
68-
final closure = transitiveClosure(graph.keys, (n) => graph[n]!);
69-
_transitiveDependencies =
70-
mapMap<String, Set<String>, String, Set<Package>>(
71-
closure,
72-
value: (depender, names) {
73-
final set = names.map((name) => packages[name]!).toSet();
74-
set.add(packages[depender]!);
75-
return set;
76-
},
77-
);
58+
final stack = [package];
59+
final visited = <String>{};
60+
while (stack.isNotEmpty) {
61+
final current = stack.removeLast();
62+
if (!visited.add(current)) continue;
63+
final currentPackage = packages[current]!;
64+
result.add(currentPackage);
65+
stack.addAll(currentPackage.dependencies.keys);
66+
if (followDevDependenciesFromRoot && current == package) {
67+
stack.addAll(currentPackage.devDependencies.keys);
68+
}
7869
}
79-
return _transitiveDependencies![package]!;
70+
return result;
8071
}
8172

8273
bool _isPackageFromImmutableSource(String package) {
@@ -98,6 +89,7 @@ class PackageGraph {
9889

9990
return transitiveDependencies(
10091
package,
92+
followDevDependenciesFromRoot: true,
10193
).any((dep) => !_isPackageFromImmutableSource(dep.name));
10294
}
10395
}

‎pubspec.lock‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,18 @@ packages:
55
dependency: transitive
66
description:
77
name: _fe_analyzer_shared
8-
sha256: e55636ed79578b9abca5fecf9437947798f5ef7456308b5cb85720b793eac92f
8+
sha256: c81659312e021e3b780a502206130ea106487b34793bce61e26dc0f9b84807af
99
url: "https://pub.dev"
1010
source: hosted
11-
version: "82.0.0"
11+
version: "83.0.0"
1212
analyzer:
1313
dependency: "direct main"
1414
description:
1515
name: analyzer
16-
sha256: "904ae5bb474d32c38fb9482e2d925d5454cda04ddd0e55d2e6826bc72f6ba8c0"
16+
sha256: "9c35a79bf2a150b3ea0d40010fbbb45b5ebea143d47096e0f82fd922a324b49b"
1717
url: "https://pub.dev"
1818
source: hosted
19-
version: "7.4.5"
19+
version: "7.4.6"
2020
args:
2121
dependency: "direct main"
2222
description:
@@ -257,6 +257,14 @@ packages:
257257
url: "https://pub.dev"
258258
source: hosted
259259
version: "2.2.0"
260+
retry:
261+
dependency: "direct main"
262+
description:
263+
name: retry
264+
sha256: "822e118d5b3aafed083109c72d5f484c6dc66707885e07c0fbcb8b986bba7efc"
265+
url: "https://pub.dev"
266+
source: hosted
267+
version: "3.1.2"
260268
shelf:
261269
dependency: "direct main"
262270
description:
@@ -474,4 +482,4 @@ packages:
474482
source: hosted
475483
version: "2.2.2"
476484
sdks:
477-
dart: ">=3.7.0 <4.0.0"
485+
dart: ">=3.8.0 <4.0.0"

‎pubspec.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ dependencies:
2020
path: ^1.9.1
2121
pool: ^1.5.1
2222
pub_semver: ^2.2.0
23+
retry: ^3.1.2
2324
shelf: ^1.4.2
2425
source_span: ^1.10.1
2526
stack_trace: ^1.12.1

‎test/global/activate/activate_package_with_hooks_test.dart‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:path/path.dart' as p;
6+
import 'package:test/test.dart';
7+
8+
import '../../descriptor.dart';
9+
import '../../test_pub.dart';
10+
11+
void main() {
12+
test(
13+
'activating a package from path gives error if package uses hooks',
14+
() async {
15+
final server = await servePackages();
16+
server.serve(
17+
'uses_hooks',
18+
'1.0.0',
19+
contents: [
20+
dir('hooks', [file('build.dart')]),
21+
],
22+
);
23+
server.serve('uses_no_hooks', '1.0.0');
24+
25+
await dir(appPath, [
26+
libPubspec(
27+
'foo',
28+
'1.2.3',
29+
extras: {
30+
'workspace': [
31+
'pkgs/foo_hooks',
32+
'pkgs/foo_dev_hooks',
33+
'pkgs/foo_no_hooks',
34+
],
35+
},
36+
sdk: '^3.5.0',
37+
),
38+
dir('pkgs', [
39+
dir('foo_hooks', [
40+
libPubspec(
41+
'foo_hooks',
42+
'1.1.1',
43+
deps: {'uses_hooks': '^1.0.0'},
44+
resolutionWorkspace: true,
45+
),
46+
]),
47+
dir('foo_dev_hooks', [
48+
libPubspec(
49+
'foo_dev_hooks',
50+
'1.1.1',
51+
devDeps: {'uses_hooks': '^1.0.0'},
52+
resolutionWorkspace: true,
53+
),
54+
]),
55+
dir('foo_no_hooks', [
56+
libPubspec(
57+
'foo_no_hooks',
58+
'1.1.1',
59+
deps: {'uses_no_hooks': '^1.0.0'},
60+
resolutionWorkspace: true,
61+
),
62+
]),
63+
]),
64+
]).create();
65+
66+
await runPub(
67+
args: ['global', 'activate', '-spath', p.join('pkgs', 'foo_hooks')],
68+
environment: {'_PUB_TEST_SDK_VERSION': '3.5.0'},
69+
error: '''
70+
The dependency of foo_hooks, uses_hooks uses hooks.
71+
72+
You currently cannot `global activate` packages relying on hooks.
73+
74+
Follow progress in https://github.com/dart-lang/sdk/issues/60889.''',
75+
exitCode: 1,
76+
);
77+
78+
await runPub(
79+
args: ['global', 'activate', '-spath', p.join('pkgs', 'foo_dev_hooks')],
80+
environment: {'_PUB_TEST_SDK_VERSION': '3.5.0'},
81+
);
82+
83+
await runPub(
84+
args: ['global', 'activate', '-spath', p.join('pkgs', 'foo_no_hooks')],
85+
environment: {'_PUB_TEST_SDK_VERSION': '3.5.0'},
86+
);
87+
},
88+
);
89+
90+
test('activating a hosted package gives error if package uses hooks in direct'
91+
' dependency', () async {
92+
final server = await servePackages();
93+
server.serve(
94+
'uses_hooks',
95+
'1.0.0',
96+
contents: [
97+
dir('hooks', [file('build.dart')]),
98+
],
99+
);
100+
server.serve('foo_hooks', '1.1.1', deps: {'uses_hooks': '^1.0.0'});
101+
server.serve(
102+
'foo_hooks_in_dev_deps',
103+
'1.0.0',
104+
pubspec: {
105+
'dev_dependencies': {'uses_hooks': '^1.0.0'},
106+
},
107+
);
108+
109+
await runPub(
110+
args: ['global', 'activate', 'uses_hooks'],
111+
environment: {'_PUB_TEST_SDK_VERSION': '3.5.0'},
112+
error: '''
113+
Package uses_hooks uses hooks.
114+
115+
You currently cannot `global activate` packages relying on hooks.
116+
117+
Follow progress in https://github.com/dart-lang/sdk/issues/60889.''',
118+
exitCode: 1,
119+
);
120+
121+
await runPub(
122+
args: ['global', 'activate', 'foo_hooks'],
123+
environment: {'_PUB_TEST_SDK_VERSION': '3.5.0'},
124+
error: '''
125+
The dependency of foo_hooks, uses_hooks uses hooks.
126+
127+
You currently cannot `global activate` packages relying on hooks.
128+
129+
Follow progress in https://github.com/dart-lang/sdk/issues/60889.''',
130+
exitCode: 1,
131+
);
132+
133+
await runPub(
134+
args: ['global', 'activate', 'foo_hooks_in_dev_deps'],
135+
environment: {'_PUB_TEST_SDK_VERSION': '3.5.0'},
136+
);
137+
});
138+
}

0 commit comments

Comments
 (0)