FIX: Don't use realpath() for module and base path

[Zauberfisch]

fixes #20 and #34

Is there a particular reason why we have been using realpath in the first place?
Can we not assume that the list of modules given to us are the paths we want?

[maxime-rainville]

Any chance you could add a unit test to cover this scenario?

[Zauberfisch]

to busy at the moment. feel free to create an issue and tag me in it for a later time though.

Any chance we can get this merged without test coverage?

[dhensby]

I think either way, this is fine re test coverage.

If we have test coverage, then that's good and we can see nothing has broken and I don't think we should have a test that explicitly allows "bad" paths, that could shoot us in the foot in the future. If we don't have any test coverage of this, then it's probably not @Zauberfisch's job to add it

[maxime-rainville]

This needs to target 1 branch.

I tried replicating this change on my local dev environment. I got this error thrown in my face by composer.

                                                                                                                            
  [InvalidArgumentException]                                                                                                
  $from (/home/maxime/Projects/vendorplugintest/public/_resources/css) and $to (themes/simple/css) must be absolute paths.  
                                                                                                                            

Exception trace:
 () at phar:///usr/local/bin/composer/src/Composer/Util/Filesystem.php:362
 Composer\Util\Filesystem->findShortestPath() at phar:///usr/local/bin/composer/src/Composer/Util/Filesystem.php:606
 Composer\Util\Filesystem->relativeSymlink() at /home/maxime/Projects/vendor-plugin/src/Methods/SymlinkMethod.php:46
 SilverStripe\VendorPlugin\Methods\SymlinkMethod->createLink() at /home/maxime/Projects/vendor-plugin/src/Methods/SymlinkMethod.php:35
 SilverStripe\VendorPlugin\Methods\SymlinkMethod->exposeDirectory() at /home/maxime/Projects/vendor-plugin/src/Methods/ChainedMethod.php:38
 SilverStripe\VendorPlugin\Methods\ChainedMethod->exposeDirectory() at /home/maxime/Projects/vendor-plugin/src/Library.php:228
 SilverStripe\VendorPlugin\Library->exposePaths() at /home/maxime/Projects/vendor-plugin/src/VendorExposeTask.php:98
 SilverStripe\VendorPlugin\VendorExposeTask->process() at /home/maxime/Projects/vendor-plugin/src/VendorPlugin.php:249
 SilverStripe\VendorPlugin\VendorPlugin->installLibrary() at /home/maxime/Projects/vendor-plugin/src/VendorPlugin.php:144
 SilverStripe\VendorPlugin\VendorPlugin->installPackage() at n/a:n/a
 call_user_func() at phar:///usr/local/bin/composer/src/Composer/EventDispatcher/EventDispatcher.php:174
 Composer\EventDispatcher\EventDispatcher->doDispatch() at phar:///usr/local/bin/composer/src/Composer/EventDispatcher/EventDispatcher.php:119
 Composer\EventDispatcher\EventDispatcher->dispatchPackageEvent() at phar:///usr/local/bin/composer/src/Composer/Installer/InstallationManager.php:431
 Composer\Installer\InstallationManager->Composer\Installer\{closure}() at phar:///usr/local/bin/composer/src/Composer/Installer/InstallationManager.php:444
 Composer\Installer\InstallationManager->executeBatch() at phar:///usr/local/bin/composer/src/Composer/Installer/InstallationManager.php:367
 Composer\Installer\InstallationManager->downloadAndExecuteBatch() at phar:///usr/local/bin/composer/src/Composer/Installer/InstallationManager.php:266
 Composer\Installer\InstallationManager->execute() at phar:///usr/local/bin/composer/src/Composer/Installer.php:698
 Composer\Installer->doInstall() at phar:///usr/local/bin/composer/src/Composer/Installer.php:535
 Composer\Installer->doUpdate() at phar:///usr/local/bin/composer/src/Composer/Installer.php:247
 Composer\Installer->run() at phar:///usr/local/bin/composer/src/Composer/Command/UpdateCommand.php:236
 Composer\Command\UpdateCommand->execute() at phar:///usr/local/bin/composer/vendor/symfony/console/Command/Command.php:245
 Symfony\Component\Console\Command\Command->run() at phar:///usr/local/bin/composer/vendor/symfony/console/Application.php:835
 Symfony\Component\Console\Application->doRunCommand() at phar:///usr/local/bin/composer/vendor/symfony/console/Application.php:185
 Symfony\Component\Console\Application->doRun() at phar:///usr/local/bin/composer/src/Composer/Console/Application.php:310
 Composer\Console\Application->doRun() at phar:///usr/local/bin/composer/vendor/symfony/console/Application.php:117
 Symfony\Component\Console\Application->run() at phar:///usr/local/bin/composer/src/Composer/Console/Application.php:122
 Composer\Console\Application->run() at phar:///usr/local/bin/composer/bin/composer:63
 require() at /usr/local/bin/composer:24

