acquia / blt

Acquia's toolset for automating Drupal 10 development, testing, and deployment.
https://docs.acquia.com/blt/
GNU General Public License v2.0
442 stars 394 forks source link

BLT 8.7.0 upgrade wipes out custom installer-paths configuration #1354

Closed geerlingguy closed 7 years ago

geerlingguy commented 7 years ago

My system information:

When I did this...

When I upgrade BLT from 8.6.15 to 8.7.0, all the installer-paths configuration gets modified inside composer.json, and since I didn't look closely enough at all the metadata modifications to composer.json (since there were a lot in this update), it wiped out some custom installer-paths we require for our project.

We have to hack around the fact that libraries and Drupal 8 aren't 'a thing' yet, so we had the following in addition to all the BLT-provided paths:

"docroot/libraries/fontawesome": [
  "fortawesome/font-awesome"
],

That, along with oomphinc/composer-installers-extender, allowed us to place Font Awesome in the site's libraries folder (accessible via docroot) instead of the inaccessible project vendor directory... but that's another discussion another day.

I expected this to happen

BLT shouldn't delete any existing configuration a project stores in its composer.json installer-paths.

grasmash commented 7 years ago

Yeah that's not supposed to happen.

greylabel commented 7 years ago

FWIW, the installer-path for libraries should work as it is now for your use case without the hack.

With:

"docroot/libraries/{$name}": [
    "type:drupal-library"
]

You can specify libraries like (note "installer-name"):

"repositories": {
  ...

  "font-awesome": {
      "type": "package",
      "package": {
          "name": "fortawesome/font-awesome",
          "version": "v4.7.0",
          "type": "drupal-library",
          "extra": {
              "installer-name": "fontawesome"
          },
          "dist": {
              "url": "https://github.com/FortAwesome/Font-Awesome/archive/v4.7.0.zip",
              "type": "zip"
          },
          "require": {
              "composer/installers": "~1.0"
          }
      }
  },  
  ...
}

And then require them like:

"require": {
  "fortawesome/font-awesome": "v4.7.0",
},
geerlingguy commented 7 years ago

@greylabel - Yeesh... the problem I have with that is that as we build sites that require 3-5 libraries, that means for every library we'd have to set up a customized repository, and that adds a lot of maintenance overhead. The way we're doing it right now I hate as well... there's sadly no way of adding libraries that aren't basically 'only for drupal' or 'drupal modules' to a Drupal 8 codebase cleanly and in a path that a Drupal 8 module expects it without adding all the library code to the codebase directly (which is a no-no and is unsupported by BLT).

Basically, libraries in Drupal 8 is a terrible DX, even for someone used to Composer. I can't imagine the poor souls who don't spend 3-5 hours hassling with Composer every week like we do :/

geerlingguy commented 7 years ago

(And in our case, we are using 3 libraries, one of which isn't even on Packagist since it's not related to PHP in any way and the maintainers don't care about PHP; I just pasted in one example to illustrate the issue.)

geerlingguy commented 7 years ago

Maybe this would be more of a documentation issue, though... just like with Configuration Management, I think 3rd party Library management should start having some 'best practices', because talking to module authors that use said libraries, most of them don't trust Libraries API anymore (since it's still not anywhere near stable in D8), and they recommend downloading a tarball and committing things to docroot/libraries/[library].

greylabel commented 7 years ago

@geerlingguy Indeed this is not ideal. We use this to manage webform's dozen or so library dependencies. Webform also recommends downloading a tarball and committing, but I found this to be more of a nightmare than the composer solution I provided. This is the most sane solution I have found for a site with a involved CI workflow.

geerlingguy commented 7 years ago

@greylabel - Yeah... I've just ignored those Webform warnings in the Status report, because that's a lot of libraries that are required :(

greylabel commented 7 years ago

@geerlingguy Unfortunately we can't because of our content security policy. Loading their libraries from the CDN URLs is not an option for us. :(

geerlingguy commented 7 years ago

See related: Font Awesome Iconpicker Composer Integration. I switched our setup to use @greylabel's suggestion (though I haven't done it for all the Webform libraries... at least not yet).

greylabel commented 7 years ago

@geerlingguy I added a working recipe for webform here: https://www.drupal.org/node/2869874#comment-12040533