[sync] Preserve relative packaged symlink targets (#185)

Author: coisa

src/Filesystem/Filesystem.php

@@ -119,7 +119,11 @@ public function remove(string|iterable $files): void
      */
     public function symlink(string $originDir, string $targetDir, bool $copyOnWindows = false): void
     {
-        $this->filesystem->symlink($this->getAbsolutePath($originDir), $this->getAbsolutePath($targetDir), $copyOnWindows);
+        $origin = Path::isAbsolute($originDir)
+            ? $this->getAbsolutePath($originDir)
+            : $originDir;
+
+        $this->filesystem->symlink($origin, $this->getAbsolutePath($targetDir), $copyOnWindows);
     }
 
     /**

src/Sync/PackagedDirectorySynchronizer.php

@@ -85,8 +85,9 @@ public function synchronize(string $targetDir, string $packagePath, string $dire
             $entryName = $packagedEntry->getFilename();
             $targetLink = Path::makeAbsolute($entryName, $targetDir);
             $sourcePath = $packagedEntry->getRealPath();
+            $isDirectory = $packagedEntry->isDir();
 
-            $this->processLink($entryName, $targetLink, $sourcePath, $result);
+            $this->processLink($entryName, $targetLink, $sourcePath, $isDirectory, $result);
         }
 
         return $result;
@@ -104,10 +105,11 @@ private function processLink(
         string $entryName,
         string $targetLink,
         string $sourcePath,
+        bool $isDirectory,
         SynchronizeResult $result,
     ): void {
         if (! $this->filesystem->exists($targetLink)) {
-            $this->createNewLink($entryName, $targetLink, $sourcePath, $result);
+            $this->createNewLink($entryName, $targetLink, $sourcePath, $isDirectory, $result);
 
             return;
         }
@@ -118,7 +120,7 @@ private function processLink(
             return;
         }
 
-        $this->processExistingSymlink($entryName, $targetLink, $sourcePath, $result);
+        $this->processExistingSymlink($entryName, $targetLink, $sourcePath, $isDirectory, $result);
     }
 
     /**
@@ -133,9 +135,13 @@ private function createNewLink(
         string $entryName,
         string $targetLink,
         string $sourcePath,
+        bool $isDirectory,
         SynchronizeResult $result,
     ): void {
-        $relativeSourcePath = $this->filesystem->makePathRelative($sourcePath, $this->filesystem->dirname($targetLink));
+        $relativeSourcePath = $this->normalizeRelativeSourcePath(
+            $this->filesystem->makePathRelative($sourcePath, $this->filesystem->dirname($targetLink)),
+            $isDirectory,
+        );
 
         $this->filesystem->symlink($relativeSourcePath, $targetLink);
         $this->logger->info('Created link: ' . $entryName . ' -> ' . $relativeSourcePath);
@@ -168,12 +174,13 @@ private function processExistingSymlink(
         string $entryName,
         string $targetLink,
         string $sourcePath,
+        bool $isDirectory,
         SynchronizeResult $result,
     ): void {
         $linkPath = $this->filesystem->readlink($targetLink, true);
 
         if (! $linkPath || ! $this->filesystem->exists($linkPath)) {
-            $this->repairBrokenLink($entryName, $targetLink, $sourcePath, $result);
+            $this->repairBrokenLink($entryName, $targetLink, $sourcePath, $isDirectory, $result);
 
             return;
         }
@@ -194,13 +201,34 @@ private function repairBrokenLink(
         string $entryName,
         string $targetLink,
         string $sourcePath,
+        bool $isDirectory,
         SynchronizeResult $result,
     ): void {
         $this->filesystem->remove($targetLink);
         $this->logger->notice('Existing link is broken: ' . $entryName . ' (removing and recreating)');
         $result->addRemovedBrokenLink($entryName);
 
-        $this->createNewLink($entryName, $targetLink, $sourcePath, $result);
+        $this->createNewLink($entryName, $targetLink, $sourcePath, $isDirectory, $result);
+    }
+
+    /**
+     * Normalizes a relative symlink target emitted by Symfony path helpers.
+     *
+     * Files MUST NOT keep the trailing slash that directory-oriented path helpers
+     * may append, otherwise link creation treats them as non-existent directories.
+     *
+     * @param string $relativeSourcePath Relative path from the consumer target directory to the packaged source
+     * @param bool $isDirectory Whether the packaged source is a directory
+     *
+     * @return string Normalized relative symlink target
+     */
+    private function normalizeRelativeSourcePath(string $relativeSourcePath, bool $isDirectory): string
+    {
+        if ($isDirectory) {
+            return $relativeSourcePath;
+        }
+
+        return rtrim($relativeSourcePath, '/');
     }
 
     /**

tests/Filesystem/FilesystemTest.php

@@ -208,6 +208,30 @@ public function symlinkAndReadlinkWillUseAbsolutePaths(): void
         self::assertSame(realpath($origin), $this->filesystem->readlink($target, true));
     }
 
+    /**
+     * @return void
+     */
+    #[Test]
+    public function symlinkWillPreserveRelativeOrigins(): void
+    {
+        $currentWorkingDirectory = getcwd();
+        $origin = $this->tempDir . '/origin';
+        $target = $this->tempDir . '/target';
+        $relativeOrigin = 'origin';
+
+        $this->filesystem->mkdir($origin);
+        chdir($this->tempDir);
+
+        try {
+            $this->filesystem->symlink($relativeOrigin, $target);
+        } finally {
+            chdir($currentWorkingDirectory);
+        }
+
+        self::assertSame($relativeOrigin, $this->filesystem->readlink($target));
+        self::assertSame(realpath($origin), $this->filesystem->readlink($target, true));
+    }
+
     /**
      * @return void
      */

tests/Sync/PackagedDirectorySynchronizerTest.php

@@ -275,7 +275,7 @@ public function synchronizeWillCreateLinksForTopLevelFiles(): void
         $targetLink = '/consumer/.agents/agents/issue-editor.md';
         $relativeEntryPath = '../../../package/.agents/agents/issue-editor.md';
 
-        $this->mockFinder($this->createEntry('issue-editor.md', $entryPath));
+        $this->mockFinder($this->createEntry('issue-editor.md', $entryPath, false));
 
         $this->filesystem->exists('/package/.agents/agents')
             ->willReturn(true);
@@ -287,7 +287,7 @@ public function synchronizeWillCreateLinksForTopLevelFiles(): void
             ->willReturn('/consumer/.agents/agents')
             ->shouldBeCalledOnce();
         $this->filesystem->makePathRelative($entryPath, '/consumer/.agents/agents')
-            ->willReturn($relativeEntryPath)
+            ->willReturn($relativeEntryPath . '/')
             ->shouldBeCalledOnce();
         $this->filesystem->symlink($relativeEntryPath, $targetLink)
             ->shouldBeCalledOnce();
@@ -329,13 +329,15 @@ private function mockFinder(SplFileInfo ...$entries): void
      *
      * @return SplFileInfo
      */
-    private function createEntry(string $entryName, string $sourcePath): SplFileInfo
+    private function createEntry(string $entryName, string $sourcePath, bool $isDirectory = true): SplFileInfo
     {
         $entry = $this->prophesize(SplFileInfo::class);
         $entry->getFilename()
             ->willReturn($entryName);
         $entry->getRealPath()
             ->willReturn($sourcePath);
+        $entry->isDir()
+            ->willReturn($isDirectory);
 
         return $entry->reveal();
     }