Packages with files autoloaders do not autoload those files

[markjaquith]

If you require a package that has a files autoloader, like php-di/php-di, that file autoloader reference disappears from Composer's autoload_files.php after mozart compose runs.

Here's a plugin that showcases the issue:

https://github.com/markjaquith/Mozart-Plugin-File-Autoloader-Bug-Example

Reproduction:

1. Check out that repo to a plugin directory.
2. Run composer install in that directory.
3. Attempt to activate the plugin.

[coenjacobs]

I've figured out what has caused this bug to appear - thanks for POC repository, Mark, that is very convenient. This bug is caused by #38 deleting the vendor directory, but the files that are in the files autoloader aren't moved (because they aren't touched by any of the logic and therefore considered fit for deleting - which is obviously a bug). You can disable removing the vendor directory (delete_vendor_directories option set to false) to see that resolving this issue.

What I think is the only way to fix this, is to move the files targeted by the files autoloader to a specific directory (regardless of the value of delete_vendor_directories) and keep them in their new home. The only thing I can't figure out in the limited time that I have, is if we then need to rewrite the autoloader bit as well, to find the files in their new home.

[markjaquith]

Completely spitballing here, but if Mozart wrote one autoload_files.php itself, that just contained includes for all the files entries in the composer.json files of all the things it was tracking, people could just (as they have to do with classmaps) add a single files autoloader to point to Mozart's one. And then Composer would take care of including it.

[coenjacobs]

Yep, I think that is the best approach to take here. I don't know what the best location is for that file to store, since the one that Composer creates (if your own package contains a files autoloader as well) can not be overwritten. The approach could be very similar to what @BrianHenryIE is doing for classmaps in #60.

[markjaquith]

Interesting. I started writing this comment with mixed feelings about the approach in #60. I don't love that you have to manually include a file in your plugin code. It feels cleaner to just keep the single vendor/autoload.php include, and use autoloaders in composer.json to indicate your classmaps autoloader and files autoloader. Composer does a lot of heavy lifting for us. But also, it complicates things!

As I brought up in one of my first tickets on the project, you run into chicken/egg situations with classmaps autoloaders, and the same is true of files autoloaders. If you have them specified in your composer.json, they must exist, or you can't even run composer install. But you need them specified for certain packages to work. It would be awkward to tell people "after adding Mozart config to composer.json you have to create your classmaps directory, perhaps persisting it with a .gitkeep and also you have to create autoload_files.php as a blank file just so that you can run composer install and Mozart can populate it".

What a confusing, error-prone developer experience.

Also, I do get that plugin authors might not want a vendor directory at all in their shipped plugin. If Mozart is managing things, the only thing you'd want to include is the vendor/autoload.php file and the vendor/composer/ directory. For people who are not doing composer install as a build step when distributing their plugin but are instead checking in the vendor directory, they have to be careful not to check in a bunch of dev dependencies. If they're not careful, they could be accidentally shipping files autoloaders that run PHP 7.2+ code in a plugin that is supposed to run on PHP 5.6. It's probably very messy and again, error-prone.

So... a proposal

Maybe we could streamline the developer experience (for composer.json, for Git, and for "building" their plugin (or not) for distribution) by doing the following:

1. Have a single Mozart directory config value, defaulting to something nice like lib/Mozart.
2. lib/Mozart should be added to .gitignore for developers who want to have a clean Git repo and who will perform a "build" step before distributing their plugin. Or... they can not add it and have noisy Git changesets with all of lib/Mozart checked in, but retain the ability to directly run the plugin from a Git checkout. Their choice!
3. Within lib/Mozart, Mozart essentially does what /vendor/autoload.php and /vendor/composer/ do. It creates a single entry point (lib/Mozart/autoload.php), and which then defers to other files for configuring other autoloaders. The plugin author, in their main plugin file, then does require 'lib/Mozart/autoload.php'; instead of require 'vendor/autoload.php'; Of course, if their plugin also includes dev dependencies, or there are dependencies they've instructed Mozart NOT to "take over", they could also (perhaps conditionally) load vendor/autoload.php too.

What would be absolutely grand is if we could leverage the guts of Composer itself to write all these files. In that way, lib/Mozart would be like a "thin" copy of vendor except with just the class files, classmaps, and autoloaded files that are needed (with namespaces prefixed, of course). I've been thinking of some crazy ideas like this:

1. Mozart makes replacements.
2. Mozart copies all relevant classes, classmaps, and autoloaded files to lib/Mozart/tmp, cleaning them out of /vendor/.
3. Mozart writes out a composer.json file to lib/Mozart/composer.json that includes a require section with all the packages and subpackages that Mozart calculated in the tree, but with custom source definitions for those, pointed at the lib/Mozart/tmp directory, and also with all of the autoload configs from those packages merged together.
4. Mozart runs composer install in lib/Mozart, and Composer itself generates lib/Mozart/vendor/, copying in all the code from tmp, and generating lib/Mozart/vendor/autoload.php and lib/Mozart/vendor/composer/*.
5. lib/Mozart/autoload.php just includes lib/Mozart/vendor/autoload.php.

It sounds nuts, and I'm probably way in over my head here, but maybe there's the kernel of a good idea somewhere in there.

[coenjacobs]

I love your idea, @markjaquith, but I have a couple remarks that might just move this proposal in the exact opposite direction. Either way, it was great food for thought and lead me to rethink the whole idea behind how we should handle files that are not being treated by either or both replacements or moving.

What a confusing, error-prone developer experience.

I absolutely agree. This is also why I think that the approach taken by @BrianHenryIE in #60 is not the right approach going forward. Allow me to explain why, but bear in mind that my idea isn't set in stone and not completely covering all issues described in this and other tickets.

The main point where this is deviating from the ideas listed above, is that I'd prefer to embrace the autoloader functionality of default Composer more. Perhaps this isn't possible, perhaps we have to end up with a structure as you explained above, but I'd like to share my two cents on why I'd prefer to stick closer to what Composer already does.

Use Composer for what it already does

In this response, I'm going to fully lean on what Composer already does, how that can be used with minimal effort and interfering from Mozart. This is intentional, leaning on what Composer does well and not deviating from that more than we absolutely have to. Mozart has always been designed this way, which is also why it is recommended to put the PSR-0/-4 files in the main namespace of your project, with a suffix (\Dependencies\ for example) and then have that namespace structure be represented by your directory structure as well (/Dependencies/). This way, the autoloader of all files stays intact.

As previously stated, without Mozart interfering, Composer does create autoloader files for classmaps and files as defined in the projects composer.json file. Mozart currently breaks this functionality by moving the files from the directory where they are originally placed (i.e. /vendor/<name>/<package>/). If we do not move these files, they will stay in their respective directory structure and the Composer autoloader will include them. We have no valid reason to move these files, other than when we want to clean up the entire vendor directory, using the delete_vendor_directories option. I will get back that.

So, assuming we do not move the files we don't have to move, everything is great. There is no extra configuration needed for the developer implementing Mozart. The files in both classmap and files autoloaders are working as they should, without Mozart doing a single thing.

So what about the /vendor/ directory?

This is where it gets tricky, in my opinion. Should we leave the files there, even when all the others are deleted by using the delete_vendor_directories option, you are stuck with the seemingly random files in there. This is definitely something that is going to be challenging for the people who do not want their /vendor/ directory checked into version control.

It is 'hard enough' to have files you need in both the /vendor/ directory (the default Composer autoloader) and in your designated target directory, that you perhaps do want to include in your version control. If we start making some directories 'stay', even when the replaced and moved class files are moved out of it, just leaving the functions files from your php-di/php-di example, in there, this is quickly turning into a mess.

As much as I want to stick as close to the original autoloader setup that Composer provides, I can't wrap my head around how I can solve this in the most universal way that is usable for all possible implementations.

Where do we go from here?

There is probably much more to say about this particular issue, or perhaps we uncover more challenges along the way. At this point, I'm seeing this issue at a crossroad between two viable options, neither without downsides. I don't think that I'm able to determine which way to go, so I'm open to any and all feedback on these both proposals. 😄

[BrianHenryIE]

I've rewritten #60 to use Composer's createMap() function, i.e. we just tell it the dep_directory path and the classmap_directory path and it does the heavy lifting. This hugely simplified the code – nothing core of Mozart was changed (whereas I had awkwardly been keeping a list of files), it just adds another command vendor/bin/mozart dump-autoload. Also, by outputting the autoload-classmap.php into the root of the respective directory, there's no need for config. I think this is all a step in the right direction.

which is also why it is recommended to put the PSR-0/-4 files in the main namespace of your project, with a suffix (\Dependencies\ for example) and then have that namespace structure be represented by your directory structure as well (/Dependencies/)

I never noticed this before. I've been prefixing in a way that doesn't match the directory structure then registering a weird autoloader to handle the difference.

It does lead me to a point I've wanted to raise for a long time but hadn't hashed out in my mind enough – but what Mark is basically saying above, can we just bang everything into a Mozart directory? Then config just needs to know the root namespace to use, which it will often be able to read from the autoload key of the project's composer.json. My_Project\Mozart\ for PSR-4 and My_Project_Mozart_ for classmap. I like the idea of people seeing "Mozart" in plugins as a means of exposure.

Is there a good reason there needs to be separate dep_directory and classmap_directory? That would simplify things, if possible. #83 is an example of where that might not be directly possible. The filenames could be prefixed with the package names when there are clashes like that.

Mark's "crazy idea" is neat but I don't think it's necessary -- just use Composer to generate the classmap and use plain PHP autoloading to autoload.

I still have to do some reading and thinking about files autoloaders themselves – it's another problem I want to fix.

I do think the right approach is to copy the files and not edit them in-place. I got burned when the directories started getting deleted (so I wrote #38 !)

[BrianHenryIE]

Using createMap() on Mozart's output directories, then un-prefixing, then looking up the files in Composer's classmap autoloader and deleting what's found could be used as a more targeted way to delete_vendor_directories, leaving behind the files that aren't being managed by Mozart.

[marcelklehr]

"delete_vendor_directories": false, does not fix this for me

[sovetski]

Same here