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

relative symlinks do not work on Windows #62

Closed
fritzmg opened this issue Nov 21, 2016 · 31 comments
Closed

relative symlinks do not work on Windows #62

fritzmg opened this issue Nov 21, 2016 · 31 comments
Assignees
Milestone

Comments

@fritzmg
Copy link
Contributor

fritzmg commented Nov 21, 2016

When trying to install a Contao 3 extension in Contao 4 that is compatible with Contao 4, the composer-plugin tries to create a relative symlink from /system/modules/… to /vendor/… » https://github.com/contao-community-alliance/composer-plugin/blob/3.0.4/src/Installer/AbstractModuleInstaller.php#L240

However, this will fail on Windows. See contao/core-bundle#208 (comment).

The contao/core-bundle for example will fall back to absolute symlinks in such a case, and so does Symfony.

@discordier discordier added this to the 3.0.5 milestone Nov 21, 2016
@aschempp aschempp self-assigned this Nov 24, 2016
@aschempp
Copy link
Collaborator

aschempp commented Nov 28, 2016

@discordier any reason we're not using AppVeyor to test on windows?

@discordier
Copy link
Member

Not really. Will set it up.

@discordier
Copy link
Member

@fritzmg could you provide a unit test? I just set up appveyor which should then point out the problem.

@fritzmg
Copy link
Contributor Author

fritzmg commented Nov 28, 2016

I don't know, I have never created unit tests 😁
I'll see what I can do.

@bezin
Copy link

bezin commented Dec 8, 2016

I just stumbled upon the same issue while trying to make an Contao 3.5 extension compatible with Contao 4 and I can confirm this.

@discordier
Copy link
Member

Unit test? Example information? Anything?

These "mee too" comments lead us to nowhere, we need test data to reproduce a problem.
This means you should supply us with the minimum file layout and configuration to reproduce the problem.

In your comment you write that:

  • you "can confirm this", this was undisputed.
  • you had the problem while trying to make an Contao 3.5 extension compatible - which extension? We can not have a look ourselves what went wrong.

Please supply relevant information. Thanks.

@fritzmg
Copy link
Contributor Author

fritzmg commented Dec 9, 2016

@aschempp
Copy link
Collaborator

@discordier do we have AppVeyor now?

@discordier
Copy link
Member

We should, however we have build issues as it seems, will bump the appveyor config from core-bundle and check if it works out.
https://ci.appveyor.com/project/contaocommunityalliance/composer-plugin

@discordier
Copy link
Member

Appveyor is up and running and failing tests like a charm.
However, there are completely different tests that are failing as it seems. some related to path naming.
Will try to sort them out.

yet the original issue stands, does the above linked test reproduce the issue by @fritzmg or do we need another unit test for this?

@discordier discordier assigned discordier and unassigned aschempp Dec 14, 2016
@discordier
Copy link
Member

Ok, I fixed several problems already. However, appveyor still breaks on 3 tests and I fail to see why.

I fired up a Windows at a customer and ran the tests there. Only one failing there.

@fritzmg You seem to have comprehensive experience with Windows debugging of the symlink issues, could you have a look on the remaining test cases and tell me why they might fail to create the links?

Funny part is, during my testing the creation of the symlink went just fine, just the validation of a relative symlink failed due to Windows failing to execute the readlink() call and erroring out with:

C:\projekte\composer-plugin>vendor\bin\phpunit.bat
PHPUnit 4.8.31 by Sebastian Bergmann and contributors.

...............E......................

Time: 24.31 seconds, Memory: 8.00MB

There was 1 error:

1) ContaoCommunityAlliance\Composer\Plugin\Test\LegacyContaoModuleInstallerTest::testSourcesOnInstallIfSymlinkIsRelative

readlink(): readlink failed to read the symbolic link ([...]/composer-plugin-test-8679005b/vendor/foo/bar/../../../system/modules/foobar/config/config.php), error 123)

