NamespaceReplacer inserts duplicate namespace

[iandillon]

I've found this behaviour since implementing the commit by costasovo for supporting dependency trees. The regex within the replace function in NamespaceReplacer.php can result in the configured mozart dep_namespace value to be inserted into namespace definitions multiple times if a file is passed to the method more than once

Changing the regex so that only namespaces which don't already contain the configured value are matched, resolves the issue.

e.g '/([^\?])(?<!'.addslashes($this->dep_namespace).')(' . addslashes($searchNamespace) . '[\|;])/U'

[coenjacobs]

@iandillon Could you have a look at #12, if this still applies on that branch? I've taken a different approach to handle the dependency tree and I think this bug is fixed with that as well. Thanks in advance!

[coenjacobs]

I believe this is no longer an issue, since the dependency tree rewrite is done differently in release 0.3.0. Please test that release to verify and feel free to reopen this issue if the issue still exists.

[ju5t]

I still have this issue. @iandillon's suggestion solves it though.

Edit: I spoke too soon. This is a breaking change as it doesn't replace everything anymore.

[coenjacobs]

@ju5t can you give some more context, perhaps share the Mozart configuration you're using?

[ju5t]

@coenjacobs Absolutely. The source can be found here:

https://github.com/sensson/whmcs-moneybird/tree/implement-mozart

$ rm -f composer.lock
$ composer install
$ grep namespace src/Dependencies/GuzzleHttp/Psr7/Response.php
namespace Sensson\Dependencies\Sensson\Dependencies\Sensson\Dependencies\GuzzleHttp\Psr7;

I know this isn't WordPress but I think that shouldn't make a difference at this stage.

It could be that I've done something wrong though.

[EvanShaw]

Sorry about the bump, but I am also experiencing this issue

[BenceSzalai]

Same issue here using 0.4.0.

What I've figured is that in order to avoid the issue, I must make sure that each package is only processed once by Mozart. So for example if I use psr/log, monolog/monolog and bradmkjr/monolog-wordpress, I can only put bradmkjr/monolog-wordpress into the packages array of Mozart section in composer.json. This is because bradmkjr/monolog-wordpress requires monolog/monolog which in turn requires psr/log. If I add all tree of them to Mozart, files in psr/log will acquire the namespace prefix 3 times, in monolog/monolog twice, and only in bradmkjr/monolog-wordpress it will be correct.

This is indeed just a workaround to only tell Mozart about the last package in the dependancy chain, as I am indeed lucky to have this issue with chained packages. But but if someone would have a fork of dependencies, like package a and b both requiring c it would be impossible to resolve the double prefixing happening in c, ergo this is an issue most likely to be fixed in Mozart and not the configuration.

[coenjacobs]

This probably might need another fix in order to solve all cases, but let me get the following straight: Mozart should only be used to rewrite the dependencies of the project itself. If those dependencies have dependencies of their own, those should not be added to the Mozart configuration. So if your project requires a and a requires b, only a should be added to the Mozart configuration. During the package discovery, Mozart will also rewrite b, because that is a dependency of a, without b being added to the Mozart configuration itself.

Can you provide an example of a composer.json that gives you these issues again, now that we're on version 0.4.0 - since the logic of discovering dependency packages has changed quite a bit since this issue has been made?

[EvanShaw]

@coenjacobs I'll post mine for you

"extra": {
        "mozart": {
            "dep_namespace": "WCEXAAP\\",
            "dep_directory": "/dist/WCEXAAP/",
            "classmap_directory": "/dist/classes/",
            "classmap_prefix": "WCEXAAP_",
            "packages": [
                "aivec/welcart-settlement-modules",
                "aivec/aauth"
            ]
        }
    },
    "require-dev": {
        "amzn/amazon-pay-sdk-php": "^3.3",
        "phpunit/phpunit": "^8"
    },
    "require": {
        "aivec/welcart-settlement-modules": "~3.0.0",
        "aivec/welcart-generic": "~2.0.1",
        "aivec/aauth": "~3.0.0",
        "coenjacobs/mozart": "^0.4.0"
    },
    "autoload": {
        "psr-4": {
            "WCEXAAP\\": "dist/WCEXAAP",
            "Aivec\\Welcart\\SettlementModules\\AmazonPay\\": "src"
        }
    },

