#600 %systemd_requires macro
Closed: Fixed None Opened 3 years ago by tibbs.

Upstream systemd provides a %systemd_The current guidelines indicate that %systemd_requires should not be used. This raises some questions:

  • Why it shouldn't be used?
  • If we made a decision to forbid it, should we reconsider?
  • If it really is not to be used, why is it there? Should we override it to produce an error?

I honestly don't remember all of the reasons. The macro simply expands to:

{{{
Requires(post): systemd
Requires(preun): systemd
Requires(postun): systemd
}}}

which doesn't seem so bad to me (though I guess it might have seemed bad to me way back when).

If we don't want it, it's not difficult to override it so that we don't have to carry a patch. But if we do want to allow it, it would cut out two lines of boilerplate....


FWIW, I'm +1 for allowing %{systemd_requires} to be used.

Actually only post and preun lines are needed. Requires(postun) could be dropped. With a macro that would be easier.

IIRC the history is that there's certain things that we thought should not be done in macros. A list of those things didn't make it into the guidelines as they're really best practices for macro writers rather than best practices for spec file writers. In this case the macro author was unwilling to listen to our guidance on this so we had to be explicit in the guidelines that the macro was not to be used even though it was available.

I think the rationale for this was that for Requires(scriplet): the Requires should be visible in the spec file as it is cannot be made self-contained, it matches with the scriptlet. If there's not a Requires in the spec it makes it much harder for users to see the correspondence between the programs that are used and the Requirements. I do not believe that this was a problem for plain Requires: in macros, just for Requires(scriptlet): in macros. I believe that tibbs specifically didn't have strong feelings but was fine with banning as it only saved a few lines. Many members of the FPC are new since this was discussed so a vote today could very well be different.

If FPC does continue to ban and it's allowed for us to redefine it to an error, that would be preferable. At the time the systemd guidelines were written with the clause not to use that macro, I think we had always relied upon package maintainers who provided the macro file in the first place to change the Fedora package to provide the "right" macros and not the "wrong" macros. This was the first case I can remember where the package maintainer was unwilling to work with us so it didn't occur to us to redefine the macro elsewhere.

If you want to document this type of guideline for macro authors, the other type of macro that I can remember us outlawing was macros that crossed or defined rpm sections. (For instance, if there was a macro for running ldconfig defined as this:
{{{
%post
/sbin/ldconfig

%postun
/sbin/ldconfig
}}}

This type of macro hides the usual sections of the rpm, hides the relationship between scriptlets and the Requirements, and makes it impossible to extend the first mentioned section (we cannot add more to the %post scriptlet in this case.)

(Note that taking these two guidelines for macros together, macros that looked like this in the spec file would probably be fine:

{{{
Requires(post): %{systemd_post_requires}
Requires(preun): %{systemd_preun_requires}

%post
%{systemd_post}

%preun
%{systemd_preun}
}}}

That way Requires(scriptlet) continues to match with the existence of the %scriptlets in question. Since there's only a single requirement being added, though, this form ends up being more verbose rather than less.)

To be quite honest, as I've become more familiar with what we can do in RPM macros, I'm far less likely to object to such macroization than I would have been in the past. If it were possible to completely hide all of the scriptlets behind a macro, I'd try to cook that up. (It may be; that would take some experimentation.)

Basically I don't think packagers should need to know about any of this in the simple case. Hiding what are essentially systemd implementation details behind macros is a win in the general case.

We discussed this at this weeks meeting (http://meetbot.fedoraproject.org/fedora-meeting-1/2016-03-03/fpc.2016-03-03-17.00.txt):

  • 600 %systemd_requires macro (geppetto, 17:08:25)

  • ACTION: Don't ban systemd_requires macro, or other
    Requires(scriptlet) macros (+1:5, 0:0, -1:0) (geppetto, 17:21:23)

Note the double negative here, we are removing the ban ... so allowing them.

For fun, the following packages have been using the macro all this time:

{{{
ceph
community-mysql
fido
kde-settings
ladvd
lcdproc
lightdm
mariadb
pacemaker
perdition
resource-agents
rhnmd
rhnsd
sbd
sddm
tigervnc
openqa
}}}

And as a bunus, community-mysql and pacemaker are kind of unclear on the concept:

{{{
%{?systemd_requires: %systemd_requires}
}}}

{{{
%if %{defined systemd_requires}
%systemd_requires
%endif
}}}

Announcement text:

The ban on the use of the %systemd_requires macro has been lifted.

Metadata Update from @zbyszek:
- Issue assigned to tibbs

2 years ago

Login to comment on this ticket.

Metadata