[...]\vendor\symfony\phpunit-bridge\DeprecationErrorHandler.php:73
[...]\tests\LegacyContaoModuleInstallerTest.php:107

FAILURES!
Tests: 38, Assertions: 93, Errors: 1.

@fritzmg
Copy link
Contributor Author

fritzmg commented Dec 14, 2016

Which PHP version is used for testing?

@discordier
Copy link
Member

5.6 and 7.0 so far.

@fritzmg
Copy link
Contributor Author

fritzmg commented Dec 14, 2016

Hm, it's weird that the creation of the relative symlink works. It still has not been fixed in any PHP version, including PHP 7: php/php-src#1243

I have no experience with AppVeyor, how exactly are these tests run? Can I run these tests locally?

@fritzmg
Copy link
Contributor Author

fritzmg commented Dec 14, 2016

I ran

php phpunit.phar --bootstrap tests/bootstrap.php tests/ContaoModuleInstallerTest

locally and got

There were 2 errors:

1) ContaoCommunityAlliance\Composer\Plugin\Test\ContaoModuleInstallerTest::testSourcesOnInstall
RuntimeException: Failed to create symlink C:/…/Temp/composer-plugin-test-700d65ff/system/modules/foobar/config/config.php

C:\…\composer-plugin\src\Installer\AbstractModuleInstaller.php:241
C:\…\composer-plugin\src\Installer\AbstractModuleInstaller.php:91
C:\…\composer-plugin\tests\ContaoModuleInstallerTest.php:122

2) ContaoCommunityAlliance\Composer\Plugin\Test\ContaoModuleInstallerTest::testSourcesOnInstallIgnoresIfLinkIsAlreadyCorrect
RuntimeException: Installation target "C:/…/Temp/composer-plugin-test-822ba422/system/modules/foobar/config/config.php" already exists

C:\…\composer-plugin\src\Installer\AbstractModuleInstaller.php:529
C:\…\composer-plugin\src\Installer\AbstractModuleInstaller.php:459
C:\…\composer-plugin\src\Installer\AbstractModuleInstaller.php:229
C:\…\composer-plugin\src\Installer\AbstractModuleInstaller.php:91
C:\…\composer-plugin\tests\ContaoModuleInstallerTest.php:158

So the creation of the symlink fails as expected.

@fritzmg
Copy link
Contributor Author

fritzmg commented Feb 13, 2017

Any updates on this? This makes developing a Contao 4 website currently rather inconvenient for me. I can make a PR for the absolute symlink fallback if you want.

@aschempp
Copy link
Collaborator

So the necessary changes is basically this?
contao/core-bundle@77c1c10#diff-eebbebd36be2522e4a8c571895b59325R183

Do you get the IOException?

@fritzmg
Copy link
Contributor Author

fritzmg commented Feb 13, 2017

So the necessary changes is basically this?
contao/core-bundle@77c1c10#diff-eebbebd36be2522e4a8c571895b59325R183

I guess, I haven't yet looked at how it was actually implemented in Contao 4.

Do you get the IOException?

No, but the IOException comes from Symfony. The composer plugin does not use Symfony for symlinks, does it?

@fritzmg
Copy link
Contributor Author

fritzmg commented Feb 13, 2017

Also Contao does not use a fallback on failure at all. It simply checks if the directory separator is \ and if so, it creates absolute symlinks: https://github.com/contao/core-bundle/blob/4.3.3/src/Util/SymlinkUtil.php#L38-L39

But currently even that fails in my testing environment with PHP 7.1.1.

@discordier
Copy link
Member

So both are failing for you? core-bundle and composer-plugin?

@fritzmg
Copy link
Contributor Author

fritzmg commented Feb 21, 2017

So both are failing for you? core-bundle and composer-plugin?

No, core-bundle works, there was some other issue. But composer-plugin still gives me the initially described problem.

