mozart icon indicating copy to clipboard operation
mozart copied to clipboard

Package without namespace won't be rewritten

Open ChristophWurst opened this issue 5 years ago • 6 comments
trafficstars

Hi,

I just found out that a package that does not use namespaces will remain global after Mozart processes it.

Steps to reproduce

  • Require https://packagist.org/packages/ezyang/htmlpurifier
  • Add ezyang/htmlpurifier to Mozart packages
  • Find HTMLPurifier in the configured path, but without a namespace.

My PR is at https://github.com/nextcloud/mail/pull/3112 if that makes it easier to understand/reproduce the problem.

Cheers from the home of Mozart (kind of :speak_no_evil: )

ChristophWurst avatar May 14 '20 20:05 ChristophWurst

Try this:

{
  "require": {
    "ezyang/htmlpurifier": "*"
  },
  "require-dev": {
    "cweagans/composer-patches": "~1.0",
    "coenjacobs/mozart": "*"
  },
  "extra": {
    "patches": {
      "coenjacobs/mozart": {
        "Classmap shizz": "https://github.com/coenjacobs/mozart/pull/39.patch",
        "Override Autoload": "https://github.com/coenjacobs/mozart/pull/41.patch",
        "Run wide update for classmap": "https://github.com/coenjacobs/mozart/pull/42.patch"
      }
    },
    "mozart": {
      "dep_namespace": "Issue44\\",
      "dep_directory": "/src/vendor/",
      "classmap_directory": "/src/dependencies/",
      "classmap_prefix": "Issue44_",
      "packages": [
        "ezyang/htmlpurifier"
      ],
      "delete_vendor_directories": false,
      "override_autoload": {
        "ezyang/htmlpurifier": {
          "classmap": [
            "library/"
          ]
        }
      }
    }
  },
  "scripts": {
      "post-install-cmd": [
        "\"vendor/bin/mozart\" compose"
      ],
      "post-update-cmd": [
        "\"vendor/bin/mozart\" compose"
      ]
  }
}

Since the package isn't namespaced, it could/can (should?) be processed by Mozart's classmap code, but ezyang/htmlpurifier's composer.json's autoload key needs to be overridden for that. I had to fix the string replacing to not replace filenames, which could still be stricter (I set it to ignore any line with require or include in it). Other fixes are ones I'd written before so better to read the PRs than me try articulate exactly what's up.

BrianHenryIE avatar May 22 '20 03:05 BrianHenryIE

For the record, the patches that are being applied in the above mentioned composer.json configuration have now all been merged into 0.6.0 Beta 3 so that can be used to test as well. 👍

coenjacobs avatar Jun 02 '20 07:06 coenjacobs

Thank you both.

I've updated my PR https://github.com/nextcloud/mail/pull/3112 with the suggestions. It now uses the beta 3 and overrides the autoload configuration. Now, it writes prefixed classes into the configured classmap directory. Is that the expected outcome? I do not fully grasp the difference between the dependency directory and the classmap directory. To me it seems incorrect to find the classes in the latter. Is my assumption correct?

ChristophWurst avatar Jun 02 '20 09:06 ChristophWurst

My understanding is the dep_directory is for classes that can be autoloaded based on their namespace/directory hierarchy, and that the classmap_directory is for classes that need to be explicitly listed in an array to be autoloaded.

Admittedly, this understanding is just from using Mozart and not because I've much experience with autoloaders.

I'd like to know what composer dump-autoload is doing in the post-install-cmd.

I have some code for generating the classmap array which I'll make a PR from eventually, assuming it's useful and I'm not doubling up on dump-autoload or otherwise.

I'm also curious if the dep_directory and classmap_directory could be the same directory? I just call my dep_directory vendor.

BrianHenryIE avatar Jun 09 '20 22:06 BrianHenryIE

I'd like to know what composer dump-autoload is doing in the post-install-cmd.

Is this a question towards me?

ChristophWurst avatar Jun 17 '20 07:06 ChristophWurst

It wasn't a question particularly at you, but one I'm unsure about. I think composer dump autoload generates a classmap file but still requires including/requiring the composer autoloader. I wrote a PR that generates a classmap file from all the files managed by Mozart: https://github.com/coenjacobs/mozart/pull/60

BrianHenryIE avatar Aug 07 '20 04:08 BrianHenryIE