Support installer-paths

[xendk]

Drupal 8 (and I suspect other frameworks) uses https://github.com/composer/installers to relocate where the installed packages is placed.

The bad news is that composer basically allows composer plugins to mess with where packages are installed. The good news is that a large amount of frameworks uses composer/installers rather than their own custom one.

Sadly, composer doesn't note the installed path of a package in the lock file. That would have been too easy.

As I see it, what is needed is:

1. Triggering on composer/installers being part of the dependencies.
2. For each package discovered, check up on installer-paths to adjust its root path.

Each path can contain {$name}, {$vendor} and {$type}. Vendor can be taken from the package name, name is either taken from "installer-name" from the "extra" section of te packages composer.json, or the package name. Type is the "type" from the composer.json or "library"...

Each path points to an array of what to install there, which can be:

1. A package name, ie. "drupal/core"
2. A vendor: "vendor:my_organization"
3. A package type: "type:drupal-module"

A bit fiddly, but not too bad...

This is step one in getting Drupal 8 working. Of course a solution that just hooks into ede-php-autoload and adjusts things for Drupal is also a possibility, but supporting composer/installers gives support for many more frameworks.

[xendk]

I've started constructing a "project" in the test folder. I love TDD. It'll probably be slow progress over the Christmas period, but better slow progress than no progress.

[stevenremot]

I totally agree with your approach, supporting composer/installers is definitely a good thing! What about creating the PR early, so that I can give you my feedback on the code in "real time"?

[xendk]

Sure. I'll create the PR when I've stubbed out the project.

…

On Mon, Dec 19, 2016, 22:29 Steven Rémot ***@***.***> wrote: I totally agree with your approach, supporting composer/installers is definitely a good thing! What about creating the PR early, so that I can give you my feedback on the code in "real time"? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#15 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAOALvUXjaFLDA28IEwxwA3gUAgUpiJdks5rJvcZgaJpZM4LQOnh> .

[xendk]

Bad news: composer/installers basically screws us over. I'd overlooked the fact that the installer-paths key in the composer.json file I was looking at is just a local override. Each framework installer in composer/installers supplies their own default paths for all their types.

So if one is using the drupal installer, and are happy with the default path that it supplies, there's no hint in the composer.json nor composer.lock where it was installed. To add insult to injury, installers might munge the name of the package in unpredictable ways.

Pondering what to do about that, as being unable to find the file of classes kills off all the fun of semantic-php...

1. One could implement the logic I outlined in the issue, and in addition to that implement the logic of all the composer installers. The downside is that it's much work, and ede-php-autoload would have to track composer/installers and implement changes in a timely manner.
2. One could fall back to the tags approach and scan all files and make an index of classes to files. But it's slow and annoying.
3. Petition composer or composer/installers to note the packages install dirs in the lock file. Probably the simplest from our point if view, but who knows if they'll like the idea.
4. Use a php script to extract the class/namespace/psr4 maps from vendor/autoload_*.php. Requires a working php command locally, and is sensitive towards changes in how composer constructs its autoloaders.

What do you think?

[stevenremot]

Yo, for now I'm trying to understand what composer/installers is doing exactly!

If there is too much work to do to implement installers, I think it will be better to just implement some hooks in ede-php-autoload and implement all the composer/installers logic in another package to keep this one focused.

But I have another suggestion: what about generating a classmap with composer and declare this classmap in your project? ede-php-autoload already has support for classmap so this can be a good first step.

Regarding your suggestion on tags, you will be better served with another semanticdb backend than ede-php-autoload if you go this way.

[stevenremot]

I had a better look at it. What about forgetting composer/installers and implementing drupal support directly? I think supporting composer/installers doesn't really worth it, as it would require doing a lot of of specific code for each installer in the end.

If you tell me what you need exactly, we can see which hooks we can put in the ede-php-autoload code to better integrate new project types, and especially drupal.

[xendk]

Well, classmap could work, but, as I understand it, it would require running composer after adding a new class. I'd rather prefer something less manual.

