Add deprecated %apply_patch, provide an alternative

[hroncok]

This is how I'd deal with the removal of %apply_patch.

I opened it as draft, because I have not tested the implementation yet, it also needs tests and documentation. But before I dive into that, I'd like to know if this is acceptable.

[pmatilai]

Adding -n is fine (funny how such obvious things don't get noticed), but I don't want %apply_patch back, and testing for the error seems over the top, just document -n to override -m and -M.

[hroncok]

What about this: I'll finally shut up about %apply_patch if you'll take a backport of %autopatch -n for RPM 4.16.x and we document it as a replacement for the removal. That way, we can update our Python spec files on Fedora 33 and further and hopefully also in CentOS Stream 9. (I'd also gladly prepare downstream-only backports.)

[pmatilai]

Sorry but I'm finding the level of noise and bargaining quite excessive for the case. For an immediately backwards compatible version (for rpm >= 4.15), use %autopatch -m <num> -M <num> and wrap it up in a one-line local helper macro for convenience. -n is certainly subject for inclusion in a future 4.16 enhancement release.

[hroncok]

I am just trying to make my life easier here, nothing more. Removals have negative impact on others :(

I'll open a PR with just %autopatch -n

[hroncok]

#1673

[keszybz]

FWIW, I think that deprecation+eventual removal is the way to go in this case. If the macro was exposed, but not actually used, it'd be OK to quickly yank it out. But clearly it is in use, and we know that the removal breaks packages that are out there. I appreciate the desire to make a clean break, but rpm is a foundational package that needs to preserve backwards compatibility.

[pmatilai]

As usual, a bit of AFK gave some helpful perspective: -n is actually a rather strange corner-case to be considering. What makes more sense is have %autopatch accept arguments, that way it's not limited to one-at-a-time special fubar and can be made to handle both paths and numbers. I'll try to supply a PR implementing that tomorrow.

[pmatilai]

So... it all ends up being rather cumbersome because -m and -M are a cumbersome way of specifying ranges, but now we're kinda stuck with them. Me thinks it'd make a whole lot more sense to have ranges expressable in -5, 50-70 100- style (multiple) arguments, in addition to individual patch numbers. Then the -m/-M thing can be handled as a special case of that and deprecated.

All of which is to say, I don't have that PR yet, but working on it.

[hroncok]

I wonder if -5 would not be recognized as an option

[pmatilai]

That's a good point, it'd require "escaping" with -- and that gets ugly. Python-style :5 as a range syntax wouldn't have that problem, but I wonder if that clashes with something else in turn...

[hroncok]

Note that Python ranges don't include the ending number. If you use them please keep the semantics to avoid confusion for Pythonistas.

[pmatilai]

Um? Maybe "range" in Python has some special meaning I'm not aware of, I guess in Python lingo what I meant is slice syntax, which can have all three variants: :5 1:5 and 5: the last I looked.

[pmatilai]

Hmm, but then we can nowadays have macros opt out of option processing. Already forgotten that... (f951643)

[hroncok]

Sorry, I've meant that ranges/slices include start but not the end. I.e. 1:3 is 1, 2. Unlike %autopatch -m 1 -M 3 which is 1, 2, 3.

Hmm, but then we can nowadays have macros opt out of option processing.

Yes, but that way, no backports would be possible.

[pmatilai]

Sorry, I've meant that ranges/slices include start but not the end. I.e. 1:3 is 1, 2. Unlike %autopatch -m 1 -M 3 which is 1, 2, 3.

Oh, that. Yeah I ran into it in Python just recently, caused some head-scratching before realizing that's how it's supposed to work. So we'd confuse somebody no matter which behavior was used...

[pmatilai]

The %apply_patch ship sailed already and resurrecting it at this point would only add to the mess, closing.

Thanks for the feedback + ideas though!