Bug 225615

Summary: Merge Review: binutils
Product: [Fedora] Fedora Reporter: Nobody's working on this, feel free to take it <nobody>
Component: Package ReviewAssignee: Jon Ciesla <limburgher>
Status: CLOSED RAWHIDE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: jakub, jan.kratochvil, nickc, opensource, pertusus, redhat-bugzilla
Target Milestone: ---Keywords: Reopened
Target Release: ---Flags: limburgher: fedora‑review+
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2013-02-07 10:18:21 EST Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Bug Depends On: 223678    
Bug Blocks:    
Attachments:
Description Flags
Proposed changes. none

Description Nobody's working on this, feel free to take it 2007-01-31 12:45:41 EST
Fedora Merge Review: binutils

http://cvs.fedora.redhat.com/viewcvs/devel/binutils/
Initial Owner: jakub@redhat.com
Comment 1 Jon Ciesla 2008-02-06 10:39:28 EST
rpmlint on srpm:

binutils.src:20: W: prereq-use /sbin/install-info
The use of PreReq is deprecated. In the majority of cases, a plain Requires
is enough and the right thing to do. Sometimes Requires(pre), Requires(post),
Requires(preun) and/or Requires(postun) can also be used instead of PreReq.

Fix.

binutils.src:22: W: unversioned-explicit-obsoletes gnupro
The specfile contains an unversioned Obsoletes: token, which will match all
older, equal and newer versions of the obsoleted thing.  This may cause update
problems, restrict future package/provides naming, and may match something it
was originally not inteded to match -- make the Obsoletes versioned if
possible.

Fix if possible.

binutils.src:47: W: prereq-use /sbin/install-info
The use of PreReq is deprecated. In the majority of cases, a plain Requires
is enough and the right thing to do. Sometimes Requires(pre), Requires(post),
Requires(preun) and/or Requires(postun) can also be used instead of PreReq.

binutils.src:303: W: macro-in-%changelog _prefix
Macros are expanded in %changelog too, which can in unfortunate cases lead
to the package not building at all, or other subtle unexpected conditions that
affect the build.  Even when that doesn't happen, the expansion results in
possibly "rewriting history" on subsequent package revisions and generally
odd entries eg. in source rpms, which is rarely wanted.  Avoid use of macros
in %changelog altogether, or use two '%'s to escape them, like '%%foo'.

binutils.src:745: W: macro-in-%changelog _prefix
Macros are expanded in %changelog too, which can in unfortunate cases lead
to the package not building at all, or other subtle unexpected conditions that
affect the build.  Even when that doesn't happen, the expansion results in
possibly "rewriting history" on subsequent package revisions and generally
odd entries eg. in source rpms, which is rarely wanted.  Avoid use of macros
in %changelog altogether, or use two '%'s to escape them, like '%%foo'.

Fix.

binutils.src: W: %ifarch-applied-patch Patch4: binutils-2.18.50.0.3-ia64-lib64.p
                                                             atch
A patch is applied inside an %ifarch block. Patches must be applied
on all architectures and may contain necessary configure and/or code
patch to be effective only on a given arch.

Not a problem.

binutils.src: W: summary-ended-with-dot A GNU collection of binary utilities.
Summary ends with a dot.

Fix.

rpmlint on rpms is clean other than the above.

Why are the .a files not in a -static package?  What would be the ramifications
of correcting this?

Otherwise, looks good, no other blockers.
Comment 2 Patrice Dumas 2008-02-06 17:53:58 EST
I think it would be better to change the perl substitution
to a sed substitution.

the gzipping of info files will be done automatically,
and install-info knows how to install/remove compressed info files.

I suggest using
%defattr(-,root,root,-) instead of %defattr(-,root,root)

