Skip to content

Scoped folder name of package not what expected? #683

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

Closed
alessandrotesoro opened this issue Sep 26, 2022 · 7 comments
Closed

Scoped folder name of package not what expected? #683

alessandrotesoro opened this issue Sep 26, 2022 · 7 comments

Comments

@alessandrotesoro
Copy link

Bug report

Question Answer
PHP-Scoper version 0.17.5
PHP version 8
Platform with version MacOS

Hi there,

In a project I'm working on, we have 2 packages with the same vendor name, example "my-company". When scoping the packages, they're both scoped, however the folder structure of one of them is completely broken.

In the configuration below: the package with name "first-package" is correctly scoped under dependencies/first-package - the 2nd package however, for some reason, it's using the entire project structure and becomes dependencies/project-name-here/vendor/my-company/second-package but what I'd need it to be is dependencies/second-package.

Any idea what's causing this?

Note: the 1st package is symlinked locally. The 2nd isn't.

scoper.inc.php
<?php

declare( strict_types=1 );

use Isolated\Symfony\Component\Finder\Finder;

return [
   // The prefix configuration. If a non null value will be used, a random prefix will be generated.
   'prefix' => 'My_Company\\Plugin\\Test_Plugin\\Dependencies',

   /**
    * By default when running php-scoper add-prefix, it will prefix all relevant code found in the current working
    * directory. You can however define which files should be scoped by defining a collection of Finders in the
    * following configuration key.
    *
    * For more see: https://github.com/humbug/php-scoper#finders-and-paths.
    */
   'finders'                    => [
       Finder::create()->
           files()->
           ignoreVCS( true )->
           ignoreDotFiles( true )->
           notName( '/LICENSE|.*\\.md|.*\\.dist|Makefile|composer\\.(json|lock)/' )->
           exclude(
               [
                   'doc',
                   'test',
                   'build',
                   'test_old',
                   'tests',
                   'Tests',
                   'vendor-bin',
                   'wp-coding-standards',
                   'squizlabs',
                   'phpcompatibility',
                   'dealerdirect',
                   'bin',
                   'vendor'
               ]
           )->
           in( [
               'vendor/my-company/first-package',
               'vendor/my-company/second-package'
           ] )->
       name( [ '*.php' ] ),
   ],

   'patchers'                   => [
       function ( string $file_path, string $prefix, string $contents ): string {
           // Change the contents here.

           return str_replace(
               'Symfony\\\\',
               sprintf( '%s\\\\Symfony\\\\', addslashes( $prefix ) ),
               $contents
           );
       },
   ],
];
Output
$ bin/php-scoper add-prefix --config .scoper.inc.php --output-dir dependencies/
> output
@theofidry
Copy link
Member

Note: the 1st package is symlinked locally. The 2nd isn't.

This is most likely the problem: IIRC the finder is configured to not follow symlinks (not certain of it though)

@alessandrotesoro
Copy link
Author

Indeed that was the issue, thanks for your help.

@LuigiPulcini
Copy link

Could this behavior be improved somehow?
At the moment, this forces using local libraries with mirroring instead of symlinking, which is not very flexible.

It is true that when php-scoper gets the real path of the files collected by the Finder (here), any symlink points to the real path of each file and that is what php-scoper uses to determine the $commonDirectoryPath (here).

Nevertheless, php-scoper disregards the original keys the Finder uses to collect all the files. Those keys are relative to the working directory and could be effectively used as array keys here, instead of the real paths.

I wonder if this behavior could be optionally introduced with an additional parameter (either in the command line or the configuration file itself).

@theofidry
Copy link
Member

Symlinks support was never added because I didn't want to bother with it, not because there is no value in it.

You are right that in theory PHP-Scoper should not care that it's a symlink: if you have a directory with a symlink in there, it could treat as if it was not a symlink and just do its job. The only difference would be to be careful to keep the file path rather than its link:

vendor/
     acme/
          symlinked-package/ <- should be copied with this path; doesn't matter what it points to

If that is the case I don't see why it would need an additional parameter (unless you think it can cause problems).

So it's definitely a feature that one can contribute to :)

@LuigiPulcini
Copy link

LuigiPulcini commented Oct 31, 2023

First of all, thank you so much for your super prompt reply, which is truly appreciated.

Regardless of whether a local library is symlinked or mirrored in the vendor directory, a Finder component always returns an array the keys of which are the path relative to the current composer configuration.
So, just to be clear, in the following scenario:

vendor/
    acme/
        library-1/                <- this is a symlink to /absolute/path/to/library-1
            class-foo.php
        library-2/                <- this is a path relative to the current project
            class-bar.php
    new-acme/
        library-3/                <- this is a path relative to the current project
            class-baz.php

For all those libraries, a Finder returns an array with the following keys:

array(
    'vendor/acme/library-1/class-foo.php',
    'vendor/acme/library-2/class-bar.php',
    'vendor/new-acme/library-3/class-baz.php',
)

When it comes to real paths, though, you have the following keys:

array(
    '/absolute/path/to/library-1/class-foo.php',
    '/absolute/path/to/my-project/vendor/acme/library-2/class-bar.php',
    '/absolute/path/to/my-project/vendor/new-acme/library-3/class-baz.php',
)

And that is what php-scoper uses as keys in the retrieveFilesWithContents() method.
So, when $commonDirectoryPath is calculated (as /absolute/path/to/) and removed from the beginning of the keys, you get the following keys:

array(
    'library-1/class-foo.php',
    'my-project/vendor/acme/library-2/class-bar.php',
    'my-project/vendor/new-acme/library-3/class-baz.php',
)

while, if you used the original keys from the finder, you would have gotten the more logical keys:

array(
    'acme/library-1/class-foo.php',
    'acme/library-2/class-bar.php',
    'new-acme/library-3/class-baz.php',
)

I say "more logical" because that is exactly the same result you would get when all libraries are distributed and copied inside the vendor folder, after the development of the symlinked library is complete. Hence, my idea of leaving it to developers whether to use the original keys from Finders as an alternative to the real path of the files.

In terms of contributing to this change, I will be more than happy to help if you like, as this may require very few changes to the current implementation.

@theofidry
Copy link
Member

When it comes to real paths, though, you have the following keys:

So maybe it is only a matter of getting the link path rather than the real path for symlinks?

In terms of contributing to this change, I will be more than happy to help if you like, as this may require very few changes to the current implementation.

I would be more than happy to receive help there :) In terms of changes I think it's best to come up with a working draft, adapting the rest can be figured out afterwards

@LuigiPulcini
Copy link

So maybe it is only a matter of getting the link path rather than the real path for symlinks?

I was suggesting to use the Finder keys as an optional, alternative possibility so that the new implementation is backward-compatible with the current one (just in case someone is already relying on the current logic).

I will submit a PR shortly.

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

No branches or pull requests

3 participants