Fixes #5991: Adds Convert Symlinks Option #5992
Conversation
| @@ -140,7 +142,8 @@ public function dump(array $options = [ | |||
| protected function prepareArchiveDir(): void | |||
| { | |||
| $this->filesystem = new Filesystem(); | |||
| $this->archiveDir = FsUtils::tmpDir(self::ARCHIVES_DIR_NAME); | |||
| // $this->archiveDir = FsUtils::tmpDir(self::ARCHIVES_DIR_NAME); | |||
There was a problem hiding this comment.
This needs to be restored to its original state
| $this->logger()->info(var_export($this->archiveDir, true)); | ||
|
|
||
| // If symlinks are disabled, convert symlinks to full content. | ||
| if (is_dir($this->archiveDir)) { |
There was a problem hiding this comment.
Shouldn't archiveDir always be a directory? If creating it might fail, then we should fail-fast above rather than nesting everything under this 'if'.
|
|
||
| $iterator = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($this->archiveDir), RecursiveIteratorIterator::SELF_FIRST); | ||
|
|
||
| foreach ($iterator as $file) { |
There was a problem hiding this comment.
It would be helpful to break this into a separate function, so that we could have a docblock explaining the purpose. The 'convert-symlinks' could be passed in as a named parameter (rather than passing all of $options).
|
@greg-1-anderson Testing added, documentation provided. |
Removed debug line.
There was a problem hiding this comment.
LGTM, thanks for providing a test. @weitzman, do you want to review?
greg-1-anderson
changed the title
|
I just noticed this needs to target the 13.x branch. Should port over without too much effort. |
| @@ -588,4 +593,71 @@ private function destinationCleanup(string $destination): string | |||
| } | |||
| return $destination; | |||
| } | |||
|
|
|||
| /** | |||
| * Converts symlinks to full content for an archive. | |||
There was a problem hiding this comment.
I'm not sure what 'full content' means in this context.
| * | ||
| * @return void | ||
| */ | ||
| public function convertSymlinks( |
There was a problem hiding this comment.
Please add param types and return types. Remove any duplicate info in the Doxygen.
| file_put_contents($linktarget, "This is a symlink target file."); | ||
| symlink($linktarget, $linkdestination); |
There was a problem hiding this comment.
We write to the sandbox and not hard code /tmp. Furthermore we register files for cleanup. This needs cleanup upon success or failure.
Resolves: #5991
This PR adds a
convert-symlinkoption to thearchive-dumpcommand which will convert symlinks to standard/static files and directories in order to resolve a bug where symlinks external to the project would cause an error on dump.