Clarification on package review issues
 

Hello Fedora Devs,



I'm currently going through the pre-review process for adding Ingres
to the Fedora distribution
(https://bugzilla.redhat.com/show_bug.cgi?id=578024). I'm getting
some conflicting information for a couple of issues and I was hoping
for some clarification.



Firstly, "%defattr()". The standard says it should be
"%defattr(-.root,root-)" unless there is a good reason for it not to
be. The vast majority of the files included in the Ingres packages
are owned by the user 'ingres', which seems (IMHO) to be a good
reason to use "%defattr(-,ingres,ingres,-)".* One reviewer
disagreed, one agreed this was probably OK but that I should check.
What's the consensus here?




Next, use of -f in the %files section. There are over 1700 files
between the 4 RPMs and the
ownership and permission for each file is maintained by one of the
build tools. The file lists are generated as part of the build
process, not stored statically in the source.
In order to add the file lists to the SPEC files, I would need to do
full build
of the exact same source outside of RPM, generate the files list,
add them to
SPEC file and then run the RPM build. This is a fairly large over
head for each
update and makes maintenance a much larger task and much more
susceptible to human error. By using the -f flag, I can
generate the lists at build time using the existing manifest and
they will
always be correct. %files -f is also used by PostgreSQL so it's not
completely outlawed. Am I OK to use it?



Thanks in advance for your help.



J



Jay Hankinson

Emerging Technologies Team

Ingres Corp



--
devel mailing list
devel@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/devel

Clarification on package review issues
 

On 06/29/2010 08:12 PM, Jay Hankinson wrote:
> Hello Fedora Devs,
>
> I'm currently going through the pre-review process for adding Ingres to
> the Fedora distribution
> (https://bugzilla.redhat.com/show_bug.cgi?id=578024). I'm getting some
> conflicting information for a couple of issues and I was hoping for some
> clarification.
>
> Firstly, "%defattr()". The standard says it should be
> "%defattr(-.root,root-)" unless there is a good reason for it not to be.
> The vast majority of the files included in the Ingres packages are owned
> by the user 'ingres', which seems (IMHO) to be a good reason to use
> "%defattr(-,ingres,ingres,-)". One reviewer disagreed, one agreed this
> was probably OK but that I should check. What's the consensus here?

If there is a valid reason for the ingres user to own those files, then
it is correct to do so as a defattr. Just be careful. :)

> Next, use of -f in the %files section. There are over 1700 files between
> the 4 RPMs and the ownership and permission for each file is maintained
> by one of the build tools. The file lists are generated as part of the
> build process, not stored statically in the source. In order to add the
> file lists to the SPEC files, I would need to do full build of the exact
> same source outside of RPM, generate the files list, add them to SPEC
> file and then run the RPM build. This is a fairly large over head for
> each update and makes maintenance a much larger task and much more
> susceptible to human error. By using the -f flag, I can generate the
> lists at build time using the existing manifest and they will always be
> correct. %files -f is also used by PostgreSQL so it's not completely
> outlawed. Am I OK to use it?

%files -f is permitted. You might find that things end up being more
complicated, especially if you have config files or locale files, as
your file list won't handle those properly.

I tend to be of the opinion that the file lists in the spec are unlikely
to change radically for most packages, except for major revision
changes, and the fact that you can leverage tricks like:

# This entry in the %%files list causes your package to own
# /usr/share/ingres/ and all of the files and directories below it
# as found in the buildroot.
%{_datadir}/ingres/

Not to mention * being valid in %files lists. If it were me, I'd try not
to use %files -f unless I absolutely had to.

~spot
--
devel mailing list
devel@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/devel

Clarification on package review issues
 

Thanks for the quick response, spot. See below...
On 6/29/10 5:25 PM, Tom "spot" Callaway wrote:
> On 06/29/2010 08:12 PM, Jay Hankinson wrote:
>> Hello Fedora Devs,
>>
>> I'm currently going through the pre-review process for adding Ingres to
>> the Fedora distribution
>> (https://bugzilla.redhat.com/show_bug.cgi?id=578024). I'm getting some
>> conflicting information for a couple of issues and I was hoping for some
>> clarification.
>>
>> Firstly, "%defattr()". The standard says it should be
>> "%defattr(-.root,root-)" unless there is a good reason for it not to be.
>> The vast majority of the files included in the Ingres packages are owned
>> by the user 'ingres', which seems (IMHO) to be a good reason to use
>> "%defattr(-,ingres,ingres,-)". One reviewer disagreed, one agreed this
>> was probably OK but that I should check. What's the consensus here?
> If there is a valid reason for the ingres user to own those files, then
> it is correct to do so as a defattr. Just be careful. :)
Thanks, I will. :-)
>> Next, use of -f in the %files section. There are over 1700 files between
>> the 4 RPMs and the ownership and permission for each file is maintained
>> by one of the build tools. The file lists are generated as part of the
>> build process, not stored statically in the source. In order to add the
>> file lists to the SPEC files, I would need to do full build of the exact
>> same source outside of RPM, generate the files list, add them to SPEC
>> file and then run the RPM build. This is a fairly large over head for
>> each update and makes maintenance a much larger task and much more
>> susceptible to human error. By using the -f flag, I can generate the
>> lists at build time using the existing manifest and they will always be
>> correct. %files -f is also used by PostgreSQL so it's not completely
>> outlawed. Am I OK to use it?
> %files -f is permitted. You might find that things end up being more
> complicated, especially if you have config files or locale files, as
> your file list won't handle those properly.
Indeed and I've had to add a few exceptions by hand. For most of the
files though, it's actually much easier for me to use the lists
generated from the existing (non-rpm) packaging info.
> I tend to be of the opinion that the file lists in the spec are unlikely
> to change radically for most packages, except for major revision
> changes, and the fact that you can leverage tricks like:
>
> # This entry in the %%files list causes your package to own
> # /usr/share/ingres/ and all of the files and directories below it
> # as found in the buildroot.
> %{_datadir}/ingres/
>
> Not to mention * being valid in %files lists. If it were me, I'd try not
> to use %files -f unless I absolutely had to.
Unfortunately, it not really possible to split files along directory or
wildcard lines. They pretty much all install into the same locations.
> ~spot
--
devel mailing list
devel@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/devel

