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

Error on Windows #810

Closed
AnirudhSingal91 opened this issue Dec 28, 2022 · 12 comments
Closed

Error on Windows #810

AnirudhSingal91 opened this issue Dec 28, 2022 · 12 comments

Comments

@AnirudhSingal91
Copy link

AnirudhSingal91 commented Dec 28, 2022

Bug report

| Question | Answer
| Getting error while running the php-scoper add-prefix command

I came across php-scoper because I was experiencing dependency hell in my custom wordpress plugin. I just want to prefix one package(guzzlehttp). I found implementing php-scoper to be extremely hard and i'm probably doing it all wrong.

I get the following error while running the php-scoper add-prefix command

In Filesystem.php line 95:

Failed to create "C:/Users/singa/source/repos/WIP/woocommerce-xero-integration/woocommerce-xero-integration/woocommerce-xero-integration/dependencies/vendorC:\Users\singa\source\repos\WIP\woocommerce-xero-integration\woocommerce-xero-integration\woocommerce-xero-integration\vendor\guzzlehttp\guzzle\src": mkdir(): No such file or directory

| ---------------
| PHP-Scoper version | x.y.z 0.18
| PHP version | x.y.z 8.1
| Platform with version | e.g. Ubuntu/Windows/MacOS Windows
| Github Repo | -

scoper.inc.php
<?php

declare(strict_types=1);

use Isolated\Symfony\Component\Finder\Finder;

return [
];
Output
$ command
> output
@theofidry
Copy link
Member

@AnirudhSingal91 sorry to hear.

Unfortunately this library is not tested under Windows. I have no Windows machine myself so it was never practical to add support for it and no one really stepped up to help out there. A few issues have occasionally been fixed for it, so maybe it works, maybe not.

I unfortunately cannot help out much there without any sort of reproducer.

You might also want to checkout #303 which talks to length about the WP use case.

@AnirudhSingal91
Copy link
Author

@theofidry.

Probably noob question, but If I use something like WSL2, will it work?

@theofidry
Copy link
Member

no idea, I've never used it myself so can't say

@janboddez
Copy link

Just wanted to mention I ran into the same thing (on Windows 10, PHP 8.1). Upgraded to 0.18 to be able to fix a different issue, and I get the same kind of error (v0.15/PHP 7.4 and 0.17/PHP 8.0, which I used up until a minute ago, did not suffer from this).

@janboddez
Copy link

If I comment out line 95 in Symfony's filesystem/Filesystem.php, I (obviously) get a new error:

Cannot rename "C:\Users\Jan\AppData\Local\Temp\aut8C1C.tmp" to "C:/Users/Jan/Sites/<etc.>/buildC:\Users\Jan\Sites\<etc.>\vendor\autoload.php"

