Fix Issue #401 - correctly apply multiple command line parameter flags for non-GNU patch

[arcanumbridge]

@amokato @mikeohara @mfb
Would you see if this version also fixes MacOS & FreeBSD? I've been only able to test on FreeBSD 13.1.

[joelpittet]

Condition incorrect

[joelpittet]

This condition should be !== because these options only work on GNU

[mfb]

FreeBSD has a non-GNU patch (version is patch 2.0-12u11 FreeBSD) and definitely has both the --posix and --batch options: https://man.freebsd.org/cgi/man.cgi?patch(1)

[arcanumbridge]

@joelpittet BSD patch in FreeBSD has these options too. I don't know why Apple wouldn't have at least pulled the latest version of patch from FreeBSD. Does the Ventura patch have --posix or --no-backup-if-mismatch? Are you able to send me a copy of the2.0-12u11-Apple patch man page and the output of the patch usage. I can replace --batch with '-t', but need a replacement for --posix or --no-backup-if-mismatch for the Apple patch case.

[joelpittet]

@arcanumbridge On Ventura 13.2 it appears to have all the flags, so maybe not straightforward to "sniff"

$ patch --version
patch 2.0-12u11-Apple

$ man patch
NAME
     patch – apply a diff file to an original

SYNOPSIS
     patch [-bCcEeflNnRstuv] [-B backup-prefix] [-D symbol] [-d directory]
           [-g vcs-option] [-F max-fuzz] [-i patchfile] [-o out-file]
           [-p strip-count] [-r rej-name] [-T | -Z] [-V t | nil | never | none]
           [-x number] [-Y prefix] [-z backup-ext] [--quoting-style style] [--posix]
           [origfile [patchfile]]
     patch <patchfile
...
     --no-backup-if-mismatch
             This option negates --backup-if-mismatch, creating backups for every
             file unless backups have been disabled with --posix or -V none.
     --posix
             Enables strict IEEE Std 1003.1-2008 (“POSIX.1”) conformance,
             specifically:

             1.   Backup files are not created unless the -b option is specified.

             2.   If unspecified, the file name used is the first of the old, new
                  and index files that exists.
     -t, --batch
             Similar to -f, in that it suppresses questions, but makes some
             different assumptions: skip patches for which a file to patch cannot be
             found (the same as -f); skip patches for which the file has the wrong
             version for the "Prereq": line in the patch; and assume that patches
             are reversed if they look like they are.

[mfb]

Ah this looks like potentially good news, if at some point Apple updated their patch to the same version as FreeBSD?

[danepowell]

I don't see how this PR could fix #423 given that it's against master and I cannot reproduce #423 in master using the test case from #423 (comment)

Edit: nevermind... explanation in my next comment

[arcanumbridge]

@danepowell Since you can't reproduce in master, I've removed the Issue #423 from the title. I've updated the condition to at least not break when using Apple patch.

[danepowell]

Ignore my above comment, the reason master is not broken is because Git patching works before the patch utility can ever be called.

However: this PR still doesn't solve the problem in #423 on Ventura. If I check out this PR and delete the Git patching logic in master (forcing it to use the patch utility), patching still hangs awaiting input (using the latest commit ef86816). If I revert a78843a, patching errors immediately, which is better than hanging waiting for input, I suppose, but only marginally! 😄

[arcanumbridge]

@mfb @cweagans I've rolled this back to just handle the FreeBSD / non-GNU case since we don't have enough info to handle the Apple patch case too. I think we're good to commit now unless you guys have anything else.

[arcanumbridge]

@mfb @danepowell @cweagans I've done one more take at this. It will still probably fail on MacOS Ventura, but hopefully it doesn't stall out. I'm testing for gpatch, patch and erroring out if nothing is found and also still sniffing for gnu patch. We want to use gpatch if it's found since it might have better compatibility than the native OS patch.

[cweagans]

This is now extremely out of date. Can you please try the behavior in main and let me know if it works for you? We can reopen this if necessary, but main doesn't use patch at all now -- it's all just git + an escape hatch that you can use to apply patches however you'd like.

[arcanumbridge]

Just looking at the updated code, can this handle multiple free-form patchers? We routinely deploy and test to multiple OS (FreeBSD, Linux, starting to add MacOS), so if we were to use the free-form patcher would we be able to handle both stock FreeBSD and MacOS Ventura in additional to Linux? We don't install git on the build nodes, so we need to fallback to the OS patch on each OS.

…

On 2/13/23 00:30, Cameron Eagans wrote: This is now extremely out of date. Can you please try the behavior in |main| and let me know if it works for you? We can reopen this if necessary, but |main| doesn't use |patch| at all now -- it's all just |git| + an escape hatch that you can use to apply patches however you'd like. — Reply to this email directly, view it on GitHub <#444 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AKDK54NYQV3LPEIZZYROXVLWXHBGRANCNFSM6AAAAAAUIZSBMQ>. You are receiving this because you were mentioned.Message ID: ***@***.***>

-- Anish Mistry

[cweagans]

No, it's just one freeform config per patch. I'm not sure why you wouldn't have git installed on a build node, but it seems way easier to just standardize on a tool rather than trying to support a bunch of different combinations.

Also: In parallel to this work, I'm looking into #474. That could potentially be a path forward for you -- you could just rely on a known version of GNU patch that is installed along with your Composer dependencies. There are some caveats though: GNU patch doesn't support git-style binary patches, and because it's a reimplementation of the Git behaviors, there might be some differences between how patch handles patches and how git handles patches.

[cweagans]

There is also https://docs.cweagans.net/composer-patches/api/capabilities/ -- you could write a Resolver to apply your patches if you need custom OS-specific functionality.

I'd encourage you to look into the feasibility of installing git on your build nodes though. It seems like a much more straightforward proposition. If that's not possible, I'd love to understand why.