Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patch for new file applied to incorrect location #43

Closed
heddn opened this issue Apr 26, 2016 · 18 comments
Closed

Patch for new file applied to incorrect location #43

heddn opened this issue Apr 26, 2016 · 18 comments

Comments

@heddn
Copy link

heddn commented Apr 26, 2016

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.

@cweagans
Copy link
Owner

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

@heddn
Copy link
Author

heddn commented Apr 26, 2016

http://sprunge.us/WPZD

@oxyc
Copy link

oxyc commented Jun 7, 2016

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?

@cweagans
Copy link
Owner

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.

@heddn
Copy link
Author

heddn commented Nov 2, 2016

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?

@DuaelFr
Copy link

DuaelFr commented Feb 6, 2017

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?

@morvans
Copy link

morvans commented Feb 6, 2017

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.

@sylus
Copy link

sylus commented Apr 10, 2017

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

@bkosborne
Copy link

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).

@geek-merlin
Copy link

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

@jhedstrom
Copy link

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 added a commit to jhedstrom/composer-patches that referenced this issue Jan 12, 2018
kevin-dutra added a commit to kevin-dutra/composer-patches that referenced this issue Jan 12, 2018
jhedstrom added a commit to jhedstrom/composer-patches that referenced this issue Jan 12, 2018
@bkosborne
Copy link

@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.

@jhedstrom
Copy link

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

@bkosborne
Copy link

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.

@MPParsley
Copy link

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

@cweagans
Copy link
Owner

cweagans commented Jun 2, 2018

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.

@john-herreno-ds
Copy link

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

Also, see #178 , #203

stucki pushed a commit to stucki/composer-patches that referenced this issue Sep 14, 2021
stucki pushed a commit to stucki/composer-patches that referenced this issue Apr 12, 2022
@cweagans
Copy link
Owner

cweagans commented Feb 7, 2023

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 cweagans closed this as completed Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.