Removing %clean (Was Re: Agenda for the 2009-05-26 Packaging Committee meeting)

Panu Matilainen pmatilai at laiskiainen.org
Sat May 30 08:15:47 UTC 2009


On Tue, 26 May 2009, Tom \spot\ Callaway wrote:

> On 05/26/2009 04:10 AM, Richard W.M. Jones wrote:
>> I vote for also removing the %clean section.
>
> So, looking at this objectively, here are the technical problems:
>
> * We're defining a BuildRoot in the spec, but that definition is no
> longer used (Fedora 10 or higher), because rpm now automagically sets it
> for us.
> * We're typing rm -rf %{buildroot} multiple times in every single Fedora
> specfile. We want to invoke it twice:
> - As the very first operation of the %install scriptlet
> - As the very first operation of the %clean scriptlet
>
> The concerns around removing the BuildRoot from the spec is that if that
> spec is taken to an older system, the spec would either
> * Not work
> * Set the BuildRoot to / and cause massive system destruction
>
> The good news is that for all the Fedora targets that we care about, if
> BuildRoot is unset, it is just unset. It never gets set to / on anything
> we care about (including RHEL 4 and 5, I checked).
> And I really don't think we need to care about anything older than RHEL
> 4 (roughly equivalent to Fedora 6). A comment in the packaging
> guidelines should be sufficient, so simply dropping the unnecessary
> BuildRoot definition as soon as Fedora 9 is EOL seems like a sane course
> of action.
>
> The %install scriptlet case is reasonably simple to solve with
> redhat-rpm-config's customized macros file, simply add:
>
> %__spec_install_pre %{___build_pre}\
>    [ "$RPM_BUILD_ROOT" != "/" ] && rm -rf "${RPM_BUILD_ROOT}"\
>    mkdir -p `dirname "$RPM_BUILD_ROOT"`\
>    mkdir "$RPM_BUILD_ROOT"\
> %{nil}
>
> This ensures that every time %install is invoked in a spec file, it
> checks that buildroot isn't / (which, it should never ever be, but given
> the past history, doesn't hurt to check), then deletes it. Next, it
> makes the basedir of $RPM_BUILD_ROOT, in case that doesn't exist, then
> makes the buildroot for us, saving additional pain in some Fedora spec
> files where the make install process is either too dumb to do this for
> us (or non-existant).

Yup, %install part is mostly a no-brainer. While at it...
redhat-rpm-config redefines %install, %build and some others as macros 
which has some strange/unwanted side-effects. Switching these to use 
templates instead wouldn't hurt.

> The %clean scriptlet case is harder. Lets get the easy case out of the
> way, removing the obligatory rm -rf %{buildroot} invocation:
>
> %__spec_clean_pre %{___build_pre}\
>    [ "$RPM_BUILD_ROOT" != "/" ] && rm -rf "${RPM_BUILD_ROOT}"\
> %{nil}
>
> With that, every time %clean is invoked in a spec file, it checks that
> buildroot isn't /, and then deletes it if it is not.
>
> However, that doesn't really resolve the deeper desire, which is as
> Richard points out, is to remove the need to explicitly state the %clean
> section at all. If we need to overload it beyond its defaults, we should
> be able to invoke it manually and append to it, but if it is not set, it
> should be invoked automagically.
>
> RPM doesn't act this way. For all scriptlets, their absence does not
> result in automatic invocation (there is no idea of "always executed"
> default scriptlets in a spec), but instead is how they are omitted. I
> can certainly see valid use cases where a package would not want %clean
> to be invoked. It might be possible to change RPM's behavior to
> introduce a disabler mechanism for default "always executed" scriptlets:
>
> <pseudocode>
> %disable %check
> </pseudocode>
>
> This would be a significant behavior change for RPM, and not something
> we could do with distribution specific macro tweaks. It would also break
> backwards compatibility with older RPM spec files, which has
> traditionally been avoided.

Another, perhaps simpler alternative would be making rpm inject default 
%clean action when spec doesn't define one. To disable/customize the 
default behavior, you'd just add an empty (or otherwise custom) %clean in 
the spec, no special disabler logic required.

It is of course a change of behavior in rpm but allows getting rid of the 
%clean section in 99% of specs and permits backwards compatibility too: if 
you want to have %clean do (or not do) whatever you want, you just keep 
the %clean section in the spec. It'd make those special cases stand out 
clearly too, all you have to do is grep for %clean.

>
> *****
>
> So, given those facts, and assuming that RPM isn't changing its
> behaviors anytime soon, we can make macro changes in redhat-rpm-config
> and change from this:
>
> BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
> ...
>
> %install
> rm -rf %{buildroot}
> make DESTDIR=%{buildroot} install
> ...
>
> %clean
> rm -rf %{buildroot}
> ...
>
> TO:
>
> ...
> %install
> make DESTDIR=%{buildroot} install
> ...
> %clean
> ...
>
> Is anyone opposed to that?

No objections to BuildRoot and %install parts, but I'd suggest leaving 
%clean out of it for the time being, as this is on direct collision 
course with the above suggestion of built-in default %clean.

 	- Panu -




More information about the fedora-devel-list mailing list