composer patches times out at first level when using multiple patch levels without -vvv flag

2dareis2do commented 5 months ago

Verification

[X] I have updated Composer to the most recent stable release (composer self-update)
[X] I have updated Composer Patches to the most recent stable release (composer update cweagans/composer-patches)
[X] I am using one of the supported PHP versions (8.0+)
[X] I have searched existing issues and discussions for my problem.
[X] My problem is not addressed in the troubleshooting guide.

What were you trying to do (and why)?

I am trying to apply a patches with both patch level 2 and 1 for drupal/core

If you specify
"patchLevel": { "drupal/core": "-p2" },

What happened? What did you expect to happen?

After running composer update:

Could not apply patch! Skipping. The error was: The process "patch '-p1' --no-backup-if-mismatch -d 'web/core' < '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/666097568bf92.patch'" exceeded the timeout of 300 seconds.

If I apply same with -vvv flag It will manually prompt me for a path. After trying and failing, it will try the next patch level on the list:

"-p1 -p0 -p2 -p4"

Now drupal/core is set to -p2 for some reason. So it looks as if this is simply timing out.

However, can we have composer patches to try the next element on the list without having to specify the -vv flag and fail the manual path 3 times?

I have tried the same with composer patches beta 2 and I do not appear to have the same issue. i.e. composer patches applies the both patch level one and two without timeout.

Seems like a big improvement here.

Contents of `composer.json`

"drupal/core": {
                "Configuration schema & required values: add test coverage for `nullable: true` validation support - https://www.drupal.org/project/drupal/issues/3364109": "https://www.drupal.org/files/issues/2024-03-19/array_null_config_fatal_errort_fix_10_2.diff",
                "Remove dependency on hardcoded classes in views template - https://www.drupal.org/project/drupal/issues/3450377": "https://www.drupal.org/files/issues/2024-06-04/views_3450377_updated2_test.patch"
            },

Contents of `patches.lock.json`

n/a

Full output of `composer patches-doctor`

n/a

Full output of relevant Composer command with the `-vvv` flag added

as above

2dareis2do commented 5 months ago

Actually I have tried version 2 and I had some issues with applying some patches. Namely


  No available patcher was able to apply patch https://www.drupal.org/files/issues/2024-02-05/3359511-38  
  -updated.patch to drupal/core

Running 1.7 seems to work only if using -vvv flag and multiple attempts, otherwise it will time out.

I do not understand the logic of having a separate patches.lock.json if there is no separate patches.json

How else can we modify values like depth per patch without updating the .lock file?

cweagans commented 5 months ago

https://docs.cweagans.net/composer-patches/usage/defining-patches/

See the "expanded definition" heading. You can specify depth on a per package or per patch basis.

I do not understand the logic of having a separate patches.lock.json if there is no separate patches.json

You can use a separate patches.json file if you want. It works OOTB. (Same docs link as above)

The lock file is the combination of all patches in the project - wherever they are defined. You should not be editing the lock file by hand. Anything that would require you to do that is either a) user error or b) a bug.

2dareis2do commented 5 months ago

