coenjacobs / mozart

Developers tool for WordPress plugins: Wraps all your projects dependencies in your own namespace, in order to prevent conflicts with other plugins loading the same dependencies in different versions.
https://coenjacobs.me/projects/mozart/
MIT License
426 stars 53 forks source link

Namespaces are still being duplicated #58

Closed EvanShaw closed 3 years ago

EvanShaw commented 4 years ago

11 doesn't seem to be fully resolved.

If I'm not mistaken, the initial issue was caused when 2 or more packages with the same dependency were included in the packages array. This would cause the namespace of said dependency to be escaped more than once. This issue appears to be resolved.

However, I still see this happening with certain libraries unrelated to the cause written above. For example, guzzlehttp/guzzle has duplicated namespaces in certain files. In some files the namespace isn't inserted at all.

Here's a sample composer.json:

{
  "name": "guzzle-demo",
  "require": {
    "guzzlehttp/guzzle": "^7.0"
  },
  "require-dev": {
    "coenjacobs/mozart": "^0.5.1"
  },
  "extra": {
    "mozart": {
      "dep_namespace": "DUPTEST\\",
      "dep_directory": "/dist/DUPTEST/",
      "classmap_directory": "/dist/classes/",
      "classmap_prefix": "DUPTEST_",
      "packages": [
        "guzzlehttp/guzzle"
      ]
    }
  },
  "autoload": {
    "psr-4": {
      "DUPTEST\\": "dist/DUPTEST"
    }
  }
}

And here is the first few lines of dist/DUPTEST/GuzzleHttp/Client.php:

<?php

namespace DUPTEST\GuzzleHttp;

use DUPTEST\GuzzleHttp\Cookie\CookieJar;
use DUPTEST\GuzzleHttp\Exception\GuzzleException;
use DUPTEST\GuzzleHttp\Exception\InvalidArgumentException;
use DUPTEST\DUPTEST\GuzzleHttp\Promise\PromiseInterface;
use Psr\Http\Message\RequestInterface;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\UriInterface;

class Client implements ClientInterface, \DUPTEST\Psr\Http\Client\ClientInterface
{
.....
EvanShaw commented 4 years ago

Just to test, I updated mozart to 0.6.0-beta-3 and that resolved the duplicate namespace issue as seen above with use DUPTEST\DUPTEST\GuzzleHttp\Promise\PromiseInterface;

The issue of no namespace being inserted at all persists, however. (use Psr\Http\Message\RequestInterface;, etc.)

BrianHenryIE commented 4 years ago

I took a look at this this week and I'm not confident I'm on the right track but feel it's best to throw it out there: Is maybe Replacer->replaceParentPackage($package, $parent) in src/Replacer.php:155 not recursing back properly (deep enough)?

markjaquith commented 4 years ago

The duplication issue is fine now, but Psr\Http\Message\ stuff is still not getting prefixed.

Current composer.json to test this:

{
  "name": "mozart/issue-58",
  "require": {
    "guzzlehttp/guzzle": "^7.0"
  },
  "require-dev": {
    "coenjacobs/mozart": "dev-master"
  },
  "extra": {
    "mozart": {
      "dep_namespace": "DUPTEST\\",
      "dep_directory": "/dist/DUPTEST/",
      "classmap_directory": "/dist/classes/",
      "classmap_prefix": "DUPTEST_",
      "packages": [
        "guzzlehttp/guzzle"
      ]
    }
  },
  "autoload": {
    "psr-4": {
      "DUPTEST\\": "dist/DUPTEST"
    }
  },
  "scripts": {
    "post-install-cmd": [
      "vendor/bin/mozart compose",
      "composer dump-autoload"
    ],
    "post-update-cmd": [
      "vendor/bin/mozart compose",
      "composer dump-autoload"
    ]
  }
}

Of note: guzzlehttp/guzzle requires guzzlehttp/psr7 which requires psr/http-message which provides the Psr\Http\Message\ PSR-4 autoloader. So there might be a "grandchild/grandparent" issue here.

BrianHenryIE commented 3 years ago

82 looks good here!

{
  "name": "guzzle-demo",
  "require": {
    "guzzlehttp/guzzle": "^7.0"
  },
  "require-dev": {
    "coenjacobs/mozart": "dev-master",
    "cweagans/composer-patches": "~1.0"
  },
  "extra": {
    "patches": {
      "coenjacobs/mozart": {
        "Replace parent packages over full tree of dependencies": "https://github.com/coenjacobs/mozart/pull/82.patch"
      }
    },
    "mozart": {
      "dep_namespace": "DUPTEST\\",
      "dep_directory": "/dist/DUPTEST/",
      "classmap_directory": "/dist/classes/",
      "classmap_prefix": "DUPTEST_",
      "packages": [
        "guzzlehttp/guzzle"
      ],
      "delete_vendor_directories": false
    }
  },
  "autoload": {
    "psr-4": {
      "DUPTEST\\": "dist/DUPTEST"
    }
  }
}

Results in the desired GuzzleHttp/Client.php:

use DUPTEST\GuzzleHttp\Cookie\CookieJar;
use DUPTEST\GuzzleHttp\Exception\GuzzleException;
use DUPTEST\GuzzleHttp\Exception\InvalidArgumentException;
use DUPTEST\GuzzleHttp\Promise as P;
use DUPTEST\GuzzleHttp\Promise\PromiseInterface;
use DUPTEST\Psr\Http\Message\RequestInterface;
use DUPTEST\Psr\Http\Message\ResponseInterface;
use DUPTEST\Psr\Http\Message\UriInterface;