Why not use %configure and why use %makeinstall? Looks like
DESTDIR is rightly used.
Comment 3 Jon Ciesla 2008-02-07 08:13:04 EST
(In reply to comment #2)
> I think it would be better to change the perl substitution
> to a sed substitution.

I concur.

> the gzipping of info files will be done automatically,
> and install-info knows how to install/remove compressed info files.

Cool, I didn't know that.

> I suggest using
> %defattr(-,root,root,-) instead of %defattr(-,root,root)

What would the advantage be?  Just curious.

> Why not use %configure and why use %makeinstall? Looks like
> DESTDIR is rightly used.

Have the concerns from the changelog on line 885 been addressed?  I've never had
a problem with %configure myself.
Comment 4 Jon Ciesla 2008-05-16 11:03:55 EDT
Any updates?
Comment 5 Jon Ciesla 2008-09-09 08:30:39 EDT
evaluating new rawhide srpm.

New in rpmlint on srpm:

binutils.src:157: W: configure-without-libdir-spec
A configure script is run without specifying the libdir. configure options
must be augmented with something like --libdir=%{_libdir} whenever the script
supports it.

Fix.

Above is still present.

rpmlint on rpms:  Summary ends with a dot.  Fix.

Other issues remain.


Adding CC to current binutils maintainer.
Comment 6 Jan Kratochvil 2008-09-11 14:14:55 EDT
Created attachment 316467 [details]
Proposed changes.

Changes attached, scratch build at:
http://koji.fedoraproject.org/koji/taskinfo?taskID=820452


(In reply to comment #1)
> binutils.src:20: W: prereq-use /sbin/install-info
> The use of PreReq is deprecated. In the majority of cases, a plain Requires
> is enough and the right thing to do. Sometimes Requires(pre), Requires(post),
> Requires(preun) and/or Requires(postun) can also be used instead of PreReq.

Fixed:
+Requires(post): /sbin/install-info
+Requires(preun): /sbin/install-info


> binutils.src:22: W: unversioned-explicit-obsoletes gnupro
> The specfile contains an unversioned Obsoletes: token, which will match all
> older, equal and newer versions of the obsoleted thing.  This may cause update
> problems, restrict future package/provides naming, and may match something it
> was originally not inteded to match -- make the Obsoletes versioned if
> possible.

Fixed:
+Obsoletes: gnupro <= 1117-1


> binutils.src:303: W: macro-in-%changelog _prefix

Not fixed as it was already used with single or double % case-by-case
appropriately. One should see during `rpm -q --changelog'
- fix multilib conflict in /usr/include/bfd.h
and not:
- fix multilib conflict in %{_prefix}/include/bfd.h


> binutils.src: W: %ifarch-applied-patch Patch4: binutils-2.18.50.0.3-ia64-lib64.p
> A patch is applied inside an %ifarch block. Patches must be applied
> on all architectures and may contain necessary configure and/or code
> patch to be effective only on a given arch.
>
> Not a problem.

Not fixed, source tree is already arch-dependent as I was told by Roland
McGrath.


> binutils.src: W: summary-ended-with-dot A GNU collection of binary utilities.
> Summary ends with a dot.

Fixed.


> Why are the .a files not in a -static package?  What would be the ramifications
> of correcting this?

libiberty.a should be in fact removed as it should not be used by anyone as
packages using libiberty have it bundled and build it on their own.

libbfd.a and libopcodes.a are not ABI compatible across versions so their
shared (.so) counterparts are not intended for linking with 3rd party
applications - they are provided only for the binaries from this rpm.
3rd party applications link bfd/opcodes statically.

There is nothing left for a possible -static package.


(In reply to comment #2)
> I think it would be better to change the perl substitution
> to a sed substitution.

Not done, IMO the Perl regex syntax is more flexible and I find the sed syntax
obsoleted nowadays.


> the gzipping of info files will be done automatically,

Fixed.


> I suggest using
> %defattr(-,root,root,-) instead of %defattr(-,root,root)

Changed (default directory attr; not in the scratch build).


> Why not use %configure and why use %makeinstall?

Changed for %configure.  There was a separate build directory before which I am
not aware how to configure while using %configure.  But as I find the separate
build directory more as an inconvenience than advantage and the practical
reasons for it (gasp) no longer exist I changed it for a unified build/source
directory back again and so even using %configure.


(In reply to comment #5)
> binutils.src:157: W: configure-without-libdir-spec
> A configure script is run without specifying the libdir. configure options
> must be augmented with something like --libdir=%{_libdir} whenever the script
> supports it.

This is a rpmlint error, --libdir is there but rpmlint did not parse it right
through the %if blocks.


Thanks, if it is fine going to commit it.
Comment 7 Jan Kratochvil 2008-09-11 14:17:17 EDT
IIRC assignee should be the reviewer, not the maintainer, sorry.
Comment 8 Jon Ciesla 2008-09-12 08:16:12 EDT
Looks good.  Commit, build, file a bug against rpmlint and we're set.  Thanks!
Comment 9 Jan Kratochvil 2008-09-15 13:18:16 EDT
Committed as: binutils-2.18.50.0.9-2.fc10
rpmlint problem submitted as: Bug 462360

I hope the review can get closed now, thanks.
Comment 10 Jon Ciesla 2008-09-15 13:26:07 EDT
Great.  APPROVED.
Comment 11 Till Maas 2010-01-17 13:46:55 EST
I object that the review is completed, the -static problem was not solved. In comment:6 it was staid that libiberty.a should be removed and probably the other .a files, too, as far as I understand the comment. But they are still shipped and there is also not -static package, so something is obviously wrong.

Btw. is there a FESCo exception to allow bundling libiberty?
https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries
https://fedoraproject.org/wiki/Packaging:Guidelines#Duplication_of_system_libraries
Comment 12 Jon Ciesla 2010-01-19 10:45:28 EST
Good catch Till, an oversight on my part.  These indeed need to be resolved.  I'd also say use the system libiberty and remove the other .a.
Comment 13 Jan Kratochvil 2010-03-10 15:15:54 EST
There should also be - IMO:
License: GPLv3+ and GPLv3+ with exceptions and GPLv2+ and GPLv2+ with
exceptions and GPL+ and LGPLv2+ and GFDL and Public Domain

following:
https://fedoraproject.org/wiki/Packaging/LicensingGuidelines

GPLv3+ with exceptions: texinfo/texinfo.tex
GPLv2+ with exceptions: libtool.m4
GPL+: include/opcode/arm.h
Other licenses are easy to find.
Comment 14 Jon Ciesla 2010-04-29 16:11:22 EDT
Ping?
Comment 15 Jakub Jelinek 2010-04-29 16:20:01 EDT
The question about bundling libiberty doesn't make any sense, libiberty it a library that is bundled with any package that needs it.  Furthermore, src repository (together with gcc repository) is the libiberty upstream.

And, binutils now has a binutils-static subpackage.
Comment 16 Till Maas 2010-04-29 17:14:15 EDT
(In reply to comment #15)
> The question about bundling libiberty doesn't make any sense, libiberty it a
> library that is bundled with any package that needs it.  Furthermore, src
> repository (together with gcc repository) is the libiberty upstream.

Then an exception needs to be granted for, because there is currently none:
https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries#Packages_granted_exceptions

I opened a ticket for this at the fesco trac. Can you maybe monitor it and answer any open questions about it, since you are the one who knows why it should be bundled?
Comment 17 Jon Ciesla 2011-03-31 12:38:32 EDT
Any updates on the bundling?
Comment 18 Jon Ciesla 2011-06-17 10:57:50 EDT
Ping?
Comment 19 Jon Ciesla 2011-10-18 11:55:48 EDT
Ping?
Comment 20 Nick Clifton 2011-10-18 12:06:59 EDT
Hi Jon,

  I have opened a fpc ticket to see if the binutils can be granted an exception to the library packaging rules.  I am not sure that it will pan out, but I feel that it is worth a try.

  https://fedorahosted.org/fpc/ticket/109

Cheers
  Nick
Comment 21 Jon Ciesla 2011-10-18 12:22:56 EDT
Thanks!
Comment 22 Jon Ciesla 2012-04-26 08:51:26 EDT
This was granted, where are we with this?
Comment 23 Jon Ciesla 2013-02-07 10:18:21 EST
Provides were added in October, forgot to close the BZ.