From 9f2ddc02f066c68fb638ff65fbb73761ece23267 Mon Sep 17 00:00:00 2001
From: Sigurd Meldgaard <sigurdm@google.com>
Date: Tue, 10 Aug 2021 09:33:10 +0200
Subject: [PATCH] Use uris for paths in git source descriptions (#3063)

---
 lib/src/source/git.dart     |  6 ++--
 test/get/git/path_test.dart | 60 ++++++++++++++++++++++++++++++++++---
 2 files changed, 60 insertions(+), 6 deletions(-)

diff --git a/lib/src/source/git.dart b/lib/src/source/git.dart
index 0e9728d58..5f1def9f0 100644
--- a/lib/src/source/git.dart
+++ b/lib/src/source/git.dart
@@ -142,6 +142,8 @@ class GitSource extends Source {
     return {'relative': relative, 'url': url};
   }
 
+  /// Returns [path] normalized.
+  ///
   /// Throws a [FormatException] if [path] isn't a relative url or null.
   String _validatedPath(dynamic path) {
     path ??= '.';
@@ -161,7 +163,7 @@ class GitSource extends Source {
           "The 'path' field of the description must not reach outside the "
           'repository.');
     }
-    return p.normalize(parsed.toString());
+    return p.url.normalize(parsed.toString());
   }
 
   /// If [description] has a resolved ref, print it out in short-form.
@@ -352,7 +354,7 @@ class BoundGitSource extends CachedSource {
 
       return Package.load(
           id.name,
-          p.join(revisionCachePath, id.description['path']),
+          p.join(revisionCachePath, p.fromUri(id.description['path'])),
           systemCache.sources);
     });
   }
diff --git a/test/get/git/path_test.dart b/test/get/git/path_test.dart
index 4831bd2ce..8bbd9e783 100644
--- a/test/get/git/path_test.dart
+++ b/test/get/git/path_test.dart
@@ -6,6 +6,8 @@
 
 import 'package:path/path.dart' as p;
 import 'package:pub/src/io.dart';
+import 'package:pub/src/lock_file.dart';
+import 'package:pub/src/source_registry.dart';
 import 'package:test/test.dart';
 
 import '../../descriptor.dart' as d;
@@ -47,14 +49,14 @@ void main() {
 
     var repo = d.git('foo.git', [
       d.dir('sub', [
-        d.dir('dir', [d.libPubspec('sub', '1.0.0'), d.libDir('sub', '1.0.0')])
+        d.dir('dir%', [d.libPubspec('sub', '1.0.0'), d.libDir('sub', '1.0.0')])
       ])
     ]);
     await repo.create();
 
     await d.appDir({
       'sub': {
-        'git': {'url': '../foo.git', 'path': 'sub/dir'}
+        'git': {'url': '../foo.git', 'path': 'sub/dir%25'}
       }
     }).create();
 
@@ -65,15 +67,65 @@ void main() {
         d.dir('cache', [d.gitPackageRepoCacheDir('foo')]),
         d.hashDir('foo', [
           d.dir('sub', [
-            d.dir('dir', [d.libDir('sub', '1.0.0')])
+            d.dir('dir%', [d.libDir('sub', '1.0.0')])
           ])
         ])
       ])
     ]).validate();
 
     await d.appPackagesFile({
-      'sub': pathInCache('git/foo-${await repo.revParse('HEAD')}/sub/dir')
+      'sub': pathInCache('git/foo-${await repo.revParse('HEAD')}/sub/dir%25')
     }).validate();
+
+    final lockFile = LockFile.load(
+        p.join(d.sandbox, appPath, 'pubspec.lock'), SourceRegistry());
+
+    expect(lockFile.packages['sub'].description['path'], 'sub/dir%25',
+        reason: 'use uris to specify the path relative to the repo');
+  });
+
+  test('depends on a package in a deep subdirectory, non-relative uri',
+      () async {
+    ensureGit();
+
+    var repo = d.git('foo.git', [
+      d.dir('sub', [
+        d.dir('dir%', [d.libPubspec('sub', '1.0.0'), d.libDir('sub', '1.0.0')])
+      ])
+    ]);
+    await repo.create();
+
+    await d.appDir({
+      'sub': {
+        'git': {
+          'url': p.toUri(p.join(d.sandbox, 'foo.git')).toString(),
+          'path': 'sub/dir%25'
+        }
+      }
+    }).create();
+
+    await pubGet();
+
+    await d.dir(cachePath, [
+      d.dir('git', [
+        d.dir('cache', [d.gitPackageRepoCacheDir('foo')]),
+        d.hashDir('foo', [
+          d.dir('sub', [
+            d.dir('dir%', [d.libDir('sub', '1.0.0')])
+          ])
+        ])
+      ])
+    ]).validate();
+
+    await d.appPackagesFile({
+      'sub': pathInCache('git/foo-${await repo.revParse('HEAD')}/sub/dir%25')
+    }).validate();
+
+    final lockFile = LockFile.load(
+        p.join(d.sandbox, appPath, 'pubspec.lock'), SourceRegistry());
+
+    expect(lockFile.packages['sub'].description['path'], 'sub/dir%25',
+        reason: 'use uris to specify the path relative to the repo');
   });
 
   test('depends on multiple packages in subdirectories', () async {