Using escapeshellarg

[tobi-pb]

I think it a good idea to escape shell argument here. In my oppionen mklink and symlink does the same. but i am really not a windows guy ;)

[Flyingmana]

thank you,

@Cash2m, you worked on this, Iam also no windows user, can you test it or give a comment if this makes sense?

[HenningCash]

I thought about escaping aswell. The code fails for me because of too many quotes. But btw: Can't we use symlink() instead of exec() for windows too?

[Flyingmana]

if it works for you, maybe. I should search for a CI environment which offers windows :/

[AydinHassan]

Apparently you can run anything on here: http://www.appveyor.com/ My friend runs a node project on there

[Flyingmana]

great link, will get added the next days :)

[LeeSaferite]

I was just researching this and since PHP 5.3.0 you can use symlink on Windows Vista/Windows Server 2008 and above. As you already have a requirement for PHP 5.3.2+ I think having a requirement for Windows users of Vista or above is reasonable. With that in mind you could drop the whole OS check for Windows and treat it the same. Maybe add something to the docs about the version needed and the fact that symlinking only works (apparently) under administrator access.

[Flyingmana]

do you know how the output differs from exec('mklink'...) and symlink(...) if you dont have administator access on windows?

[HenningCash]

This is how symfony deals with it

https://github.com/symfony/Filesystem/blob/master/Filesystem.php#L313

[LeeSaferite]

The '\\' === DIRECTORY_SEPARATOR bit seems a little hacky, but it's a good solution overall.

[Flyingmana]

what is the difference between '\\' === DIRECTORY_SEPARATOR and strtoupper(substr(PHP_OS, 0, 3)) == 'WIN'?

[LeeSaferite]

Well, the PHP_OS value could be CYGWIN which could be a problem, right?

[Flyingmana]

does CYGWIN behave like win or linux? Or is it just undefined :/ Thats probably another issue

[HenningCash]

CYGWIN behaves excactly like default Windows. DIRECTORY_SEPARATOR is \ and symlink() without admin privileges also throws the error

array(4) {
  ["type"]=>
  int(2)
  ["message"]=>
  string(50) "symlink(): Cannot create symlink, error code(1314)"
  ["file"]=>
  string(24) "C:\Users\foo\test.php"
  ["line"]=>
  int(3)
}

So it's save to check only for backslash, even if it's hacky

[barryvdh]

Why not just let Symfony handle it altogether. Just require symfony/filesystem 2.x and use that instead of the Composer filesystem?

[Flyingmana]

@barryvdh you are right, I remember some problems in the past, but its time to move to better reusable parts and reduce our own codebase. Not now, but during one of the next releases the symfony/filesystem component will become a part of the installer. For now this fix should do well