Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFE: support conditional patch application in %autosetup #3110

Open
pmatilai opened this issue May 20, 2024 · 3 comments
Open

RFE: support conditional patch application in %autosetup #3110

pmatilai opened this issue May 20, 2024 · 3 comments
Labels
design Complicated design issue RFE

Comments

@pmatilai
Copy link
Member

pmatilai commented May 20, 2024

Currently %autosetup only supports conditional patch application by either conditionalizing the patch declarations (which leads to inconsistent src.rpms). The other alternative is falling back to %autosetup -N and manual patch application with (ranged) %autopatch and/or plain %patch, both of which are error-prone.

While ideally you don't carry conditional patches, real world often disagrees. We need a better way to handle this, something that removes the need for humans to deal with individual patch numbers while preserving them all in the src.rpm. One possible idea could be supporting multiple (named) %patchlist sections that are generally applied in the order they appear in the spec, but allow conditionalizing by the name/keyword. Something like %patchlist -k s390 whose patches are only applied if %bcond s390 is true.

@pmatilai pmatilai added the RFE label May 20, 2024
@pmatilai pmatilai added the design Complicated design issue label May 23, 2024
@berolinux
Copy link
Contributor

Adding the use case that was the reason for #3109: It would be good to have a way to apply different groups of patches at different times in the spec file, and in different locations.

For example, when building something that insists on bundling internal modified copies of various libraries (e.g. chromium -- and yes I agree they should be sent to hell for internalizing half an OS) and keeping patches in sync with the corresponding system libraries, or when dealing with something like wine-staging (where it may be necessary to apply some patches only after the staging script has run).

e.g. in wine-staging

%autosetup -p1
[regular patches are applied]
./staging/patchinstall.py
%autopatch -p1 -k staging
[patches specific to -staging applied now]

or for a chromium-like example

%autosetup -p1
[regular patches are applied]
cd third_party/libevent
%autopatch -p1 -k  libevent
[libevent patches are applied]

Of course in the latter example, you could also modify the patch files to prefix third_party/libevent to all the files being patched, but that would make it harder to keep the patches in sync with the system libevent package.

@pmatilai
Copy link
Member Author

pmatilai commented May 27, 2024

Oh absolutely. The default behavior of %autosetup should be some "just make it so" thing, but underneath itm a -k(eyword) argument to %autopatch seems like a nice way to achieve it.

Amusingly I just realized we already support multiple %autopatch sections from the start, I'd just totally forgotten about it.

@pmatilai
Copy link
Member Author

Thinking along these lines, we could maybe use Patch(<keyword>): foo.patch syntax for independent patches, and %patchlist -k <keyword> for patchlists. Then this basically becomes just a one thing to store alongside the patch itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Complicated design issue RFE
Projects
Status: Backlog
Development

No branches or pull requests

2 participants