$ composer require fritzmg/contao-inherit-article
Using version ^1.3 for fritzmg/contao-inherit-article
./composer.json has been updated
Loading composer repositories with package information
Updating dependencies (including require-dev)
  - Installing contao-community-alliance/composer-plugin (3.0.4)
    Downloading: 100%

  - Installing fritzmg/contao-inherit-article (1.3.3)
    Downloading: 100%


Installation failed, reverting ./composer.json to its original content.


  [RuntimeException]
  Failed to create symlink C:/xampp/htdocs/c43dev/system/modules/inherit_article

@aschempp
Copy link
Collaborator

@discordier I saw you added a branch related to this, can this be finalized so we can release 3.0.5 ?

@discordier
Copy link
Member

We can surely try to finalize this, yet I lack the time to do this right now.

Maybe I could try to squeeze it in next week.

@m-vo
Copy link
Contributor

m-vo commented Apr 5, 2017

(I stumbled across this topic when trying to add codefog/haste (which requires the ComposerPlugin) to a contao 4.3 installation on a windows machine (PHP7) resulting in the same Failed to create symlink xyz error.)

...so just to sum it up:

  • addSymlinks(...) gets called during install and normalizes source and target paths
  • it then calls Composer\Util\Filesystem->relativeSymlink(...)
[...]
public function relativeSymlink($target, $link) {
  $cwd = getcwd();
  $relativePath = $this->findShortestPath($link, $target);
  chdir(dirname($link));
  $result = @symlink($relativePath, $link);
  chdir($cwd);
  return (bool) $result;
}
  • there @symlink($relativePath, $link) fails with the error symlink(): Could not fetch file information(error 3)
  • this is exactly the same output I get if running the commands (chdir(...) + symlink(...)) directly on the command line
  • if I understand correctly this is the expected behaviour as creating relative symlinks unfortunately does not work with PHP on windows (at least not in this way)

Am I right to assume that the only working solution then is to use absolute links as a fallback? Sorry if this was clear all the time - I am just trying to understand the core of the problem and maybe help debugging/testing. 😉

EDIT: btw., thats how the actual paths look like:

  • $link = "X:/xampp/htdocs/test/system/modules/haste"
  • $relativePath = "../../vendor/codefog/contao-haste"

@fritzmg
Copy link
Contributor Author

fritzmg commented Apr 5, 2017

Am I right to assume that the only working solution then is to use absolute links as a fallback?

Yes 😉 . Symfony does the same fallback.

@m-vo
Copy link
Contributor

m-vo commented Apr 5, 2017

Right, I did a quick test and it seems to work without a problem so far!

For my test, I just replaced this (https://github.com/contao-community-alliance/composer-plugin/blob/3.0.4/src/Installer/AbstractModuleInstaller.php#L240-L242):

if (!$this->filesystem->relativeSymlink($source, $target)) {
    throw new \RuntimeException('Failed to create symlink ' . $target);
}

with this:

if('\\' == DIRECTORY_SEPARATOR) {
    if (!@symlink($source, $target)) {
        throw new \RuntimeException('Failed to create absolute symlink (fallback) ' . $target);
    }
}
else {
    if (!$this->filesystem->relativeSymlink($source, $target)) {
        throw new \RuntimeException('Failed to create symlink ' . $target);
    }
}

@fritzmg
Copy link
Contributor Author

fritzmg commented Apr 21, 2017

@m-vo looks good, tested it as well. Can you please make a PR?

@m-vo
Copy link
Contributor

m-vo commented Apr 23, 2017

@fritzmg Allright, I'll try my best (might take until mid next week, though).

@fritzmg
Copy link
Contributor Author

fritzmg commented Apr 23, 2017

Since it's a simple fix, you could also just use the edit button here on GitHub ;)

@m-vo
Copy link
Contributor

m-vo commented Apr 23, 2017

You're so right :-) First time I tried the online editor, hope it worked.

@aschempp
Copy link
Collaborator

Fixed in #67

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

No branches or pull requests

5 participants