Opened 6 months ago

Closed 3 months ago

Last modified 2 months ago

#69649 closed defect (fixed)

Avoid /usr/bin/patch silently applying patches in reverse

Reported by: ryandesign (Ryan Carsten Schmidt) Owned by:
Priority: Normal Milestone: MacPorts 2.10.0
Component: base Version: 2.9.1
Keywords: Cc: mascguy (Christopher Nielsen)
Port:

Description

The patch program shipping with macOS 13 and later sometimes silently applies patches in reverse (https://github.com/macports/macports-ports/pull/23318) and sometimes hangs. It's really unsuitable for our use. Can MacPorts use an alternative?

Change History (11)

comment:1 Changed 6 months ago by ryandesign (Ryan Carsten Schmidt)

Version: 2.9.22.9.1

comment:2 Changed 6 months ago by jmroot (Joshua Root)

I noticed the assuming patches are reversed behaviour while experimenting with using -t, but it seemed to go away when I stopped using that option.

comment:3 Changed 6 months ago by ryandesign (Ryan Carsten Schmidt)

We have already multiple times had port updates committed without removing a patch that had been incorporated upstream, with the effect that it was reversed, installation succeeded, and once it was discovered the patch had to be removed and the port had to be revbumped.

comment:4 Changed 6 months ago by jmroot (Joshua Root)

Looks like we could add -N to avoid that at least?

comment:5 in reply to:  4 Changed 6 months ago by ryandesign (Ryan Carsten Schmidt)

Replying to jmroot:

Looks like we could add -N to avoid that at least?

A PR for this was submitted at https://github.com/macports/macports-base/pull/321

comment:6 Changed 6 months ago by ryandesign (Ryan Carsten Schmidt)

From the macOS 13 build log, here's where it automatically reversed a patch because the new patch command defaults to "yes" when asking if we want to apply a reversed patch:

--->  Applying patches to borgbackup
--->  Applying patch-allow-msgpack-1.0.8.diff
DEBUG: Environment: 
CC_PRINT_OPTIONS='YES'
CC_PRINT_OPTIONS_FILE='/opt/local/var/macports/build/_opt_bblocal_var_buildworker_ports_build_ports_sysutils_borgbackup/borgbackup/work/.CC_PRINT_OPTIONS'
CPATH='/opt/local/include'
DEVELOPER_DIR='/Library/Developer/CommandLineTools'
LIBRARY_PATH='/opt/local/lib'
MACOSX_DEPLOYMENT_TARGET='13.0'
SDKROOT='/Library/Developer/CommandLineTools/SDKs/MacOSX13.sdk'
Executing:  cd "/opt/local/var/macports/build/_opt_bblocal_var_buildworker_ports_build_ports_sysutils_borgbackup/borgbackup/work/borgbackup-1.2.8" && /usr/bin/patch -p0 < '/opt/bblocal/var/buildworker/ports/build/ports/sysutils/borgbackup/files/patch-allow-msgpack-1.0.8.diff'
DEBUG: system:  cd "/opt/local/var/macports/build/_opt_bblocal_var_buildworker_ports_build_ports_sysutils_borgbackup/borgbackup/work/borgbackup-1.2.8" && /usr/bin/patch -p0 < '/opt/bblocal/var/buildworker/ports/build/ports/sysutils/borgbackup/files/patch-allow-msgpack-1.0.8.diff'
patching file setup.py
Reversed (or previously applied) patch detected!  Assume -R? [y] 
patching file 'src/borg/helpers/msgpack.py'
Reversed (or previously applied) patch detected!  Assume -R? [y] 

In contrast, the macOS 12 build log shows that its patch defaults to "no" for that question:

--->  Applying patches to borgbackup
--->  Applying patch-allow-msgpack-1.0.8.diff
DEBUG: Environment: 
CC_PRINT_OPTIONS='YES'
CC_PRINT_OPTIONS_FILE='/opt/local/var/macports/build/_opt_bblocal_var_buildworker_ports_build_ports_sysutils_borgbackup/borgbackup/work/.CC_PRINT_OPTIONS'
CPATH='/opt/local/include'
DEVELOPER_DIR='/Library/Developer/CommandLineTools'
LIBRARY_PATH='/opt/local/lib'
MACOSX_DEPLOYMENT_TARGET='12.0'
SDKROOT='/Library/Developer/CommandLineTools/SDKs/MacOSX12.sdk'
Executing:  cd "/opt/local/var/macports/build/_opt_bblocal_var_buildworker_ports_build_ports_sysutils_borgbackup/borgbackup/work/borgbackup-1.2.8" && /usr/bin/patch -p0 < '/opt/bblocal/var/buildworker/ports/build/ports/sysutils/borgbackup/files/patch-allow-msgpack-1.0.8.diff'
DEBUG: system:  cd "/opt/local/var/macports/build/_opt_bblocal_var_buildworker_ports_build_ports_sysutils_borgbackup/borgbackup/work/borgbackup-1.2.8" && /usr/bin/patch -p0 < '/opt/bblocal/var/buildworker/ports/build/ports/sysutils/borgbackup/files/patch-allow-msgpack-1.0.8.diff'
patching file setup.py
Reversed (or previously applied) patch detected!  Assume -R? [n] 
Apply anyway? [n] 
Skipping patch.
1 out of 1 hunk ignored -- saving rejects to file setup.py.rej
patching file src/borg/helpers/msgpack.py
Reversed (or previously applied) patch detected!  Assume -R? [n] 
Apply anyway? [n] 
Skipping patch.
1 out of 1 hunk ignored -- saving rejects to file src/borg/helpers/msgpack.py.rej
Command failed:  cd "/opt/local/var/macports/build/_opt_bblocal_var_buildworker_ports_build_ports_sysutils_borgbackup/borgbackup/work/borgbackup-1.2.8" && /usr/bin/patch -p0 < '/opt/bblocal/var/buildworker/ports/build/ports/sysutils/borgbackup/files/patch-allow-msgpack-1.0.8.diff'
Exit code: 1

I still need to find an example of where the new patch command hangs to understand the conditions in which that happens.

comment:7 Changed 6 months ago by ryandesign (Ryan Carsten Schmidt)

Hangs were observed in #66780, #66851, #66861, and #66954 due to incorrect worksrcdir (so they would have failed with the macOS ≤ 12 patch program too, just with a better error message). Ports having incorrect worksrcdir should decrease over time as they're found and fixed. If that's the only condition leading to a hang, and we can work around the automatic applying of a reversed patch by adding -N or --forward, then maybe we can tolerate continued use of /usr/bin/patch.

comment:8 Changed 3 months ago by mascguy (Christopher Nielsen)

Cc: mascguy added

comment:9 Changed 3 months ago by ryandesign (Ryan Carsten Schmidt)

Milestone: MacPorts Future

comment:10 Changed 3 months ago by jmroot (Joshua Root)

Resolution: fixed
Status: newclosed
Summary: Avoid using /usr/bin/patchAvoid /usr/bin/patch silently applying patches in reverse

Sure.

comment:11 Changed 2 months ago by jmroot (Joshua Root)

Milestone: MacPorts FutureMacPorts 2.10.0
Note: See TracTickets for help on using tickets.