Classmap nested path duplication

[Spreeuw]

I'm not sure if this was already reported before because there seem to be a number of issues regarding namespace duplication, but this appears to be different. config:

{
    "require": {
        "iio/libmergepdf": "^4.0"
    },
    "require-dev": {
        "coenjacobs/mozart": "dev-master"
    },
    "extra": {
        "mozart": {
            "dep_namespace": "MyLibMerge\\Vendor",
            "dep_directory": "/lib/packages/",
            "classmap_directory": "/lib/classes/",
            "classmap_prefix": "MyLibMerge_",
            "excluded_packages": [
                "tecnickcom/tcpdf"
            ],
            "override_autoload": {
            },
            "delete_vendor_directories": true
        }
    },
    "scripts": {
        "post-install-cmd": [
            "\"vendor/bin/mozart\" compose",
            "composer dump-autoload"
        ],
        "post-update-cmd": [
            "\"vendor/bin/mozart\" compose",
            "composer dump-autoload"
        ]
    }
}

results in the following path:

lib\classes\iio\libmergepdf\vendor\iio\libmergepdf\tcpdi

The classes themselves don't seem to be mapped correctly, but in my particular application it seems they are not used.

I saw a similar thing in #89 though I'm not sure it's the same issue/root cause

[BrianHenryIE]

I can't reproduce that on MacOS 10.15 using Bash shell. I presume since you're writing \ that you're on Windows?

I think the problem is in Mover::moveFile() at Mover.php:179, which for me works as:

// iio/libmergepdf
$namespacePath = $package->config->name;

// lib/classes//iio/libmergepdf
$replaceWith = $this->config->classmap_directory . DIRECTORY_SEPARATOR . $namespacePath;

// /lib/classes//iio/libmergepdf/vendor/iio/libmergepdf/tcddi/tcpdi.php
$targetFile = str_replace($this->workingDir, $replaceWith, $file->getPathname());

// /vendor/iio/libmergepdf/
$packageVendorPath = DIRECTORY_SEPARATOR . 'vendor' . DIRECTORY_SEPARATOR . $package->config->name
                     . DIRECTORY_SEPARATOR;

// /vendor/iio/libmergepdf/
$packageVendorPath = str_replace('/', DIRECTORY_SEPARATOR, $packageVendorPath);

// /lib/classes//iio/libmergepdf/tcpdi/tcpdi.pdf
$targetFile = str_replace($packageVendorPath, DIRECTORY_SEPARATOR, $targetFile);

So I think change $namespacePath = $package->config->name; to $namespacePath = str_replace('/', DIRECTORY_SEPARATOR, $package->config->name);

[Spreeuw]

I'm not Windows indeed, but that suggestion doesn't fix this. I'm getting:

// iio/libmergepdf
$namespacePath = $package->config->name;

// /lib/classes/\iio/libmergepdf
$replaceWith = $this->config->classmap_directory . DIRECTORY_SEPARATOR . $namespacePath;

// /lib/classes/\iio/libmergepdf\vendor\iio/libmergepdf\tcpdi\tcpdi.php
$targetFile = str_replace($this->workingDir, $replaceWith, $file->getPathname());

// \vendor\iio/libmergepdf\
$packageVendorPath = DIRECTORY_SEPARATOR . 'vendor' . DIRECTORY_SEPARATOR . $package->config->name
                     . DIRECTORY_SEPARATOR;

// \vendor\iio\libmergepdf\
$packageVendorPath = str_replace('/', DIRECTORY_SEPARATOR, $packageVendorPath);

// /lib/classes/\iio/libmergepdf\vendor\iio/libmergepdf\tcpdi\tcpdi.php
$targetFile = str_replace($packageVendorPath, DIRECTORY_SEPARATOR, $targetFile);

which is an interesting mix of separators 🌪️

With your change it's:

iio\libmergepdf
/lib/classes/\iio\libmergepdf
/lib/classes/\iio\libmergepdf\vendor\iio/libmergepdf\tcpdi\tcpdi.php
\vendor\iio/libmergepdf\
\vendor\iio\libmergepdf\
/lib/classes/\iio\libmergepdf\vendor\iio/libmergepdf\tcpdi\tcpdi.php

[BrianHenryIE]

Try replace that whole } else { and the following few lines with this:

} else {

    $clean = function($path) { return trim( preg_replace('/[\/\\\\]+/', DIRECTORY_SEPARATOR, $path), DIRECTORY_SEPARATOR ); };

    $namespacePath = $clean( $package->config->name );
    $classmapDirectory = $clean( $this->config->classmap_directory );

    $sourceFile = $file->getPathname();

    $packageVendorPath = 'vendor' . DIRECTORY_SEPARATOR . $namespacePath;

    $mozartClassesPath = $classmapDirectory . DIRECTORY_SEPARATOR . $namespacePath;

    $targetFile = str_replace($packageVendorPath, $mozartClassesPath, $sourceFile);
}

