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

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

Open
5 tasks done
2dareis2do opened this issue Jun 5, 2024 · 15 comments
Labels
bug Inconsistencies or issues which will cause a problem for users or implementors.

Comments

@2dareis2do
Copy link

Verification

  • I have updated Composer to the most recent stable release (composer self-update)
  • I have updated Composer Patches to the most recent stable release (composer update cweagans/composer-patches)
  • I am using one of the supported PHP versions (8.0+)
  • I have searched existing issues and discussions for my problem.
  • 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 2dareis2do added the bug Inconsistencies or issues which will cause a problem for users or implementors. label Jun 5, 2024
@2dareis2do 2dareis2do changed the title composer patches times out at first level ^1.6 composer patches times out at first level ^1.7 Jun 5, 2024
@2dareis2do
Copy link
Author

2dareis2do commented Jun 5, 2024

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?

@2dareis2do 2dareis2do changed the title composer patches times out at first level ^1.7 composer patches times out at first level ^1.7 when using different patch levels Jun 5, 2024
@2dareis2do 2dareis2do changed the title composer patches times out at first level ^1.7 when using different patch levels composer patches times out at first level when using multiple patch levels without -vvv flag Jun 5, 2024
@cweagans
Copy link
Owner

cweagans commented Jun 5, 2024

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
Copy link
Author

2dareis2do commented Jun 6, 2024

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?

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

@2dareis2do
Copy link
Author

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
Copy link
Owner

 "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
Copy link
Owner

Opened #582 to capture the annoying compact -> expanded format conversion.

@2dareis2do
Copy link
Author

@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
Copy link
Owner

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
Copy link
Author

2dareis2do commented Jun 22, 2024

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
Copy link
Author

2dareis2do commented Jun 22, 2024

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
Copy link
Author

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
Copy link
Author

2dareis2do commented Jun 22, 2024

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
Copy link
Author

2dareis2do commented Jun 22, 2024

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
Copy link
Author

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

@2dareis2do
Copy link
Author

2dareis2do commented Jun 24, 2024

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'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Inconsistencies or issues which will cause a problem for users or implementors.
Projects
None yet
Development

No branches or pull requests

2 participants