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

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



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 -


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