Bug 452211 - Review Request: spu-binutils - Binutils for the SPU on IBM Cell processors
Summary: Review Request: spu-binutils - Binutils for the SPU on IBM Cell processors
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
low
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 462968
TreeView+ depends on / blocked
 
Reported: 2008-06-20 08:48 UTC by Aidan Delaney
Modified: 2008-10-17 18:17 UTC (History)
8 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-10-17 18:17:26 UTC
Type: ---
Embargoed:
redhat: fedora-review+


Attachments (Terms of Use)

Description Aidan Delaney 2008-06-20 08:48:10 UTC
Spec URL: http://foss.it.brighton.ac.uk/downloads/spu-binutils.spec
SRPM URL: http://foss.it.brighton.ac.uk/downloads/spu-binutils-2.18.50.0.6-1.src.rpm

These are based on the standard binutils package found in Fedora.  I envisage it
being updated in lock-step with the vanilla binutils package.  It's called
spu-binutils (rather than binutils-spu) in following with the convention set by
avr-binutils and arm-gp2x-linux-binutils.

Description: This package contains the assembler and linker for ELF binaries on
the SPU architecture found in the IBM Cell processor.

Comment 1 Ralf Corsepius 2008-06-20 10:02:15 UTC
I regret having to say so, but this submission will need a lot of improvements,
before it can make it into Fedora. Please make yourself familiar with the Fedora
packaging guidelines and with cross-building binutils. Did you compare your spec
against those of the avr-binutils?

Just 3 points for you to investigate:
* Don't replace packages from other packages (e.g. bfd)
* Don't use %makeinstall
* Check how to  install infos and check if your binutils's info install at all
(Hint: For cross-binutils built from FSF sources, you need to --disable-infos,
because they clash with those of a native binutils).



Comment 2 Jochen Roth 2008-07-14 11:34:11 UTC
I just created a spec file for spu-binutils based on the same source as the
system binutils is.
I used the avr-binutils as a reference for cross toolchain packages.

Spec URL: ftp://testcase.software.ibm.com/fromibm/linux/spu-binutils.spec
SRPM URL:
ftp://testcase.software.ibm.com/fromibm/linux/spu-binutils-2.18.50.0.6-1.fc9.src.rpm



Comment 3 Jochen Roth 2008-08-28 15:28:02 UTC
Is there anyone on CC who can help us with reviewing this package?
We still need someone who volunteers to review it. Thanks!

Comment 4 Aidan Delaney 2008-08-30 09:26:12 UTC
This is my first package review.  I've read both the maintainers guide and the reviewers guide.

Following the reviewer guide verbatim I find the following issues
• rpmlint reports the creation of non-standard /usr/spu directory
• BuildRequires should include gcc
• internationalisation is disabled
• ${RPM_BUILD_ROOT} used instead of %{buildroot}
I also proffer the opinion that
• ExclusiveArch may be a little restrictive, it /should/ work on x86_64

I think I can argue against some of my strict reading of the reviewers guidelines.  A non-standard /usr/spu directory is created as this is what was agreed for Fedora cross compilers.  Furthermore, internationalisation is disabled because otherwise spu-binutils would overwrite the .mo files.

I believe that BuildRequires must include gcc and I suspect that the ${RPM_BUILD_ROOT} and ExclusiveArch issues are because of the style employed by the packager, something I'm unqualified to critique.

I find the spec file easy to read and have been using binaries built from this spec file on my own systems.

Comment 5 Aidan Delaney 2008-08-30 09:29:41 UTC
I should have said

"Internationalisation is disabled because otherwise spu-binutils would overwrite the .mo files provided by binutils."

