Skip to content

Commit 3bd1765

Browse files
sigurdmsigurdm
committed
Address review comments
1 parent f8ee397 commit 3bd1765

File tree

4 files changed

+126
-74
lines changed

4 files changed

+126
-74
lines changed

‎lib/src/package.dart‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,9 @@ See $workspacesDocUrl for more information.
287287
try {
288288
link.resolveSymbolicLinksSync();
289289
} on FileSystemException catch (e) {
290+
if (!link.existsSync()) {
291+
return;
292+
}
290293
throw DataException(
291294
'Could not resolve symbolic link $path. $e',
292295
);
@@ -300,6 +303,10 @@ See $workspacesDocUrl for more information.
300303
///
301304
/// Cache the symlink resolutions here.
302305
final symlinkResolvedDirs = <String, String>{};
306+
String resolveDirSymlinks(String path) {
307+
return symlinkResolvedDirs[path] ??=
308+
Directory(path).resolveSymbolicLinksSync();
309+
}
303310

304311
final result = Ignore.listFiles(
305312
beneath: beneath,
@@ -309,11 +316,9 @@ See $workspacesDocUrl for more information.
309316

310317
{
311318
final canonicalized = p.canonicalize(resolvedDir);
312-
final symlinkResolvedDir = symlinkResolvedDirs[canonicalized] ??=
313-
Directory(canonicalized).resolveSymbolicLinksSync();
319+
final symlinkResolvedDir = resolveDirSymlinks(canonicalized);
314320
for (final parent in parentDirs(p.dirname(canonicalized))) {
315-
final symlinkResolvedParent = symlinkResolvedDirs[parent] ??=
316-
Directory(parent).resolveSymbolicLinksSync();
321+
final symlinkResolvedParent = resolveDirSymlinks(parent);
317322
if (p.equals(symlinkResolvedDir, symlinkResolvedParent)) {
318323
dataError('''
319324
Pub does not support symlink cycles.

‎test/package_list_files_test.dart‎

Lines changed: 113 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ import 'package:pub/src/system_cache.dart';
1111
import 'package:test/test.dart';
1212

1313
import 'descriptor.dart' as d;
14+
import 'link_descriptor.dart';
1415
import 'test_pub.dart';
15-
import 'validator/utils.dart';
1616

1717
late String root;
1818
Entrypoint? entrypoint;
@@ -58,12 +58,9 @@ void main() {
5858
d.file('file2.txt', 'contents'),
5959
d.dir('subdir', [
6060
d.dir('a', [d.file('file')]),
61+
link('symlink', 'a', forceDirectory: true),
6162
]),
6263
]).create();
63-
createDirectorySymlink(
64-
p.join(d.sandbox, appPath, 'subdir', 'symlink'),
65-
'a',
66-
);
6764

6865
createEntrypoint();
6966

@@ -84,12 +81,9 @@ void main() {
8481
d.dir('subdir', [
8582
d.file('.pubignore', 'symlink'),
8683
d.dir('a', [d.file('file')]),
84+
link('symlink', 'a', forceDirectory: true),
8785
]),
8886
]).create();
89-
createDirectorySymlink(
90-
p.join(d.sandbox, appPath, 'subdir', 'symlink'),
91-
'a',
92-
);
9387

9488
createEntrypoint();
9589

@@ -109,12 +103,9 @@ void main() {
109103
d.dir('subdir', [
110104
d.file('.pubignore', 'symlink'),
111105
d.dir('a', [d.file('file')]),
106+
link('symlink', 'b', forceDirectory: true),
112107
]),
113108
]).create();
114-
createDirectorySymlink(
115-
p.join(d.sandbox, appPath, 'subdir', 'symlink'),
116-
'b',
117-
);
118109

119110
createEntrypoint();
120111

@@ -134,12 +125,9 @@ void main() {
134125
d.file('file2.txt', 'contents'),
135126
d.dir('subdir', [
136127
d.dir('a', [d.file('file')]),
128+
link('symlink', '..', forceDirectory: true),
137129
]),
138130
]).create();
139-
createDirectorySymlink(
140-
p.join(d.sandbox, appPath, 'subdir', 'symlink'),
141-
'..',
142-
);
143131

144132
createEntrypoint();
145133

@@ -164,12 +152,9 @@ void main() {
164152
d.file('file2.txt', 'contents'),
165153
d.dir('subdir', [
166154
d.dir('a', [d.file('file')]),
155+
link('symlink', 'symlink', forceDirectory: true),
167156
]),
168157
]).create();
169-
createDirectorySymlink(
170-
p.join(d.sandbox, appPath, 'subdir', 'symlink'),
171-
'symlink',
172-
);
173158

174159
createEntrypoint();
175160

@@ -191,29 +176,94 @@ void main() {
191176
d.file('file1.txt', 'contents'),
192177
d.file('file2.txt', 'contents'),
193178
d.dir('subdir', [
194-
d.dir('a', [d.file('file')]),
195-
d.dir('b'),
196-
d.dir('c'),
179+
d.dir('a', [
180+
d.file('file'),
181+
link('symlink1', p.join('..', 'b'), forceDirectory: true),
182+
]),
183+
d.dir('b', [
184+
link('symlink2', p.join('..', 'c'), forceDirectory: true),
185+
]),
186+
d.dir('c', [
187+
link('symlink3', p.join('..', 'a'), forceDirectory: true),
188+
]),
189+
link('symlink', 'a', forceDirectory: true),
197190
]),
198191
]).create();
199-
createDirectorySymlink(
200-
p.join(d.sandbox, appPath, 'subdir', 'symlink'),
201-
'a',
202-
);
203-
createDirectorySymlink(
204-
p.join(d.sandbox, appPath, 'subdir', 'a', 'symlink1'),
205-
p.join('..', 'b'),
192+
193+
createEntrypoint();
194+
195+
expect(
196+
() => entrypoint!.workspaceRoot.listFiles(),
197+
throwsA(
198+
isA<DataException>().having(
199+
(e) => e.message,
200+
'message',
201+
contains('Pub does not support symlink cycles.'),
202+
),
203+
),
206204
);
207-
createDirectorySymlink(
208-
p.join(d.sandbox, appPath, 'subdir', 'b', 'symlink2'),
209-
p.join('..', 'c'),
205+
});
206+
207+
test('throws on link to loop', () async {
208+
await d.dir(appPath, [
209+
d.pubspec({'name': 'myapp'}),
210+
link('symlink', p.join(d.sandbox, 'loop'), forceDirectory: true),
211+
]).create();
212+
await link('loop', 'loop', forceDirectory: true).create();
213+
214+
createEntrypoint();
215+
216+
expect(
217+
() => entrypoint!.workspaceRoot.listFiles(),
218+
throwsA(
219+
isA<DataException>().having(
220+
(e) => e.message,
221+
'message',
222+
contains('Could not resolve symbolic link'),
223+
),
224+
),
210225
);
211-
createDirectorySymlink(
212-
p.join(d.sandbox, appPath, 'subdir', 'c', 'symlink3'),
213-
p.join('..', 'a'),
226+
});
227+
228+
test('throws on link to loop back to parent directory', () async {
229+
await d.dir('src', [
230+
d.dir(appPath, [
231+
d.pubspec({'name': 'myapp'}),
232+
link('symlink', p.join(d.sandbox, 'source'), forceDirectory: true),
233+
]),
234+
]).create();
235+
await link('source', p.join(d.sandbox, 'src'), forceDirectory: true)
236+
.create();
237+
238+
createEntrypoint(p.join('src', appPath));
239+
240+
expect(
241+
() => entrypoint!.workspaceRoot.listFiles(),
242+
throwsA(
243+
isA<DataException>().having(
244+
(e) => e.message,
245+
'message',
246+
contains('Pub does not support symlink cycles.'),
247+
),
248+
),
214249
);
250+
});
215251

216-
createEntrypoint();
252+
test('throws on link to subdirectory of loop back to parent directory',
253+
() async {
254+
await d.dir('src', [
255+
d.dir(appPath, [
256+
d.pubspec({'name': 'myapp'}),
257+
link('symlink', p.join(d.sandbox, 'source'), forceDirectory: true),
258+
]),
259+
]).create();
260+
await link(
261+
'source',
262+
p.join(d.sandbox, 'src'),
263+
forceDirectory: true,
264+
).create();
265+
266+
createEntrypoint(p.join('source', appPath));
217267

218268
expect(
219269
() => entrypoint!.workspaceRoot.listFiles(),
@@ -227,6 +277,25 @@ void main() {
227277
);
228278
});
229279

280+
test('Does not throw when publishing via symlink', () async {
281+
await d.dir('src', [
282+
d.dir(appPath, [
283+
d.pubspec({'name': 'myapp'}),
284+
]),
285+
]).create();
286+
await link(
287+
'source',
288+
p.join(d.sandbox, 'src'),
289+
forceDirectory: true,
290+
).create();
291+
292+
createEntrypoint(p.join('source', appPath));
293+
294+
expect(entrypoint!.workspaceRoot.listFiles(), {
295+
p.join(root, 'pubspec.yaml'),
296+
});
297+
});
298+
230299
test('not throws on ignored link', () async {
231300
await d.dir(appPath, [
232301
d.pubspec({'name': 'myapp'}),
@@ -235,12 +304,9 @@ void main() {
235304
d.dir('subdir', [
236305
d.file('.pubignore', 'symlink'),
237306
d.dir('a', [d.file('file')]),
307+
link('symlink', '..', forceDirectory: true),
238308
]),
239309
]).create();
240-
createDirectorySymlink(
241-
p.join(d.sandbox, appPath, 'subdir', 'symlink'),
242-
'..',
243-
);
244310

245311
createEntrypoint();
246312

@@ -259,20 +325,11 @@ void main() {
259325
d.file('file2.txt', 'contents'),
260326
d.dir('subdir', [
261327
d.dir('a', [d.file('file')]),
328+
link('symlink1', 'a', forceDirectory: true),
329+
link('symlink2', 'a', forceDirectory: true),
262330
]),
331+
link('symlink3', p.join('subdir', 'a')),
263332
]).create();
264-
createDirectorySymlink(
265-
p.join(d.sandbox, appPath, 'subdir', 'symlink1'),
266-
'a',
267-
);
268-
createDirectorySymlink(
269-
p.join(d.sandbox, appPath, 'subdir', 'symlink2'),
270-
'a',
271-
);
272-
createDirectorySymlink(
273-
p.join(d.sandbox, appPath, 'symlink3'),
274-
p.join('subdir', 'a'),
275-
);
276333

277334
createEntrypoint();
278335

@@ -299,8 +356,8 @@ void main() {
299356
]),
300357
]).create();
301358

302-
final root = p.join(d.sandbox, 'symlink');
303-
createDirectorySymlink(root, appPath);
359+
await link('symlink', appPath).create();
360+
root = p.join(d.sandbox, 'symlink');
304361

305362
final entrypoint = Entrypoint(
306363
p.join(d.sandbox, 'symlink'),

‎test/validator/gitignore_test.dart‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ import 'package:pub/src/exit_codes.dart' as exit_codes;
99
import 'package:test/test.dart';
1010

1111
import '../descriptor.dart' as d;
12+
import '../link_descriptor.dart';
1213
import '../test_pub.dart';
13-
import 'utils.dart';
1414

1515
Future<void> expectValidation(
1616
Matcher error,
@@ -159,10 +159,11 @@ void main() {
159159
await pubGet(
160160
workingDirectory: packageRoot,
161161
);
162-
createDirectorySymlink(
162+
await link(
163163
p.join(d.sandbox, appPath, 'dir_with_symlink', 'symlink'),
164164
'..',
165-
);
165+
forceDirectory: true,
166+
).create();
166167
await git.commit();
167168

168169
await expectValidation(

‎test/validator/utils.dart‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
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 'dart:io';
6-
75
import 'package:pub/src/exit_codes.dart';
86
import 'package:test/test.dart';
97

@@ -26,15 +24,6 @@ Future<void> expectValidationDeprecated(
2624
expect(validator.hints, hints ?? isEmpty);
2725
}
2826

29-
// On windows symlinks to directories are distinct from symlinks to files.
30-
void createDirectorySymlink(String path, String target) {
31-
if (Platform.isWindows) {
32-
Process.runSync('cmd', ['/c', 'mklink', '/D', path, target]);
33-
} else {
34-
Link(path).createSync(target);
35-
}
36-
}
37-
3827
Future<void> expectValidation({
3928
Object? error,
4029
int exitCode = 0,

0 commit comments

Comments
 (0)