Look like the "to" path is wrong. Instead of build\vendor\autoload.php it is build<the-absolute-path-to-the-vendor-dir>\autoload.php (if you get what I'm trying to say).

@janboddez
Copy link

I know it's super hacky, but if I essentially undo #806 by replacing ConsoleScoper.php and functions.php with the version from just before, it again works.

@theofidry theofidry changed the title Error while running add-prefix command Error on Windows Feb 22, 2023
@saas786
Copy link

saas786 commented Sep 27, 2023

I am encountering the same issue with https://github.com/humbug/php-scoper/releases/tag/0.18.3.

I have traced it back to this PR: https://github.com/humbug/php-scoper/pull/806/files
and specifically, this line of code: https://github.com/humbug/php-scoper/blob/0.18.3/src/Console/ConsoleScoper.php#L185.

In my case, there are two different separators used for $commonDirectoryPath and $inputFileTuple[0], one using \ and the other using /. As a result, str_replace fails.

Normalizing the paths can help address this issue. I have tested the code on v0.17.5, and it works. However, I want to utilize v0.18.3 due to Symfony patches and other enhancements.

@janboddez
Copy link

janboddez commented Sep 27, 2023

Looks like Symfony's Path::getDirectory() calls self::canonicalize(), which turns backslashes into forward slashes. See https://github.com/symfony/filesystem/blob/6.3/Path.php#L145.

Can we not also run $filesWithContent and $excludedFilesWithContents through Path::canonicalize()? Or Path::normalize()? Or would that break something else?

@saas786
Copy link

saas786 commented Sep 27, 2023

        // Fix path.
        $outputDir = str_replace(array('/', '\\'), DIRECTORY_SEPARATOR, $outputDir);
        $commonDirectoryPath = str_replace(array('/', '\\'), DIRECTORY_SEPARATOR, $commonDirectoryPath);

Using this just before: https://github.com/humbug/php-scoper/blob/0.18.3/src/Console/ConsoleScoper.php#L182

fixes it for me.


Additionally

        $commonDirectoryPath = Path::getLongestCommonBasePath(
            ...array_map(
                static fn (string $path) => Path::getDirectory($path),
                array_keys($filesWithContent),
            ),
            ...array_map(
                static fn (string $path) => Path::getDirectory($path),
                array_keys($excludedFilesWithContents),
            ),
        );

produces path C:\project', which than results in output as C:\project\vendor_prefixed\vendor\packages`, which is not ideal.

I would rather prefer C:\project\vendor_prefixed\packages, is there something at config level I could set, to avoid this situation?


Also when I use

// Example of collecting files to include in the scoped build but to not scope
// leveraging the isolated finder.
$excludedFiles = array_map(
    static fn( SplFileInfo $fileInfo ) => $fileInfo->getPathName(),
    iterator_to_array(
        Finder::create()->files()->in( __DIR__ ),
        false
    )
);

it instead of excluding the files, include these files (which are infact config files scoper.inc.php.

@EusebiuOprinoiu
Copy link

This is a really frustrating issue.
The latest version that can be used (without modifying the package files) is 0.18.0.

I think the problem can be solved by normalizing all paths to use consistent slashes.
On Windows, paths use \ slashes, while Unix systems use / slashes.
Most people use Unix-style slashes in their code since the overwhelming majority of PHP servers run on Linux, but without any kind of normalization, this leads to issues when paths are created dynamically in Windows environments.

One possible solution would be to check the path of the original vendor directory, check what kind of slashes it contains, and normalize all paths to use that kind of slash.
As an alternative, you could normalize all paths to use / regardless of operating system. I believe this is what WordPress does.
The point is, paths can't use a mix of \ and / slashes.

@theofidry
Copy link
Member

AFAIK all the user defined paths are actually normalized. At least that is one part of the process that was done at some point to bring compatibility with Windows. As mentioned elsewhere though, I don't have a Windows machine so trying to keep track is not worth it, as if the CI fails I still have no way to really debug this locally.

I think the root of the issue in this case though is switching from Webmozart's PathUtil to the Symfony Path. And ideally this could be fixed by patching it (this could be tested by here first to validate the issue).

@EusebiuOprinoiu
Copy link

EusebiuOprinoiu commented Oct 1, 2023

I am willing to test any changes you make, but there is no need to switch libraries.
I believe I found the issue. As you said, user-defined paths are normalized, but $inputFileTuple is not.
I updated the code between lines 182-186 from src/Console/ConsoleScoper.php like this, and the issue is gone.

Original code:

$mapFiles = static fn (array $inputFileTuple) => [
    $inputFileTuple[0],
    $inputFileTuple[1],
    $outputDir.str_replace($commonDirectoryPath, '', $inputFileTuple[0]),
];

New code with normalized input files:

$mapFiles = static fn (array $inputFileTuple) => [
    Path::canonicalize($inputFileTuple[0]),
    $inputFileTuple[1],
    $outputDir.str_replace($commonDirectoryPath, '', Path::canonicalize($inputFileTuple[0])),
];

I'm not familiar with your codebase and I might be wrong, but as far as I can tell you normalize only the path and the output-dir.
You should normalize all variables that contain paths, whether they are user-defined or not.
(especially when dealing with a mix of static and programmatically generated values)

This includes $filesWithContents and $excludedFilesWithContents, which right now are not normalized.
For best results, the normalization should be done early, when the $config object is built, to avoid the need to normalize the values every time they are used.

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

5 participants