PSR-0/4 dependencies are not correctly prefixed

polevaultweb commented 4 years ago

Bug report

This is related to https://github.com/humbug/php-scoper/issues/127, but it feels like a regression has happened and now PSR-4 and PSR-0 namespaces aren't prefixed in the Composer autoloader files.

Using the test repo here,

composer install
./vendor/bin/php-scoper add-prefix --output-dir=build-prefixed --force --prefix=Mutant

`autoload_static.php`:

public static $prefixLengthsPsr4 = array(
        'S' => array(
            'Symfony\\Polyfill\\Php73\\'       => 23,
            'Symfony\\Polyfill\\Mbstring\\'    => 26,
            'Symfony\\Polyfill\\Ctype\\'       => 23,
            'Symfony\\Contracts\\Service\\'    => 26,
            'Symfony\\Component\\Finder\\'     => 25,
            'Symfony\\Component\\Filesystem\\' => 29,
            'Symfony\\Component\\Console\\'    => 26,
            'ScoperBug\\'                      => 10,
        ),
        'P' => array(
            'Psr\\Container\\'  => 14,
            'PhpParser\\'       => 10,
            'PackageVersions\\' => 16,
        ),
        'H' => array( 'Humbug\\PhpScoper\\' => 17 ),
    );
    public static $prefixDirsPsr4 = array(
        'Symfony\\Polyfill\\Php73\\'       => array( 0 => __DIR__ . '/..' . '/symfony/polyfill-php73' ),
        'Symfony\\Polyfill\\Mbstring\\'    => array( 0 => __DIR__ . '/..' . '/symfony/polyfill-mbstring' ),
        'Symfony\\Polyfill\\Ctype\\'       => array( 0 => __DIR__ . '/..' . '/symfony/polyfill-ctype' ),
        'Symfony\\Contracts\\Service\\'    => array( 0 => __DIR__ . '/..' . '/symfony/service-contracts' ),
        'Symfony\\Component\\Finder\\'     => array( 0 => __DIR__ . '/..' . '/symfony/finder' ),
        'Symfony\\Component\\Filesystem\\' => array( 0 => __DIR__ . '/..' . '/symfony/filesystem' ),
        'Symfony\\Component\\Console\\'    => array( 0 => __DIR__ . '/..' . '/symfony/console' ),
        'ScoperBug\\'                      => array( 0 => __DIR__ . '/../..' . '/src' ),
        'Psr\\Container\\'                 => array( 0 => __DIR__ . '/..' . '/psr/container/src' ),
        'PhpParser\\'                      => array( 0 => __DIR__ . '/..' . '/nikic/php-parser/lib/PhpParser' ),
        'PackageVersions\\'                => array( 0 => __DIR__ . '/..' . '/ocramius/package-versions/src/PackageVersions' ),
        'Humbug\\PhpScoper\\'              => array( 0 => __DIR__ . '/..' . '/humbug/php-scoper/src' ),
    );
    public static $prefixesPsr0 = array( 'P' => array( 'Pimple' => array( 0 => __DIR__ . '/..' . '/pimple/pimple/src' ) ) );

