From 0189750480d383aa094f744bfcfa023ab9b01920 Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Wed, 19 Jan 2022 02:50:27 +0700 Subject: [PATCH 01/58] don't warn on ignored directory symlinks (cherry picked from commit 25fd5bf1118dad3278bf35301a075c74dd93f974) --- lib/src/validator/gitignore.dart | 11 ++++++----- test/package_list_files_test.dart | 10 ++-------- test/validator/gitignore_test.dart | 21 +++++++++++++++++++++ test/validator/utils.dart | 11 +++++++++++ 4 files changed, 40 insertions(+), 13 deletions(-) diff --git a/lib/src/validator/gitignore.dart b/lib/src/validator/gitignore.dart index 809c53707..c9ad02f61 100644 --- a/lib/src/validator/gitignore.dart +++ b/lib/src/validator/gitignore.dart @@ -53,10 +53,8 @@ class GitignoreValidator extends Validator { beneath: beneath, listDir: (dir) { var contents = Directory(resolve(dir)).listSync(); - return contents - .where((e) => !(linkExists(e.path) && dirExists(e.path))) - .map((entity) => p.posix - .joinAll(p.split(p.relative(entity.path, from: root)))); + return contents.map((entity) => + p.posix.joinAll(p.split(p.relative(entity.path, from: root)))); }, ignoreForDir: (dir) { final gitIgnore = resolve('$dir/.gitignore'); @@ -65,7 +63,10 @@ class GitignoreValidator extends Validator { ]; return rules.isEmpty ? null : Ignore(rules); }, - isDir: (dir) => dirExists(resolve(dir)), + isDir: (dir) { + final resolved = resolve(dir); + return dirExists(resolved) && !linkExists(resolved); + }, ).map((file) { final relative = p.relative(resolve(file), from: entrypoint.root.dir); return Platform.isWindows diff --git a/test/package_list_files_test.dart b/test/package_list_files_test.dart index 013f3a19f..23938b177 100644 --- a/test/package_list_files_test.dart +++ b/test/package_list_files_test.dart @@ -12,6 +12,7 @@ import 'package:test/test.dart'; import 'descriptor.dart' as d; import 'test_pub.dart'; +import 'validator/utils.dart'; late String root; Entrypoint? entrypoint; @@ -45,14 +46,7 @@ void main() { ])); }); - // On windows symlinks to directories are distinct from symlinks to files. - void createDirectorySymlink(String path, String target) { - if (Platform.isWindows) { - Process.runSync('cmd', ['/c', 'mklink', '/D', path, target]); - } else { - Link(path).createSync(target); - } - } + test('throws on directory symlinks', () async { await d.dir(appPath, [ diff --git a/test/validator/gitignore_test.dart b/test/validator/gitignore_test.dart index 3cd4a119a..bbc25d7e0 100644 --- a/test/validator/gitignore_test.dart +++ b/test/validator/gitignore_test.dart @@ -10,6 +10,7 @@ import 'package:test/test.dart'; import '../descriptor.dart' as d; import '../test_pub.dart'; +import 'utils.dart'; Future expectValidation( error, @@ -117,4 +118,24 @@ void main() { await expectValidation(contains('Package has 0 warnings.'), 0, workingDirectory: packageRoot); }); + + test('Should consider symlinks to be valid files and not list them as gitignored', () async { + final git = d.git(appPath, [ + ...d.validPackage.contents, + d.dir('dir_with_symlink', [ + d.file('.pubignore', '/symlink'), + ]), + ]); + await git.create(); + final packageRoot = p.join(d.sandbox, appPath); + await pubGet( + environment: {'_PUB_TEST_SDK_VERSION': '1.12.0'}, + workingDirectory: packageRoot); + createDirectorySymlink( + p.join(d.sandbox, appPath, 'dir_with_symlink', 'symlink'), '..'); + git.commit(); + + await expectValidation(contains('Package has 0 warnings.'), 0, + workingDirectory: packageRoot); + }); } diff --git a/test/validator/utils.dart b/test/validator/utils.dart index 8a1c0e2b4..1861d6ed4 100644 --- a/test/validator/utils.dart +++ b/test/validator/utils.dart @@ -2,6 +2,8 @@ // 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 '../test_pub.dart'; @@ -16,3 +18,12 @@ Future expectValidation(ValidatorCreator fn, expect(validator.warnings, warnings ?? isEmpty); expect(validator.hints, hints ?? isEmpty); } + +// On windows symlinks to directories are distinct from symlinks to files. +void createDirectorySymlink(String path, String target) { + if (Platform.isWindows) { + Process.runSync('cmd', ['/c', 'mklink', '/D', path, target]); + } else { + Link(path).createSync(target); + } +} From 7320081f898225a46f96b41f3aaa3f434ced30b3 Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Wed, 19 Jan 2022 02:52:07 +0700 Subject: [PATCH 02/58] Don't crash on directory symlinks (default behavior is copy-paste of symlinked directories in published archive) (cherry picked from commit 163d23c838a3c1046bc2d8ae6b585eeacecc4090) --- lib/src/package.dart | 22 ++++++++++------------ test/package_list_files_test.dart | 23 +++++++++++++++++++++++ 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index 4d90311eb..e9869cd5b 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -238,17 +238,6 @@ class Package { contents = contents.where((entity) => entity is! Directory).toList(); } return contents.map((entity) { - if (linkExists(entity.path)) { - final target = Link(entity.path).targetSync(); - if (dirExists(entity.path)) { - throw DataException( - '''Pub does not support publishing packages with directory symlinks: `${entity.path}`.'''); - } - if (!fileExists(entity.path)) { - throw DataException( - '''Pub does not support publishing packages with non-resolving symlink: `${entity.path}` => `$target`.'''); - } - } final relative = p.relative(entity.path, from: root); if (Platform.isWindows) { return p.posix.joinAll(p.split(relative)); @@ -313,7 +302,16 @@ class Package { ); }, isDir: (dir) => dirExists(resolve(dir)), - ).map(resolve).toList(); + ).map(resolve).map((path) { + if (linkExists(path)) { + final target = Link(path).targetSync(); + if (!fileExists(path)) { + throw DataException( + '''Pub does not support publishing packages with non-resolving symlink: `${path}` => `$target`.'''); + } + } + return path; + }).toList(); } } diff --git a/test/package_list_files_test.dart b/test/package_list_files_test.dart index 23938b177..2e384eb40 100644 --- a/test/package_list_files_test.dart +++ b/test/package_list_files_test.dart @@ -76,6 +76,29 @@ void main() { ); }); + test('not throws on ignored directory symlinks', () async { + await d.dir(appPath, [ + d.pubspec({'name': 'myapp'}), + d.file('file1.txt', 'contents'), + d.file('file2.txt', 'contents'), + d.dir('subdir', [ + d.file('.pubignore', 'symlink'), + d.dir('a', [d.file('file')]), + ]), + ]).create(); + createDirectorySymlink( + p.join(d.sandbox, appPath, 'subdir', 'symlink'), 'a'); + + createEntrypoint(); + + expect(entrypoint!.root.listFiles(), { + p.join(root, 'pubspec.yaml'), + p.join(root, 'file1.txt'), + p.join(root, 'file2.txt'), + p.join(root, 'subdir', 'a', 'file'), + }); + }); + test('can list a package inside a symlinked folder', () async { await d.dir(appPath, [ d.pubspec({'name': 'myapp'}), From 963b6454f4ff0bbaa034dbc1587034dce7da094e Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Tue, 1 Feb 2022 19:00:09 +0300 Subject: [PATCH 03/58] add missing await --- test/validator/gitignore_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/validator/gitignore_test.dart b/test/validator/gitignore_test.dart index bbc25d7e0..5041bb300 100644 --- a/test/validator/gitignore_test.dart +++ b/test/validator/gitignore_test.dart @@ -133,7 +133,7 @@ void main() { workingDirectory: packageRoot); createDirectorySymlink( p.join(d.sandbox, appPath, 'dir_with_symlink', 'symlink'), '..'); - git.commit(); + await git.commit(); await expectValidation(contains('Package has 0 warnings.'), 0, workingDirectory: packageRoot); From b87e533072d45883b93f96a3f773f81091dcd138 Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Tue, 1 Feb 2022 19:00:54 +0300 Subject: [PATCH 04/58] remove unnecessary brackets in string interpolation --- lib/src/package.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index e9869cd5b..d54fff80a 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -307,7 +307,7 @@ class Package { final target = Link(path).targetSync(); if (!fileExists(path)) { throw DataException( - '''Pub does not support publishing packages with non-resolving symlink: `${path}` => `$target`.'''); + '''Pub does not support publishing packages with non-resolving symlink: `$path` => `$target`.'''); } } return path; From c087295aa5647d73eb4845a103a5337087fdf0b1 Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Tue, 1 Feb 2022 19:06:42 +0300 Subject: [PATCH 05/58] codestyle --- test/package_list_files_test.dart | 2 -- test/validator/gitignore_test.dart | 4 +++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/package_list_files_test.dart b/test/package_list_files_test.dart index 2e384eb40..3ced90867 100644 --- a/test/package_list_files_test.dart +++ b/test/package_list_files_test.dart @@ -46,8 +46,6 @@ void main() { ])); }); - - test('throws on directory symlinks', () async { await d.dir(appPath, [ d.pubspec({'name': 'myapp'}), diff --git a/test/validator/gitignore_test.dart b/test/validator/gitignore_test.dart index 5041bb300..9520f0f13 100644 --- a/test/validator/gitignore_test.dart +++ b/test/validator/gitignore_test.dart @@ -119,7 +119,9 @@ void main() { workingDirectory: packageRoot); }); - test('Should consider symlinks to be valid files and not list them as gitignored', () async { + test( + 'Should consider symlinks to be valid files and not list them as gitignored', + () async { final git = d.git(appPath, [ ...d.validPackage.contents, d.dir('dir_with_symlink', [ From 6e23dcc96089379a2c182935f20f2b7bbff2a868 Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Tue, 1 Feb 2022 19:35:03 +0300 Subject: [PATCH 06/58] add symlink cycle test --- test/package_list_files_test.dart | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/package_list_files_test.dart b/test/package_list_files_test.dart index 3ced90867..61f2e2068 100644 --- a/test/package_list_files_test.dart +++ b/test/package_list_files_test.dart @@ -97,6 +97,33 @@ void main() { }); }); + test('throws on symlink cycles', () async { + await d.dir(appPath, [ + d.pubspec({'name': 'myapp'}), + d.file('file1.txt', 'contents'), + d.file('file2.txt', 'contents'), + d.dir('subdir', [ + d.dir('a', [d.file('file')]), + ]), + ]).create(); + createDirectorySymlink( + p.join(d.sandbox, appPath, 'subdir', 'symlink'), '..'); + + createEntrypoint(); + + expect( + () => entrypoint!.root.listFiles(), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains( + 'Pub does not support publishing packages with non-resolving symlink:'), + ), + ), + ); + }); + test('can list a package inside a symlinked folder', () async { await d.dir(appPath, [ d.pubspec({'name': 'myapp'}), From b9b931436a9e44f1c0e495f8c644284264b111f4 Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Tue, 1 Feb 2022 20:03:00 +0300 Subject: [PATCH 07/58] use ignore rule without delimiter in checked-in but ignored warning test --- test/validator/gitignore_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/validator/gitignore_test.dart b/test/validator/gitignore_test.dart index 9520f0f13..c5f5b9668 100644 --- a/test/validator/gitignore_test.dart +++ b/test/validator/gitignore_test.dart @@ -125,7 +125,7 @@ void main() { final git = d.git(appPath, [ ...d.validPackage.contents, d.dir('dir_with_symlink', [ - d.file('.pubignore', '/symlink'), + d.file('.pubignore', 'symlink'), ]), ]); await git.create(); From 4486f006c89b970d9efb0fd2156bdf90075784ca Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Thu, 3 Feb 2022 16:45:48 +0700 Subject: [PATCH 08/58] probable fix for cycles on windows --- lib/src/package.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index d54fff80a..e31ad5551 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -233,7 +233,7 @@ class Package { return Ignore.listFiles( beneath: beneath, listDir: (dir) { - var contents = Directory(resolve(dir)).listSync(); + var contents = Directory(resolve(dir)).listSync(recursive: true); if (!recursive) { contents = contents.where((entity) => entity is! Directory).toList(); } From e49c45be281e597d3ce860a37a19ea1186abcb40 Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Thu, 3 Feb 2022 17:07:20 +0700 Subject: [PATCH 09/58] don't follow links --- lib/src/package.dart | 2 +- lib/src/validator/gitignore.dart | 2 +- test/package_list_files_test.dart | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index e31ad5551..69b4e8a49 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -233,7 +233,7 @@ class Package { return Ignore.listFiles( beneath: beneath, listDir: (dir) { - var contents = Directory(resolve(dir)).listSync(recursive: true); + var contents = Directory(resolve(dir)).listSync(followLinks: false); if (!recursive) { contents = contents.where((entity) => entity is! Directory).toList(); } diff --git a/lib/src/validator/gitignore.dart b/lib/src/validator/gitignore.dart index c9ad02f61..9b104d686 100644 --- a/lib/src/validator/gitignore.dart +++ b/lib/src/validator/gitignore.dart @@ -52,7 +52,7 @@ class GitignoreValidator extends Validator { final unignoredByGitignore = Ignore.listFiles( beneath: beneath, listDir: (dir) { - var contents = Directory(resolve(dir)).listSync(); + var contents = Directory(resolve(dir)).listSync(followLinks: false); return contents.map((entity) => p.posix.joinAll(p.split(p.relative(entity.path, from: root)))); }, diff --git a/test/package_list_files_test.dart b/test/package_list_files_test.dart index 61f2e2068..e52da132f 100644 --- a/test/package_list_files_test.dart +++ b/test/package_list_files_test.dart @@ -118,7 +118,8 @@ void main() { (e) => e.message, 'message', contains( - 'Pub does not support publishing packages with non-resolving symlink:'), + 'Pub does not support publishing packages with directory symlinks', + ), ), ), ); From 7e1cb20facb0f921510e0c146a23002ffa889ecb Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Thu, 3 Feb 2022 18:30:35 +0700 Subject: [PATCH 10/58] use appropriate error message (wrongly cherry-picked) --- test/package_list_files_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/package_list_files_test.dart b/test/package_list_files_test.dart index e52da132f..e2abfeffe 100644 --- a/test/package_list_files_test.dart +++ b/test/package_list_files_test.dart @@ -118,7 +118,7 @@ void main() { (e) => e.message, 'message', contains( - 'Pub does not support publishing packages with directory symlinks', + 'Pub does not support publishing packages with non-resolving symlink:', ), ), ), From 05f6cd9b65654080ae4db83a24e756bc1bbbd6c4 Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Fri, 4 Feb 2022 02:42:40 +0700 Subject: [PATCH 11/58] double check for directory symlinks --- lib/src/package.dart | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index 69b4e8a49..feb0f11c5 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -235,7 +235,13 @@ class Package { listDir: (dir) { var contents = Directory(resolve(dir)).listSync(followLinks: false); if (!recursive) { - contents = contents.where((entity) => entity is! Directory).toList(); + contents = contents + .where( + (entity) => + entity is! Directory && + !(linkExists(entity.path) && dirExists(entity.path)), + ) + .toList(); } return contents.map((entity) { final relative = p.relative(entity.path, from: root); From 0a6ffaa89d4f976e7981b24d45d339cc736d30cf Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Fri, 4 Feb 2022 02:46:26 +0700 Subject: [PATCH 12/58] replace "throws" => "not throws on valid directory symlinks" --- test/package_list_files_test.dart | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/test/package_list_files_test.dart b/test/package_list_files_test.dart index e2abfeffe..465d1e22d 100644 --- a/test/package_list_files_test.dart +++ b/test/package_list_files_test.dart @@ -46,7 +46,7 @@ void main() { ])); }); - test('throws on directory symlinks', () async { + test('not throws on valid directory symlinks', () async { await d.dir(appPath, [ d.pubspec({'name': 'myapp'}), d.file('file1.txt', 'contents'), @@ -60,18 +60,13 @@ void main() { createEntrypoint(); - expect( - () => entrypoint!.root.listFiles(), - throwsA( - isA().having( - (e) => e.message, - 'message', - contains( - 'Pub does not support publishing packages with directory symlinks', - ), - ), - ), - ); + expect(entrypoint!.root.listFiles(), { + p.join(root, 'pubspec.yaml'), + p.join(root, 'file1.txt'), + p.join(root, 'file2.txt'), + p.join(root, 'subdir', 'a', 'file'), + p.join(root, 'subdir', 'symlink', 'file'), + }); }); test('not throws on ignored directory symlinks', () async { From 10b8e3ea4d77d19a74a1d7382f6f1a75714c563c Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Sat, 12 Feb 2022 10:08:45 +0700 Subject: [PATCH 13/58] Revert "double check for directory symlinks" This reverts commit db790fbd --- lib/src/package.dart | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index feb0f11c5..69b4e8a49 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -235,13 +235,7 @@ class Package { listDir: (dir) { var contents = Directory(resolve(dir)).listSync(followLinks: false); if (!recursive) { - contents = contents - .where( - (entity) => - entity is! Directory && - !(linkExists(entity.path) && dirExists(entity.path)), - ) - .toList(); + contents = contents.where((entity) => entity is! Directory).toList(); } return contents.map((entity) { final relative = p.relative(entity.path, from: root); From f6bd8d242ffb01b68755d66dbb3f2a60cbf1a0ed Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Sat, 12 Feb 2022 10:09:38 +0700 Subject: [PATCH 14/58] Revert "don't follow links" This reverts commit f3713146 --- lib/src/package.dart | 2 +- lib/src/validator/gitignore.dart | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index 69b4e8a49..e31ad5551 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -233,7 +233,7 @@ class Package { return Ignore.listFiles( beneath: beneath, listDir: (dir) { - var contents = Directory(resolve(dir)).listSync(followLinks: false); + var contents = Directory(resolve(dir)).listSync(recursive: true); if (!recursive) { contents = contents.where((entity) => entity is! Directory).toList(); } diff --git a/lib/src/validator/gitignore.dart b/lib/src/validator/gitignore.dart index 9b104d686..c9ad02f61 100644 --- a/lib/src/validator/gitignore.dart +++ b/lib/src/validator/gitignore.dart @@ -52,7 +52,7 @@ class GitignoreValidator extends Validator { final unignoredByGitignore = Ignore.listFiles( beneath: beneath, listDir: (dir) { - var contents = Directory(resolve(dir)).listSync(followLinks: false); + var contents = Directory(resolve(dir)).listSync(); return contents.map((entity) => p.posix.joinAll(p.split(p.relative(entity.path, from: root)))); }, From bfc30e5ba5c3da84af8e15a7108091742a74e3d2 Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Sat, 12 Feb 2022 10:10:10 +0700 Subject: [PATCH 15/58] Revert "probable fix for cycles on windows" This reverts commit 47bb026a --- lib/src/package.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index e31ad5551..d54fff80a 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -233,7 +233,7 @@ class Package { return Ignore.listFiles( beneath: beneath, listDir: (dir) { - var contents = Directory(resolve(dir)).listSync(recursive: true); + var contents = Directory(resolve(dir)).listSync(); if (!recursive) { contents = contents.where((entity) => entity is! Directory).toList(); } From b1b69bc2b6f5c39e16dc143d122d72921606a5c8 Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Sat, 12 Feb 2022 10:23:53 +0700 Subject: [PATCH 16/58] try to resolve symbolic link manually before listSync invocation --- lib/src/package.dart | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index d54fff80a..d404b3548 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -233,7 +233,18 @@ class Package { return Ignore.listFiles( beneath: beneath, listDir: (dir) { - var contents = Directory(resolve(dir)).listSync(); + final resolvedDir = resolve(dir); + if (linkExists(resolvedDir)) { + try { + Link(resolvedDir).resolveSymbolicLinksSync(); + } on FileSystemException catch (_) { + final target = Link(resolvedDir).targetSync(); + throw DataException( + '''Pub does not support publishing packages with non-resolving symlink: `$path` => `$target`.'''); + } + } + + var contents = Directory(resolvedDir).listSync(); if (!recursive) { contents = contents.where((entity) => entity is! Directory).toList(); } From d97bfe2256b147694f695c0aec3c29a2d848e9a0 Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Sat, 12 Feb 2022 10:42:52 +0700 Subject: [PATCH 17/58] Revert "Revert "probable fix for cycles on windows"" This reverts commit fbe23f17 --- lib/src/package.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index d404b3548..d2ce72294 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -244,7 +244,7 @@ class Package { } } - var contents = Directory(resolvedDir).listSync(); + var contents = Directory(resolvedDir).listSync(recursive: true); if (!recursive) { contents = contents.where((entity) => entity is! Directory).toList(); } From add6e5c7e073042e5f170339356b8e639e97dfba Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Sat, 12 Feb 2022 10:43:17 +0700 Subject: [PATCH 18/58] Revert "Revert "don't follow links"" This reverts commit aab94fb6 --- lib/src/package.dart | 2 +- lib/src/validator/gitignore.dart | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index d2ce72294..957872375 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -244,7 +244,7 @@ class Package { } } - var contents = Directory(resolvedDir).listSync(recursive: true); + var contents = Directory(resolvedDir).listSync(followLinks: false); if (!recursive) { contents = contents.where((entity) => entity is! Directory).toList(); } diff --git a/lib/src/validator/gitignore.dart b/lib/src/validator/gitignore.dart index c9ad02f61..9b104d686 100644 --- a/lib/src/validator/gitignore.dart +++ b/lib/src/validator/gitignore.dart @@ -52,7 +52,7 @@ class GitignoreValidator extends Validator { final unignoredByGitignore = Ignore.listFiles( beneath: beneath, listDir: (dir) { - var contents = Directory(resolve(dir)).listSync(); + var contents = Directory(resolve(dir)).listSync(followLinks: false); return contents.map((entity) => p.posix.joinAll(p.split(p.relative(entity.path, from: root)))); }, From 380a1e07741b7d582a871dcecf83b44aa5e5bd17 Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Sat, 12 Feb 2022 10:43:26 +0700 Subject: [PATCH 19/58] Revert "Revert "double check for directory symlinks"" This reverts commit 1ca429ee --- lib/src/package.dart | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index 957872375..e931c6c4f 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -246,7 +246,13 @@ class Package { var contents = Directory(resolvedDir).listSync(followLinks: false); if (!recursive) { - contents = contents.where((entity) => entity is! Directory).toList(); + contents = contents + .where( + (entity) => + entity is! Directory && + !(linkExists(entity.path) && dirExists(entity.path)), + ) + .toList(); } return contents.map((entity) { final relative = p.relative(entity.path, from: root); From 6623ea26d74dd7a30dcc21b2e985fb828d3d3983 Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Sat, 12 Feb 2022 10:52:11 +0700 Subject: [PATCH 20/58] add additional cycles tests --- test/package_list_files_test.dart | 148 ++++++++++++++++++------------ 1 file changed, 87 insertions(+), 61 deletions(-) diff --git a/test/package_list_files_test.dart b/test/package_list_files_test.dart index 465d1e22d..83a8a3083 100644 --- a/test/package_list_files_test.dart +++ b/test/package_list_files_test.dart @@ -46,78 +46,104 @@ void main() { ])); }); - test('not throws on valid directory symlinks', () async { - await d.dir(appPath, [ - d.pubspec({'name': 'myapp'}), - d.file('file1.txt', 'contents'), - d.file('file2.txt', 'contents'), - d.dir('subdir', [ - d.dir('a', [d.file('file')]) - ]), - ]).create(); - createDirectorySymlink( - p.join(d.sandbox, appPath, 'subdir', 'symlink'), 'a'); + group('directory symlinks', () { + test('not throws on valid directory symlinks', () async { + await d.dir(appPath, [ + d.pubspec({'name': 'myapp'}), + d.file('file1.txt', 'contents'), + d.file('file2.txt', 'contents'), + d.dir('subdir', [ + d.dir('a', [d.file('file')]) + ]), + ]).create(); + createDirectorySymlink( + p.join(d.sandbox, appPath, 'subdir', 'symlink'), 'a'); - createEntrypoint(); + createEntrypoint(); - expect(entrypoint!.root.listFiles(), { - p.join(root, 'pubspec.yaml'), - p.join(root, 'file1.txt'), - p.join(root, 'file2.txt'), - p.join(root, 'subdir', 'a', 'file'), - p.join(root, 'subdir', 'symlink', 'file'), + expect(entrypoint!.root.listFiles(), { + p.join(root, 'pubspec.yaml'), + p.join(root, 'file1.txt'), + p.join(root, 'file2.txt'), + p.join(root, 'subdir', 'a', 'file'), + p.join(root, 'subdir', 'symlink', 'file'), + }); }); - }); - test('not throws on ignored directory symlinks', () async { - await d.dir(appPath, [ - d.pubspec({'name': 'myapp'}), - d.file('file1.txt', 'contents'), - d.file('file2.txt', 'contents'), - d.dir('subdir', [ - d.file('.pubignore', 'symlink'), - d.dir('a', [d.file('file')]), - ]), - ]).create(); - createDirectorySymlink( - p.join(d.sandbox, appPath, 'subdir', 'symlink'), 'a'); + test('not throws on ignored directory symlinks', () async { + await d.dir(appPath, [ + d.pubspec({'name': 'myapp'}), + d.file('file1.txt', 'contents'), + d.file('file2.txt', 'contents'), + d.dir('subdir', [ + d.file('.pubignore', 'symlink'), + d.dir('a', [d.file('file')]), + ]), + ]).create(); + createDirectorySymlink( + p.join(d.sandbox, appPath, 'subdir', 'symlink'), 'a'); - createEntrypoint(); + createEntrypoint(); - expect(entrypoint!.root.listFiles(), { - p.join(root, 'pubspec.yaml'), - p.join(root, 'file1.txt'), - p.join(root, 'file2.txt'), - p.join(root, 'subdir', 'a', 'file'), + expect(entrypoint!.root.listFiles(), { + p.join(root, 'pubspec.yaml'), + p.join(root, 'file1.txt'), + p.join(root, 'file2.txt'), + p.join(root, 'subdir', 'a', 'file'), + }); }); - }); - test('throws on symlink cycles', () async { - await d.dir(appPath, [ - d.pubspec({'name': 'myapp'}), - d.file('file1.txt', 'contents'), - d.file('file2.txt', 'contents'), - d.dir('subdir', [ - d.dir('a', [d.file('file')]), - ]), - ]).create(); - createDirectorySymlink( - p.join(d.sandbox, appPath, 'subdir', 'symlink'), '..'); + group('cycles', () { + test('throws on included link', () async { + await d.dir(appPath, [ + d.pubspec({'name': 'myapp'}), + d.file('file1.txt', 'contents'), + d.file('file2.txt', 'contents'), + d.dir('subdir', [ + d.dir('a', [d.file('file')]), + ]), + ]).create(); + createDirectorySymlink( + p.join(d.sandbox, appPath, 'subdir', 'symlink'), '..'); - createEntrypoint(); + createEntrypoint(); - expect( - () => entrypoint!.root.listFiles(), - throwsA( - isA().having( - (e) => e.message, - 'message', - contains( - 'Pub does not support publishing packages with non-resolving symlink:', + expect( + () => entrypoint!.root.listFiles(), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains( + 'Pub does not support publishing packages with non-resolving symlink:', + ), + ), ), - ), - ), - ); + ); + }); + test('not throws on ignored link', () async { + await d.dir(appPath, [ + d.pubspec({'name': 'myapp'}), + d.file('file1.txt', 'contents'), + d.file('file2.txt', 'contents'), + d.dir('subdir', [ + d.file('.pubignore', 'symlink'), + d.dir('a', [d.file('file')]), + ]), + ]).create(); + createDirectorySymlink( + p.join(d.sandbox, appPath, 'subdir', 'symlink'), '..'); + + createEntrypoint(); + + expect(entrypoint!.root.listFiles(), { + p.join(root, 'pubspec.yaml'), + p.join(root, 'file1.txt'), + p.join(root, 'file2.txt'), + p.join(root, 'subdir', 'a', 'file'), + }); + }); + }); }); test('can list a package inside a symlinked folder', () async { From 8e6aa02c9a55e753d3a9b569ecb2da4aeb90e3c9 Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Sun, 13 Feb 2022 21:11:54 +0700 Subject: [PATCH 21/58] try to see detailed log on windows --- lib/src/package.dart | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index e931c6c4f..9d0b4631f 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -243,8 +243,17 @@ class Package { '''Pub does not support publishing packages with non-resolving symlink: `$path` => `$target`.'''); } } - - var contents = Directory(resolvedDir).listSync(followLinks: false); + List contents; + try { + contents = Directory(resolvedDir).listSync(followLinks: false); + } catch (e) { + log.warning('dir: $dir'); + log.warning('resolvedDir: $resolvedDir'); + log.warning('linkExists(resolvedDir): ${linkExists(resolvedDir)}'); + log.warning( + 'Link(resolvedDir).resolveSymbolicLinksSync(): ${Link(resolvedDir).resolveSymbolicLinksSync()}'); + rethrow; + } if (!recursive) { contents = contents .where( From aa391dc2194e7f05042b17b2622eaaa271b6322d Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Fri, 3 Jun 2022 23:26:01 +0700 Subject: [PATCH 22/58] maintain list of visited symlink dirs --- lib/src/package.dart | 49 +++++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 26 deletions(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index 9d0b4631f..3bb656a88 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -230,30 +230,26 @@ class Package { return p.join(root, path); } + // maintain list of visited links to detect cyclical symlinks + final linkTargets = {}; + return Ignore.listFiles( beneath: beneath, listDir: (dir) { final resolvedDir = resolve(dir); + if (linkExists(resolvedDir)) { - try { - Link(resolvedDir).resolveSymbolicLinksSync(); - } on FileSystemException catch (_) { - final target = Link(resolvedDir).targetSync(); + final link = Link(resolvedDir); + if (linkTargets.add(link.targetSync())) { throw DataException( - '''Pub does not support publishing packages with non-resolving symlink: `$path` => `$target`.'''); + 'Pub does not support publishing packages with cyclical symlinks: ' + '`$resolvedDir` => `${link.targetSync()}`.', + ); } } - List contents; - try { - contents = Directory(resolvedDir).listSync(followLinks: false); - } catch (e) { - log.warning('dir: $dir'); - log.warning('resolvedDir: $resolvedDir'); - log.warning('linkExists(resolvedDir): ${linkExists(resolvedDir)}'); - log.warning( - 'Link(resolvedDir).resolveSymbolicLinksSync(): ${Link(resolvedDir).resolveSymbolicLinksSync()}'); - rethrow; - } + + var contents = Directory(resolvedDir).listSync(followLinks: false); + if (!recursive) { contents = contents .where( @@ -328,16 +324,17 @@ class Package { ); }, isDir: (dir) => dirExists(resolve(dir)), - ).map(resolve).map((path) { - if (linkExists(path)) { - final target = Link(path).targetSync(); - if (!fileExists(path)) { - throw DataException( - '''Pub does not support publishing packages with non-resolving symlink: `$path` => `$target`.'''); - } - } - return path; - }).toList(); + ).map(resolve).map(assertLinkResolvable).toList(); + } + + String assertLinkResolvable(String path) { + final target = Link(path).targetSync(); + if (!fileExists(path)) { + throw DataException( + 'Pub does not support publishing packages with non-resolving symlink: ' + '`$path` => `$target`.'); + } + return path; } } From dfb47e50677e93f635784883105ce5fd660b0426 Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Fri, 3 Jun 2022 23:38:33 +0700 Subject: [PATCH 23/58] use resolvedLinkTargets --- lib/src/package.dart | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index 3bb656a88..9d5b0a798 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -230,8 +230,8 @@ class Package { return p.join(root, path); } - // maintain list of visited links to detect cyclical symlinks - final linkTargets = {}; + // maintain list of resolved symlinks targets to detect cycles + final resolvedLinkTargets = {}; return Ignore.listFiles( beneath: beneath, @@ -240,7 +240,10 @@ class Package { if (linkExists(resolvedDir)) { final link = Link(resolvedDir); - if (linkTargets.add(link.targetSync())) { + final linkTarget = Directory(link.targetSync()); + final resolvedLinkTarget = linkTarget.resolveSymbolicLinksSync(); + final wasVisited = resolvedLinkTargets.add(resolvedLinkTarget); + if (wasVisited) { throw DataException( 'Pub does not support publishing packages with cyclical symlinks: ' '`$resolvedDir` => `${link.targetSync()}`.', From 270541087c4b928817eea614c6bdf9460639c37d Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Sat, 4 Jun 2022 00:16:32 +0700 Subject: [PATCH 24/58] fix assertLinksResolvable --- lib/src/package.dart | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index 9d5b0a798..efc235648 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -327,10 +327,13 @@ class Package { ); }, isDir: (dir) => dirExists(resolve(dir)), - ).map(resolve).map(assertLinkResolvable).toList(); + ).map(resolve).map(assertLinksResolvable).toList(); } - String assertLinkResolvable(String path) { + String assertLinksResolvable(String path) { + if (!linkExists(path)) { + return path; + } final target = Link(path).targetSync(); if (!fileExists(path)) { throw DataException( From f3aa7c38075d6fddbacbddf494c298e91694151c Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Sat, 4 Jun 2022 00:43:45 +0700 Subject: [PATCH 25/58] update message --- test/package_list_files_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/package_list_files_test.dart b/test/package_list_files_test.dart index 83a8a3083..1087fe3a4 100644 --- a/test/package_list_files_test.dart +++ b/test/package_list_files_test.dart @@ -115,7 +115,7 @@ void main() { (e) => e.message, 'message', contains( - 'Pub does not support publishing packages with non-resolving symlink:', + 'Pub does not support publishing packages with cyclical symlinks:', ), ), ), From ad092c503a1a5198d031c9c4242b253bf30f637c Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Sat, 4 Jun 2022 01:30:15 +0700 Subject: [PATCH 26/58] add a bit more tests --- test/package_list_files_test.dart | 57 ++++++++++++++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-) diff --git a/test/package_list_files_test.dart b/test/package_list_files_test.dart index 1087fe3a4..619ff8a7e 100644 --- a/test/package_list_files_test.dart +++ b/test/package_list_files_test.dart @@ -115,12 +115,41 @@ void main() { (e) => e.message, 'message', contains( - 'Pub does not support publishing packages with cyclical symlinks:', + 'Pub does not support publishing packages with non-resolving symlink:', ), ), ), ); }); + + test('throws on instant loop', () async { + await d.dir(appPath, [ + d.pubspec({'name': 'myapp'}), + d.file('file1.txt', 'contents'), + d.file('file2.txt', 'contents'), + d.dir('subdir', [ + d.dir('a', [d.file('file')]), + ]), + ]).create(); + createDirectorySymlink( + p.join(d.sandbox, appPath, 'subdir', 'symlink'), 'symlink'); + + createEntrypoint(); + + expect( + () => entrypoint!.root.listFiles(), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains( + 'Pub does not support publishing packages with non-resolving symlink:', + ), + ), + ), + ); + }); + test('not throws on ignored link', () async { await d.dir(appPath, [ d.pubspec({'name': 'myapp'}), @@ -197,6 +226,32 @@ void main() { ); }); + test('throws on loop file symlinks', () async { + await d.dir(appPath, [ + d.pubspec({'name': 'myapp'}), + d.file('file1.txt', 'contents'), + d.file('file2.txt', 'contents'), + d.dir('subdir', [ + d.dir('a', [d.file('file')]) + ]), + ]).create(); + Link(p.join(d.sandbox, appPath, 'subdir', 'symlink')).createSync('symlink'); + + createEntrypoint(); + + expect( + () => entrypoint!.root.listFiles(), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains( + 'Pub does not support publishing packages with non-resolving symlink:'), + ), + ), + ); + }); + test('throws on reciprocal symlinks', () async { await d.dir(appPath, [ d.pubspec({'name': 'myapp'}), From 003a59d50543cc1456c84bf38f653a72f1d64cff Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Sat, 4 Jun 2022 01:31:01 +0700 Subject: [PATCH 27/58] try to resolve or else count occurrences --- lib/src/package.dart | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index efc235648..4ed9d2dea 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -231,7 +231,7 @@ class Package { } // maintain list of resolved symlinks targets to detect cycles - final resolvedLinkTargets = {}; + final resolvedTargetToLink = >{}; return Ignore.listFiles( beneath: beneath, @@ -240,15 +240,31 @@ class Package { if (linkExists(resolvedDir)) { final link = Link(resolvedDir); - final linkTarget = Directory(link.targetSync()); - final resolvedLinkTarget = linkTarget.resolveSymbolicLinksSync(); - final wasVisited = resolvedLinkTargets.add(resolvedLinkTarget); - if (wasVisited) { + final target = link.targetSync(); + final String resolvedTarget; + + // default try to resolve a link + try { + resolvedTarget = link.resolveSymbolicLinksSync(); + } on FileSystemException catch (_) { throw DataException( - 'Pub does not support publishing packages with cyclical symlinks: ' + 'Pub does not support publishing packages with non-resolving symlink: ' '`$resolvedDir` => `${link.targetSync()}`.', ); } + + // fallback for windows + if (Platform.isWindows) { + final isFirstOccurrence = resolvedTargetToLink + .putIfAbsent(resolvedTarget, () => {}) + .add(target); + if (!isFirstOccurrence) { + throw DataException( + 'Pub does not support publishing packages with non-resolving symlinks: ' + '`$resolvedDir` => `${link.targetSync()}`.', + ); + } + } } var contents = Directory(resolvedDir).listSync(followLinks: false); From 73455c6123ece2152ec9fedba299a8eea0afb0ef Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Sat, 4 Jun 2022 02:14:49 +0700 Subject: [PATCH 28/58] typo: symlinks => symlink --- lib/src/package.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index 4ed9d2dea..29ea24874 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -260,7 +260,7 @@ class Package { .add(target); if (!isFirstOccurrence) { throw DataException( - 'Pub does not support publishing packages with non-resolving symlinks: ' + 'Pub does not support publishing packages with non-resolving symlink: ' '`$resolvedDir` => `${link.targetSync()}`.', ); } From fa09a85057be754b87cc8130516f2da1548679d1 Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Sun, 5 Jun 2022 02:36:48 +0700 Subject: [PATCH 29/58] rewrite symlinks detection --- lib/src/package.dart | 67 ++++++++++++++++++++++---------------------- 1 file changed, 33 insertions(+), 34 deletions(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index 29ea24874..3578855ee 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -230,42 +230,14 @@ class Package { return p.join(root, path); } - // maintain list of resolved symlinks targets to detect cycles - final resolvedTargetToLink = >{}; + // maintain set of visited symlinks for every directory + final visitedSymlinks = >{resolve(beneath): {}}; return Ignore.listFiles( beneath: beneath, listDir: (dir) { final resolvedDir = resolve(dir); - - if (linkExists(resolvedDir)) { - final link = Link(resolvedDir); - final target = link.targetSync(); - final String resolvedTarget; - - // default try to resolve a link - try { - resolvedTarget = link.resolveSymbolicLinksSync(); - } on FileSystemException catch (_) { - throw DataException( - 'Pub does not support publishing packages with non-resolving symlink: ' - '`$resolvedDir` => `${link.targetSync()}`.', - ); - } - - // fallback for windows - if (Platform.isWindows) { - final isFirstOccurrence = resolvedTargetToLink - .putIfAbsent(resolvedTarget, () => {}) - .add(target); - if (!isFirstOccurrence) { - throw DataException( - 'Pub does not support publishing packages with non-resolving symlink: ' - '`$resolvedDir` => `${link.targetSync()}`.', - ); - } - } - } + assertSymlinkLoop(resolvedDir, visitedSymlinks); var contents = Directory(resolvedDir).listSync(followLinks: false); @@ -283,6 +255,7 @@ class Package { if (Platform.isWindows) { return p.posix.joinAll(p.split(relative)); } + visitedSymlinks[resolve(relative)] = visitedSymlinks[resolvedDir]!; return relative; }); }, @@ -343,15 +316,41 @@ class Package { ); }, isDir: (dir) => dirExists(resolve(dir)), - ).map(resolve).map(assertLinksResolvable).toList(); + ).map(resolve).map(assertFileLinksResolvable).toList(); + } + + void assertSymlinkLoop( + String resolvedDir, + Map> visitedSymlinks, + ) { + final link = Link(resolvedDir); + if (link.existsSync()) { + // "normalize" link path by resolving all links above it. + final resolvedLinkPath = p.join( + link.parent.resolveSymbolicLinksSync(), + link.uri.pathSegments.last, + ); + + // copy on write + final currentSymlinks = + visitedSymlinks[resolvedDir] = visitedSymlinks[resolvedDir]!.toSet(); + if (!currentSymlinks.add(resolvedLinkPath)) { + final link = Link(resolvedDir); + final target = link.targetSync(); + throw DataException( + 'Pub does not support publishing packages with non-resolving symlink: ' + '`$resolvedDir` => `$target`.', + ); + } + } } - String assertLinksResolvable(String path) { + String assertFileLinksResolvable(String path) { if (!linkExists(path)) { return path; } - final target = Link(path).targetSync(); if (!fileExists(path)) { + final target = Link(path).targetSync(); throw DataException( 'Pub does not support publishing packages with non-resolving symlink: ' '`$path` => `$target`.'); From 1a72dc3e937a3a35e3863a0df0fd145fe0237c56 Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Sun, 5 Jun 2022 03:17:23 +0700 Subject: [PATCH 30/58] create and append in single assertion --- lib/src/package.dart | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index 3578855ee..771c9f5be 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -231,13 +231,12 @@ class Package { } // maintain set of visited symlinks for every directory - final visitedSymlinks = >{resolve(beneath): {}}; - + final visitedSymlinks = >{}; return Ignore.listFiles( beneath: beneath, listDir: (dir) { final resolvedDir = resolve(dir); - assertSymlinkLoop(resolvedDir, visitedSymlinks); + assertSymlinkLoop(dir, resolvedDir, visitedSymlinks); var contents = Directory(resolvedDir).listSync(followLinks: false); @@ -255,7 +254,6 @@ class Package { if (Platform.isWindows) { return p.posix.joinAll(p.split(relative)); } - visitedSymlinks[resolve(relative)] = visitedSymlinks[resolvedDir]!; return relative; }); }, @@ -320,6 +318,7 @@ class Package { } void assertSymlinkLoop( + String dir, String resolvedDir, Map> visitedSymlinks, ) { @@ -328,12 +327,12 @@ class Package { // "normalize" link path by resolving all links above it. final resolvedLinkPath = p.join( link.parent.resolveSymbolicLinksSync(), - link.uri.pathSegments.last, + p.basename(resolvedDir), ); // copy on write - final currentSymlinks = - visitedSymlinks[resolvedDir] = visitedSymlinks[resolvedDir]!.toSet(); + final currentSymlinks = visitedSymlinks[p.dirname(dir)] ?? {}; + visitedSymlinks[dir] = currentSymlinks; if (!currentSymlinks.add(resolvedLinkPath)) { final link = Link(resolvedDir); final target = link.targetSync(); From 3a58e929e138c92d3d2284ef46f785bd4e8c7014 Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Sun, 5 Jun 2022 03:18:44 +0700 Subject: [PATCH 31/58] posix dirname for internal representation --- lib/src/package.dart | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index 771c9f5be..23fc3ae87 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -318,7 +318,7 @@ class Package { } void assertSymlinkLoop( - String dir, + String posixDir, String resolvedDir, Map> visitedSymlinks, ) { @@ -331,8 +331,9 @@ class Package { ); // copy on write - final currentSymlinks = visitedSymlinks[p.dirname(dir)] ?? {}; - visitedSymlinks[dir] = currentSymlinks; + final currentSymlinks = + visitedSymlinks[p.posix.dirname(posixDir)] ?? {}; + visitedSymlinks[posixDir] = currentSymlinks; if (!currentSymlinks.add(resolvedLinkPath)) { final link = Link(resolvedDir); final target = link.targetSync(); From ce5544ab40c4c0df3c1df853be2aaab82670c744 Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Sun, 5 Jun 2022 17:53:43 +0700 Subject: [PATCH 32/58] fix copy on write solution propagate visited symlinks through ordinary directories --- lib/src/package.dart | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index 23fc3ae87..3198c8283 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -323,17 +323,21 @@ class Package { Map> visitedSymlinks, ) { final link = Link(resolvedDir); + + var currentSymlinks = + visitedSymlinks[p.posix.dirname(posixDir)] ?? {}; + visitedSymlinks[posixDir] = currentSymlinks; + if (link.existsSync()) { + // copy on write + visitedSymlinks[posixDir] = currentSymlinks = currentSymlinks.toSet(); + // "normalize" link path by resolving all links above it. final resolvedLinkPath = p.join( link.parent.resolveSymbolicLinksSync(), p.basename(resolvedDir), ); - // copy on write - final currentSymlinks = - visitedSymlinks[p.posix.dirname(posixDir)] ?? {}; - visitedSymlinks[posixDir] = currentSymlinks; if (!currentSymlinks.add(resolvedLinkPath)) { final link = Link(resolvedDir); final target = link.targetSync(); From d5b595bd175dd469c010ef9bc266d2ea62a19784 Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Sun, 5 Jun 2022 17:56:05 +0700 Subject: [PATCH 33/58] code style --- lib/src/package.dart | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index 3198c8283..fde030069 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -324,13 +324,12 @@ class Package { ) { final link = Link(resolvedDir); - var currentSymlinks = - visitedSymlinks[p.posix.dirname(posixDir)] ?? {}; - visitedSymlinks[posixDir] = currentSymlinks; + var currentSymlinks = visitedSymlinks[p.posix.dirname(posixDir)]; + currentSymlinks ??= {}; if (link.existsSync()) { // copy on write - visitedSymlinks[posixDir] = currentSymlinks = currentSymlinks.toSet(); + currentSymlinks = currentSymlinks.toSet(); // "normalize" link path by resolving all links above it. final resolvedLinkPath = p.join( @@ -347,6 +346,8 @@ class Package { ); } } + + visitedSymlinks[posixDir] = currentSymlinks; } String assertFileLinksResolvable(String path) { From 7745fc7be231b12df746bb34d0992fd9e25b1bda Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Sun, 5 Jun 2022 18:09:40 +0700 Subject: [PATCH 34/58] explicitly say that error happened because of loop --- lib/src/package.dart | 5 +++-- test/package_list_files_test.dart | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index fde030069..bc2ed8ac6 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -341,8 +341,9 @@ class Package { final link = Link(resolvedDir); final target = link.targetSync(); throw DataException( - 'Pub does not support publishing packages with non-resolving symlink: ' - '`$resolvedDir` => `$target`.', + 'Pub does not support publishing packages with symlinks loop: ' + '`$resolvedDir` => `$target`. ' + 'Full list of visited symlinks: $currentSymlinks', ); } } diff --git a/test/package_list_files_test.dart b/test/package_list_files_test.dart index 619ff8a7e..0408dd80d 100644 --- a/test/package_list_files_test.dart +++ b/test/package_list_files_test.dart @@ -115,7 +115,7 @@ void main() { (e) => e.message, 'message', contains( - 'Pub does not support publishing packages with non-resolving symlink:', + 'Pub does not support publishing packages with symlinks loop:', ), ), ), @@ -143,7 +143,7 @@ void main() { (e) => e.message, 'message', contains( - 'Pub does not support publishing packages with non-resolving symlink:', + 'Pub does not support publishing packages with symlinks loop:', ), ), ), From af93eb1e3a82423bec69471eeba0378e43a6ae21 Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Sun, 5 Jun 2022 18:10:00 +0700 Subject: [PATCH 35/58] not throws on ignored broken directory symlinks --- test/package_list_files_test.dart | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test/package_list_files_test.dart b/test/package_list_files_test.dart index 0408dd80d..db5a0793f 100644 --- a/test/package_list_files_test.dart +++ b/test/package_list_files_test.dart @@ -93,6 +93,29 @@ void main() { }); }); + test('not throws on ignored broken directory symlinks', () async { + await d.dir(appPath, [ + d.pubspec({'name': 'myapp'}), + d.file('file1.txt', 'contents'), + d.file('file2.txt', 'contents'), + d.dir('subdir', [ + d.file('.pubignore', 'symlink'), + d.dir('a', [d.file('file')]), + ]), + ]).create(); + createDirectorySymlink( + p.join(d.sandbox, appPath, 'subdir', 'symlink'), 'b'); + + createEntrypoint(); + + expect(entrypoint!.root.listFiles(), { + p.join(root, 'pubspec.yaml'), + p.join(root, 'file1.txt'), + p.join(root, 'file2.txt'), + p.join(root, 'subdir', 'a', 'file'), + }); + }); + group('cycles', () { test('throws on included link', () async { await d.dir(appPath, [ From 8aa575d4d733b1d3c0c6888c037e10a4e86457d4 Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Sun, 5 Jun 2022 18:10:09 +0700 Subject: [PATCH 36/58] not throws on valid links to the same directory --- test/package_list_files_test.dart | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/package_list_files_test.dart b/test/package_list_files_test.dart index db5a0793f..06f336e12 100644 --- a/test/package_list_files_test.dart +++ b/test/package_list_files_test.dart @@ -195,6 +195,35 @@ void main() { p.join(root, 'subdir', 'a', 'file'), }); }); + + test('not throws on valid links to the same directory', () async { + await d.dir(appPath, [ + d.pubspec({'name': 'myapp'}), + d.file('file1.txt', 'contents'), + d.file('file2.txt', 'contents'), + d.dir('subdir', [ + d.dir('a', [d.file('file')]), + ]), + ]).create(); + createDirectorySymlink( + p.join(d.sandbox, appPath, 'subdir', 'symlink1'), 'a'); + createDirectorySymlink( + p.join(d.sandbox, appPath, 'subdir', 'symlink2'), 'a'); + createDirectorySymlink( + p.join(d.sandbox, appPath, 'symlink3'), p.join('subdir', 'a')); + + createEntrypoint(); + + expect(entrypoint!.root.listFiles(), { + p.join(root, 'pubspec.yaml'), + p.join(root, 'file1.txt'), + p.join(root, 'file2.txt'), + p.join(root, 'subdir', 'a', 'file'), + p.join(root, 'subdir', 'symlink1', 'file'), + p.join(root, 'subdir', 'symlink2', 'file'), + p.join(root, 'symlink3', 'file'), + }); + }); }); }); From 9700d92606cdbd481c2f8f0db2f1ed0bb4f9e85d Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Sun, 5 Jun 2022 18:25:24 +0700 Subject: [PATCH 37/58] instant loop is actually non-resolving --- test/package_list_files_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/package_list_files_test.dart b/test/package_list_files_test.dart index 06f336e12..47d3561c2 100644 --- a/test/package_list_files_test.dart +++ b/test/package_list_files_test.dart @@ -166,7 +166,7 @@ void main() { (e) => e.message, 'message', contains( - 'Pub does not support publishing packages with symlinks loop:', + 'Pub does not support publishing packages with non-resolving symlink:', ), ), ), From 0dcf5572e65a0ca091003e20e90c0e41c0732a51 Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Sun, 5 Jun 2022 18:55:08 +0700 Subject: [PATCH 38/58] add detailed comment with explanation --- lib/src/package.dart | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index bc2ed8ac6..bf5cf248c 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -230,8 +230,20 @@ class Package { return p.join(root, path); } - // maintain set of visited symlinks for every directory + // Maintain set of visited symlinks for every directory. + // If the same symlink is visited twice while moving down the tree, + // then we have faced a loop. + // + // additional complexity: + // N - number of directory symlinks + // memory and time complexity are roughly the same: + // from O(N) (without nested symlinks): + // single set is created for each symlink and never copied + // up to O(N^2) (each symlink is nested in previous one): + // for each symlink clone set with all visited symlinks. + // i-th set contains i symlinks. final visitedSymlinks = >{}; + return Ignore.listFiles( beneath: beneath, listDir: (dir) { From 9c2c78d778a0d9487e8f0b26f0b2633ce67d5c7c Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Sun, 5 Jun 2022 18:57:57 +0700 Subject: [PATCH 39/58] make assert methods static --- lib/src/package.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index bf5cf248c..fdfeecf71 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -329,7 +329,7 @@ class Package { ).map(resolve).map(assertFileLinksResolvable).toList(); } - void assertSymlinkLoop( + static void assertSymlinkLoop( String posixDir, String resolvedDir, Map> visitedSymlinks, @@ -363,7 +363,7 @@ class Package { visitedSymlinks[posixDir] = currentSymlinks; } - String assertFileLinksResolvable(String path) { + static String assertFileLinksResolvable(String path) { if (!linkExists(path)) { return path; } From 42b0821fa11048e621e40e8a0d04873bc2c98d2a Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Sun, 5 Jun 2022 22:55:11 +0700 Subject: [PATCH 40/58] add test for nested loop --- test/package_list_files_test.dart | 36 +++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/test/package_list_files_test.dart b/test/package_list_files_test.dart index 47d3561c2..76ea5e371 100644 --- a/test/package_list_files_test.dart +++ b/test/package_list_files_test.dart @@ -173,6 +173,42 @@ void main() { ); }); + test('throws on nested loop', () async { + await d.dir(appPath, [ + d.pubspec({'name': 'myapp'}), + d.file('file1.txt', 'contents'), + d.file('file2.txt', 'contents'), + d.dir('subdir', [ + d.dir('a', [d.file('file')]), + d.dir('b'), + d.dir('c'), + ]), + ]).create(); + createDirectorySymlink( + p.join(d.sandbox, appPath, 'subdir', 'symlink'), 'a'); + createDirectorySymlink( + p.join(d.sandbox, appPath, 'subdir', 'a', 'symlink1'), '../b'); + createDirectorySymlink( + p.join(d.sandbox, appPath, 'subdir', 'b', 'symlink2'), '../c'); + createDirectorySymlink( + p.join(d.sandbox, appPath, 'subdir', 'c', 'symlink3'), '../a'); + + createEntrypoint(); + + expect( + () => entrypoint!.root.listFiles(), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains( + 'Pub does not support publishing packages with symlinks loop:', + ), + ), + ), + ); + }); + test('not throws on ignored link', () async { await d.dir(appPath, [ d.pubspec({'name': 'myapp'}), From 020f465737f9dde057126a150ad8117054c7472b Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Sun, 5 Jun 2022 22:56:11 +0700 Subject: [PATCH 41/58] rename: posixDir => internalDir --- lib/src/package.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index fdfeecf71..8ea43cd7f 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -330,13 +330,13 @@ class Package { } static void assertSymlinkLoop( - String posixDir, + String internalDir, String resolvedDir, Map> visitedSymlinks, ) { final link = Link(resolvedDir); - var currentSymlinks = visitedSymlinks[p.posix.dirname(posixDir)]; + var currentSymlinks = visitedSymlinks[p.posix.dirname(internalDir)]; currentSymlinks ??= {}; if (link.existsSync()) { @@ -360,7 +360,7 @@ class Package { } } - visitedSymlinks[posixDir] = currentSymlinks; + visitedSymlinks[internalDir] = currentSymlinks; } static String assertFileLinksResolvable(String path) { From 07b8cf9e8e3ae3cdc52324d00a5a232f899c47d8 Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Sun, 5 Jun 2022 22:57:06 +0700 Subject: [PATCH 42/58] make assertions private --- lib/src/package.dart | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index 8ea43cd7f..831855a94 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -248,7 +248,7 @@ class Package { beneath: beneath, listDir: (dir) { final resolvedDir = resolve(dir); - assertSymlinkLoop(dir, resolvedDir, visitedSymlinks); + _assertSymlinkLoop(dir, resolvedDir, visitedSymlinks); var contents = Directory(resolvedDir).listSync(followLinks: false); @@ -326,10 +326,10 @@ class Package { ); }, isDir: (dir) => dirExists(resolve(dir)), - ).map(resolve).map(assertFileLinksResolvable).toList(); + ).map(resolve).map(_assertFileLinksResolvable).toList(); } - static void assertSymlinkLoop( + static void _assertSymlinkLoop( String internalDir, String resolvedDir, Map> visitedSymlinks, @@ -363,7 +363,7 @@ class Package { visitedSymlinks[internalDir] = currentSymlinks; } - static String assertFileLinksResolvable(String path) { + static String _assertFileLinksResolvable(String path) { if (!linkExists(path)) { return path; } From 68831dc9e8acfb6aec350280d69cdf6e61049f8f Mon Sep 17 00:00:00 2001 From: Bogdan Lukin Date: Mon, 6 Jun 2022 01:33:49 +0700 Subject: [PATCH 43/58] use join instead of raw posix path --- test/package_list_files_test.dart | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/test/package_list_files_test.dart b/test/package_list_files_test.dart index 76ea5e371..02f72e785 100644 --- a/test/package_list_files_test.dart +++ b/test/package_list_files_test.dart @@ -187,11 +187,17 @@ void main() { createDirectorySymlink( p.join(d.sandbox, appPath, 'subdir', 'symlink'), 'a'); createDirectorySymlink( - p.join(d.sandbox, appPath, 'subdir', 'a', 'symlink1'), '../b'); + p.join(d.sandbox, appPath, 'subdir', 'a', 'symlink1'), + p.join('..', 'b'), + ); createDirectorySymlink( - p.join(d.sandbox, appPath, 'subdir', 'b', 'symlink2'), '../c'); + p.join(d.sandbox, appPath, 'subdir', 'b', 'symlink2'), + p.join('..', 'c'), + ); createDirectorySymlink( - p.join(d.sandbox, appPath, 'subdir', 'c', 'symlink3'), '../a'); + p.join(d.sandbox, appPath, 'subdir', 'c', 'symlink3'), + p.join('..', 'a'), + ); createEntrypoint(); From cdb66d3428f4501ea1cf30f665d983e25d1e20fa Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Tue, 7 Jun 2022 12:30:21 +0000 Subject: [PATCH 44/58] Use FileSystemEntity.resolveSymbolicLinks --- lib/src/package.dart | 73 ++++++++++++++++---------------------------- 1 file changed, 26 insertions(+), 47 deletions(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index 831855a94..9c9928962 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -242,13 +242,27 @@ class Package { // up to O(N^2) (each symlink is nested in previous one): // for each symlink clone set with all visited symlinks. // i-th set contains i symlinks. - final visitedSymlinks = >{}; + final visitedSymlinks = {}; - return Ignore.listFiles( + final result = Ignore.listFiles( beneath: beneath, listDir: (dir) { final resolvedDir = resolve(dir); - _assertSymlinkLoop(dir, resolvedDir, visitedSymlinks); + + if (Link(resolvedDir).existsSync()) { + final canonicalLink = p.canonicalize( + p.join( + Directory(p.dirname(resolvedDir)).resolveSymbolicLinksSync(), + p.basename(resolvedDir), + ), + ); + if (!visitedSymlinks.add(canonicalLink)) { + throw DataException( + 'Pub does not support publishing packages with symlinks loop: ' + '`$resolvedDir` => `$canonicalLink`. ', + ); + } + } var contents = Directory(resolvedDir).listSync(followLinks: false); @@ -326,54 +340,19 @@ class Package { ); }, isDir: (dir) => dirExists(resolve(dir)), - ).map(resolve).map(_assertFileLinksResolvable).toList(); - } + ).map(resolve).toList(); - static void _assertSymlinkLoop( - String internalDir, - String resolvedDir, - Map> visitedSymlinks, - ) { - final link = Link(resolvedDir); - - var currentSymlinks = visitedSymlinks[p.posix.dirname(internalDir)]; - currentSymlinks ??= {}; - - if (link.existsSync()) { - // copy on write - currentSymlinks = currentSymlinks.toSet(); - - // "normalize" link path by resolving all links above it. - final resolvedLinkPath = p.join( - link.parent.resolveSymbolicLinksSync(), - p.basename(resolvedDir), - ); - - if (!currentSymlinks.add(resolvedLinkPath)) { - final link = Link(resolvedDir); - final target = link.targetSync(); + // Check that all symlinks in [result] are valid. + for (final file in result) { + try { + File(file).resolveSymbolicLinksSync(); + } on IOException { throw DataException( - 'Pub does not support publishing packages with symlinks loop: ' - '`$resolvedDir` => `$target`. ' - 'Full list of visited symlinks: $currentSymlinks', - ); + 'Pub does not support publishing packages with non-resolving symlink: ' + '`$path`'); } } - - visitedSymlinks[internalDir] = currentSymlinks; - } - - static String _assertFileLinksResolvable(String path) { - if (!linkExists(path)) { - return path; - } - if (!fileExists(path)) { - final target = Link(path).targetSync(); - throw DataException( - 'Pub does not support publishing packages with non-resolving symlink: ' - '`$path` => `$target`.'); - } - return path; + return result; } } From 599a42ff5f7b534d21f15581fed3dcb4ba3f1494 Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Mon, 21 Oct 2024 14:04:52 +0000 Subject: [PATCH 45/58] publish integration test --- test/lish/symlinks_test.dart | 69 ++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 test/lish/symlinks_test.dart diff --git a/test/lish/symlinks_test.dart b/test/lish/symlinks_test.dart new file mode 100644 index 000000000..ced358633 --- /dev/null +++ b/test/lish/symlinks_test.dart @@ -0,0 +1,69 @@ +// Copyright (c) 2025, 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:path/path.dart' as p; +import 'package:tar/tar.dart'; +import 'package:test/test.dart'; + +import '../descriptor.dart'; +import '../test_pub.dart'; + +Future main() async { + test('symlink directories are replaced by their targets', () async { + await validPackage().create(); + await dir('a', [file('aa', 'aaa')]).create(); + await file('t', 'ttt').create(); + + await dir(appPath, [ + dir('b', [file('bb', 'bbb')]), + ]).create(); + Link(p.join(sandbox, appPath, 'symlink_to_dir_outside_package')) + .createSync(p.join(sandbox, 'a')); + Link(p.join(sandbox, appPath, 'symlink_to_dir_outside_package_relative')) + .createSync(p.join('..', 'a')); + Link(p.join(sandbox, appPath, 'symlink_to_dir_inside_package')) + .createSync(p.join(sandbox, appPath, 'b')); + Link(p.join(sandbox, appPath, 'symlink_to_dir_inside_package_relative')) + .createSync('b'); + Link(p.join(sandbox, appPath, 'b', 'l')).createSync(p.join(sandbox, 't')); + + await runPub(args: ['publish', '--to-archive=archive.tar.gz']); + + final reader = TarReader( + File(p.join(sandbox, appPath, 'archive.tar.gz')) + .openRead() + .transform(GZipCodec().decoder), + ); + + while (await reader.moveNext()) { + final current = reader.current; + expect(current.type, isNot(TypeFlag.symlink)); + } + + await runPub(args: ['cache', 'preload', 'archive.tar.gz']); + + await dir('test_pkg-1.0.0', [ + ...validPackage().contents, + dir('symlink_to_dir_outside_package', [ + file('aa', 'aaa'), + ]), + dir('symlink_to_dir_outside_package_relative', [ + file('aa', 'aaa'), + ]), + dir('b', [file('bb', 'bbb')]), + dir('symlink_to_dir_inside_package', [ + file('bb', 'bbb'), + file('l', 'ttt'), + ]), + dir('symlink_to_dir_inside_package_relative', [ + file('bb', 'bbb'), + file('l', 'ttt'), + ]), + ]).validate( + p.join(sandbox, cachePath, 'hosted', 'pub.dev'), + ); + }); +} From 4ed382a3462452e835b1309a6ae64c1ed037aab3 Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Tue, 22 Oct 2024 12:21:56 +0000 Subject: [PATCH 46/58] Rely on system detecting cycles --- lib/src/package.dart | 66 +++++++++-------------------- test/package_list_files_test.dart | 67 +++++++++++++++++------------- test/validator/gitignore_test.dart | 21 ++++++---- 3 files changed, 71 insertions(+), 83 deletions(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index ab4da34ec..626087bfc 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -278,50 +278,31 @@ See $workspacesDocUrl for more information. return p.join(root, path); } - // Maintain set of visited symlinks for every directory. - // If the same symlink is visited twice while moving down the tree, - // then we have faced a loop. - // - // additional complexity: - // N - number of directory symlinks - // memory and time complexity are roughly the same: - // from O(N) (without nested symlinks): - // single set is created for each symlink and never copied - // up to O(N^2) (each symlink is nested in previous one): - // for each symlink clone set with all visited symlinks. - // i-th set contains i symlinks. - final visitedSymlinks = {}; + /// Throws if [path] is a link that cannot resolve. + /// + /// Circular links will fail to resolve at some depth defined by the os. + void verifyLink(String path) { + final link = Link(path); + if (link.existsSync()) { + try { + link.resolveSymbolicLinksSync(); + } on FileSystemException catch (e) { + throw DataException( + 'Could not resolve symbolic link $path. $e', + ); + } + } + } final result = Ignore.listFiles( beneath: beneath, listDir: (dir) { final resolvedDir = resolve(dir); - - if (Link(resolvedDir).existsSync()) { - final canonicalLink = p.canonicalize( - p.join( - Directory(p.dirname(resolvedDir)).resolveSymbolicLinksSync(), - p.basename(resolvedDir), - ), - ); - if (!visitedSymlinks.add(canonicalLink)) { - throw DataException( - 'Pub does not support publishing packages with symlinks loop: ' - '`$resolvedDir` => `$canonicalLink`. ', - ); - } - } - + verifyLink(resolvedDir); var contents = Directory(resolvedDir).listSync(followLinks: false); if (!recursive) { - contents = contents - .where( - (entity) => - entity is! Directory && - !(linkExists(entity.path) && dirExists(entity.path)), - ) - .toList(); + contents = contents.where((entity) => entity is! Directory).toList(); } return contents.map((entity) { final relative = p.relative(entity.path, from: root); @@ -392,17 +373,8 @@ See $workspacesDocUrl for more information. isDir: (dir) => dirExists(resolve(dir)), includeDirs: includeDirs, ).map(resolve).toList(); - - // Check that all symlinks in [result] are valid. - for (final file in result) { - try { - File(file).resolveSymbolicLinksSync(); - } on IOException { - throw DataException( - 'Pub does not support publishing packages with non-resolving symlink: ' - '`$file`', - ); - } + for (final f in result) { + verifyLink(f); } return result; } diff --git a/test/package_list_files_test.dart b/test/package_list_files_test.dart index 05fd9ccf2..349a6f807 100644 --- a/test/package_list_files_test.dart +++ b/test/package_list_files_test.dart @@ -57,11 +57,13 @@ void main() { d.file('file1.txt', 'contents'), d.file('file2.txt', 'contents'), d.dir('subdir', [ - d.dir('a', [d.file('file')]) + d.dir('a', [d.file('file')]), ]), ]).create(); createDirectorySymlink( - p.join(d.sandbox, appPath, 'subdir', 'symlink'), 'a'); + p.join(d.sandbox, appPath, 'subdir', 'symlink'), + 'a', + ); createEntrypoint(); @@ -85,7 +87,9 @@ void main() { ]), ]).create(); createDirectorySymlink( - p.join(d.sandbox, appPath, 'subdir', 'symlink'), 'a'); + p.join(d.sandbox, appPath, 'subdir', 'symlink'), + 'a', + ); createEntrypoint(); @@ -108,7 +112,9 @@ void main() { ]), ]).create(); createDirectorySymlink( - p.join(d.sandbox, appPath, 'subdir', 'symlink'), 'b'); + p.join(d.sandbox, appPath, 'subdir', 'symlink'), + 'b', + ); createEntrypoint(); @@ -131,7 +137,9 @@ void main() { ]), ]).create(); createDirectorySymlink( - p.join(d.sandbox, appPath, 'subdir', 'symlink'), '..'); + p.join(d.sandbox, appPath, 'subdir', 'symlink'), + '..', + ); createEntrypoint(); @@ -142,7 +150,7 @@ void main() { (e) => e.message, 'message', contains( - 'Pub does not support publishing packages with symlinks loop:', + 'Could not resolve symbolic link', ), ), ), @@ -159,7 +167,9 @@ void main() { ]), ]).create(); createDirectorySymlink( - p.join(d.sandbox, appPath, 'subdir', 'symlink'), 'symlink'); + p.join(d.sandbox, appPath, 'subdir', 'symlink'), + 'symlink', + ); createEntrypoint(); @@ -169,11 +179,7 @@ void main() { isA().having( (e) => e.message, 'message', - contains( - 'Pub does not support publishing packages with ' - 'non-resolving symlink: ' - '${p.join(d.sandbox, appPath, 'subdir', 'symlink')}', - ), + contains('Could not resolve symbolic link'), ), ), ); @@ -191,7 +197,9 @@ void main() { ]), ]).create(); createDirectorySymlink( - p.join(d.sandbox, appPath, 'subdir', 'symlink'), 'a'); + p.join(d.sandbox, appPath, 'subdir', 'symlink'), + 'a', + ); createDirectorySymlink( p.join(d.sandbox, appPath, 'subdir', 'a', 'symlink1'), p.join('..', 'b'), @@ -213,9 +221,7 @@ void main() { isA().having( (e) => e.message, 'message', - contains( - 'Pub does not support publishing packages with symlinks loop:', - ), + contains('Could not resolve symbolic link'), ), ), ); @@ -232,7 +238,9 @@ void main() { ]), ]).create(); createDirectorySymlink( - p.join(d.sandbox, appPath, 'subdir', 'symlink'), '..'); + p.join(d.sandbox, appPath, 'subdir', 'symlink'), + '..', + ); createEntrypoint(); @@ -254,11 +262,17 @@ void main() { ]), ]).create(); createDirectorySymlink( - p.join(d.sandbox, appPath, 'subdir', 'symlink1'), 'a'); + p.join(d.sandbox, appPath, 'subdir', 'symlink1'), + 'a', + ); createDirectorySymlink( - p.join(d.sandbox, appPath, 'subdir', 'symlink2'), 'a'); + p.join(d.sandbox, appPath, 'subdir', 'symlink2'), + 'a', + ); createDirectorySymlink( - p.join(d.sandbox, appPath, 'symlink3'), p.join('subdir', 'a')); + p.join(d.sandbox, appPath, 'symlink3'), + p.join('subdir', 'a'), + ); createEntrypoint(); @@ -322,8 +336,7 @@ void main() { (e) => e.message, 'message', contains( - 'Pub does not support publishing packages with ' - 'non-resolving symlink:', + 'Could not resolve symbolic link', ), ), ), @@ -336,7 +349,7 @@ void main() { d.file('file1.txt', 'contents'), d.file('file2.txt', 'contents'), d.dir('subdir', [ - d.dir('a', [d.file('file')]) + d.dir('a', [d.file('file')]), ]), ]).create(); Link(p.join(d.sandbox, appPath, 'subdir', 'symlink')).createSync('symlink'); @@ -350,7 +363,8 @@ void main() { (e) => e.message, 'message', contains( - 'Pub does not support publishing packages with non-resolving symlink:'), + 'Could not resolve symbolic link', + ), ), ), ); @@ -377,10 +391,7 @@ void main() { isA().having( (e) => e.message, 'message', - contains( - 'Pub does not support publishing packages with ' - 'non-resolving symlink:', - ), + contains('Could not resolve symbolic link'), ), ), ); diff --git a/test/validator/gitignore_test.dart b/test/validator/gitignore_test.dart index 475b11963..a8aa17798 100644 --- a/test/validator/gitignore_test.dart +++ b/test/validator/gitignore_test.dart @@ -146,10 +146,10 @@ void main() { }); test( - 'Should consider symlinks to be valid files and not list them as gitignored', - () async { + 'Should consider symlinks to be valid files and not list ' + 'them as gitignored', () async { final git = d.git(appPath, [ - ...d.validPackage.contents, + ...d.validPackage().contents, d.dir('dir_with_symlink', [ d.file('.pubignore', 'symlink'), ]), @@ -157,13 +157,18 @@ void main() { await git.create(); final packageRoot = p.join(d.sandbox, appPath); await pubGet( - environment: {'_PUB_TEST_SDK_VERSION': '1.12.0'}, - workingDirectory: packageRoot); + workingDirectory: packageRoot, + ); createDirectorySymlink( - p.join(d.sandbox, appPath, 'dir_with_symlink', 'symlink'), '..'); + p.join(d.sandbox, appPath, 'dir_with_symlink', 'symlink'), + '..', + ); await git.commit(); - await expectValidation(contains('Package has 0 warnings.'), 0, - workingDirectory: packageRoot); + await expectValidation( + contains('Package has 0 warnings.'), + 0, + workingDirectory: packageRoot, + ); }); } From 0c56b15831429f80dcc32706ea75537a53b2d25c Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Tue, 22 Oct 2024 12:24:13 +0000 Subject: [PATCH 47/58] fmt --- lib/src/package.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index 626087bfc..011c95ee7 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -279,7 +279,7 @@ See $workspacesDocUrl for more information. } /// Throws if [path] is a link that cannot resolve. - /// + /// /// Circular links will fail to resolve at some depth defined by the os. void verifyLink(String path) { final link = Link(path); From f620e186059425d89141186f1f2c7f25abc43bac Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Tue, 22 Oct 2024 12:25:22 +0000 Subject: [PATCH 48/58] Remove deprecated lint --- analysis_options.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/analysis_options.yaml b/analysis_options.yaml index 715ebeab2..5e2f708a2 100644 --- a/analysis_options.yaml +++ b/analysis_options.yaml @@ -22,7 +22,6 @@ linter: - missing_whitespace_between_adjacent_strings - no_adjacent_strings_in_list - no_runtimeType_toString - - package_api_docs - prefer_const_declarations - prefer_final_locals - require_trailing_commas From 7679721a1096d548c61c1ab6f06261a3e4abdec3 Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Fri, 25 Oct 2024 10:35:15 +0000 Subject: [PATCH 49/58] Detect cycles --- lib/src/package.dart | 26 +++++++++++++++++++++++++- test/package_list_files_test.dart | 4 ++-- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index 011c95ee7..3e4a8dc6b 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -294,11 +294,35 @@ See $workspacesDocUrl for more information. } } + /// We check each directory that it doesn't symlink-resolve to the + /// symlink-resolution of any parent directory of itself. This avoids + /// cycles. + /// + /// Cache the symlink resolutions here. + final symlinkResolvedDirs = {}; + final result = Ignore.listFiles( beneath: beneath, listDir: (dir) { - final resolvedDir = resolve(dir); + final resolvedDir = p.normalize(resolve(dir)); verifyLink(resolvedDir); + + { + final symlinkResolvedDir = symlinkResolvedDirs[resolvedDir] ??= + Directory(resolvedDir).resolveSymbolicLinksSync(); + + for (final parent in parentDirs(p.dirname(resolvedDir))) { + final symlinkResolvedParent = symlinkResolvedDirs[parent] ??= + Directory(parent).resolveSymbolicLinksSync(); + if (p.equals(symlinkResolvedDir, symlinkResolvedParent)) { + dataError(''' +Pub does not support symlink cycles. + +$resolvedDir => ${p.canonicalize(parent)} +'''); + } + } + } var contents = Directory(resolvedDir).listSync(followLinks: false); if (!recursive) { diff --git a/test/package_list_files_test.dart b/test/package_list_files_test.dart index 349a6f807..7b4a06b83 100644 --- a/test/package_list_files_test.dart +++ b/test/package_list_files_test.dart @@ -150,7 +150,7 @@ void main() { (e) => e.message, 'message', contains( - 'Could not resolve symbolic link', + 'Pub does not support symlink cycles.', ), ), ), @@ -221,7 +221,7 @@ void main() { isA().having( (e) => e.message, 'message', - contains('Could not resolve symbolic link'), + contains('Pub does not support symlink cycles.'), ), ), ); From f8ee39710812140dd1fe8e0c54afa239e51c3ff1 Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Fri, 25 Oct 2024 12:18:40 +0000 Subject: [PATCH 50/58] canonicalize before examining parents (gets rid of /.) --- lib/src/package.dart | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index 3e4a8dc6b..11c346e4e 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -308,17 +308,17 @@ See $workspacesDocUrl for more information. verifyLink(resolvedDir); { - final symlinkResolvedDir = symlinkResolvedDirs[resolvedDir] ??= - Directory(resolvedDir).resolveSymbolicLinksSync(); - - for (final parent in parentDirs(p.dirname(resolvedDir))) { + final canonicalized = p.canonicalize(resolvedDir); + final symlinkResolvedDir = symlinkResolvedDirs[canonicalized] ??= + Directory(canonicalized).resolveSymbolicLinksSync(); + for (final parent in parentDirs(p.dirname(canonicalized))) { final symlinkResolvedParent = symlinkResolvedDirs[parent] ??= Directory(parent).resolveSymbolicLinksSync(); if (p.equals(symlinkResolvedDir, symlinkResolvedParent)) { dataError(''' Pub does not support symlink cycles. -$resolvedDir => ${p.canonicalize(parent)} +$symlinkResolvedDir => ${p.canonicalize(symlinkResolvedParent)} '''); } } From 6d2ef9225964503ee2ccaa768f94f89d6987958b Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Tue, 29 Oct 2024 13:26:14 +0100 Subject: [PATCH 51/58] Update test/package_list_files_test.dart Co-authored-by: Jonas Finnemann Jensen --- test/package_list_files_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/package_list_files_test.dart b/test/package_list_files_test.dart index 7b4a06b83..ee8d33b2e 100644 --- a/test/package_list_files_test.dart +++ b/test/package_list_files_test.dart @@ -126,7 +126,7 @@ void main() { }); }); - group('cycles', () { + group('symlink cycles', () { test('throws on included link', () async { await d.dir(appPath, [ d.pubspec({'name': 'myapp'}), From 3bd17650e7498fef2ece47329247da71c0158909 Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Thu, 31 Oct 2024 12:40:25 +0000 Subject: [PATCH 52/58] Address review comments --- lib/src/package.dart | 13 ++- test/package_list_files_test.dart | 169 +++++++++++++++++++---------- test/validator/gitignore_test.dart | 7 +- test/validator/utils.dart | 11 -- 4 files changed, 126 insertions(+), 74 deletions(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index 11c346e4e..be6ebaad1 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -287,6 +287,9 @@ See $workspacesDocUrl for more information. try { link.resolveSymbolicLinksSync(); } on FileSystemException catch (e) { + if (!link.existsSync()) { + return; + } throw DataException( 'Could not resolve symbolic link $path. $e', ); @@ -300,6 +303,10 @@ See $workspacesDocUrl for more information. /// /// Cache the symlink resolutions here. final symlinkResolvedDirs = {}; + String resolveDirSymlinks(String path) { + return symlinkResolvedDirs[path] ??= + Directory(path).resolveSymbolicLinksSync(); + } final result = Ignore.listFiles( beneath: beneath, @@ -309,11 +316,9 @@ See $workspacesDocUrl for more information. { final canonicalized = p.canonicalize(resolvedDir); - final symlinkResolvedDir = symlinkResolvedDirs[canonicalized] ??= - Directory(canonicalized).resolveSymbolicLinksSync(); + final symlinkResolvedDir = resolveDirSymlinks(canonicalized); for (final parent in parentDirs(p.dirname(canonicalized))) { - final symlinkResolvedParent = symlinkResolvedDirs[parent] ??= - Directory(parent).resolveSymbolicLinksSync(); + final symlinkResolvedParent = resolveDirSymlinks(parent); if (p.equals(symlinkResolvedDir, symlinkResolvedParent)) { dataError(''' Pub does not support symlink cycles. diff --git a/test/package_list_files_test.dart b/test/package_list_files_test.dart index 7b4a06b83..525d0330e 100644 --- a/test/package_list_files_test.dart +++ b/test/package_list_files_test.dart @@ -11,8 +11,8 @@ import 'package:pub/src/system_cache.dart'; import 'package:test/test.dart'; import 'descriptor.dart' as d; +import 'link_descriptor.dart'; import 'test_pub.dart'; -import 'validator/utils.dart'; late String root; Entrypoint? entrypoint; @@ -58,12 +58,9 @@ void main() { d.file('file2.txt', 'contents'), d.dir('subdir', [ d.dir('a', [d.file('file')]), + link('symlink', 'a', forceDirectory: true), ]), ]).create(); - createDirectorySymlink( - p.join(d.sandbox, appPath, 'subdir', 'symlink'), - 'a', - ); createEntrypoint(); @@ -84,12 +81,9 @@ void main() { d.dir('subdir', [ d.file('.pubignore', 'symlink'), d.dir('a', [d.file('file')]), + link('symlink', 'a', forceDirectory: true), ]), ]).create(); - createDirectorySymlink( - p.join(d.sandbox, appPath, 'subdir', 'symlink'), - 'a', - ); createEntrypoint(); @@ -109,12 +103,9 @@ void main() { d.dir('subdir', [ d.file('.pubignore', 'symlink'), d.dir('a', [d.file('file')]), + link('symlink', 'b', forceDirectory: true), ]), ]).create(); - createDirectorySymlink( - p.join(d.sandbox, appPath, 'subdir', 'symlink'), - 'b', - ); createEntrypoint(); @@ -134,12 +125,9 @@ void main() { d.file('file2.txt', 'contents'), d.dir('subdir', [ d.dir('a', [d.file('file')]), + link('symlink', '..', forceDirectory: true), ]), ]).create(); - createDirectorySymlink( - p.join(d.sandbox, appPath, 'subdir', 'symlink'), - '..', - ); createEntrypoint(); @@ -164,12 +152,9 @@ void main() { d.file('file2.txt', 'contents'), d.dir('subdir', [ d.dir('a', [d.file('file')]), + link('symlink', 'symlink', forceDirectory: true), ]), ]).create(); - createDirectorySymlink( - p.join(d.sandbox, appPath, 'subdir', 'symlink'), - 'symlink', - ); createEntrypoint(); @@ -191,29 +176,94 @@ void main() { d.file('file1.txt', 'contents'), d.file('file2.txt', 'contents'), d.dir('subdir', [ - d.dir('a', [d.file('file')]), - d.dir('b'), - d.dir('c'), + d.dir('a', [ + d.file('file'), + link('symlink1', p.join('..', 'b'), forceDirectory: true), + ]), + d.dir('b', [ + link('symlink2', p.join('..', 'c'), forceDirectory: true), + ]), + d.dir('c', [ + link('symlink3', p.join('..', 'a'), forceDirectory: true), + ]), + link('symlink', 'a', forceDirectory: true), ]), ]).create(); - createDirectorySymlink( - p.join(d.sandbox, appPath, 'subdir', 'symlink'), - 'a', - ); - createDirectorySymlink( - p.join(d.sandbox, appPath, 'subdir', 'a', 'symlink1'), - p.join('..', 'b'), + + createEntrypoint(); + + expect( + () => entrypoint!.workspaceRoot.listFiles(), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('Pub does not support symlink cycles.'), + ), + ), ); - createDirectorySymlink( - p.join(d.sandbox, appPath, 'subdir', 'b', 'symlink2'), - p.join('..', 'c'), + }); + + test('throws on link to loop', () async { + await d.dir(appPath, [ + d.pubspec({'name': 'myapp'}), + link('symlink', p.join(d.sandbox, 'loop'), forceDirectory: true), + ]).create(); + await link('loop', 'loop', forceDirectory: true).create(); + + createEntrypoint(); + + expect( + () => entrypoint!.workspaceRoot.listFiles(), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('Could not resolve symbolic link'), + ), + ), ); - createDirectorySymlink( - p.join(d.sandbox, appPath, 'subdir', 'c', 'symlink3'), - p.join('..', 'a'), + }); + + test('throws on link to loop back to parent directory', () async { + await d.dir('src', [ + d.dir(appPath, [ + d.pubspec({'name': 'myapp'}), + link('symlink', p.join(d.sandbox, 'source'), forceDirectory: true), + ]), + ]).create(); + await link('source', p.join(d.sandbox, 'src'), forceDirectory: true) + .create(); + + createEntrypoint(p.join('src', appPath)); + + expect( + () => entrypoint!.workspaceRoot.listFiles(), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('Pub does not support symlink cycles.'), + ), + ), ); + }); - createEntrypoint(); + test('throws on link to subdirectory of loop back to parent directory', + () async { + await d.dir('src', [ + d.dir(appPath, [ + d.pubspec({'name': 'myapp'}), + link('symlink', p.join(d.sandbox, 'source'), forceDirectory: true), + ]), + ]).create(); + await link( + 'source', + p.join(d.sandbox, 'src'), + forceDirectory: true, + ).create(); + + createEntrypoint(p.join('source', appPath)); expect( () => entrypoint!.workspaceRoot.listFiles(), @@ -227,6 +277,25 @@ void main() { ); }); + test('Does not throw when publishing via symlink', () async { + await d.dir('src', [ + d.dir(appPath, [ + d.pubspec({'name': 'myapp'}), + ]), + ]).create(); + await link( + 'source', + p.join(d.sandbox, 'src'), + forceDirectory: true, + ).create(); + + createEntrypoint(p.join('source', appPath)); + + expect(entrypoint!.workspaceRoot.listFiles(), { + p.join(root, 'pubspec.yaml'), + }); + }); + test('not throws on ignored link', () async { await d.dir(appPath, [ d.pubspec({'name': 'myapp'}), @@ -235,12 +304,9 @@ void main() { d.dir('subdir', [ d.file('.pubignore', 'symlink'), d.dir('a', [d.file('file')]), + link('symlink', '..', forceDirectory: true), ]), ]).create(); - createDirectorySymlink( - p.join(d.sandbox, appPath, 'subdir', 'symlink'), - '..', - ); createEntrypoint(); @@ -259,20 +325,11 @@ void main() { d.file('file2.txt', 'contents'), d.dir('subdir', [ d.dir('a', [d.file('file')]), + link('symlink1', 'a', forceDirectory: true), + link('symlink2', 'a', forceDirectory: true), ]), + link('symlink3', p.join('subdir', 'a')), ]).create(); - createDirectorySymlink( - p.join(d.sandbox, appPath, 'subdir', 'symlink1'), - 'a', - ); - createDirectorySymlink( - p.join(d.sandbox, appPath, 'subdir', 'symlink2'), - 'a', - ); - createDirectorySymlink( - p.join(d.sandbox, appPath, 'symlink3'), - p.join('subdir', 'a'), - ); createEntrypoint(); @@ -299,8 +356,8 @@ void main() { ]), ]).create(); - final root = p.join(d.sandbox, 'symlink'); - createDirectorySymlink(root, appPath); + await link('symlink', appPath).create(); + root = p.join(d.sandbox, 'symlink'); final entrypoint = Entrypoint( p.join(d.sandbox, 'symlink'), diff --git a/test/validator/gitignore_test.dart b/test/validator/gitignore_test.dart index a8aa17798..1dc841384 100644 --- a/test/validator/gitignore_test.dart +++ b/test/validator/gitignore_test.dart @@ -9,8 +9,8 @@ import 'package:pub/src/exit_codes.dart' as exit_codes; import 'package:test/test.dart'; import '../descriptor.dart' as d; +import '../link_descriptor.dart'; import '../test_pub.dart'; -import 'utils.dart'; Future expectValidation( Matcher error, @@ -159,10 +159,11 @@ void main() { await pubGet( workingDirectory: packageRoot, ); - createDirectorySymlink( + await link( p.join(d.sandbox, appPath, 'dir_with_symlink', 'symlink'), '..', - ); + forceDirectory: true, + ).create(); await git.commit(); await expectValidation( diff --git a/test/validator/utils.dart b/test/validator/utils.dart index d1f91b245..3b3016cf4 100644 --- a/test/validator/utils.dart +++ b/test/validator/utils.dart @@ -2,8 +2,6 @@ // 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:pub/src/exit_codes.dart'; import 'package:test/test.dart'; @@ -26,15 +24,6 @@ Future expectValidationDeprecated( expect(validator.hints, hints ?? isEmpty); } -// On windows symlinks to directories are distinct from symlinks to files. -void createDirectorySymlink(String path, String target) { - if (Platform.isWindows) { - Process.runSync('cmd', ['/c', 'mklink', '/D', path, target]); - } else { - Link(path).createSync(target); - } -} - Future expectValidation({ Object? error, int exitCode = 0, From 66b1ae395b9aba4f9387820c155a7207e6e3a7ab Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Thu, 31 Oct 2024 12:54:13 +0000 Subject: [PATCH 53/58] Add missing file --- test/link_descriptor.dart | 56 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 test/link_descriptor.dart diff --git a/test/link_descriptor.dart b/test/link_descriptor.dart new file mode 100644 index 000000000..e7bf6ef97 --- /dev/null +++ b/test/link_descriptor.dart @@ -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:path/path.dart' as p; +import 'package:test/test.dart'; + +import 'descriptor.dart' as d; + +/// Describes a symlink. +class LinkDescriptor extends d.Descriptor { + /// On windows symlinks to directories are distinct from symlinks to files. + final bool forceDirectory; + final String target; + LinkDescriptor(super.name, this.target, {this.forceDirectory = false}); + + @override + Future create([String? parent]) async { + final path = p.join(parent ?? d.sandbox, name); + if (forceDirectory) { + if (Platform.isWindows) { + Process.runSync('cmd', ['/c', 'mklink', '/D', path, target]); + } else { + Link(path).createSync(target); + } + } else { + Link(path).createSync(target); + } + } + + @override + String describe() { + return 'symlink at $name targeting $target'; + } + + @override + Future validate([String? parent]) async { + final link = Link(p.join(parent ?? d.sandbox, name)); + try { + final actualTarget = link.targetSync(); + expect( + actualTarget, + target, + reason: 'Link doesn\'t point where expected.', + ); + } on FileSystemException catch (e) { + fail('Could not read link at $name $e'); + } + } +} + +d.Descriptor link(String name, String target, {bool forceDirectory = false}) { + return LinkDescriptor(name, target, forceDirectory: forceDirectory); +} From d4a89e00b59921dfc375b6b402983c2eceb678e1 Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Thu, 31 Oct 2024 13:06:41 +0000 Subject: [PATCH 54/58] attempt at windows fix --- test/package_list_files_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/package_list_files_test.dart b/test/package_list_files_test.dart index 00120bee9..873ea4604 100644 --- a/test/package_list_files_test.dart +++ b/test/package_list_files_test.dart @@ -328,7 +328,7 @@ void main() { link('symlink1', 'a', forceDirectory: true), link('symlink2', 'a', forceDirectory: true), ]), - link('symlink3', p.join('subdir', 'a')), + link('symlink3', p.join('subdir', 'a'), forceDirectory: true), ]).create(); createEntrypoint(); From 8909acfad00ffab114542650edd1de6a6a7cdaa2 Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Thu, 31 Oct 2024 14:05:21 +0000 Subject: [PATCH 55/58] Use link descriptors and prefix in lish/symlinks_test --- test/lish/symlinks_test.dart | 64 +++++++++++++++++++----------------- 1 file changed, 34 insertions(+), 30 deletions(-) diff --git a/test/lish/symlinks_test.dart b/test/lish/symlinks_test.dart index ced358633..5be2ca204 100644 --- a/test/lish/symlinks_test.dart +++ b/test/lish/symlinks_test.dart @@ -8,32 +8,36 @@ import 'package:path/path.dart' as p; import 'package:tar/tar.dart'; import 'package:test/test.dart'; -import '../descriptor.dart'; +import '../descriptor.dart' as d; +import '../link_descriptor.dart'; import '../test_pub.dart'; Future main() async { test('symlink directories are replaced by their targets', () async { - await validPackage().create(); - await dir('a', [file('aa', 'aaa')]).create(); - await file('t', 'ttt').create(); + await d.validPackage().create(); + await d.dir('a', [d.file('aa', 'aaa')]).create(); + await d.file('t', 'ttt').create(); - await dir(appPath, [ - dir('b', [file('bb', 'bbb')]), + await d.dir(appPath, [ + d.dir('b', [d.file('bb', 'bbb'), link('l', p.join(d.sandbox, 't'))]), + link( + 'symlink_to_dir_outside_package', + p.join(d.sandbox, 'a'), + forceDirectory: true, + ), + link( + 'symlink_to_dir_outside_package_relative', + p.join('..', 'a'), + forceDirectory: true, + ), + link('symlink_to_dir_inside_package', p.join(d.sandbox, appPath, 'b')), + link('symlink_to_dir_inside_package_relative', 'b', forceDirectory: true), ]).create(); - Link(p.join(sandbox, appPath, 'symlink_to_dir_outside_package')) - .createSync(p.join(sandbox, 'a')); - Link(p.join(sandbox, appPath, 'symlink_to_dir_outside_package_relative')) - .createSync(p.join('..', 'a')); - Link(p.join(sandbox, appPath, 'symlink_to_dir_inside_package')) - .createSync(p.join(sandbox, appPath, 'b')); - Link(p.join(sandbox, appPath, 'symlink_to_dir_inside_package_relative')) - .createSync('b'); - Link(p.join(sandbox, appPath, 'b', 'l')).createSync(p.join(sandbox, 't')); await runPub(args: ['publish', '--to-archive=archive.tar.gz']); final reader = TarReader( - File(p.join(sandbox, appPath, 'archive.tar.gz')) + File(p.join(d.sandbox, appPath, 'archive.tar.gz')) .openRead() .transform(GZipCodec().decoder), ); @@ -45,25 +49,25 @@ Future main() async { await runPub(args: ['cache', 'preload', 'archive.tar.gz']); - await dir('test_pkg-1.0.0', [ - ...validPackage().contents, - dir('symlink_to_dir_outside_package', [ - file('aa', 'aaa'), + await d.dir('test_pkg-1.0.0', [ + ...d.validPackage().contents, + d.dir('symlink_to_dir_outside_package', [ + d.file('aa', 'aaa'), ]), - dir('symlink_to_dir_outside_package_relative', [ - file('aa', 'aaa'), + d.dir('symlink_to_dir_outside_package_relative', [ + d.file('aa', 'aaa'), ]), - dir('b', [file('bb', 'bbb')]), - dir('symlink_to_dir_inside_package', [ - file('bb', 'bbb'), - file('l', 'ttt'), + d.dir('b', [d.file('bb', 'bbb')]), + d.dir('symlink_to_dir_inside_package', [ + d.file('bb', 'bbb'), + d.file('l', 'ttt'), ]), - dir('symlink_to_dir_inside_package_relative', [ - file('bb', 'bbb'), - file('l', 'ttt'), + d.dir('symlink_to_dir_inside_package_relative', [ + d.file('bb', 'bbb'), + d.file('l', 'ttt'), ]), ]).validate( - p.join(sandbox, cachePath, 'hosted', 'pub.dev'), + p.join(d.sandbox, cachePath, 'hosted', 'pub.dev'), ); }); } From 778e35c13f48fa64ab84c5493c84fe7582997c95 Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Thu, 31 Oct 2024 14:09:45 +0000 Subject: [PATCH 56/58] Move link() -> descriptor.dart --- test/descriptor.dart | 5 +++ test/link_descriptor.dart | 4 -- test/lish/symlinks_test.dart | 15 ++++--- test/package_list_files_test.dart | 72 +++++++++++++++++------------- test/validator/gitignore_test.dart | 13 +++--- 5 files changed, 63 insertions(+), 46 deletions(-) diff --git a/test/descriptor.dart b/test/descriptor.dart index ab991d385..ae5e05441 100644 --- a/test/descriptor.dart +++ b/test/descriptor.dart @@ -18,6 +18,7 @@ import 'descriptor/git.dart'; import 'descriptor/package_config.dart'; import 'descriptor/tar.dart'; import 'descriptor/yaml.dart'; +import 'link_descriptor.dart'; import 'test_pub.dart'; export 'package:test_descriptor/test_descriptor.dart'; @@ -403,3 +404,7 @@ Descriptor flutterVersion(String version) { FileDescriptor sdkPackagesConfig(SdkPackageConfig sdkPackageConfig) { return YamlDescriptor('sdk_packages.yaml', yaml(sdkPackageConfig.toMap())); } + +Descriptor link(String name, String target, {bool forceDirectory = false}) { + return LinkDescriptor(name, target, forceDirectory: forceDirectory); +} diff --git a/test/link_descriptor.dart b/test/link_descriptor.dart index e7bf6ef97..d30d00f1c 100644 --- a/test/link_descriptor.dart +++ b/test/link_descriptor.dart @@ -50,7 +50,3 @@ class LinkDescriptor extends d.Descriptor { } } } - -d.Descriptor link(String name, String target, {bool forceDirectory = false}) { - return LinkDescriptor(name, target, forceDirectory: forceDirectory); -} diff --git a/test/lish/symlinks_test.dart b/test/lish/symlinks_test.dart index 5be2ca204..e67fd50bf 100644 --- a/test/lish/symlinks_test.dart +++ b/test/lish/symlinks_test.dart @@ -9,7 +9,6 @@ import 'package:tar/tar.dart'; import 'package:test/test.dart'; import '../descriptor.dart' as d; -import '../link_descriptor.dart'; import '../test_pub.dart'; Future main() async { @@ -19,19 +18,23 @@ Future main() async { await d.file('t', 'ttt').create(); await d.dir(appPath, [ - d.dir('b', [d.file('bb', 'bbb'), link('l', p.join(d.sandbox, 't'))]), - link( + d.dir('b', [d.file('bb', 'bbb'), d.link('l', p.join(d.sandbox, 't'))]), + d.link( 'symlink_to_dir_outside_package', p.join(d.sandbox, 'a'), forceDirectory: true, ), - link( + d.link( 'symlink_to_dir_outside_package_relative', p.join('..', 'a'), forceDirectory: true, ), - link('symlink_to_dir_inside_package', p.join(d.sandbox, appPath, 'b')), - link('symlink_to_dir_inside_package_relative', 'b', forceDirectory: true), + d.link('symlink_to_dir_inside_package', p.join(d.sandbox, appPath, 'b')), + d.link( + 'symlink_to_dir_inside_package_relative', + 'b', + forceDirectory: true, + ), ]).create(); await runPub(args: ['publish', '--to-archive=archive.tar.gz']); diff --git a/test/package_list_files_test.dart b/test/package_list_files_test.dart index 873ea4604..621bb02b8 100644 --- a/test/package_list_files_test.dart +++ b/test/package_list_files_test.dart @@ -11,7 +11,6 @@ import 'package:pub/src/system_cache.dart'; import 'package:test/test.dart'; import 'descriptor.dart' as d; -import 'link_descriptor.dart'; import 'test_pub.dart'; late String root; @@ -58,7 +57,7 @@ void main() { d.file('file2.txt', 'contents'), d.dir('subdir', [ d.dir('a', [d.file('file')]), - link('symlink', 'a', forceDirectory: true), + d.link('symlink', 'a', forceDirectory: true), ]), ]).create(); @@ -81,7 +80,7 @@ void main() { d.dir('subdir', [ d.file('.pubignore', 'symlink'), d.dir('a', [d.file('file')]), - link('symlink', 'a', forceDirectory: true), + d.link('symlink', 'a', forceDirectory: true), ]), ]).create(); @@ -103,7 +102,7 @@ void main() { d.dir('subdir', [ d.file('.pubignore', 'symlink'), d.dir('a', [d.file('file')]), - link('symlink', 'b', forceDirectory: true), + d.link('symlink', 'b', forceDirectory: true), ]), ]).create(); @@ -125,7 +124,7 @@ void main() { d.file('file2.txt', 'contents'), d.dir('subdir', [ d.dir('a', [d.file('file')]), - link('symlink', '..', forceDirectory: true), + d.link('symlink', '..', forceDirectory: true), ]), ]).create(); @@ -152,7 +151,7 @@ void main() { d.file('file2.txt', 'contents'), d.dir('subdir', [ d.dir('a', [d.file('file')]), - link('symlink', 'symlink', forceDirectory: true), + d.link('symlink', 'symlink', forceDirectory: true), ]), ]).create(); @@ -178,15 +177,15 @@ void main() { d.dir('subdir', [ d.dir('a', [ d.file('file'), - link('symlink1', p.join('..', 'b'), forceDirectory: true), + d.link('symlink1', p.join('..', 'b'), forceDirectory: true), ]), d.dir('b', [ - link('symlink2', p.join('..', 'c'), forceDirectory: true), + d.link('symlink2', p.join('..', 'c'), forceDirectory: true), ]), d.dir('c', [ - link('symlink3', p.join('..', 'a'), forceDirectory: true), + d.link('symlink3', p.join('..', 'a'), forceDirectory: true), ]), - link('symlink', 'a', forceDirectory: true), + d.link('symlink', 'a', forceDirectory: true), ]), ]).create(); @@ -207,9 +206,9 @@ void main() { test('throws on link to loop', () async { await d.dir(appPath, [ d.pubspec({'name': 'myapp'}), - link('symlink', p.join(d.sandbox, 'loop'), forceDirectory: true), + d.link('symlink', p.join(d.sandbox, 'loop'), forceDirectory: true), ]).create(); - await link('loop', 'loop', forceDirectory: true).create(); + await d.link('loop', 'loop', forceDirectory: true).create(); createEntrypoint(); @@ -229,10 +228,15 @@ void main() { await d.dir('src', [ d.dir(appPath, [ d.pubspec({'name': 'myapp'}), - link('symlink', p.join(d.sandbox, 'source'), forceDirectory: true), + d.link( + 'symlink', + p.join(d.sandbox, 'source'), + forceDirectory: true, + ), ]), ]).create(); - await link('source', p.join(d.sandbox, 'src'), forceDirectory: true) + await d + .link('source', p.join(d.sandbox, 'src'), forceDirectory: true) .create(); createEntrypoint(p.join('src', appPath)); @@ -254,14 +258,20 @@ void main() { await d.dir('src', [ d.dir(appPath, [ d.pubspec({'name': 'myapp'}), - link('symlink', p.join(d.sandbox, 'source'), forceDirectory: true), + d.link( + 'symlink', + p.join(d.sandbox, 'source'), + forceDirectory: true, + ), ]), ]).create(); - await link( - 'source', - p.join(d.sandbox, 'src'), - forceDirectory: true, - ).create(); + await d + .link( + 'source', + p.join(d.sandbox, 'src'), + forceDirectory: true, + ) + .create(); createEntrypoint(p.join('source', appPath)); @@ -283,11 +293,13 @@ void main() { d.pubspec({'name': 'myapp'}), ]), ]).create(); - await link( - 'source', - p.join(d.sandbox, 'src'), - forceDirectory: true, - ).create(); + await d + .link( + 'source', + p.join(d.sandbox, 'src'), + forceDirectory: true, + ) + .create(); createEntrypoint(p.join('source', appPath)); @@ -304,7 +316,7 @@ void main() { d.dir('subdir', [ d.file('.pubignore', 'symlink'), d.dir('a', [d.file('file')]), - link('symlink', '..', forceDirectory: true), + d.link('symlink', '..', forceDirectory: true), ]), ]).create(); @@ -325,10 +337,10 @@ void main() { d.file('file2.txt', 'contents'), d.dir('subdir', [ d.dir('a', [d.file('file')]), - link('symlink1', 'a', forceDirectory: true), - link('symlink2', 'a', forceDirectory: true), + d.link('symlink1', 'a', forceDirectory: true), + d.link('symlink2', 'a', forceDirectory: true), ]), - link('symlink3', p.join('subdir', 'a'), forceDirectory: true), + d.link('symlink3', p.join('subdir', 'a'), forceDirectory: true), ]).create(); createEntrypoint(); @@ -356,7 +368,7 @@ void main() { ]), ]).create(); - await link('symlink', appPath).create(); + await d.link('symlink', appPath).create(); root = p.join(d.sandbox, 'symlink'); final entrypoint = Entrypoint( diff --git a/test/validator/gitignore_test.dart b/test/validator/gitignore_test.dart index 1dc841384..14f7a0433 100644 --- a/test/validator/gitignore_test.dart +++ b/test/validator/gitignore_test.dart @@ -9,7 +9,6 @@ import 'package:pub/src/exit_codes.dart' as exit_codes; import 'package:test/test.dart'; import '../descriptor.dart' as d; -import '../link_descriptor.dart'; import '../test_pub.dart'; Future expectValidation( @@ -159,11 +158,13 @@ void main() { await pubGet( workingDirectory: packageRoot, ); - await link( - p.join(d.sandbox, appPath, 'dir_with_symlink', 'symlink'), - '..', - forceDirectory: true, - ).create(); + await d + .link( + p.join(d.sandbox, appPath, 'dir_with_symlink', 'symlink'), + '..', + forceDirectory: true, + ) + .create(); await git.commit(); await expectValidation( From b179ccf19fc75652ceb70d3cded5804cb20f0304 Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Thu, 31 Oct 2024 14:11:36 +0000 Subject: [PATCH 57/58] move test/link_descriptor.dart -> test/descriptor/linkdescriptor.dart --- test/descriptor.dart | 2 +- test/{ => descriptor}/link_descriptor.dart | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename test/{ => descriptor}/link_descriptor.dart (97%) diff --git a/test/descriptor.dart b/test/descriptor.dart index ae5e05441..c760f983e 100644 --- a/test/descriptor.dart +++ b/test/descriptor.dart @@ -15,10 +15,10 @@ import 'package:pub/src/sdk/sdk_package_config.dart'; import 'package:test_descriptor/test_descriptor.dart'; import 'descriptor/git.dart'; +import 'descriptor/link_descriptor.dart'; import 'descriptor/package_config.dart'; import 'descriptor/tar.dart'; import 'descriptor/yaml.dart'; -import 'link_descriptor.dart'; import 'test_pub.dart'; export 'package:test_descriptor/test_descriptor.dart'; diff --git a/test/link_descriptor.dart b/test/descriptor/link_descriptor.dart similarity index 97% rename from test/link_descriptor.dart rename to test/descriptor/link_descriptor.dart index d30d00f1c..9f66aae33 100644 --- a/test/link_descriptor.dart +++ b/test/descriptor/link_descriptor.dart @@ -7,7 +7,7 @@ import 'dart:io'; import 'package:path/path.dart' as p; import 'package:test/test.dart'; -import 'descriptor.dart' as d; +import '../descriptor.dart' as d; /// Describes a symlink. class LinkDescriptor extends d.Descriptor { From 2d8ec9b501d96b43459df7f06eed9cddbefa7fc0 Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Fri, 1 Nov 2024 13:12:47 +0000 Subject: [PATCH 58/58] attempt at windows fix2 --- test/lish/symlinks_test.dart | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/lish/symlinks_test.dart b/test/lish/symlinks_test.dart index e67fd50bf..b2b3ab7ec 100644 --- a/test/lish/symlinks_test.dart +++ b/test/lish/symlinks_test.dart @@ -29,7 +29,11 @@ Future main() async { p.join('..', 'a'), forceDirectory: true, ), - d.link('symlink_to_dir_inside_package', p.join(d.sandbox, appPath, 'b')), + d.link( + 'symlink_to_dir_inside_package', + p.join(d.sandbox, appPath, 'b'), + forceDirectory: true, + ), d.link( 'symlink_to_dir_inside_package_relative', 'b',