getgrav / grav

Modern, Crazy Fast, Ridiculously Easy and Amazingly Powerful Flat-File CMS powered by PHP, Markdown, Twig, and Symfony
https://getgrav.org
MIT License
14.48k stars 1.4k forks source link

docker : rename(/var/www/html/cache/tmp/Grav-56a5f25571331/getgrav-grav-plugin-relatedpages-97423fe/,/var/www/html/user/plugins/relatedpages): Invalid cross-device link #635

Closed luluprat closed 7 years ago

luluprat commented 8 years ago

when i install a plugin i have this error bin/gpm install relatedpages Preparing to install Related Pages [v1.1.1] |- Downloading package... 100% |- Checking destination... ok |- Installing package...

[RuntimeException]
rename(/var/www/html/cache/tmp/Grav-56a5f25571331/getgrav-grav-plugin-relatedpages-97423fe/,/var/www/html/user/plugins/relatedpages): Invalid cross-device link

flaviocopes commented 8 years ago

Looks like a issue specific to Docker, the two paths /var/www/html/cache/ and /var/www/html/user/ are on separate filesystems?

Please provide as much information as you can to replicate the error. Dockerfile, configuration, etc.

luluprat commented 8 years ago

here is my dockerfile

FROM php:7.0-apache

# Based on the official Wordpress docker file

RUN a2enmod rewrite expires

# install the PHP extensions we need
RUN apt-get update && apt-get install -y git libpng12-dev libjpeg-dev zlib1g-dev && rm -rf /var/lib/apt/lists/* \
    && docker-php-ext-configure gd --with-png-dir=/usr --with-jpeg-dir=/usr \
    && docker-php-ext-install gd mysqli opcache zip mbstring

# set recommended PHP.ini settings
# see https://secure.php.net/manual/en/opcache.installation.php
RUN { \
        echo 'opcache.memory_consumption=128'; \
        echo 'opcache.interned_strings_buffer=8'; \
        echo 'opcache.max_accelerated_files=4000'; \
        echo 'opcache.revalidate_freq=60'; \
        echo 'opcache.fast_shutdown=1'; \
        echo 'opcache.enable_cli=1'; \
    } > /usr/local/etc/php/conf.d/opcache-recommended.ini

ENV GRAV_VERSION 1.0.8
RUN curl -o grav.tar.gz -SL https://github.com/getgrav/grav/archive/${GRAV_VERSION}.tar.gz \
    && mkdir -p /tmp/grav \
    && tar -xzf grav.tar.gz -C /tmp \
    && rsync -a /tmp/grav-${GRAV_VERSION}/ /var/www/html --exclude user \
    && /var/www/html/bin/grav install \
    && chown -R www-data:www-data /var/www/html

COPY docker-entrypoint.sh /entrypoint.sh

ENTRYPOINT ["/entrypoint.sh"]
CMD ["apache2-foreground"]
rhukster commented 8 years ago

I think the issue is that rename() in php does not work on an NFS share.. However, it appears mv() does, so the fix might be as simple as changing rename to mv in the GPM installer. Will have to track that down so you can try it.

flaviocopes commented 8 years ago

To run this Dockerfile this file https://github.com/benjick/docker/blob/master/grav/docker-entrypoint.sh is also needed to be present in the folder where you run it. Please be as specific as possible in order to be able to recreate issues.

I built an image with this Dockerfile and I have no issues with it. Plugins install just fine. Not sure what might be failing there, unless you have more details in your setup we should know.

flaviocopes commented 8 years ago

Unless your docker-entrypoint.sh is different. In that case paste that too!

rhukster commented 8 years ago

Any more updates on this? If not will close soon...

flaviocopes commented 8 years ago

Closing for inactivity

bauer01 commented 7 years ago

@rhukster its definitely bug...you should use mv() instead of rename() ... rename can be used only with FILES, not DIRS if you copy over devices....the reason why @flaviocopes Dockerfile works, is because he does not create any volume....you can also see explanation here https://bugs.php.net/bug.php?id=54097&edit=1

flaviocopes commented 7 years ago

In this case we should change rename() to an exec system call as there is no mv() in PHP

nsteinmetz commented 7 years ago

Good to know you found the solution for this bug as I meet it since I use volumes too.

I have a docker-compose file here if you want to test it : http://code.cerenit.fr/cerenit/docker-grav

flaviocopes commented 7 years ago

Even without using a Docker setup, it was enough to have the plugins folder symlinked to an external hard drive to replicate the problem. Should now be fixed in develop, nice if you can test it too @bauer01 @nsteinmetz

flaviocopes commented 7 years ago

Reverted and made a PR instead https://github.com/getgrav/grav/pull/1214 for more proper testing

nsteinmetz commented 7 years ago

Tested right now by fetching Folder.php file in a vanillia Grav 1.9 instance with auser directory being 1.8 based and it works as expected :)

Plugins were correctly upgraded.

Thanks !

mahagr commented 7 years ago

I think the new approach is wrong.

"Warnings may be generated if the destination filesystem doesn't permit chown() or chmod() system calls to be made on files — for example, if the destination filesystem is a FAT filesystem."

More explicitly, rename() may still return (bool) true, despite the warnings that result from the underlying calls to chown() or chmod(). This behavior can be misleading absent a deeper understanding of the underlying mechanics. To rename across filesystems, PHP "fakes it" by calling copy(), unlink(), chown(), and chmod() (not necessarily in that order). See PHP bug #50676 for more information.

On UNIX-like operating systems, filesystems may be mounted with an explicit uid and/or gid (for example, with mount options "uid=someuser,gid=somegroup"). Attempting to call rename() with such a destination filesystem will cause an "Operation not permitted" warning, even though the file is indeed renamed and rename() returns (bool) true.

This is not a bug. Either handle the warning as is appropriate to your use-case, or call copy() and then unlink(), which will avoid the doomed calls to chown() and chmod(), thereby eliminating the warning.

Though as stated in https://bugs.php.net/bug.php?id=54097 I would silence the warnings and do $this->copy() and $this->delete() separately in case if moving fails (eg. returns false).

flaviocopes commented 7 years ago

Replaced by https://github.com/getgrav/grav/commit/61005360a5edf718d17de55255950fd37bf66d9a