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

Turn %prep into a normal build script #2205

Closed
ffesti opened this issue Sep 26, 2022 · 8 comments · Fixed by #2730
Closed

Turn %prep into a normal build script #2205

ffesti opened this issue Sep 26, 2022 · 8 comments · Fixed by #2730
Assignees
Labels
handsfree Packaging automation and convenience
Milestone

Comments

@ffesti
Copy link
Contributor

ffesti commented Sep 26, 2022

%prep is very special. It is first parsed completely and only then are %setup and %patch replaced. While these two need to do special things they should still be internal macros that are processed right when they are encountered.

One side effect of this is #1870

@pmatilai
Copy link
Member

There are some "interesting" gotchas in this direction. I have draft patches to turn %patch and %setup into actual macros so they get processed with the normal spec readLine() machinery but as long as the macro engine cannot handle multiple options of the same type, this is a no-go. The other, ahem, specialty is that %patch1 style syntax will forever need pre-processing to turn it into %patch 1 basically. Until those two are eliminated, %prep will require that very special processing.

@voxik
Copy link
Contributor

voxik commented Sep 27, 2022

%patch1 style syntax will forever need pre-processing to turn it into %patch 1 basically.

👀 This is new to me. Could there be warning that %patch1 is obsolete?

@pmatilai
Copy link
Member

pmatilai commented Sep 28, 2022

Yes, and there will be. I actually planned one for 4.18 already but withdrew it, didn't have the energy to fight that battle just now 😅

For some stats, out of 22000+ packages in Fedora, about 6300 have Patch declarations, of which roughly 3500 are still using the %patch1 style syntax. and a whopping 25 are using other variants, including defaulting to 0 and using a combo of -P and positional arguments etc. The rest are presumably using %autosetup (~8700 packages)

@ferdnyc
Copy link
Contributor

ferdnyc commented Oct 31, 2022

Doesn't help that the only example (end of that section) of %patch usage that I can find in the Fedora Packaging Guidelines still uses %patch0. Seems this memo really hasn't been distributed widely enough. (News to me, as well.)

Guidelines examples can be corrected easily enough, of course, if the updated syntax is universally preferred/correct. How long has e.g. %patch 0 worked in RPM, relative to how long %patch0 has been legal? Is there any extreme legacy use case or whatever reason that doing the equivalent of s/%patch([0-9]+)/%patch $1/ across the board would be a bad idea? (I'm assuming %patch0 -p1, like in that FPG example, is canonically supposed to be %patch 0 -p1?)

@ferdnyc
Copy link
Contributor

ferdnyc commented Oct 31, 2022

For some stats, out of 22000+ packages in Fedora, about 6300 have Patch declarations, of which roughly 3500 are still using the %patch1 style syntax. and a whopping 25 are using other variants, including defaulting to 0 and using a combo of -P and positional arguments etc. The rest are presumably using %autosetup (~8700 packages)

Hey, wait... those numbers don't add up. If there are 6300 PatchN-packages, and 3525 use %patchN or %patch N, then at most 2775 are using %autosetup.

Or is ~8700 the total number of %autosetup packages, including those with no PatchN declarations?

(If so, that's also depressing: After all this time, only 39.54% of packages are using %autosetup? I know packages using conditional patching can't use it, but at most that's the other 2775 with Patch declarations.)

@pmatilai
Copy link
Member

pmatilai commented Nov 1, 2022

The %autosetup number is indeed total packages using it, whether they currently have patches or not (because that's a situation that can and does change release by release). This ticket is about %prep though, lets keep the %patch discussion in #2209 (the compat info is already there btw)

pmatilai added a commit to pmatilai/rpm that referenced this issue Jan 26, 2023
Instead of treating `%patch` with no options and errors the same as
`%patch 0`, just raise an error. There will be folks who have missed the
warning-only phase, but we don't want to wait another ten years to finally
rid ourselves of these legacy quirks.

Related: rpm-software-management#2205
ffesti pushed a commit that referenced this issue Jan 26, 2023
Instead of treating `%patch` with no options and errors the same as
`%patch 0`, just raise an error. There will be folks who have missed the
warning-only phase, but we don't want to wait another ten years to finally
rid ourselves of these legacy quirks.

Related: #2205
@pmatilai pmatilai added the handsfree Packaging automation and convenience label Sep 14, 2023
@pmatilai
Copy link
Member

(this is "handsfree" item because the aforementioned, ah, specialty prevents all sorts of interesting developements)

@jengelh
Copy link
Contributor

jengelh commented Sep 20, 2023

Hey, wait... those numbers don't add up. If there are 6300 PatchN-packages, and 3525 use %patchN or %patch N, then at most 2775 are using %autosetup.

It is possible to utilize both %autosetup and %patch (admittedly somewhat unusual):

%autosetup
%if 0%{?somecond}
%patch93 -R
%endif

pmatilai added a commit to pmatilai/rpm that referenced this issue Oct 20, 2023
Allows %build, %install, %check, %clean and %generate_depends to be
augmented arbitrary number of times by appending or prepending to them.
The main use-case is to support automatic population of these sections
(declarative builds) while still allowing packagers to easily tweak them,
but could also be useful for complicated conditional specs and such.

%prep is missing from the list because it's way too special at the
moment, it'll gain this capability when rpm-software-management#2205 is fixed.

Related: rpm-software-management#1087
pmatilai added a commit to pmatilai/rpm that referenced this issue Oct 20, 2023
Allows %build, %install, %check, %clean and %generate_depends to be
augmented arbitrary number of times by appending or prepending to them.
The main use-case is to support automatic population of these sections
(declarative builds) while still allowing packagers to easily tweak them,
but could also be useful for complicated conditional specs and such.

%prep is missing from the list because it's way too special at the
moment, it'll gain this capability when rpm-software-management#2205 is fixed.

Related: rpm-software-management#1087
pmatilai added a commit to pmatilai/rpm that referenced this issue Oct 23, 2023
Now that we can, implement %setup and %patch as auxiliary macros that are
defined only during %prep parsing. And voila, %prep is no longer special
at all. Not much anyhow.

Notably rpmspec --parse now emits %build where it belongs.
The error message on %patchN is now very much an ugly hack, but at least
it's something we can drop one sunny day.

Fixes: rpm-software-management#2205
@pmatilai pmatilai self-assigned this Oct 23, 2023
@pmatilai pmatilai added this to the 4.20.0 milestone Oct 23, 2023
pmatilai added a commit that referenced this issue Oct 27, 2023
Now that we can, implement %setup and %patch as auxiliary macros that are
defined only during %prep parsing. And voila, %prep is no longer special
at all. Not much anyhow.

Notably rpmspec --parse now emits %build where it belongs.
The error message on %patchN is now very much an ugly hack, but at least
it's something we can drop one sunny day.

Fixes: #2205
dmnks pushed a commit to dmnks/rpm that referenced this issue Aug 8, 2024
Instead of treating `%patch` with no options and errors the same as
`%patch 0`, just raise an error. There will be folks who have missed the
warning-only phase, but we don't want to wait another ten years to finally
rid ourselves of these legacy quirks.

Related: rpm-software-management#2205
(cherry picked from commit b3adc8c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
handsfree Packaging automation and convenience
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants