[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

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"\

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).

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}"\

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:

%disable %check

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.


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)

rm -rf %{buildroot}
make DESTDIR=%{buildroot} install

rm -rf %{buildroot}


make DESTDIR=%{buildroot} install

Is anyone opposed to that?


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]