Thing is, doing Drupal (and other framework) support completly outside of ede-php-autoload is either going to be half-hearted or duplicate much of ede-php-autoload. A naive implementation would have to guess whether drupal/core is installed in core or web/core and fail miserably when someone decides on docroot/core. Trying harder would mean reading the composer.json and checking the installer-paths, which would be more elegantly done with some support from ede-php-autoload.

The thought about using the old filescan method came from pondering how IDEs like PHPStorm might do it. I suspect that might actually be what they're doing, but with the advantage of smart use of file notifications and background scanning. Not easily duplicated.

It still seems to me that the simplest solution would be for composer to note the installed path of packages in composer.lock. Whether the composer team would agree is another question, possibly they'd object to ede-php-autoload reading the lock file in the first place.

What I was thinking might be realistically workable (save help from composer), was for ede-php-autoload to understand composer/installers enough to:

1. Notice the type property in the lock file and
2. Either using the path specified in intaller-paths, if one is given,
3. Or map up types to directories using a customize variable so end users can add the common paths themselves.
4. Let a hook fiddle with it for the odd case where a simple mapping wont do.

I think 1 to 3 will cover 99% of all users needs, and the last hook gives people something to work with.

Given the right hooks in ede-php-autoload, it could be implemented in a ede-php-composer-installers package. The minimum required would be for ede-php-autoload to hand over the package name and the type from the lock file, and get back a base directory to use instead of vendor/<vendor>/<package>. I could work with that at least.

My lisp-fu is weak, but isn't ede-php-autoload-composer--get-third-party-dir basically the place to hook in? I figure I could just redefine it with my own function to make it do what I want, though a more proper hooking mechanism would be preferable. The only problem with that is figuring out whether composer/installers are installed by the composer.json, as nothing special should be done in the case where it isn't.

Oh, and bear in mind that there's seems to exist other custom installers works much like composer/installers but are for a specific framework (typo3/cms-composer-installers).

So a ede-php-autoload-composer--get-third-party-dir hook basically needs to have the list of packages form composer.json (too see if it's installed), the extras section from same (it's where customizations are specified), the package name and the composer.lock section for the package. The first two could be handed to some sort of init function, but the init function would need to have some place to store state in the project for the hook.

Can you think up a nice API for that?

[stevenremot]

I was actually thinking about a more general hook to be able to parse any arbitrary configuration from the composer file and add the necesary autoloads. It would let you define a new composer "visitor" like this:

(defun my-custom-composer-visitor (composer-data composer-lock base-directory autoloads)
  ;; Add your own autoloads in the autoloads var, or clone it and enrich it
  autoloads)

