Changing syntax for per-package configs

[derhasi]

Rebase of #4 on develop. This would fix #2 and #5.

With this PR the plugin will use the same syntax as composer/installers for custom-install-paths. This allows paths to be defined per type and per package name.

composer/installers and davidbarratt/custom-installer can be used side-by-side without problems, when they do not try to alter the same package (or custom-installer-all-package-types is set to true).

Example for the proposed syntax.

{
    "extra": {
        "custom-installer": {
            "web/": ["type:drupal-core"],
            "web/sites/{$name}/": ["type:drupal-site"],
            "custom/{$type}/{$vendor}/{$name}/": ["type:random-type"],
            "web/sites/all/libraries/ckeditor": ["ckeditor/ckeditor"]
        },
        "custom-installer-all-package-types": true
    }
}

[davidbarratt]

@derhasi,

Ok, I think this looks pretty good. I like that you built in backwards comparability for Alpha 1.

Here's some simple changes we should probably make:

• Mark Alpha1 functions as being deprecated or move them out of the that class and into a new one as you suggested in the @todo
• Add a new example to the README which a kitchen-sink type example. Basically, I didn't understand that you could have more than one package per path, or more than one type, or both.

Also, I have some questions for you:
When a user sets custom-installer-all-package-types to true and they have a package type they have not defined in their composer.json what happens? i.e. if I have a package of type drupal-core and I haven't defined where that goes, what will Composer do? Also, what will it do if I have Composer Installers loaded?

I guess I don't fully understand what custom-installer-all-package-types does. I see that this tells Composer that we are supporting every type:

public function isPackageTypeSupported($packageType)
    {
        if ($this->allPackageTypes) {
            return true;
        } else {
            return isset($this->types[$packageType]);
        }
    }

but if that is the case, how will types the user has not defined be handled? or is the user forced to define every type they are using? If they are not forced to define every type they are using, then I'd just remove the setting and this function should always return true. If they are forced to define every type, then I'd still remove this setting and just set it to true if they've defined anything other than a type: (and this should be documented either way).

it's interesting how @mnsami solved this problem (if it is one):
https://github.com/mnsami/composer-custom-directory-installer/blob/master/src/Composer/CustomDirectoryInstaller/LibraryInstaller.php

I can go in and test this myself, but I thought I'd just ask because I figured you might already have the answer. :)

[derhasi]

The installer should explicitly define, what package types it supports, so other installers can handle other package types. In the case custom installer and composer installers define the same type, I guess the result could be unpredictable.

I added custom-installer-all-package-types so someone, that only uses custom-installer, can directly handle all packages, regardless of the the package type. The default location would be the vendor directory (as it is the default in LibraryInstaller).
In case you want to alter the path of a single package and that property is not defined, you would have to add a definition for the package type too, so custom-installer knows, that it shall support that package type, and therefore can provide the path for the single package.

Example for ckeditor/ckeditor being of package type library:

{
    "extra": {
        "custom-installer": {
            "vendor/{$vendor}/{$name}/": ["type:library"],
            "web/sites/all/libraries/ckeditor": ["ckeditor/ckeditor"]
        }
    }
}

As far as I see, https://github.com/mnsami/composer-custom-directory-installer can only work for packages of type "library", as it extends the core LibraryInstaller (see https://github.com/composer/composer/blob/master/src/Composer/Installer/LibraryInstaller.php#L61)

[davidbarratt]

@derhasi,

You're right, I did a little test with:

{
  "require": {
    "mnsami/composer-custom-directory-installer": "1.0.*",
    "drupal/core": "~8.0"
  },
  "minimum-stability": "dev",
  "prefer-stable": true,
  "config": {
    "preferred-install": "dist",
    "autoloader-suffix": "Drupal8"
  },
  "extra": {
    "installer-paths": {
      "./core/": ["drupal/core"]
    }
  }
}

and Drupal core ends up in the /vendor/drupal directory.

Well, we have a few choices. we could either:

1. Use the custom-installer-all-package-types to allow people to completely takeover the installation. I think this is a little confusing (at least it was for me). I think it's hard to understand why someone would want or need to do that and what might break if a user chooses that option.
2. Automatically takeover all types when someone defines a package name. I think this is the more "intuitive" solution, but it also means that if someone is using some other installer, the other installer will break. :/ but really the same thing would happen if they enabled custom-installer-all-package-types
3. Do neither of the above and use the type:library workaround as you've described. I think this is actually ok. It looks nasty, but if it's understood that Custom Installer is a drop-in replacement for Composer Installers (as you've proposed) then the user should understand (and this should be documented) that they need to predefine all types they are using.

I think 3 is probably the way to go. Honestly it would be nice if Composer would pass the entire package object into the supports method, which would fix the problem for us.

[davidbarratt]

ok, I've opened composer/composer#4058 to address that shortcoming, that would fix a lot of our issues.

[derhasi]

Yes, I think Option 3) would be the best for now.
Maybe we also can kick BC, if you still want to rename the plugin in a separate group (like drupal-composer).

[derhasi]

I finally found time to rework the PR with your proposed changes. A review would be great :)

💃

[derhasi]

@davidbarratt , could you test that out? I'd love to use the changes in an official release :)

[davidbarratt]

@derhasi,

certainly! I'll take a look at it as soon as possible, I apologize for my neglect.

[davidbarratt]

@derhasi,

I don't understand how you can specify a custom package (without a type). Based on Configuration::isPackageTypeSupported() it looks like it only looks at $this->types and not $this->packages. Am I missing something?

[derhasi]

@davidbarratt , yes, that Is because of the way the composer API currently works. Because of that you always have to specify a declaration for the package type of your single package, even if it simply is vendor/{$vendor}/{$name}/ and therefore the default.

Every package has a type. If none is explicitly specified, the type defaults to library.

Example

{
    "extra": {
        "custom-installer": {
            "vendor/{$vendor}/{$name}/": ["type:library"],
            "web/sites/all/libraries/ckeditor": ["ckeditor/ckeditor"]
        }
    }
}

[derhasi]

Better to say, you always have to specify the type. You never can specify a custom package without specifying its type.