In the above config, aivec/welcart-settlement-modules has aivec/welcart-generic as a dependency. Because of this, as described by @Grapestain, adding aivec/welcart-generic to the packages array in the above config will duplicate the WCEXAAP namespace in all files generated by mozart under dist/WCEXAAP/Aivec/Welcart/Generic.

[ChristophWurst]

Hi :wave:

I stumbled over your project when I was looking for ways to scope composer packages for Nextcloud apps (plugins). As far as I understood we're facing the same problem as Wordpress developers do.

In any case I found the same issue reported here to block us from the adoption of this package. Our goal is to bundle sentry/sdk in a way that it's Guzzle dependency doesn't conflict with other Nextcloud plugins.

With sentry/sdk in the packages config, Mozart didn't write any new files. With sentry/sentry it does, but the namespace of the Symfony OptionResolver is rewritten to OCA\Sentry\Vendor\OCA\Sentry\Vendor\Symfony\Component\OptionsResolver when it should be OCA\Sentry\Vendor\Symfony\Component\OptionsResolver. Thus the Sentry package doesn't work.

The file is located at lib/Vendor/Symfony/Component/OptionsResolver/OptionsResolver.php

You can find my PR at https://github.com/ChristophWurst/nextcloud_sentry/pull/172. It should be possible to reproduce the issue without any knowledge about Nextcloud. Just treat is as any other composer package or Wordpress plugin :)

Is there anything we can do to help you diagnose or solve the problem?

Cheers and thanks for this package :rocket:

[EvanShaw]

@ChristophWurst Just as a heads up, I have tried to package Guzzle by itself before with mozart and mozart cannot do it. It almost works, but after calling mozart compose you'll have to manually fix several things.

Not really related to the issue in this thread but just thought I would let you know.

[ChristophWurst]

Thanks for the heads-up, @EvanShaw. That is quite a bummer as Guzzle is our #​1 conflicting dependency in the Nextcloud ecosystem right now.

[BrianHenryIE]

Potentially fixed with: https://github.com/coenjacobs/mozart/pull/40. I didn't see the issue until after the PR.

To quickly test it, add:

 "require-dev": {
  "cweagans/composer-patches": "~1.0"
 },
 "extra": {
  "patches": {
   "coenjacobs/mozart": {
    "Fixed escaping of backslash": "https://github.com/coenjacobs/mozart/pull/40.patch"
   }
  }
 }

[ChristophWurst]

@BrianHenryIE good stuff! I tried with the patch and the original bug is gone. Guzzle is not rewritten with the prefix twice. Nice!

I still can't get Guzzle to work here because it's used (detected) via httplug and Mozart does not rewrite all imports, thus it does not detect the implementation.

So, even with this follow-up problem, I think you patch is a great improvement :) Thanks a lot :heart:

[coenjacobs]

The original problem for which this issue was raised is fixed thanks to @BrianHenryIE. Therefore, I'm closing this issue. This can now be tested with the 0.6.0 beta 1 release.

@ChristophWurst could you do me a favour and test that to see if the followup issues you ran into, are also fixed with that beta release? There's been significant improvements in the replace logic in parent/using classes as well, in that same release. If not, it would be greatly appreciated if you could open a separate issue for the remaining issue, so we can tackle those as well.

[ju5t]

I will test it next Monday, thanks! 👍

[ChristophWurst]

@ChristophWurst could you do me a favour and test that to see if the followup issues you ran into, are also fixed with that beta release?

The duplicate namespace issue is fixed. The failing auto discovery with httplug remains. I'll try to create a minimal example project to demonstrate the issue.