ok atm my patches look like so:

 "patches": {
            "drupal/migrate_file": {
                "STATUS_PERMANENT constant not found - https://www.drupal.org/project/migrate_file/issues/3339613": "https://www.drupal.org/files/issues/2023-02-06/migrate_file_fix_STATUS_PERMANENT_patch.patch",
                "Add support for image attributes on remote images - https://www.drupal.org/project/migrate_file/issues/3418059": "https://www.drupal.org/files/issues/2024-02-01/project_migrate_file_issues_3418059_test3.patch"
            },
            "drupal/backup_migrate": {
                "Add way to skip certain rows in config table - https://www.drupal.org/project/backup_migrate/issues/3412141": "https://www.drupal.org/files/issues/2024-02-06/backup_migrate_3412141ccc_test.patch"
            },
            "drupal/token": {
                "Summary token not fully handled in fields - https://www.drupal.org/project/token/issues/2924873#comment-15104758": "https://www.drupal.org/files/issues/2024-05-21/tokens_body_with_summary-2924873-11stre_new.patch"
            },
            "drupal/core": {
                "Claro - fix issues with icon sizes - https://www.drupal.org/project/drupal/issues/3414419": "https://www.drupal.org/files/issues/2024-01-12/claro_add_px2rem_selector_blacklist_for_toolbar_icon_pseudo_class_updated_test_1_1.patch",
                "Claro - Add support for additional coloured notices - https://www.drupal.org/project/drupal/issues/3437924": "https://www.drupal.org/files/issues/2024-04-04/3437924_new_working.patch",
                "Claro - Table layout breaks with long strings and content - https://www.drupal.org/project/drupal/issues/3438269": "https://www.drupal.org/files/issues/2024-04-04/3438269_new3_working.patch",
                "Modal dialogue Views Messages breaks form usability - https://www.drupal.org/project/drupal/issues/3161840": "https://www.drupal.org/files/issues/2024-01-03/project_drupal_issues_3161840b.patch",
                "Total Views Counters is always 0 - https://www.drupal.org/project/drupal/issues/2962763#comment-15413322": "https://www.drupal.org/files/issues/2024-01-25/drupal_issues_2962763_test2.patch",
                "[regression] missing menu active trail in Drupal 9.5.9 - https://www.drupal.org/project/drupal/issues/3359511": "https://www.drupal.org/files/issues/2024-02-05/3359511-38-updated.patch",
                "Add token for a Views Display description - https://www.drupal.org/project/drupal/issues/3419323#comment-15430210": "https://www.drupal.org/files/issues/2024-02-06/drupal_issues_341932b_test.patch",
                "More link is missing in pager when using the \"Some\" pager and there are more records than shown - https://www.drupal.org/project/drupal/issues/3381979": "https://www.drupal.org/files/issues/2024-05-29/3381979-13_test.patch",
                "Configuration schema & required values: add test coverage for `nullable: true` validation support - https://www.drupal.org/project/drupal/issues/3364109": "https://www.drupal.org/files/issues/2024-03-19/array_null_config_fatal_errort_fix_10_2.diff",
                "Remove dependency on hardcoded classes in views template - https://www.drupal.org/project/drupal/issues/3450377": "https://www.drupal.org/files/issues/2024-06-04/views_3450377_updated2_test.patch"
            },

Now if I want to use the expanded format that is used in 2.x format is like

        "the/project": [
                {
                    "description": "This is the description of the patch",
                    "url": "https://www.example.com/path/to/file.patch"
                },

so specifically I just want to enforce a level 2 patch for say

       "drupal/core": {
                "Remove dependency on hardcoded classes in views template - https://www.drupal.org/project/drupal/issues/3450377": "https://www.drupal.org/files/issues/2024-06-04/views_3450377_updated2_test.patch"
            },

How can I do that?

key value is already declared, we can't have 2 keys with the say name
I have to refactor every patch to the new format
I had issue with the last patch applying using 2.0 (on one project)

2dareis2do commented 5 months ago

The main issue here is that 1.7.x fails to apply level patches other than the first level unless using -vvv patch. If this worked as expected there would be no need to specify the patchLevel in composer.json

cweagans commented 5 months ago

 "the/project": [
{ "description": "This is the description of the patch", "url": "https://www.example.com/path/to/file.patch", "depth": 2 }
]

would let you set it for that specific patch.

However, patch depth of 2 is already the default for drupal/core: https://github.com/cweagans/composer-patches/blob/main/src/Util.php#L19-L21

The main issue here is that 1.7.x fails to apply level patches other than the first level unless using -vvv patch. If this worked as expected there would be no need to specify the patchLevel in composer.json

1.x is effectively unsupported at this point.

cweagans commented 5 months ago

Opened https://github.com/cweagans/composer-patches/issues/582 to capture the annoying compact -> expanded format conversion.

2dareis2do commented 5 months ago

@cweagans If 1.x is closed am I correct in assuming this issue is also in 2.x branch? i.e. patch levels are not applied unless using -vvv flag

Also with regards the patch level for Drupal core, I believe the default is 2 for drupal/core and 1 for Drupal contributed modules.

https://www.previousnext.com.au/blog/patch-drupal-core-without-things-ending-up-corecore-or-coreb

cweagans commented 5 months ago

No, 2.x works completely differently. Give it a go.

Depth of 1 is the base default in 2.x as well (since that's the default for git apply).

2dareis2do commented 5 months ago

I tried it once and decided to roll back.

It still looks like 2.x is in beta stage, so i find it puzzling you are no longer supporting 1.x

I think we need something like this that retries 3 times before failing

https://stackoverflow.com/a/25002806/3592441

2dareis2do commented 5 months ago

Ok I have tried to enable composer patches so that I can step though the code but am not quite sure what I am doing

Currently getting:

The editor could not be opened due to an unexpected error: Unable to resolve resource phar:/usr/local/bin/composer/src/Composer/Factory.php

when applying COMPOSER_ALLOW_XDEBUG=1 composer update -vvv

?

Any way, looking at the code we have this:

    // Attempt to apply with git apply.
    $patched = $this->applyPatchWithGit($install_path, $patch_levels, $filename);

    // In some rare cases, git will fail to apply a patch, fallback to using
    // the 'patch' command.
    if (!$patched) {
      foreach ($patch_levels as $patch_level) {
        // --no-backup-if-mismatch here is a hack that fixes some
        // differences between how patch works on windows and unix.
        if ($patched = $this->executeCommand("patch %s --no-backup-if-mismatch -d %s < %s", $patch_level, $install_path, $filename)) {
          break;
        }
      }
    }

So in my case it looks like it first looks to apply a patch with git. If this is unsuccessful it iterates through each patch level and then tries to apply the patch.

It looks to me as if the following message is actually coming from patch command

e.g.

 https://www.drupal.org/files/issues/2024-06-04/views_3450377_updated2_test.patch (Remove dependency on hardcoded classes in views template - https://www.drupal.org/project/drupal/issues/3450377)
Downloading https://www.drupal.org/files/issues/2024-06-04/views_3450377_updated2_test.patch
string(3) "-p1"
patch '-p1' --no-backup-if-mismatch -d 'web/core' < '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/6677166890eb7.patch'
Executing command (CWD): patch '-p1' --no-backup-if-mismatch -d 'web/core' < '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/6677166890eb7.patch'
File to patch: 
web
No file found--skip this patch? [n] 
web/
patch: **** can't find web

string(3) "-p0"
patch '-p0' --no-backup-if-mismatch -d 'web/core' < '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/6677166890eb7.patch'
Executing command (CWD): patch '-p0' --no-backup-if-mismatch -d 'web/core' < '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/6677166890eb7.patch'
File to patch: 
web/core/
No file found--skip this patch? [n] 

patch: **** can't find 'web/core/'

string(3) "-p2"
patch '-p2' --no-backup-if-mismatch -d 'web/core' < '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/6677166890eb7.patch'
Executing command (CWD): patch '-p2' --no-backup-if-mismatch -d 'web/core' < '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/6677166890eb7.patch'
patching file 'modules/views/config/schema/views.data_types.schema.yml'

patching file 'modules/views/src/Plugin/views/style/DefaultStyle.php'

patching file 'modules/views/src/Plugin/views/style/StylePluginBase.php'

patching file 'modules/views/templates/views-view.html.twig'

patching file 'modules/views/views.theme.inc'

> post-package-install: Drupal\Composer\Plugin\Scaffold\Plugin_composer_tmp8->postPackage

so specifically:

File to patch: 
web
No file found--skip this patch? [n] 
web/
patch: **** can't find web

If -vvv command is not entered then the prompt is not visible and it times out.

2dareis2do commented 5 months ago

When traditional patch asked the user a question, it sent the question to standard error and looked for an answer from the first file in the following list that was a terminal: standard error, standard output, /dev/tty, and standard input. Now patch sends questions to standard output and gets answers from /dev/tty. Defaults for some answers have been changed so that patch never goes into an infinite loop when using default answers. https://www.man7.org/linux/man-pages/man1/patch.1.html

2dareis2do commented 5 months ago

Ok I think we can use force here:

-f or --force Assume that the user knows exactly what he or she is doing, and do not ask any questions. Skip patches whose headers do not say which file is to be patched; patch files even though they have the wrong version for the Prereq: line in the patch; and assume that patches are not reversed even if they look like they are. This option does not suppress commentary; use -s for that.

https://www.man7.org/linux/man-pages/man1/patch.1.html

        if ($patched = $this->executeCommand("patch %s -f --no-backup-if-mismatch -d %s < %s", $patch_level, $install_path, $filename)) {
          break;
        }

2dareis2do commented 5 months ago

cweagan-patch-timeout-fix.patch

diff --git a/src/Patches.php b/src/Patches.php
index 31f5225..756a9ed 100644
--- a/src/Patches.php
+++ b/src/Patches.php
@@ -398,7 +398,7 @@ class Patches implements PluginInterface, EventSubscriberInterface {
       foreach ($patch_levels as $patch_level) {
         // --no-backup-if-mismatch here is a hack that fixes some
         // differences between how patch works on windows and unix.
-        if ($patched = $this->executeCommand("patch %s --no-backup-if-mismatch -d %s < %s", $patch_level, $install_path, $filename)) {
+        if ($patched = $this->executeCommand("patch %s -f --no-backup-if-mismatch -d %s < %s", $patch_level, $install_path, $filename)) {
           break;
         }
       }

I've tested this locally and it seems to work. Correct patch is applied without time out or having to use -vvv flag with multiple failed attempts. No need to specify patch level if either level, 1, 0, 2 or 4 - in that order

2dareis2do commented 5 months ago

How can I apply this patch? Can I apply with cweagans/composer-patches ?

2dareis2do commented 5 months ago

After manually applying patch, here is the output with patch applied:

Downloading https://www.drupal.org/files/issues/2024-06-04/views_3450377_updated2_test.patch
patch '-p1' -f --no-backup-if-mismatch -d '/Users/danlobo/Sites/streathamlifed8/web/core' < '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/66793aa23a68d.patch'
Executing command (CWD): patch '-p1' -f --no-backup-if-mismatch -d '/Users/danlobo/Sites/streathamlifed8/web/core' < '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/66793aa23a68d.patch'
No file to patch.  Skipping...

1 out of 1 hunks ignored--saving rejects to 'core/modules/views/config/schema/views.data_types.schema.yml.rej'

Can't create '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/patchrp5oe4aH3es', output is in '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/patchrp5oe4aH3es': No such file or directory
No file to patch.  Skipping...

1 out of 1 hunks ignored--saving rejects to 'core/modules/views/src/Plugin/views/style/DefaultStyle.php.rej'
Can't create '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/patchrp5oe4aH3es', output is in '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/patchrp5oe4aH3es': No such file or directory

No file to patch.  Skipping...

6 out of 6 hunks ignored--saving rejects to 'core/modules/views/src/Plugin/views/style/StylePluginBase.php.rej'

Can't create '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/patchrp5oe4aH3es', output is in '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/patchrp5oe4aH3es': No such file or directory
No file to patch.  Skipping...

1 out of 1 hunks ignored--saving rejects to 'core/modules/views/templates/views-view.html.twig.rej'

Can't create '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/patchrp5oe4aH3es', output is in '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/patchrp5oe4aH3es': No such file or directory
No file to patch.  Skipping...

2 out of 2 hunks ignored--saving rejects to 'core/modules/views/views.theme.inc.rej'

Can't create '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/patchrp5oe4aH3es', output is in '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/patchrp5oe4aH3es': No such file or directory

patch '-p0' -f --no-backup-if-mismatch -d '/Users/danlobo/Sites/streathamlifed8/web/core' < '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/66793aa23a68d.patch'
Executing command (CWD): patch '-p0' -f --no-backup-if-mismatch -d '/Users/danlobo/Sites/streathamlifed8/web/core' < '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/66793aa23a68d.patch'
No file to patch.  Skipping...

1 out of 1 hunks ignored--saving rejects to 'a/core/modules/views/config/schema/views.data_types.schema.yml.rej'

Can't create '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/patchrBDfg3EK1Wa', output is in '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/patchrBDfg3EK1Wa': No such file or directory

No file to patch.  Skipping...

1 out of 1 hunks ignored--saving rejects to 'a/core/modules/views/src/Plugin/views/style/DefaultStyle.php.rej'

Can't create '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/patchrBDfg3EK1Wa', output is in '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/patchrBDfg3EK1Wa': No such file or directory
No file to patch.  Skipping...

6 out of 6 hunks ignored--saving rejects to 'a/core/modules/views/src/Plugin/views/style/StylePluginBase.php.rej'

Can't create '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/patchrBDfg3EK1Wa', output is in '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/patchrBDfg3EK1Wa': No such file or directory
No file to patch.  Skipping...

1 out of 1 hunks ignored--saving rejects to 'a/core/modules/views/templates/views-view.html.twig.rej'

Can't create '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/patchrBDfg3EK1Wa', output is in '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/patchrBDfg3EK1Wa': No such file or directory
No file to patch.  Skipping...

2 out of 2 hunks ignored--saving rejects to 'a/core/modules/views/views.theme.inc.rej'
Can't create '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/patchrBDfg3EK1Wa', output is in '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/patchrBDfg3EK1Wa': No such file or directory

patch '-p2' -f --no-backup-if-mismatch -d '/Users/danlobo/Sites/streathamlifed8/web/core' < '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/66793aa23a68d.patch'
Executing command (CWD): patch '-p2' -f --no-backup-if-mismatch -d '/Users/danlobo/Sites/streathamlifed8/web/core' < '/var/folders/pn/ngzsxydj2ng4d1yjdflg6tx40000gn/T/66793aa23a68d.patch'
patching file 'modules/views/config/schema/views.data_types.schema.yml'

patching file 'modules/views/src/Plugin/views/style/DefaultStyle.php'

patching file 'modules/views/src/Plugin/views/style/StylePluginBase.php'

patching file 'modules/views/templates/views-view.html.twig'

patching file 'modules/views/views.theme.inc'

cweagans / composer-patches