Clarification on package review issues
 

On Tue, Jun 29, 2010 at 05:58:42PM -0700, Jay Hankinson wrote:
> Thanks for the quick response, spot. See below...
> On 6/29/10 5:25 PM, Tom "spot" Callaway wrote:
> > On 06/29/2010 08:12 PM, Jay Hankinson wrote:
> >> Hello Fedora Devs,
> >>
> >> I'm currently going through the pre-review process for adding Ingres to
> >> the Fedora distribution
> >> (https://bugzilla.redhat.com/show_bug.cgi?id=578024). I'm getting some
> >> conflicting information for a couple of issues and I was hoping for some
> >> clarification.
> >>
> >> Firstly, "%defattr()". The standard says it should be
> >> "%defattr(-.root,root-)" unless there is a good reason for it not to be.
> >> The vast majority of the files included in the Ingres packages are owned
> >> by the user 'ingres', which seems (IMHO) to be a good reason to use
> >> "%defattr(-,ingres,ingres,-)". One reviewer disagreed, one agreed this
> >> was probably OK but that I should check. What's the consensus here?
> > If there is a valid reason for the ingres user to own those files, then
> > it is correct to do so as a defattr. Just be careful. :)
> Thanks, I will. :-)
> >> Next, use of -f in the %files section. There are over 1700 files between
> >> the 4 RPMs and the ownership and permission for each file is maintained
> >> by one of the build tools. The file lists are generated as part of the
> >> build process, not stored statically in the source. In order to add the
> >> file lists to the SPEC files, I would need to do full build of the exact
> >> same source outside of RPM, generate the files list, add them to SPEC
> >> file and then run the RPM build. This is a fairly large over head for
> >> each update and makes maintenance a much larger task and much more
> >> susceptible to human error. By using the -f flag, I can generate the
> >> lists at build time using the existing manifest and they will always be
> >> correct. %files -f is also used by PostgreSQL so it's not completely
> >> outlawed. Am I OK to use it?
> > %files -f is permitted. You might find that things end up being more
> > complicated, especially if you have config files or locale files, as
> > your file list won't handle those properly.
> Indeed and I've had to add a few exceptions by hand. For most of the
> files though, it's actually much easier for me to use the lists
> generated from the existing (non-rpm) packaging info.

One thing to watch out for here is being able to change your file lists to
use macros. ie: if your file list has the files:
/usr/bin/ingresclient
/usr/sbin/ingresserver

You'd need to modify them to have:
%{_bindir}/ingresclient
%{_sbindir}/ingresserver

Also be careful about config files and things that don't have the default
user/group. For instance, %{_bindir}/ingresclient might need to have user
and group root. So you'd need to modify the file list to have:

%attr(0755, root, root) %{_bindir}/ingresclient

A config file, /etc/ingres/server.cfg would need to be modified to read:

%config(noreplace) %{_sysconfdir}/ingres/server.cfg

Directories also need to be marked. If you can't use wildcards in most
places, you'll need to explicitly list the directories which means changing
things like /usr/share/ingres into:

%dir %attr(0755, root, root) %{_datadir}/ingres

-Toshio
--
devel mailing list
devel@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/devel