$this->filesystem->copy(
    str_replace($this->workingDir, '', $file->getPathname()),
    str_replace($this->workingDir, '', $targetFile)
);

[Spreeuw]

Unfortunately that doesn't solve it. I get:

File already exists at path: vendor/tecnickcom/tcpdf/tcpdf.php

That is with PR #91 applied. (without #91 there is no difference, but since that PR attempts to address the file already exists issue I though to test both versions)

the variables in that function evolve like this:

$namespacePath = "iio\libmergepdf"
$classmapDirectory  = "lib\classes"
$sourceFile  = "C:\tests\libmerge-mozart\vendor\iio/libmergepdf\tcpdi\fpdf_tpl.php"
$packageVendorPath  = "vendor\iio\libmergepdf"
$mozartClassesPath  = "lib\classes\iio\libmergepdf"
$targetFile  = "C:\tests\libmerge-mozart\vendor\iio/libmergepdf\tcpdi\fpdf_tpl.php"

Is there a reason why you didn't apply the clean function to the $sourceFile ? Because that appears to be the problem here. If I do apply clean there too, the issue is in fact resolved.

By the way, as you can see the workingDir is not properly stripped, probably for the same DIRECTORY_SEPARATOR inconsistencies

[BrianHenryIE]

Because that appears to be the problem here. If I do apply clean there too, the issue is in fact resolved.

I did not apply $clean to $file->getPathname() because I assumed it was good since it came from Symphony Finder!

After posting here, I tidied it up a bit more and posted it in 43 because I think it's the same issue. Now that I look at that code again, every input is run through $clean, so I'm optimistic it will work.

I think workingDir was not being stripped because (as you've shown) $file->getPathname() is using / – although again I'm just assuming it's good based on the source, maybe it does need $clean.

[Spreeuw]

Yes, that cleaned up code works nicely indeed, I get the following variable progression:

$sourceFilePath    = "vendor\iio\libmergepdf\tcpdi\tcpdi.php";
$namespacePath     = "iio\libmergepdf";
$classmapDirectory = "lib\classes";
$packageVendorPath = "vendor\iio\libmergepdf";
$mozartClassesPath = "lib\classes\iio\libmergepdf";
$targetFilePath    = "lib\classes\iio\libmergepdf\tcpdi\tcpdi.php";

Would be great if you can include this in master!

[BrianHenryIE]

When I have time to write some tests around it I'll make a PR.

[BrianHenryIE]

Try this:

{
  "require": {
    "iio/libmergepdf": "^4.0"
  },
  "require-dev": {
    "coenjacobs/mozart": "dev-master",
    "cweagans/composer-patches": "*"
  },
  "extra": {
    "patches": {
      "coenjacobs/mozart": {
        "Move each file only once (classmap) Fixes #89": "https://github.com/coenjacobs/mozart/pull/91.patch",
        "Do NOT parse words in comments as class names": "https://github.com/coenjacobs/mozart/pull/102.patch",
        "Directory separator and its duplicates": "https://github.com/coenjacobs/mozart/pull/103.patch"
      }
    },
    "mozart": {
      "dep_namespace": "Mozart",
      "dep_directory": "/dep_directory/",
      "classmap_directory": "/classmap_directory/",
      "classmap_prefix": "Mozart_",
      "delete_vendor_directories": false
    }
  },
  "scripts": {
    "post-install-cmd": [
      "\"vendor/bin/mozart\" compose"
    ],
    "post-update-cmd": [
      "\"vendor/bin/mozart\" compose"
    ]
  }
}

And if you could run the actual tests on Windows, that'd be great: https://github.com/coenjacobs/mozart/pull/103#issuecomment-744762475

[Spreeuw]

Unfortunately his throws an error, apparently caused by prefixing the path with a backslash:

In Local.php line 112:

Impossible to create the root directory "\C:\Users\Ewout\Documents\GitHub\libmerge-mozart". mkdir(): No such file or directory

[BrianHenryIE]

Damn. Thank you. The opening slash is used on Unix (MacOS for me) to indicate an absolute path.

I'll have to spin up a Windows VM and do some reading. Unfortunately it's not imminent – I've only 20 GB free on my laptop!

At a glance, a regex ~ /^\w+:\/ on $workingDir should work to filter/differentiate, but there's probably a more appropriate solution. Maybe "cleaning" $workingDir isn't even needed since (I think) it's coming from getcwd() (but that's the same assumption I made about other libraries' paths being correct).

[Spreeuw]

if there's anything I can test for you, let me know! I tried your suggestion of replacing https://github.com/coenjacobs/mozart/blob/832a5587e4ad3ccc200a0395d200d9e67c3a92c7/src/Mover.php#L45 with:

        $this->workingDir = $workingDir;

Works flawlessly, but I don't know if that's an issue on other OSes.

[BrianHenryIE]

Great. That works on MacOS too. I've committed the changed to the PR.

I'll also take a look at getting a Windows GitHub Actions workflow running the tests. As I realised in the PR's thread, I'll need to update the file paths in the tests before I expect they'll run.