[Bug 226495] Merge Review: tmpwatch

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Merge Review: tmpwatch


https://bugzilla.redhat.com/show_bug.cgi?id=226495





------- Additional Comments From mitr at redhat.com  2008-01-17 21:37 EST -------
Created an attachment (id=292099)
 --> (https://bugzilla.redhat.com/attachment.cgi?id=292099&action=view)
Spec file for tmpwatch-2.9.12-3

Thank you both for the reviews.

(In reply to comment #1)
> * Parallel make:
> Append to make %{?_smp_mflags} in %{build}, unless you have a reason not to
do
> so (package does not build with parallel make).
It does not make any difference right now, because (make) runs only one command
- but future-proofing the spec file doesn't hurt.  Fixed.

> * Macro usage:
> 
> Use %{_sysconfdir} instead of /etc
I'm not sure it makes sense for /etc/cron.daily;  I understand one can override
%{_prefix} and have a special set of packages rooted e.g. under
/opt/fedora-backports, but cron.daily/tmpwatch must be in a directory that is
actually processed by the system crontab, not under /opt/fedora-backports. 
This is all hypothetical though, I'm quite willing to use the macro if you
think it is necessary.


(In reply to comment #2)
> I there's really no upstream, please make a note of that in the spec. 
> http://fedoraproject.org/wiki/Packaging/SourceURL
Added.
> If it's hosted internally to Red Hat, please consider transferring it to some

> externally visible site such as fedorahosted.org.
It is actually on elvis.  I should really move all my packages to hosted, but
there's always some more urgent work.

> Parallel make: there's only one source file so there's really no point, but
if
> you're fixin things you might as well future-proof thins.
Added.

> Then there's this rpmlint complaint:
> %{_sysconfdir} is generally preferred to /etc.
See above.

>   tmpwatch.x86_64: E: executable-marked-as-config-file
/etc/cron.daily/tmpwatch
> This one is interesting.  I'm guessing that this was marked as %config in the

> past because someone was annoyed that a package update wiped out their
changes.
>  However:
>   A shell script is a really poor configuration interface.
>   If we end up having to protect an additional directory in /tmp from
cleanup,
>    there will be issues because we can't update that script.
> 
> Would it be at all possible to move the bits that people might want to
configure
> into a file in /etc/sysconfig?  It doesn't look like it should be difficult
at all.
People "might want to configure" most of the script (it would be "reasonable"
to add at least 6 variables), and I'm afraid I don't know how to make the
script configurable in a reasonable future-proof way that wouldn't encourage
adding progressively convoluted variables every few months, and wouldn't be
horribly overengineered.

> The only licensing information I can see is in tmpwatch.c, which says only
> "under the terms of the GPL".  Isn't Red Hat developed code supposed to be
GPLv2
> only?  I'm told it's corporate policy.
Yes; I guess this was an oversight - but over time, patches from external
contributors were accepted and I don't think this would be fair to them.

I'll ask our lawyers.

> BuildRoot: is not correct; it needs to have at least %release, but would be
> better if it were it were one of the recommended values.
Fixed.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.