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 with Moving Directories During Update Package Installation #49

Open
Dheyvidj opened this issue Aug 9, 2024 · 5 comments
Open

Comments

@Dheyvidj
Copy link

Dheyvidj commented Aug 9, 2024

We are experiencing an issue with the update package installation process in our application. Specifically, the problem occurs when trying to move directories using the rename() function, which results in an error. The error message is as follows:

yii\base\ErrorException: rename(): The first argument to copy() function cannot be a directory

Detailed Problem:

The error occurs when attempting to move directories such as vendor, humhub, and static to the backup directory and replacing these directories with new ones from the update package. The current code uses the rename() function for this task, which does not support directory copy operations.

Steps to Reproduce:

  1. Execute the install() function from the UpdatePackage class.
  2. The code attempts to move the vendor, humhub, and static directories to the backup directory and replace them with new directories.
  3. Observe the error that occurs.

Impact:

  • The rename() function fails when trying to move directories, resulting in the failure of the update package installation.
  • The update process does not complete correctly, which may lead to compatibility issues or incomplete updates.

Proposed Solution:

Replace the use of the rename() function with a method that correctly copies and removes directories. It is suggested to use the copyDirectory() function to copy the directories and then remove the old directories using FileHelper::removeDirectory().

Current Code (for reference):

// Current code using rename()
if (is_dir($this->getNewFileDirectory() . DIRECTORY_SEPARATOR . 'vendor')) {
    rename(Yii::getAlias('@webroot/protected/vendor'), $this->getBackupPath() . DIRECTORY_SEPARATOR . 'vendor_' . time());
    rename($this->getNewFileDirectory() . DIRECTORY_SEPARATOR . 'vendor', Yii::getAlias('@webroot/protected/vendor'));
}

Suggested Code (with fix):

// Suggested code using copyDirectory() and FileHelper::removeDirectory()
if (is_dir($this->getNewFileDirectory() . DIRECTORY_SEPARATOR . 'vendor')) {
    copyDirectory(Yii::getAlias('@webroot/protected/vendor'), $this->getBackupPath() . DIRECTORY_SEPARATOR . 'vendor_' . time());
    FileHelper::removeDirectory(Yii::getAlias('@webroot/protected/vendor'), ['traverseSymlinks' => true]);
    copyDirectory($this->getNewFileDirectory() . DIRECTORY_SEPARATOR . 'vendor', Yii::getAlias('@webroot/protected/vendor'));
}

The fix will allow proper movement of directories during the update process.

@luke-
Copy link
Contributor

luke- commented Aug 9, 2024

@Dheyvidj It is interesting that "rename" causes a "copy" on your installation.
Are the directories on your installation maybe located on different file systems?

@Dheyvidj
Copy link
Author

Dheyvidj commented Aug 9, 2024

@Dheyvidj It is interesting that "rename" causes a "copy" on your installation. Are the directories on your installation maybe located on different file systems?

My installation is in a Docker container that uses several volumes. I believe these volumes might be directly influencing the issue. However, I made some modifications to the code that worked for me, but I am not certain about its effectiveness in different installation environments.

I suspect that the use of Docker volumes might be causing rename() to fail and triggering an internal copy() operation.

@luke-
Copy link
Contributor

luke- commented Aug 10, 2024

Ok, thanks for the details.
Actually, I would like to stay with a move operation, as it happens almost instantly compared to a copy operation.

Maybe we could add a FileHelper::moveDirectory() functions, which:

  1. Tries to move via mv system command (if available) - it's safe to move via several volumes
  2. Tries to use PHP rename()
  3. Fallback to Copy

@dreua
Copy link

dreua commented Aug 10, 2024

Sounds like a bug in yii PHP to me, unless it is clearly documented that it can't be used with directories across volumes. (At least the error message should be better.)

  1. Tries to move via mv system command (if available) - it's save to move via several volumes

Note that mv can't move between filesystems either, it will, however, fall back to copy and delete without errors in most cases. If you rely on an atomic move or persistent file pointers that may not work. (I'd need to check the documentation for details.)

@dreua
Copy link

dreua commented Aug 11, 2024

Found the bug reported in 2011: PHP :: Bug #54097 :: rename() of dirs accross devices produces confusing copy error

Is this language actually maintained any more? 🤔 Loads of people must have wasted their time with this being unfixed and undocumented. Be it on Windows, with tmp file systems and especially since containerization started.

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

No branches or pull requests

3 participants