composer-patches icon indicating copy to clipboard operation
composer-patches copied to clipboard

Patch for new file applied to incorrect location

Open heddn opened this issue 9 years ago • 17 comments

https://www.drupal.org/files/issues/drupal-migrate_skip_on_value-2711949-2.patch

I'm using drupal-composer/drupal-project and when I try to apply the patch listed above, it creates it as web/core/core/core/modules/migrate/src/Plugin/migrate/process/SkipOnValue.php, etc.

heddn avatar Apr 26 '16 16:04 heddn

Can you please provide a complete composer.json file that will allow me to reproduce the problem?

cweagans avatar Apr 26 '16 16:04 cweagans

http://sprunge.us/WPZD

heddn avatar Apr 26 '16 16:04 heddn

We ran into the same issue:

{
    "name": "drupal-composer/drupal-project",
    "description": "Project template for Drupal 8 projects with composer",
    "type": "project",
    "license": "GPL-2.0+",
    "authors": [
        {
            "name": "",
            "role": ""
        }
    ],
    "repositories": [
        {
            "type": "composer",
            "url": "https://packagist.drupal-composer.org"
        }
    ],
    "require": {
        "composer/installers": "^1.0.20",
        "cweagans/composer-patches": "~1.0",
        "drupal/core": "~8.0"
    },
    "extra": {
        "installer-paths": {
            "web/core": ["type:drupal-core"],
            "web/modules/contrib/{$name}": ["type:drupal-module"],
            "web/profiles/contrib/{$name}": ["type:drupal-profile"],
            "web/themes/contrib/{$name}": ["type:drupal-theme"],
            "drush/contrib/{$name}": ["type:drupal-drush"]
        },
        "patches": {
            "drupal/core": {
                "Reintroduce Views integration for Book": "https://www.drupal.org/files/issues/reintroduce_views-1853524-90.patch"
            }
        }
    }
}

The issue is that as drupal/core is a subtree split of only the core/ directory. All patches posted on DO contains the core/ prefix, while that subdirectory doesn't exist in the composer package

diff --git a/core/modules/book/book.views.inc b/core/modules/book/book.views.inc

This means that if a patch modifies a file p0 and p1 will always fail and p2 will be used. If the patch only introduces new code p1 will pass and create the file in the incorrect location.

Are there any workarounds for this?

oxyc avatar Jun 07 '16 10:06 oxyc

Not presently, I'm afraid. 2.x will give you the ability to manually set your p value, though, so this issue would go away. I'm hoping to have a beta out the door this weekend, but that may change depending on some other things that are going on.

cweagans avatar Jun 15 '16 19:06 cweagans

Seeing this again on another migrate process plugin patch. The namespace is migrate, because that is where the plugins exist. But the module it should be applied to is migrate_plus, not core migrate.

Did any functionality for 2.x ever get contributed?

heddn avatar Nov 02 '16 22:11 heddn

It looks like it's only happening on patches that only include new files without and file change (change on permissions are working). How can it work so well when there is at least one changed file in the patch?

DuaelFr avatar Feb 06 '17 11:02 DuaelFr

Patching "drupal/core" requires -p2 because the package root is the "core" subfolder of the Drupal repo. The getAndApplyPatch() method tries with -p1, -p0 then -p2. When a Drupal core patch is applied, one can notice (with --verbose) that -p2 is used. The issue here is that a patch consisting of only file addition won't incur a failure with "-p1" because git has no way to tell that paths don't match. The only way around here would be to be able to "force" the patch level for a given package. Working on a PR now.

morvans avatar Feb 06 '17 11:02 morvans

Came across this issue as well with a migrate process plugin.

sylus avatar Apr 10 '17 14:04 sylus

Here's a workaround for anyone else running into this with a Drupal core patch. Just copy the patch to local storage and modify it to remove "core/" from the patch lines (this allows the patch to apply properly with -p1).

bkosborne avatar Apr 13 '17 13:04 bkosborne

With this it should be not too difficult to massage the patch: https://github.com/ptlis/diff-parser

geek-merlin avatar May 13 '17 14:05 geek-merlin

After upgrading to the latest (1.6.4), we're seeing any core patch that only adds new files applied to core/core, so the patches are effectively lost. The referenced PR above (#101) mentions this is configurable in the 2.0 version, but that branch seems somewhat out of date with the 1.x branch.

Swapping the patch levels to run -p2 first resolves the issue, but I'm not sure what other effects that might have on non-core patches.

jhedstrom avatar Jan 12 '18 18:01 jhedstrom

@jhedstrom Do you have a sample core patch that's failing in the manner you described? My project uses many Drupal core patches but I believe most of them modify other files in addition to adding new ones.

I have composer-patches 1.6.4, Git version 2.16.1, and patch version 2.7.5.

The behavior I see is that core patches ARE applied, but new files that the patches introduce are ALSO placed in a core/core and core/b folder. This problem has its own dedicated issue now in #178.

bkosborne avatar Feb 02 '18 20:02 bkosborne

@bkosborne yep, this patch only adds new files: https://www.drupal.org/project/drupal/issues/2920310#comment-12418762

jhedstrom avatar Feb 02 '18 21:02 jhedstrom

Thanks, confirmed it's a problem. Yeah, we need to be able to specify the patch level to use for applying patches, but I think it needs to be done per-patch and not per-project. Drupal core patches will always have the /b/core/ path in the patch, so they need p2, but contrib modules don't have the extra level, so they need to be applied with p1.

bkosborne avatar Feb 04 '18 20:02 bkosborne

The same issue occurs for the patches in https://www.drupal.org/project/drupal/issues/2835825

MPParsley avatar Apr 11 '18 10:04 MPParsley

Another instance of needing to specify patch depth on a per-patch basis. That's a 2.0.0 thing, and is tracked on #93.

cweagans avatar Jun 02 '18 07:06 cweagans

For current 1.x, patchLevel should help with patches for drupal/core.

Also, see #178 , #203

john-herreno-ds avatar Apr 07 '21 16:04 john-herreno-ds

main allows you to specify depth on a per-patch basis now. I'll be adding the ability to specify a default per-package in the next day or two.

cweagans avatar Feb 07 '23 19:02 cweagans