update [--with WITH] [--prefer-source] [--prefer-dist] [--dry-run] [--dev] [--no-dev] [--lock] [--no-install] [--no-autoloader] [--no-scripts] [--no-suggest] [--no-progress] [-w|--with-dependencies] [-W|--with-all-dependencies] [-v|vv|vvv|--verbose] [-o|--optimize-autoloader] [-a|--classmap-authoritative] [--apcu-autoloader] [--apcu-autoloader-prefix APCU-AUTOLOADER-PREFIX] [--ignore-platform-req IGNORE-PLATFORM-REQ] [--ignore-platform-reqs] [--prefer-stable] [--prefer-lowest] [-i|--interactive] [--root-reqs] [--] [<packages>]...

This is the composer file I was using for testing. I had a version of vendor-plugin minus the realpath call and of silverstripe/cms one level up from my test project.

{
    "name": "silverstripe/installer",
    "type": "silverstripe-recipe",
    "description": "The SilverStripe Framework Installer",
    "require": {
        "php": "^7.1 || ^8",
        "silverstripe/recipe-plugin": "^1.2",
        "silverstripe/recipe-cms": "4.x-dev",
        "silverstripe-themes/simple": "~3.2.0",
        "silverstripe/login-forms": "4.x-dev"
    },
    "require-dev": {
        "sminnee/phpunit": "^5.7",
        "sminnee/phpunit-mock-objects": "^3.4.5"
    },
    "extra": {
        "resources-dir": "_resources"
    },
    "config": {
        "process-timeout": 600
    },
    "prefer-stable": true,
    "minimum-stability": "dev",
    "repositories": [
        {
            "type": "path",
            "url": "../vendor-plugin"
        },
        {
            "type": "path",
            "url": "../silverstripe-cms"
        }
    ]
}

[jonom]

I encountered this problem recently when trying to update some modules for SS5... the same symlink strategy that has always worked for me is ruined by this bug 😅 I don't know why I haven't experienced it before but it's quite a PITA for local module development. I'm surprised this isn't affecting all/most module developers?

Anyway... I poked around for a couple of hours tonight and think I might have found the missing piece. This fix from @Zauberfisch worked perfectly for me - but I don't use a theme installed by composer (no shade, but... does anyone?). So I tried installing the simple theme and got the same error @maxime-rainville hit at the vendor-expose step. I thought maybe we were looking at an edge case for themes, since they're not installed in to the vendor directory. BUT right after hitting that error during composer update, I ran composer vendor-expose - and this time I didn't get an error. Symlinking worked just fine. I was able to reproduce this by uninstalling and re-installing the theme.

So I figured there must be a difference between doing the expose command at install time, vs running the 'expose all' command.

I found that in VendorPlugin::getLibrary() a Library is instantiated with (pseudo-code) new Library($this->getProjectPath(), $composerInstaller->getInstallPath($package)). The problem is that Composer's getInstallPath returns a relative path, as opposed to the absolute ones you get in VendorExposeCommand::getAllLibraries().

If I patch VendorPlugin::getLibrary() so the last line reads:

return new Library($this->getProjectPath(), $this->getProjectPath() . '/' . $path);

...then everything seems to work just fine without realpath.

I'm sure there's a 'right' way to join the paths as opposed to the hack I'm doing there, but I don't really know what it is. I also don't think I'll be able to find any more time to work on this - sorry - but I hope this provides a path to fixing this issue! As @Zauberfisch pointed out in #34, symlinking in a local repo is the 'official' way to do module development as far as I'm aware so it would be great to get this fixed.

[GuySartorelli]

The master branch is being deleted, which would close this PR. Based on #43 (review) I will instead retarget this PR at the 1.7 branch so if it ever gets merged it can be released as a patch.