allow any posix-compliant PATCH(1)

[dimpase]

currently we require GNU patch, version at least 2.7 (from 2009).
In particular, BSD patch does not know about --no-backup-if-mismatch, something that Sage used. An equivalent, and known to BSD patch as well as GNU patch, option is --posix.

So we could use --posix and change spkg-configure to accept BSD patch too.
I am not sure what's needed to be checked, besides that --posix is accepted. Something to learn from https://savannah.gnu.org/forum/forum.php?forum_id=7361 ?

Depends on #30668

CC: @thierry-FreeBSD @orlitzky

Component: build: configure

Author: Dima Pasechnik

Branch/Commit: u/dimpase/config/acceptposixpatch @ e0fcafc

Issue created by migration from https://trac.sagemath.org/ticket/30481

[orlitzky]

comment:1

Sounds good to me. Additionally we could check to see if the system patch supports a --posix flag. If it does, we should pass it; otherwise we may simply assume that the implementation respects POSIX (and does not create backups).

[mkoeppe]

comment:2

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

[dimpase]

comment:3

need to check whether an oldish macOS patch (GNU patch 2.5.8, it says) is OK.

---

New commits:

9bfbc41	update for autoconf 2.71
2791d92	Merge branch 'u/dimpase/ticket/30668' of trac.sagemath.org:sage into u/dimpase/config/acceptposixpatch
e0fcafc	accept any POSIX-compatible patch(1)

[dimpase]

Branch: u/dimpase/config/acceptposixpatch

[dimpase]

Dependencies: #30668

[dimpase]

Author: Dima Pasechnik

[dimpase]

Commit: e0fcafc

[orlitzky]

comment:4

This will actually only find a non-POSIX patch, since the --posix option only exists in non-conforming implementations =)

I think printf "" | patch --posix might be a more reliable test of support for the --posix flag. And if --posix is not supported, we should assume that "patch" conforms, i.e.

1. Find "patch" with AC_PATH_PROG
2. Try printf "" | ${PATCH} --posix. If it works, set SAGE_PATCH_ARGS="--posix".
3. Rename build/bin/sage-apply-patches with an .in suffix and add it to AC_CONFIG_FILES
4. Set patch_args="@SAGE_PATCH_ARGS@" in build/bin/sage-apply-patches.in.

That way, if --posix is supported, we use it. Otherwise, we assume POSIX conformance.

[dimpase]

comment:5

meanwhile I am finding out that we cannot call patch
with --posix option, as it doesn't allow file creation.
So the patch in psutil spkg does not apply.

[dimpase]

comment:6

on the other hand I see no harm in just dropping --no-backup-if-mismatch as it doesn't buy us anything.

[orlitzky]

comment:7

Replying to @dimpase:

meanwhile I am finding out that we cannot call patch
with --posix option, as it doesn't allow file creation.
So the patch in psutil spkg does not apply.

What's the problem with file creation?

The --posix docs for GNU patch mention that it will become impossible to remove files (you can only empty them), but don't say anything about file creation.

It may be important: if we start to accept any "patch", then we have to be prepared for one that follows the POSIX behavior. In other words, if the patches we ship only work with GNU/BSD patch, then we should probably check for one of those and not for any POSIX-compatible patch. (In that case, we can still test for the backup flag with printf "" | patch --no-backup-if-mismatch)

[mkoeppe]

comment:8

Relaxing the spkg-configure.m4 of patch sounds to me like asking for trouble. Recall the insanity that had to be worked around in #30403?

[dimpase]

comment:9

Replying to @orlitzky:

Replying to @dimpase:

meanwhile I am finding out that we cannot call patch
with --posix option, as it doesn't allow file creation.
So the patch in psutil spkg does not apply.

What's the problem with file creation?

The --posix docs for GNU patch mention that it will become impossible to remove files (you can only empty them), but don't say anything about file creation.

I suppose it's due to the way patch --posix determines the filename to patch, and gathers it's /dev/null, oops.
(this is what GNU patch does, I didn't look at what's BSD patch does.)

It may be important: if we start to accept any "patch", then we have to be prepared for one that follows the POSIX behavior. In other words, if the patches we ship only work with GNU/BSD patch, then we should probably check for one of those and not for any POSIX-compatible patch. (In that case, we can still test for the backup flag with printf "" | patch --no-backup-if-mismatch)

I agree, some more testing is needed.

[dimpase]

comment:10

Replying to @mkoeppe:

Relaxing the spkg-configure.m4 of patch sounds to me like asking for trouble. Recall the insanity that had to be worked around in #30403?

I don't think it's so hard, it's just an executable after all.
And as a bonus we can just get rid of Sage's patch spkg, after this is done.

[mkoeppe]

comment:11

There are no problems with Sage's patch spkg, please don't create new problems

[mkoeppe]

comment:12

And until someone is sufficiently invested in a *BSD port to actually set up automatic testing, as discussed several times, let's please not use any BSD platform as motivation for changes like this.

[dimpase]

comment:13

Replying to @mkoeppe:

There are no problems with Sage's patch spkg, please don't create new problems

my primary issue with patch is that it's part of the Sage's toolchain, and as such 1st in line to be kicked out.

[mkoeppe]

comment:14

comment 11 covers this.

[dimpase]

comment:15

Replying to @mkoeppe:

comment 11 covers this.

How? I don't see why we need to keep toolchain components vendored.

[mkoeppe]

comment:16

We are not vendoring it. We are providing a fallback installation script for the benefit of users who don't have it on their machines. It is working well: It installs correctly and has not caused other maintenance issues. (That's in contrast to the problematic gcc package - which is a fallback that works sometimes but not always; and requires relatively frequent upgrades.) So, let's please not solve nonexisting problems.

[dimpase]

comment:17

so far one outcome is that --no-backup-if-mismatch may be just dropped (this option is just cosmetic), and in fact native macOS patch, which is GNU patch 2.5.8, may be used just as well.

I gather that the fallback is simply not needed, just as it's not needed for tar (tar used to be a Sage spkg).

Whether BSD patch should be allowed is another question.

Oh, and iml spkg has a patch for a non-existing file, something I found out using --posix option :-)

[dimpase]

@jhpalmieri - it seems we can just drop the requirement that the option --no-backup-if-mismatch is supported, we can accept Apple patch. The only place patch is invoked with this option is build/bin/sage-apply-patches, and this is only used in sage-spkg for applying patches supplied with spkgs. Otherwise it should work, I think.

[jhpalmieri]

Note that Apple's current version of patch supports --no-backup-if-mismatch, according to their man page (which is dated April 2022). Could we just test to see whether the system's patch supports that option and if so, use it?

Otherwise, a brief skim of the discussion here suggests that we need to test whether the version of patch supports --posix or is already posix-compliant.

[dimpase]

--posix option is meaningless for us, it's too restrictive. For me "/usr/bin/patch -h" on macOS does not show that no-backup-if... option, so we need to check on "real" files?