Comment 6 Jochen Roth 2008-09-10 15:36:37 UTC
(In reply to comment #4)
> Following the reviewer guide verbatim I find the following issues
> • BuildRequires should include gcc

Isn't gcc part of the standard packages? 

> • ${RPM_BUILD_ROOT} used instead of %{buildroot}
> I also proffer the opinion that
> • ExclusiveArch may be a little restrictive, it /should/ work on x86_64

Ok, I'll remove the ExclusiveArch statement. 

> I believe that BuildRequires must include gcc and I suspect that the
> ${RPM_BUILD_ROOT} and ExclusiveArch issues are because of the style employed by
> the packager, something I'm unqualified to critique.

Quoting the CreatePackageHowTo http://fedoraproject.org/wiki/PackageMaintainers/CreatingPackageHowTo

"You may have $RPM_BUILD_ROOT instead of %{buildroot}; just be consistent."

Comment 7 Jan Kratochvil 2008-09-22 15:43:52 UTC
Please note Rawhide binutils now supports custom %binutils_target:
https://www.redhat.com/archives/fedora-devel-list/2008-July/msg00209.html
(in binutils-2.18.50.0.9-5.fc10)

rpmbuild --define "binutils_target spu" -bb binutils.spec

But still there is no such prebuilt spu package in Fedora.  Linker now supports spu as a secondary target on ppc* 455242 but for example gas does not have the spu support there.

While it would be most straightforward to build the binary packages out of the main binutils package it could be equally done even for avr and other targets which does not scale much.  Does it make sense to just %define binutils_target and build spu-binutils from the main binutils package?

Comment 8 Aidan Delaney 2008-09-25 08:27:07 UTC
Jan,
Do you mean to 
(a) define a new %package spu target in binutils.spec or 
(b) to use the binutils.src.rpm as Source0 in the spu-binutils.spec?

I don't believe (a) is a good option as any upstream stability issues in SPU could delay the release of new packages for more popular architectures such as i386 etc...

I've done some initial hacking on (b) trying to have spu-binutils.spec using binutils%{version}.src.rpm as Source0.  It appears that RPM isn't designed for such build configurations as, for example, %setup does not know about .src.rpm files.  One has to manually setup the binutils.src.rpm and then rpmbuild --bc binutils.spec and finally use the %install and %files directives of spu-binutils.spec to create the spu-binutils.rpm.  It appears, to me, that the only advantage of this approach is to maintain spu-binutils.spec as having the same binutils version as binutils.spec.  This leaves us in the same position as (a) as we may sometimes need to lag behind the version of binutils built by binutils.spec.

Overall, I think Jochen's approach is the most straightforward and uses conventions established by other cross toolsets.

Comment 9 Jan Kratochvil 2008-09-26 01:49:05 UTC
I was discussing (a) as a possibility but saying I would not like it much.

I was suggesting something like (b) but more by:
cp -a binutils/*.patch spu-binutils/
(echo '%define binutils_target spu'; cat binutils/binutils.spec) \
  >spu-binutils/spu-binutils.spec

Another possibility is to introduce some "virtual packages" to Koji which would be created based on other packages with specified --define parameters.  But that would be a new Koji FEAT request.

Comment 10 Aidan Delaney 2008-09-28 21:27:06 UTC
Jan,
A spu-binutils.spec based on your binutils.spec is available here:
http://foss.it.brighton.ac.uk/~balor/rpm/spu-binutils/spu-binutils.spec

It works for me on simple tests.  I'll put it through its paces over the next week or so.  Would you prefer this version to go upstream?

Jochen:  Do you care whether your spu-binutils.spec is used or a spu-binutils.spec based on binutils.spec is used?  Particularly given that we have to target Fedora 11 at this stage?

Comment 11 Jochen Roth 2008-09-29 16:29:16 UTC
(In reply to comment #10)
> Jochen:  Do you care whether your spu-binutils.spec is used or a
> spu-binutils.spec based on binutils.spec is used?  Particularly given that we
> have to target Fedora 11 at this stage?

No, that's fine for me. The only question I have is how we'd keep binutils.spec and spu-binutils.spec in sync. Two different spec files would still mean that we have two different packages (binutils and spu-binutils), wouldn't it? Or is there any way besides the "virtual packaging" like running rpmbuild within a .spec file? 

I thought that the easiest and fastest way might be to create the spu-binutils package according to the way avr-binutils does.

Comment 12 Jan Kratochvil 2008-09-29 17:04:21 UTC
(In reply to comment #10)
> A spu-binutils.spec based on your binutils.spec is available here:
> http://foss.it.brighton.ac.uk/~balor/rpm/spu-binutils/spu-binutils.spec

Just the used binutils version it was based on is pretty old, even without the
changes for the package review.  The latest binutils Rawhide release:
http://koji.fedoraproject.org/koji/buildinfo?buildID=63802
http://cvs.fedora.redhat.com/viewvc/rpms/binutils/devel/


> Would you prefer this version to go upstream?

This is your package, you should like it. :-)


(In reply to comment #11)
> No, that's fine for me. The only question I have is how we'd keep binutils.spec
> and spu-binutils.spec in sync.

Without some rocket science of automatic rebuilds etc. I think it is simple to
register for watching the CVS commits of each other:
https://admin.fedoraproject.org/pkgdb/packages/name/binutils
https://admin.fedoraproject.org/pkgdb/packages/name/avr-binutils

> Two different spec files would still mean that we have two different packages
> (binutils and spu-binutils), wouldn't it?

Yes.

> Or is there any way besides the "virtual packaging" like running rpmbuild
> within a .spec file?

IMO not worth the problems.


> I thought that the easiest and fastest way might be to create the spu-binutils
> package according to the way avr-binutils does.

It is based on an old upstream release (not the H.J.Lu's snapshots).
On the other hand the Fedora binutils patches are mostly arch-specific and 
therefore unrelated to SPU.  Still they do not affect the SPU build anyhow.

Comment 13 Aidan Delaney 2008-09-30 13:13:14 UTC
Updated version is here:
http://foss.it.brighton.ac.uk/~balor/rpm/spu-binutils/r1/spu-binutils.spec

I intend to keep spu-binutils.spec and binutils.spec in some sort of synch via a manual process.  Any automagic process is probably overkill.

Comment 14 Jochen Roth 2008-09-30 13:55:25 UTC
(In reply to comment #12)
> Without some rocket science of automatic rebuilds etc. I think it is simple to
> register for watching the CVS commits of each other:
> https://admin.fedoraproject.org/pkgdb/packages/name/binutils
> https://admin.fedoraproject.org/pkgdb/packages/name/avr-binutils

Great, lets do it this way. 
Can you do the package review for this package, please? Or do you know someone who would volunteer? 

(In reply to comment #13)
> Updated version is here:
> http://foss.it.brighton.ac.uk/~balor/rpm/spu-binutils/r1/spu-binutils.spec
> 
> I intend to keep spu-binutils.spec and binutils.spec in some sort of synch via
> a manual process.  Any automagic process is probably overkill.

Looks good! Thanks!

Comment 15 Jan Kratochvil 2008-10-01 18:02:17 UTC
(In reply to comment #14)
> (In reply to comment #12)
> > Without some rocket science of automatic rebuilds etc. I think it is simple
> > to register for watching the CVS commits of each other:
> > https://admin.fedoraproject.org/pkgdb/packages/name/binutils
> > https://admin.fedoraproject.org/pkgdb/packages/name/avr-binutils
> 
> Great, lets do it this way. 
> Can you do the package review for this package, please? Or do you know someone
> who would volunteer? 

Bug 225615 review for the main binutils package is closed so this one may be closed as its DUPLICATE now? :-)

Comment 16 Robert Scheck 2008-10-01 18:53:32 UTC
I think, you need some kind of ExclusiveArch or ExcludeArch for that, because 
on i386, I'm getting:

+ ./configure --build=i686-redhat-linux-gnu --host=i686-redhat-linux-gnu --target=i386-redhat-linux --program-prefix= --prefix=/usr --exec-prefix=/usr --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib --libexecdir=/usr/libexec --localstatedir=/var --sharedstatedir=/usr/com --mandir=/usr/share/man --infodir=/usr/share/info --build=i386-redhat-linux --host=i386-redhat-linux --target=spu --enable-targets=i686-redhat-linux-gnu --with-sysroot=/usr/spu/sys-root --program-prefix=spu- --disable-shared --disable-werror --with-bugurl=http://bugzilla.redhat.com/bugzilla/
checking build system type... i386-redhat-linux-gnu
checking host system type... i386-redhat-linux-gnu
checking target system type... Invalid configuration `spu': machine `spu' not recognized
configure: error: /bin/sh ./config.sub spu failed

Comment 17 Aidan Delaney 2008-10-02 10:26:00 UTC
@Robert Scheck:
I cannot replicate this issue on my F9 i686 system on which autoconf autodetects the host as i386-redhat-linux-gnu.  Can you provide more info on your system such as the OS version?

Comment 18 Jochen Roth 2008-10-02 11:24:03 UTC
(In reply to comment #16)
> I think, you need some kind of ExclusiveArch or ExcludeArch for that, because 
> on i386, I'm getting:

Hi Robert, 

I wasn't able to reproduce the problem you are facing. Have a look for my build result here:  https://koji.fedoraproject.org/koji/getfile?taskID=856414&name=build.log

Comment 19 Robert Scheck 2008-10-02 15:38:30 UTC
Looks like this was a local issue on single box which seems to be a bit
broken. Finally a scratch build in Koji worked as expected. Package looks
good to me, thus:

  APPROVED

Note, that I'm not really able to test the spu functionally itself, just
the packaging and around. And please fix the last remaining warning before 
importing or shortly after:

spu-binutils.i386: W: incoherent-version-in-changelog 2.18.50.0.9-6 2.18.50.0.9-5.fc10

Comment 20 Jochen Roth 2008-10-02 15:58:24 UTC
(In reply to comment #19)
>   APPROVED

Thanks a lot!
 
> spu-binutils.i386: W: incoherent-version-in-changelog 2.18.50.0.9-6
> 2.18.50.0.9-5.fc10

Yes, I'm sorry I accidentally created the SRPM for the scratch build with the wrong .spec file. I already fixed that.

Comment 21 Jochen Roth 2008-10-02 15:59:18 UTC
New Package CVS Request
=======================
Package Name: spu-binutils
Short Description: A GNU collection of binary utilities
Owners: jroth
Branches: EL-5
InitialCC: adrian

Comment 22 Chitlesh GOORAH 2008-10-02 16:26:36 UTC
Hello Jochen,

I'm curious to know why you didn't add the Branches F-9 and F-10 ?

I'm willing to list this package to be part of the Fedora Electronic Lab.

I'm inviting you guys to subscribe to this low traffic mailing list :)
https://www.redhat.com/mailman/listinfo/fedora-electronic-lab-list

Comment 23 Kevin Fenzi 2008-10-03 16:57:53 UTC
cvs done. Feel free to request more branches in another request.

Comment 24 Jochen Roth 2008-10-06 09:02:53 UTC
(In reply to comment #22)
> I'm curious to know why you didn't add the Branches F-9 and F-10 ?

Indeed, I forgot that there is a F-10 branch already. I didn't add F-8 and F-9 because it would break the SDK installations of the available Cell SDKs which provide spu-binutils. 

> I'm willing to list this package to be part of the Fedora Electronic Lab.

Thanks!

> I'm inviting you guys to subscribe to this low traffic mailing list :)
> https://www.redhat.com/mailman/listinfo/fedora-electronic-lab-list

OK, I'm registered now.

Comment 25 Jochen Roth 2008-10-06 10:17:25 UTC
New Package CVS Request
=======================
Package Name: spu-binutils
Short Description: A GNU collection of binary utilities
Owners: jroth
Branches: F-10
InitialCC: adrian

Comment 26 Kevin Fenzi 2008-10-07 17:30:24 UTC
Note that all packages will get a F-10 branch when we mass branch for it.
You can request one now if you need one to land lots of changes in the rawhide branch for after F-10 release. 

Do you need a F-10 early branch?

Comment 27 Jochen Roth 2008-10-08 08:06:44 UTC
(In reply to comment #26)
> Note that all packages will get a F-10 branch when we mass branch for it.
> You can request one now if you need one to land lots of changes in the rawhide
> branch for after F-10 release. 
> 
> Do you need a F-10 early branch?

Yes, thanks I noticed that but I thought that I have to create an extra request for new packages. In this case I don't need an extra F-10 early branch. Thanks!


Note You need to log in before you can comment on or make changes to this bug.