The public static $classMap = array( is correctly prefixed

`autoload_real.php`

ClassLoader isn't prefixed here -

    private static $loader;
    public static function loadClassLoader($class)
    {
        if ('Composer\\Autoload\\ClassLoader' === $class) {
            require __DIR__ . '/ClassLoader.php';
        }
    }

`autoload_psr4.php`

The array key strings of the class aren't namespaced.

// autoload_psr4.php @generated by Composer
$vendorDir = \dirname( \dirname( __FILE__ ) );
$baseDir   = \dirname( $vendorDir );

return array( 'Symfony\\Polyfill\\Php73\\'       => array( $vendorDir . '/symfony/polyfill-php73' ),
              'Symfony\\Polyfill\\Mbstring\\'    => array( $vendorDir . '/symfony/polyfill-mbstring' ) );

`autoload_namespaces.php`

Same as autoload_psr4.php.

Question	Answer
Box version	0.12.4
PHP version	7.2.1
Platform with version	MacOS
Github Repo	https://github.com/polevaultweb/php-scoper-bug

scoper.inc.php

```php

Output

```bash $ command > output ```

theofidry commented 4 years ago

You need to do a composer dump-autoload afterwards, it is done automatically if you are using Box but still needs to be done if you are using PHP-Scoper alone

polevaultweb commented 4 years ago

hi @theofidry sorry of course, but I think I'm going in circles. Doing that fixes the issue, but then I lose my namespace prefix from the Composer classes, like I reported in https://github.com/humbug/php-scoper/issues/353

But that has the undesired effect of removing my custom prefix from the Composer autoloader classes.

theofidry commented 4 years ago

I'm a bit confused:

353 was about trying to exclude php-scoper from your package to which there a couple of solutions suggested, the most obvious one being to remove the dep dependencies by including the --no-dev flag when installing/dumping the autoloader
PHP-Scoper is unable to properly scope the autoloader as it would require a very tight coupling on the autoloader mechanism; since the Composer autoloader is already pretty unique (in terms of namespace/class collision) there is also no need to actually scope it

theofidry commented 4 years ago

Maybe it would be easier if you shared your "build" script or "building" process?

polevaultweb commented 4 years ago

PHP-Scoper is unable to properly scope the autoloader as it would require a very tight coupling on the autoloader mechanism; since the Composer autoloader is already pretty unique (in terms of namespace/class collision) there is also no need to actually scope it

So the main issue is the ClassLoader class, which isn't uniquely named. I had an issue where another WordPress plugin had some packages included with an older version of Composer, their ClassLoader was loaded and setClassMapAuthoritative didn't exist.

My build process is as you suggested in #353 to remove the dev dependencies

rm -rf ./build
mkdir ./build
cp ./composer.json ./build/composer.json
cp ./scoper.inc.php ./build/scoper.inc.php
composer install --no-dev --optimize-autoloader --working-dir build
cd build
PREFIX=MY_PREFIX
php -d memory_limit=728M ../vendor/bin/php-scoper add-prefix --prefix=${PREFIX} --no-interaction --force --output-dir=prefixed

What about performing the scoping, dumping the autoloader, then copying the prefixed/vendor/composer dir to another directory and just scoping that and moving it back later?

theofidry commented 4 years ago

So the main issue is the ClassLoader class, which isn't uniquely named. I had an issue where another WordPress plugin had some packages included with an older version of Composer, their ClassLoader was loaded and setClassMapAuthoritative didn't exist.

Hm that could be indeed; To which there is two "solutions":

To not care: it requires Composer to be updated accordingly
To take a look at how Composer can be scoped

I did not dig into the last one since I wanted to avoid any coupling with Composer internals. However maybe I missed something and there is a way to get it done without too much hacking, e.g. just scoping the ClassLoader for example.

What about performing the scoping, dumping the autoloader, then copying the prefixed/vendor/composer dir to another directory and just scoping that and moving it back later?

Related to the previous point actually: the issue is that there is the Composer executable which you are using (and is not scoped) and scoping the "dumped" autoloader (`vendor/composer/autoload_*) is horribly complex.

polevaultweb commented 4 years ago

Ok so maybe some clever manual patching of the ClassLoader class after scoping has taken place?

theofidry commented 4 years ago

Either that, or we can try to find a way to do it in a more automated way. If you find a reproducible way to do it via a simple script for example, that could be enough and documented

theofidry commented 4 years ago

Had a little chat about Nils about it. It looks like it won't be easy to change ClassLoader since it could be typehinted elsewhere as well, so I only see 3 solutions:

Stick with "patch it manually" (no idea how easy/hard it is)
Say "not my problem, use the last Composer executable available"
Manage to scope the autoloader removing the need to dump it again

I hope you could help out with that, at least to report how hard/easy it is.
Is the most sensitive answer IMO: Composer is relatively stable, it's not like updating it should break anything so pushing people to update it is not too much of a stretch IMO; especially if you are already using PHP-Scoper you are already on the edge! :D
I don't see this happening unless you are ready and committed to work on this for the next years because it's gonna require frequent updates as Composer (internals) changes and is always going to be extremely fragile

So I hope you can give a shot to the first and report back on it! Depending on the result we can discuss further to see if there is something that can be done to make it easier

polevaultweb commented 4 years ago

Thanks for your time and help @theofidry - will try 1. and report back 👍

polevaultweb commented 4 years ago

@theofidry got this working for me https://gist.github.com/polevaultweb/de44d747b57191a851947d5f5175e922

theofidry commented 4 years ago

👍 Might be worth adding a doc entry for it? Alternatively could even be a dedicated command added to PHP-Scoper

polevaultweb commented 4 years ago

Where are the docs? the README.md you mean?

Alternatively could even be a dedicated command added to PHP-Scoper

That would be cool!

theofidry commented 4 years ago

Yep the readme

theofidry commented 4 years ago

Closing as the original issue has been fixed. Will gladly accept a PR to update the readme to include your work!

humbug / php-scoper