Re: Clean up your spec files

[Miroslav Suchý]

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
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org

Re: Clean up your spec files

[Neal Gompa]

On Fri, Feb 9, 2018 at 4:08 AM, Panu Matilainen  wrote:
> On 02/08/2018 04:53 PM, Neal Gompa wrote:
>>
>> On Thu, Feb 8, 2018 at 9:49 AM, Brett Lentz  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!
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org

Re: Clean up your spec files

[Todd Zullinger]

Chenxiong Qi wrote:
> On Thu, Feb 8, 2018 at 9:09 PM, Miroslav Suchý  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



signature.asc
Description: PGP signature
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org

Re: Clean up your spec files

[Chenxiong Qi]

On Thu, Feb 8, 2018 at 9:09 PM, Miroslav Suchý  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@lists.fedoraproject.org
> To unsubscribe send an email to devel-le...@lists.fedoraproject.org



-- 
Regards,
Chenxiong Qi
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org

Re: Clean up your spec files

[Panu Matilainen]

On 02/08/2018 04:53 PM, Neal Gompa wrote:

On Thu, Feb 8, 2018 at 9:49 AM, Brett Lentz  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 -
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org

Re: Clean up your spec files

[Matthew Miller]

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

Fedora Project Leader
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org

Re: Clean up your spec files

[Tomasz Kłoczko]

On 8 February 2018 at 15:39, Kamil Dudka  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.spec.in)
| 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 .spec it will be possible to
build such package by

$ rpmbuild -ta .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 *



>
> Kamil
>
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org

Re: Clean up your spec files

[Daniel P . Berrangé]

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  wrote:
> > https://fedoraproject.org/wiki/Packaging:Guidelines#Spec_Maintenance_and_Canonicity
> >
> > 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 :|
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org

Re: Clean up your spec files

[Ben Rosser]

On Thu, Feb 8, 2018 at 9:53 AM, Neal Gompa  wrote:
> On Thu, Feb 8, 2018 at 9:49 AM, Brett Lentz  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
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org

Re: Clean up your spec files

[Neal Gompa]

On Thu, Feb 8, 2018 at 10:45 AM, Vít Ondruch  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  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_Canonicity
>
> 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!
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org

Re: Clean up your spec files

[Vít Ondruch]

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  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_Canonicity

Not saying it contradicts the guideline above, just FYI.


V.
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org

Re: Clean up your spec files

[Rob Crittenden]

Tomasz Kłoczko wrote:
> On 8 February 2018 at 15:03, Kamil Dudka  > 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
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org

Re: Clean up your spec files

[Kamil Dudka]

On Thursday, February 8, 2018 4:21:53 PM CET Tomasz Kłoczko wrote:
> On 8 February 2018 at 15:03, Kamil Dudka  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
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org

Re: Clean up your spec files

[Tomasz Kłoczko]

On 8 February 2018 at 15:03, Kamil Dudka  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 *
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org

Re: Clean up your spec files

[Kamil Dudka]

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_id=8392731=Fedora_format=advanced_desc=%25check%20RPM%20macro%20used%20in%20%25changelog%20needs%20to%20be%20escaped_desc_type=allwordssubstr

Kamil

> Miroslav
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org

Re: Clean up your spec files

[Neal Gompa]

On Thu, Feb 8, 2018 at 9:49 AM, Brett Lentz  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!
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org

Re: Clean up your spec files

[Brett Lentz]

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.

---Brett.


signature.asc
Description: PGP signature
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org

Re: Clean up your spec files

[Germano Massullo]

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
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org

Clean up your spec files

[Miroslav Suchý]

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
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org