(ede-php-autoload-composer-define-visitor #'my-custom-composer-visitor)

The ede-php-autoload internal code will use it too. For example, ede-php-autoload--merge-composer-autoloads would be rewritten like this:

(defun ede-php-autoload--merge-composer-autoloads (composer-data composer-lock base-dir autoloads)
  "Load the autoload information in COMPOSER-DATA and merge it with AUTOLOADS.

COMPOSER-DATA is the parsed composer.json file.
BASE-DIR is the prefix dir to add to each autoload path."
  (let ((composer-autoloads (append (cdr (assoc 'autoload composer-data))
                                    (cdr (assoc 'autoload-dev composer-data))))
        key spec base-spec)
    (dolist (autoload-part composer-autoloads)
      (when (member (car autoload-part) '(psr-0 psr-4))
        (setq key (intern (concat ":" (symbol-name (car autoload-part))))

              spec (mapcar #'(lambda (pair)
                               (ede-php-autoload--format-composer-pair pair base-dir (car autoload-part)))
                           (cdr autoload-part))

              base-spec (plist-get autoloads key))

        (setq autoloads (plist-put autoloads key (append base-spec spec)))))
    autoloads))

(ede-php-autoload-composer-define-visitor #'ede-php-autoload--merge-composer-autoloads)

(Nothing changes except the argument list)

ede-php-autoload--merge-lock-file-data would be rewritten to use the same system.

ede-php-autoload--append-composer-data would be rewritten to be a big reduce on all visitors.

This way, it will be possible to create a new emacs package that defines its own visitor to scan the composer.json and add its own Psr0 / Psr4 / whatever (you can create your own class loaders with ede-php-autoload-class-loader-define-factory). You can create a visitor that searches the composer entries for the "type" property and add the autoload entries corresponding to the project type.

The only thing I'm not sure is the argument list. I think sending a alist may be better than sending 4 arguments, because this list of arguments could grow with time and become difficult to manage...

Regarding your suggestion on having the info in the composer.lock file, I think this is worth discussing with the composer developers. If they don't agree with us scanning the composer.lock file, they may give us a better solution.

Anyway, do you think the API is good enough to allow other packages extending ede-php-autoload? If so, I will implement it even if we're not sure you will have to use it. It will only make the code better :-)

[xendk]

Well yeah, I figure that the API makes it possible to implement the ede-php-composer-installers previously outlined, though it's a bit difficult to follow for a lisp-beginner like me.

Two things:

1. Order of visitors? I'll need to get in after the one that loads the lock file, and other visitors might have other requirements.
2. You talk about adding class loaders, but I just need to adjust the paths of the ones that ede-php-autoload--merge-lock-file-data already added. Will I have to loop through autoloads to find the ones I need to fix up?

[stevenremot]

To answer your questions:

1. All visitors will be provided with composer.json and composer.lock content, already parsed in a lisp structure
2. Mmmmmh, it depends, I don't really understand the details in composer/installers. Will you have to "fix" wrong autoloads, or to create brand new ones? If you have to create new ones, it will be easy. Your visitor will receive the current autoloads set by previous visitors. You just have to append the new ones, and to return the new list of autoloads in your visitor.

[xendk]

Fix wrong autoloads. All composer/installers really do is change the path of the package directory from vendor/<vendor>/<package> to, say web/modules/contrib/<package-name>. Unless ede-php-autoload checks the directory for existence, it'll just think the package is in the wrong place.

So unless the autoloads are indexed in some way, I'll have to loop through the composer.lock data and then search through autoloads for the found projects. Sounds a bit ineffective to me.

[stevenremot]

So actually the autoloads created by composer/installers can be found in the composer.lock? I didn't understand that. But you are right, this can be ineffective for big composer.lock files...

If I understand correctly, in the case of Drupal, the third-party packages are put out of vendor, and Drupal itself takes care of autoloading without the help of composer, right? If that's the case, what about forgetting composer/installers totally, and implement the Drupal autoload logic? I don't know Drupal enough to know if it's a sane solution or not.

[xendk]

That's correct. I believe there's no difference to the composer.lock, with or without composer/installers.

Only partly. Drupal uses the composer autoloader for the core classes, but these are moved to, for instancew core/lib by composer/installers. How composer/installers does this, I don't know, but I can see Drupal make a showing in vendor/composer/autoload*

How it loads classes of modules (which might not be installed by composer, might even not have a composer.json file) I haven't figured out yet. But yeah, I'll have to figure out an autoloader for that.

But I see where you're going. If I implement the autoloading for modules, it could also handle Drupal core. But then I'd need to suppress the Drupal autoloads created by ede-php-autoload-composer, as they'd still point down at vendor/. Or fix them up. But I'll still need the composer autoloads for non-Drupal libraries.

[stevenremot]

I have an idea in order to let you suppress wrong autoloads created by ede-php-autoload. It should not be abused, but what do you think about being able to give priority to visitors, so that you could define a visitor that would be run after the standard composer stuffs ? You will be able to remove the wrong autoload composer created, and to add your drupal autoloads.

It would be a definition like this:

(ede-php-autoload-composer-define-visitor
 #'add-drupal-autoloads
 :after)

(I'm not really sure how to name priorities)

[xendk]

I've got a bit of difficulty picturing things in my head by now, but I guess yeah? Can't I just adjust the path of the "wrong" autoloads? They're still valid, it's just the path.

For a first stab, I can just do a visitor that adjusts packages of certain known types to the path that suits me. If I can get that working, using the installer-paths from the composer file is the next step.