Fix Issue #401 - correctly apply multiple command line parameter flags for FreeBSD

[arcanumbridge]

This is a fix for issue #401 to individually escape the flags to patch.

[mfb]

👍 Tested on FreeBSD, it now runs patch '-p2' '--posix' '--batch' -d 'web/core' < '/tmp//6253330a8bb7f.patch'

(I do see a mix of code styles e.g. foreach( and if( vs foreach ( and if ( - not sure if this package has code style preferences or not :)

[grappler]

I have just tested the patch and can confirm it is working as expected.

An alternative to the nested if statement would be to use continue instead, like I did here: wearerequired@d1a08bd

[mfb]

Tested again on FreeBSD and still working fine - it executes patch '-p1' '--posix' '--batch' rather than the broken patch '-p1' '--posix --batch'

Now I guess we need an actual maintainer to review :)

[arcanumbridge]

@cweagans @danepowell can we get a maintainer to review and merge this? This fixes FreeBSD support.

[mfb]

@arcanumbridge I found some other issues mentioning non-GNU patch also being broken on macOS. I'm not sure if macOS uses "BSD" as PHP_OS_FAMILY or something else? If "BSD," then hopefully this PR also fixes macOS. If something else, then maybe this code should be checking whether or not patch is GNU patch rather than looking at the operating system family.

[mfb]

Realized I could just check the php source code :) Looks like macOS uses "Darwin" as the PHP_OS_FAMILY, so this PR won't help there. But, could we tweak the logic to check if patch --version is not GNU patch? See #426 for a way to check this. And if not GNU patch, apply the BSD options.

[mfb]

@arcanumbridge should we roll this back to the version that I tested, with BSD OS-specific logic?

[cweagans]

@mfb Can you please take a look at #447 (in particular, src/Patcher/*) and let me know if I've captured what needs captured?

[cweagans]

(for context, 2.x is essentially a rewrite and it breaks a ton of things out of the main plugin so that they're individually testable)

[danepowell]

I don't have time to test it this weekend, but I like #447 and it looks like it should fix the issue. I like that the patch providers are more structured and test the patch version directly instead trying to infer it based on OS info.

[arcanumbridge]

@cweagans I've made a few comments regarding some consistency of using ->patchTool() and testing for gpatch, but I think it essentially captures what we want.

[cweagans]

Great! I'm going to close this then and focus on getting 2.x out the door as quickly as possible.

(also thank you for the review -- super helpful)