7:09 a.m.
Hi,
I am sometimes reviewing spec files and I very often see common mistakes. I mean in
packages which are already in
Fedora. For a long time and they have some dust from past times.
I am not going to file bug reports as those are not bugs. I will just point it here and
leave it up to you to check your
spec files:
* Group: System Environment/Base
Please remove it. Group was intended for something (sort apps in menus), but it never
actually worked. It was required
for EL5 packages. Since EL6 it can be omitted. And nowadays it is recommended to remove it
completely.
* rm -rf $RPM_BUILD_ROOT
In the past, it was necessary to clean the buildroot at the beginning of %install and the
end of %clean. This is no
longer true and not needed since F12.
* %defattr(-,root,root,-)
If you have this at the top of your %files section, then you can safely remove it. This is
default since rpm 4.2, so it
is not needed even for RHEL5.
* Buildroot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
RPM define builroot variable since F12 (and EL6). There is no need to define it yourself.
Miroslav
7:49 a.m.
Miroslav thank you for the hints, I will check my packages, but I think
Igor Gnatenko already removed such stuff because he made a quick review
of them.
I would also say that we should increase the usage of *comments* in spec
files because they are very useful for new packagers
8:53 a.m.
On Thu, Feb 8, 2018 at 9:49 AM, Brett Lentz <blentz(a)redhat.com> wrote:
On 08/02/18 14:09 +0100, Miroslav Suchý wrote:
>
>
> * rm -rf $RPM_BUILD_ROOT
>
rpmdev-newspec still inserts this. It may be worthwhile to file a bug to get
it to stop.
The only reason I haven't dropped it yet is because SLE 11 still is
supported, and it requires it.
I could see into adding some magic into removing it when newer rpm is
detected, but I'm not sure it's worth it for a single line.
--
真実はいつも一つ！/ Always, there's only one truth!
10:08 a.m.
On Thu, Feb 8, 2018 at 9:53 AM, Neal Gompa <ngompa13(a)gmail.com> wrote:
On Thu, Feb 8, 2018 at 9:49 AM, Brett Lentz <blentz(a)redhat.com>
wrote:
> On 08/02/18 14:09 +0100, Miroslav Suchý wrote:
>>
>>
>> * rm -rf $RPM_BUILD_ROOT
>>
>
> rpmdev-newspec still inserts this. It may be worthwhile to file a bug to get
> it to stop.
>
The only reason I haven't dropped it yet is because SLE 11 still is
supported, and it requires it.
I could see into adding some magic into removing it when newer rpm is
detected, but I'm not sure it's worth it for a single line.
Well... personally, it's pretty annoying to have to remove this from
every spec I generate using rpmdev-newspec. It also gives the
impression that the newspec-generated specs might be outdated in other
respects too.
I don't know how other people feel, though.
Ben Rosser
10:45 a.m.
On Thu, Feb 08, 2018 at 09:53:01AM -0500, Neal Gompa wrote:
The only reason I haven't dropped it yet is because SLE 11 still
is
supported, and it requires it.
I could see into adding some magic into removing it when newer rpm is
detected, but I'm not sure it's worth it for a single line.
Isn't the framework for this already in the tool with --rpm-version?
--
Matthew Miller
<mattdm(a)fedoraproject.org>
Fedora Project Leader
3:08 a.m.
On 02/08/2018 04:53 PM, Neal Gompa wrote:
On Thu, Feb 8, 2018 at 9:49 AM, Brett Lentz <blentz(a)redhat.com>
wrote:
> On 08/02/18 14:09 +0100, Miroslav Suchý wrote:
>>
>>
>> * rm -rf $RPM_BUILD_ROOT
>>
>
> rpmdev-newspec still inserts this. It may be worthwhile to file a bug to get
> it to stop.
>
The only reason I haven't dropped it yet is because SLE 11 still is
supported, and it requires it.
Does it, really? IIRC Suse started doing this long long before we did -
we basically copied it from them:
%__spec_install_pre %{___build_pre}\
[ "$RPM_BUILD_ROOT" != "/" ] && rm -rf
"${RPM_BUILD_ROOT}"\
mkdir -p `dirname "$RPM_BUILD_ROOT"`\
mkdir "$RPM_BUILD_ROOT"\
%{nil}
I could see into adding some magic into removing it when newer rpm is
detected, but I'm not sure it's worth it for a single line.
That single line is not just entirely harmless junk, it inserts an
insecurity into the picture which the above _install_pre snippet fixes,
and adding sections that might not even be needed can have other
unwanted side-effects, witness
https://bugzilla.redhat.com/show_bug.cgi?id=1542743 where a package
actually fails to build because there's a bogus/redundant %install
section containing that one line.
So please, remove it. If SLE 11 really requires it then handle it that
old dog specially. It is worth it.
- Panu -
7:32 a.m.
On Fri, Feb 9, 2018 at 4:08 AM, Panu Matilainen <pmatilai(a)redhat.com> wrote:
On 02/08/2018 04:53 PM, Neal Gompa wrote:
>
> On Thu, Feb 8, 2018 at 9:49 AM, Brett Lentz <blentz(a)redhat.com> wrote:
>>
>> On 08/02/18 14:09 +0100, Miroslav Suchý wrote:
>>>
>>>
>>>
>>> * rm -rf $RPM_BUILD_ROOT
>>>
>>
>> rpmdev-newspec still inserts this. It may be worthwhile to file a bug to
>> get
>> it to stop.
>>
>
> The only reason I haven't dropped it yet is because SLE 11 still is
> supported, and it requires it.
Does it, really? IIRC Suse started doing this long long before we did - we
basically copied it from them:
%__spec_install_pre %{___build_pre}\
[ "$RPM_BUILD_ROOT" != "/" ] && rm -rf
"${RPM_BUILD_ROOT}"\
mkdir -p `dirname "$RPM_BUILD_ROOT"`\
mkdir "$RPM_BUILD_ROOT"\
%{nil}
>
> I could see into adding some magic into removing it when newer rpm is
> detected, but I'm not sure it's worth it for a single line.
That single line is not just entirely harmless junk, it inserts an
insecurity into the picture which the above _install_pre snippet fixes, and
adding sections that might not even be needed can have other unwanted
side-effects, witness https://bugzilla.redhat.com/show_bug.cgi?id=1542743
where a package actually fails to build because there's a bogus/redundant
%install section containing that one line.
So please, remove it. If SLE 11 really requires it then handle it that old
dog specially. It is worth it.
Alright, I'll double check and remove it accordingly.
--
真実はいつも一つ！/ Always, there's only one truth!
9:03 a.m.
On Thursday, February 8, 2018 2:09:07 PM CET Miroslav Suchý wrote:
Hi,
I am sometimes reviewing spec files and I very often see common mistakes.
The issues you are mentioning below hardly classify as mistakes in my view.
I mean in packages which are already in
Fedora. For a long time and they
have some dust from past times.
I am not going to file bug reports as those are not bugs. I will just point
it here and leave it up to you to check your
spec files:
* Group: System Environment/Base
Please remove it. Group was intended for something (sort apps in menus), but
it never actually worked. It was required
for EL5 packages. Since EL6 it
can be omitted. And nowadays it is recommended to remove it completely.
* rm -rf $RPM_BUILD_ROOT
In the past, it was necessary to clean the buildroot at the beginning of
%install and the end of %clean. This is no
longer true and not needed
since F12.
* %defattr(-,root,root,-)
If you have this at the top of your %files section, then you can safely
remove it. This is default since rpm 4.2, so it
is not needed even for
RHEL5.
* Buildroot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
RPM define builroot variable since F12 (and EL6). There is no need to define
it yourself.
While all the above things are safe to be removed in Fedora, they are pretty
much harmless. I would not spend too much energy to remove them manually in
each single package.
There might be valid reasons for the old stuff appearing in _some_ spec files
beyond your knowledge, for example specfile maintained by upstream, usable not
only by Fedora.
I would rather suggest to spend time on fixing spec file issues that may cause
real-world problems, such as C/C++ package being built in %install instead of
%build, or unescaped RPM macros in %changelog:
https://bugzilla.redhat.com/buglist.cgi?classification=Fedora&list_id...
Kamil
> Miroslav
9:21 a.m.
On 8 February 2018 at 15:03, Kamil Dudka <kdudka(a)redhat.com> wrote:
[..]
There might be valid reasons for the old stuff appearing in _some_
spec
files
beyond your knowledge, for example specfile maintained by upstream, usable
not
only by Fedora.
Theoretically you may be right. In practice .. nope.
There is no any reasons to use in Fedora spec file which is not
readable/simple as it is only possible because someone who is not focused
on Fedora want to make it universal without testing it on all possible
distributions on every new version released.
kloczek
--
Tomasz Kłoczko | LinkedIn: *http://lnkd.in/FXPWxH <http://lnkd.in/FXPWxH>*
9:39 a.m.
On Thursday, February 8, 2018 4:21:53 PM CET Tomasz Kłoczko wrote:
On 8 February 2018 at 15:03, Kamil Dudka <kdudka(a)redhat.com>
wrote:
[..]
> There might be valid reasons for the old stuff appearing in _some_ spec
> files
> beyond your knowledge, for example specfile maintained by upstream, usable
> not
> only by Fedora.
Theoretically you may be right. In practice .. nope.
There is no any reasons to use in Fedora spec file which is not
readable/simple as it is only possible because someone who is not focused
on Fedora want to make it universal without testing it on all possible
distributions on every new version released.
kloczek
For example logrotate upstream maintains a spec file that is regularly updated
and CI-tested by Travis:
https://github.com/logrotate/logrotate/commits/master/logrotate.spec.in
Spec files of csdiff, cscppc, csmock, and cswrap are produced by make-srpm.sh
maintained in the upstream git repositories.
Kamil
9:45 a.m.
Dne 8.2.2018 v 16:39 Kamil Dudka napsal(a):
On Thursday, February 8, 2018 4:21:53 PM CET Tomasz Kłoczko wrote:
> On 8 February 2018 at 15:03, Kamil Dudka <kdudka(a)redhat.com> wrote:
> [..]
>
>> There might be valid reasons for the old stuff appearing in _some_ spec
>> files
>> beyond your knowledge, for example specfile maintained by upstream, usable
>> not
>> only by Fedora.
> Theoretically you may be right. In practice .. nope.
> There is no any reasons to use in Fedora spec file which is not
> readable/simple as it is only possible because someone who is not focused
> on Fedora want to make it universal without testing it on all possible
> distributions on every new version released.
>
> kloczek
For example logrotate upstream maintains a spec file that is regularly updated
and CI-tested by Travis:
https://github.com/logrotate/logrotate/commits/master/logrotate.spec.in
Spec files of csdiff, cscppc, csmock, and cswrap are produced by make-srpm.sh
maintained in the upstream git repositories.
https://fedoraproject.org/wiki/Packaging:Guidelines#Spec_Maintenance_and_...
Not saying it contradicts the guideline above, just FYI.
V.
10:05 a.m.
On Thu, Feb 8, 2018 at 10:45 AM, Vít Ondruch <vondruch(a)redhat.com> wrote:
Dne 8.2.2018 v 16:39 Kamil Dudka napsal(a):
> On Thursday, February 8, 2018 4:21:53 PM CET Tomasz Kłoczko wrote:
>> On 8 February 2018 at 15:03, Kamil Dudka <kdudka(a)redhat.com> wrote:
>> [..]
>>
>>> There might be valid reasons for the old stuff appearing in _some_ spec
>>> files
>>> beyond your knowledge, for example specfile maintained by upstream, usable
>>> not
>>> only by Fedora.
>> Theoretically you may be right. In practice .. nope.
>> There is no any reasons to use in Fedora spec file which is not
>> readable/simple as it is only possible because someone who is not focused
>> on Fedora want to make it universal without testing it on all possible
>> distributions on every new version released.
>>
>> kloczek
> For example logrotate upstream maintains a spec file that is regularly updated
> and CI-tested by Travis:
>
> https://github.com/logrotate/logrotate/commits/master/logrotate.spec.in
>
> Spec files of csdiff, cscppc, csmock, and cswrap are produced by make-srpm.sh
> maintained in the upstream git repositories.
https://fedoraproject.org/wiki/Packaging:Guidelines#Spec_Maintenance_and_...
Not saying it contradicts the guideline above, just FYI.
In practice, there are several projects that blatantly ignore this.
Off the top of my head:
* OpenStack libraries and client packages (RDO)
* libvirt stack packages (excluding php-libvirt, as I maintain that
and have no rights to the libvirt-php repo)
* Cockpit packages
There's some hand-waviness with a few that I know of:
* Many PHP stack packages maintained by Remi (remirepo<->fedora)
* Compiler/toolchain packages (SCLized for non-Fedora)
I'm somewhat doing this with one package:
* snapd (Fedora is the canonical location, but changes are synced
between upstream and Fedora regularly for upstream CI support)
--
真実はいつも一つ！/ Always, there's only one truth!
10:16 a.m.
On Thu, Feb 08, 2018 at 11:05:38AM -0500, Neal Gompa wrote:
On Thu, Feb 8, 2018 at 10:45 AM, Vít Ondruch
<vondruch(a)redhat.com> wrote:
> https://fedoraproject.org/wiki/Packaging:Guidelines#Spec_Maintenance_and_...
>
> Not saying it contradicts the guideline above, just FYI.
>
In practice, there are several projects that blatantly ignore this.
Off the top of my head:
* libvirt stack packages (excluding php-libvirt, as I maintain that
and have no rights to the libvirt-php repo)
Claiming we ignore the guideline is wrong. As the primary package
maintainer, I do consider upstream repos to have the canonical
copy of the spec file, but do *not* just blindly overwrite changes
made in Fedora. If someone proposes a cleanup to the Fedora libvirt
related specs, we'll encourage them to send that as a patch to upstream.
If they don't, and just make the change to Fedora specs, we'll none the
less apply it to upstream copy ourselves, assuming it was functionally
correct. When updating to new libvirt releases, I'll do a diff between
upstream and Fedora specs to make sure I don't mistakenly kill some
change a Fedora provenpackager made that I missed upstream. This all
works very well and ensures Fedora packages get their spec updated
accurately when upstream releases come with new features that impact
on packaging.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
10:24 a.m.
On 8 February 2018 at 15:39, Kamil Dudka <kdudka(a)redhat.com> wrote:
[..]
For example logrotate upstream maintains a spec file that is
regularly
updated
and CI-tested by Travis:
https://github.com/logrotate/logrotate/commits/master/logrotate.spec.in
OK. Please compare what is one that URL with
https://src.fedoraproject.org/rpms/logrotate/raw/master/f/logrotate.spec
Just try to use command like:
$ diff -u <(curl -s
https://src.fedoraproject.org/rpms/logrotate/raw/master/f/logrotate.spec)
<(curl -s
https://raw.githubusercontent.com/logrotate/logrotate/master/logrotate.sp...)
| less
Additionally to compare this spec.in with spec used in SuSE, altlinux and
few other ..
Spec files of csdiff, cscppc, csmock, and cswrap are produced by
make-srpm.sh
maintained in the upstream git repositories.
In such cases you will be manually adding %changelog.
In all those cases we are talking about so simple packages that one time
written correctly spec file can be used for years.
Looks like someone who found idea make-srpm.sh scripts didn't know that if
in source tar ball will be included <package>.spec it will be possible to
build such package by
$ rpmbuild -ta <package>.tar.gz
In other words instead including such script should be included spec
generated on "make dist-check" when tar ball with new version will be
created by maintainer.
Nevertheless logrote and other packages which you've mentioned by you
already have own history of changes in Fedora and have as well higher
release numbers (all of them).
It will be really better if source tree maintainers will be only focused on
maintaining such tree and will leave packaging to the people who are
maintaining packages in different OSes (what about *BSD,
Solaris/OpenIdiana?) and OSes distributions (what about Debian, OpenWRT/Lede
.. ?).
Usually effort like in examples which you gave are pure waste of time
because maintaining packages it is not the same as maintaining source tree
of exact package.
On both areas is required quite different knowledge.
kloczek
--
Tomasz Kłoczko | LinkedIn: *http://lnkd.in/FXPWxH <http://lnkd.in/FXPWxH>*
Kamil
9:42 a.m.
Tomasz Kłoczko wrote:
On 8 February 2018 at 15:03, Kamil Dudka <kdudka(a)redhat.com
<mailto:kdudka@redhat.com>> wrote:
[..]
There might be valid reasons for the old stuff appearing in _some_
spec files
beyond your knowledge, for example specfile maintained by upstream,
usable not
only by Fedora.
Theoretically you may be right. In practice .. nope.
On what do you base your assertion? Are you saying that nobody does this?
There is no any reasons to use in Fedora spec file which is not
readable/simple as it is only possible because someone who is not
focused on Fedora want to make it universal without testing it on all
possible distributions on every new version released.
Or it may be due to complex
spec files that are difficult to diff
between releases so maintaining them in one place (upstream) is far more
efficient and less error-prone.
rob
1:53 a.m.
Dne 8.2.2018 v 16:03 Kamil Dudka napsal(a):
There might be valid reasons for the old stuff appearing in _some_
spec files
beyond your knowledge, for example specfile maintained by upstream, usable not
only by Fedora.
AFAIK all mentioned parts are not needed in Fedora and all supported RHELs. I would be
really surprised if any
distribution requires it.
they are pretty much harmless.
Technically it is harmless.
But a lot of newbies and 3rd party developers take Fedora packages as example for *their*
packages. And they copy those
bits over and over...
Miroslav
9:40 p.m.
On Thu, Feb 8, 2018 at 9:09 PM, Miroslav Suchý <msuchy(a)redhat.com> wrote:
Hi,
I am sometimes reviewing spec files and I very often see common mistakes. I mean in
packages which are already in
Fedora. For a long time and they have some dust from past times.
I am not going to file bug reports as those are not bugs. I will just point it here and
leave it up to you to check your
spec files:
* Group: System Environment/Base
Please remove it. Group was intended for something (sort apps in menus), but it never
actually worked. It was required
for EL5 packages. Since EL6 it can be omitted. And nowadays it is recommended to remove
it completely.
* rm -rf $RPM_BUILD_ROOT
In the past, it was necessary to clean the buildroot at the beginning of %install and the
end of %clean. This is no
longer true and not needed since F12.
Not needed in EL6 as well?
* %defattr(-,root,root,-)
If you have this at the top of your %files section, then you can safely remove it. This
is default since rpm 4.2, so it
is not needed even for RHEL5.
* Buildroot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
RPM define builroot variable since F12 (and EL6). There is no need to define it
yourself.
Miroslav
_______________________________________________
devel mailing list -- devel(a)lists.fedoraproject.org
To unsubscribe send an email to devel-leave(a)lists.fedoraproject.org
--
Regards,
Chenxiong Qi
10 p.m.
Chenxiong Qi wrote:
On Thu, Feb 8, 2018 at 9:09 PM, Miroslav Suchý
<msuchy(a)redhat.com> wrote:
> * rm -rf $RPM_BUILD_ROOT
>
> In the past, it was necessary to clean the buildroot at
> the beginning of %install and the end of %clean. This is
> no longer true and not needed since F12.
Not needed in EL6 as well?
I don't think so.
redhat-rpm-config-9.0.3-51.el6.centos has the following
code in /usr/lib/rpm/redhat/macros:
#---------------------------------------------------------------------
# Expanded at beginning of %install scriptlet.
#
%__spec_install_pre %{___build_pre}\
[ "$RPM_BUILD_ROOT" != "/" ] && rm -rf
"${RPM_BUILD_ROOT}"\
mkdir -p `dirname "$RPM_BUILD_ROOT"`\
mkdir "$RPM_BUILD_ROOT"\
%{nil}
But if I'm reading things incorrectly, I welcome corrections
from the real rpm experts here. :)
--
Todd
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Suppose I were a member of Congress, and suppose I were an idiot. But,
I repeat myself.
-- Mark Twain
2265
days inactive
2269
days old
18 comments
14 participants
participants (14)
-
Ben Rosser -
Brett Lentz -
Chenxiong Qi -
Daniel P. Berrangé -
Germano Massullo -
Kamil Dudka -
Matthew Miller -
Miroslav Suchý -
Neal Gompa -
Panu Matilainen -
Rob Crittenden -
Todd Zullinger -
Tomasz Kłoczko -
Vít Ondruch