Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #5991 -- Adds Convert Symlinks Option #5992

Open
wants to merge 8 commits into
base: 12.x
Choose a base branch
from

Conversation

rwagner00
Copy link
Contributor

@rwagner00 rwagner00 commented May 8, 2024

Resolves: #5991

This PR adds a convert-symlink option to the archive-dump command 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.

@@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants