Incompatible with cweagans/composer-patches

[shahinam]

When adding or deleting patches, paths are not preserved.

This is the test composer.json I am using.

{
  "name": "example",
  "description": "Example config for a D7 site using composer based build process.",
  "repositories": [{
    "type": "composer",
    "url": "https://packagist.drupal-composer.org"
  }],
  "require": {
    "composer/installers": "~1.0",
    "cweagans/composer-patches": "~1.0",
    "derhasi/composer-preserve-paths": "0.1.*",
    "drupal/drupal": "7.43",
    "drupal/views": "~7.0",
    "drupal/ctools": "~7.0",
    "drupal/memcache": "~7.0",
    "drupal/panels": "~7.0"
  },
  "conflict": {
    "drupal/core": "8.*"
  },
  "scripts": {
    "post-create-project-cmd": [
      "rm README.md LICENSE .travis.yml phpunit.xml.dist"
    ]
  },
  "config": {
    "vendor-dir": "vendor"
  },
  "minimum-stability": "dev",
  "prefer-stable": true,
  "extra": {
    "_readme": [
      "This is an example comment!"
    ],
    "installer-paths": {
      "docroot/": ["type:drupal-core"],
      "docroot/sites/all/modules/contrib/{$name}/": ["type:drupal-module"],
      "docroot/sites/all/themes/contrib/{$name}/": ["type:drupal-theme"],
      "docroot/sites/all/libraries/{$name}/": ["type:drupal-library"],
      "docroot/sites/all/drush/{$name}/": ["type:drupal-drush"],
      "docroot/profiles/{$name}/": ["type:drupal-profile"]
    },
    "patches": {
      "drupal/drupal": {
        "Node access grants should be statically cached": "https://www.drupal.org/files/issues/node_access_grants-static-cache-11.patch",
        "Invalid image style URLs should return 404, not 403.": "https://www.drupal.org/files/issues/drupal-image-style-not-found-2211429-4.patch",
        "Save and restore css/js/head with block cache.": "https://www.drupal.org/files/issues/d7-block-cache-1460766-34.patch",
        "Ignore front end vendor folders to improve directory search performance": "https://www.drupal.org/files/issues/ignore_frontend_folders-2329453-101-7.x.patch"
      }
    },
    "preserve-paths": [
      "docroot/robots.txt",
      "docroot/.htaccess",
      "docroot/sites/all/modules/contrib",
      "docroot/sites/all/themes/contrib",
      "docroot/sites/all/libraries",
      "docroot/sites/all/drush",
      "docroot/sites/default/settings.php",
      "docroot/sites/default/files"
    ]
  }
}

After installing, I am making changes in robots.txt which I want to preserve.
Adding or updating any packages works fine and preserves the changes.

Now If I add/remove a patch and do composer update, all the changes are gone.

I did a short debugging, and here is what I think is happening:
For cleanly applying patches composer-patches plugin deletes package files (in this case docroot) by responding to ScriptEvents::PRE_INSTALL_CMD event which happens to be before the ScriptEvents::PRE_PACKAGE_INSTALL / ScriptEvents::PRE_PACKAGE_UPDATE events in event loop - where composer-preserve-paths plugin backs up the files. So when composer-preserve-paths gets chance to backup files, they are already removed, so nothing to backup and hence restore.

[cweagans]

I think we can set weights for the event listeners. @derhasi, if you want to coordinate setting our listener weights appropriately, I'm happy to change things on my end to ensure that your plugin is supported by mine and vice-versa!

[derhasi]

@cweagans I try to have a closer look on this in the next days.

[eelkeblok]

Any update on this issue? Is there any workaround possible in the mean time?

[Goon3r]

@eelkeblok this package works fine with: "netresearch/composer-patches-plugin"

[cweagans]

I haven't looked into it, as I'm presently in the middle of a rewrite of my plugin. If you open a corresponding issue in my queue, I'll make sure 2.x is compatible.

[eelkeblok]

It looks like cweagans/composer-patches#42 is basically the equivalent of this issue in the composer-patches queue.

[SebCorbin]

Related question composer/composer#6261

[derhasi]

Is this still an issue? I haven't used the plugin in a while.

[bartek-flp]

Yes, this is not resolved yet.

[SebCorbin]

For those interested, you can try my ugly patch here SebCorbin/composer-patches@1cb9bac.
If it works, then report it and it may set a direction for the maintainers...

[wesley-jones]

The patch by @SebCorbin worked for me. I simply added the following to composer.json and my paths were preserved:

extra": {
    "patches": {
        "cweagans/composer-patches": {
            "Fix to preserve paths" : "https://github.com/SebCorbin/composer-patches/commit/1cb9bacba51f8bba0c5f3f322c3bb61e4ceae974.patch"
        }
        ... (lots of other patches)
    }
}

[clemens-tolboom]

@SebCorbin thanks for the patch

@shahinam @Others is this only a Drupal core 7 problem then let's alter the title and add a Drupal 7 only label.

My experience is this is only happening on D7 core only.

[coreykck]

Same as @clemens-tolboom

[wesley-jones]

My experience was with Drupal 7

[eelkeblok]

I think it is correct that this happens with Drupal 7, not 8. IIRC, it has something to do with the file system structure. Plus, these plugins seem to be used almost exclusively in Drupal projects, so in practice Drupal 7 is likely the only project affected. I am a bit hesitant to tag it as D7 exclusive, but it should definitely be noted in the issue summary.

[clemens-tolboom]

What I learned so far is Drupal 8 is installed/added as a component like any normal composer require then installed in its own /web/core directory. To make Drupal 8 work it needs drupal-composer/drupal-scaffold to install the files outside /core like index.php and .htaccess.

OTOH Drupal 7 lives in /web together with ie /web/sites/sites/* which derhasi/composer-preserve-paths tries to preserve but fails.

We @work decided to live with this and go with the Drupal 7 composer and do core updates manually. Unfortunately unavailable time prevents testing this.

What would you do?

[coreykck]

I put my modules themes and libraries outside web folder and use composer-post install script to link inside Drupal 7 structure; this fails under Windows but is good enough to me

[eelkeblok]

We've decided to stick with drush make 😇 Composer is neat, but maintaining one of the components (Drupal core) manually - especially with patches - sounds like a step back, to me. YMMV.

[jcnventura]

This seems to be a bug in composer-patches, as per the comment from @SebCorbin above.

Nothing to change here, so I'd suggest to close this issue.

[clemens-tolboom]

The patch from #10 (comment) is now added to https://github.com/drupal-composer/drupal-project/commits/7.x in
drupal-composer/drupal-project@8f67c76

[drzraf]

this patch does not apply anymore.

[dmp1ce]

@drzraf The patch #10 (comment) seems to apply for me using 1.6.5 of cweagans/composer-patches.

[codebymikey]

@dmp1ce The patch doesn't apply on the initial composer install because cweagans/composer-patches isn't yet installed so it can't patch itself.

Subsequent composer installs will apply the patch - this isn't good as it's inconsistent behaviour and can potentially break your pipelines/deploys if the patch isn't applied (I found this out the hard way).

@cweagans is there a reason the
https://github.com/SebCorbin/composer-patches/commit/1cb9bacba51f8bba0c5f3f322c3bb61e4ceae974.patch patch itself hasn't been added in the main project?

It shouldn't break anything, and actually makes your plugin more flexible with other composer plugins.

edit:
Just noticed your cweagans/composer-patches#42 issue.

We can add an extras.composer-patches.dispatch-uninstall-hooks option which determines if it should dispatch the events if you're worried about breaking compatibility? But I doubt it'd break anything.

[cweagans]

cweagans/composer-patches#42 (comment) is really my main concern at this point.

[codebymikey]

Following my comment, I did some research into it and I think it should be fine to merge that code in.

The parameters of note are passed in perfectly as per the original implementation.

The only ones which could do with some work are the $policy, $pool and $request - but those objects are only really useful when you're updating/installing a package or something and will rarely be useful on uninstalls - if a plugin runs into an issue with those parameter values, then we'll cross that bridge when we get to it.

The current version of the plugin is unstable as it actively breaks composer in a pretty big way by not notifying other plugins when an uninstall is occurring, so I would prioritize not breaking that API contract and create a separate issue which addresses the creation of the $policy, $pool and $request parameters properly.

The ideal solution would be to replicate the \Composer\Installer's logic for creating the $policy type parameters, but their implementation is pretty convoluted and requires access to certain objects which aren't easily accessible to the plugin.

[driskell]

It's looking like the $policy, $pool and $request don't exist at the moment in Composer 2 so perhaps this is now much easier to implement?