Bug 442507 (libspe2)
Summary: | Review Request: libspe2 - SPE Runtime Management Library | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Jochen Roth <jroth> |
Component: | Package Review | Assignee: | Adrian Reber <adrian> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | low | ||
Version: | rawhide | CC: | adrian, asayama, ejratl, fedora-package-review, hannsj_uhl, itamar, jwboyer, notting, redhat-bugzilla, stenzel, susi.lehtola |
Target Milestone: | --- | Flags: | adrian:
fedora-review+
kevin: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | ppc64 | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2008-09-27 08:27:09 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: |
Description
Jochen Roth
2008-04-15 07:37:21 UTC
Is it possible to download package from a alternative place. The package doesn't seems present on the ftp website even if anonymous FTP is used. Hello Red Hat, yes, the ftp web site is cleared after a couple of days .. ... but this package is also upstream at http://sourceforge.net/projects/libspe Can you please check the package from there ..? Thanks in advance for your support. Neither of us is with Red Hat; Fedora is a community project so you'll find that many of the people you'll interact with do not work for them. Could you provide working links to the specfile and the src.rpm which you would like to have reviewed? I only see tarballs at sourceforge. Jochen, just before I'm seeing the spec file one thing: libspe2-2.2.80-95.tar.gz is the upstream tarball name, so your package can't be libspe2-2.2.80-95 at all, because "Release" tag is reserved to the packager (except for some special cases such as CVS or SVN checkouts, pre-releases). I think, here the same scheme as for e.g. ImageMagick applies, libspe2-2.2.80-95 gets libspe2-2.2.80.95-X where X can be incremented for each build or change the packager does to the package. Version tag is always the whole upstream version. I guess for review the following should do: http://www.bsc.es/projects/deepcomputing/linuxoncell/cellsimulator/sdk3.0/SRPMS/libspe2-2.2.0-91.src.rpm even, if it is not the latest source level, the spec file is the more or less up to date. Thanks for your help. I never did a full review, but rpmlint isn't silent for me here and I got many packages reviewed until now, so let's try to fix the main things first. libspe2.src:30: E: hardcoded-library-path in /usr/lib -> %_libdir redefined libspe2.src:107: W: setup-not-quiet - Replace "%setup" by "%setup -q" libspe2.src: W: mixed-use-of-spaces-and-tabs (spaces: line 151, tab: line 127) -> Yeah, don't mix spaces and tabs inside of the spec file ;-) libspe2.src: W: non-standard-group Cell Development Libraries -> Please choose a valid group from /usr/share/doc/rpm-*/GROUPS, I think for the library packages, "System Environment/Libraries" should maybe fit and for packages with header and development files, maybe "Development/Libraries", just have a look to the list yourself. libspe2.src: W: invalid-license LGPL -> Please be more precise and select a valid license tag from the wiki list http://fedoraproject.org/wiki/Licensing libspe2.src: E: unknown-key GPG#77550217 -> Did you put a key somewhere into the package?! libspe2.src: W: strange-permission libspe2-2.2.0-91.tar.gz 0600 libspe2.src: W: strange-permission libspe2.spec 0600 -> chmod 644 to both files before executing rpmbuild Aside of rpmlint: - Please kill/remove the vendor tag, Fedora inserts it's own - Set buildroot tag to something valid from the Packaging Guidelines - Kill/remove the distribution tag, not needed/wished in Fedora - Don't do things like "%define _libdir /usr/lib", this is all already in the rpm-redhat-config rpm package - Change "Requires: %{name} = %{version}" to "Requires: %{name} = %{version}- %{release}" - Don't abuse release tag as mentioned in comment #4 - Does %WITH_WRAPPER really make sense? For Fedora you can't define such switches directly as you never can't call rpmbuild directly, maybe decide for one flavor or build both in parallel, if they can co-exist?! - Don't define %{_initdir} and friends, already in rpm-redhat-config rpm package - What is %_adabindingdir and %_includedir2? They don't look LSB conform, includes have to go into %{_includedir} or in a subdirectory, not directly into /usr/somewhere - Please remove %_unpackaged_files_terminate_build - remove unneeded files or use the %exclude macro - Initscript should contain maybe an LSB/upstart compatible header section as well - at least it would be nice for the future. - Is it necessary do enable the initscript/service per default? - Does the initscript really show [ OK ] and [FAILED]? I don't think so... - Please use %doc e.g. for README, COPYING, LICENSE or whatelse exists - Eliminate %set_optflags macro usage, just use OPTFLAGS="%{optflags}" at the make command itself, %optflags also knows how to handle noarch packages - I can't see any build requires. Please note, the package has to rebuild successfully in mock (https://fedoraproject.org/wiki/Projects/Mock) of Fedora so you have to list all main dependencies to build the package - Don't ship static libs, please (no *.a) P.S.: I had a talk with Jochen, I thought he wrote a more sane spec file - can we maybe see that one and maybe review his one, as Jochen told me that he tried to follow the guidelines of Fedora already more than the current SRPM is doing. FYI, https://fedoraproject.org/wiki/Packaging/Guidelines explains many of the things I maybe just wrote short more detailed and with reasons et al. Sorry for the confusion. I'm at the LinuxTag and I have only limited access to the internet. I'll upload the files to the link mentioned in comment #1 as soon as possible. Some of the point mentioned by Robert are already addressed. I just had a talk with Robert. Thanks for your interest. The spec file and the src.rpm are available again on: Spec URL: ftp://testcase.software.ibm.com/fromibm/linux/libspe2.spec SRPM URL: ftp://testcase.software.ibm.com/fromibm/linux/libspe2-2.2.80-95.src.rpm I'll reply to your comments as soon as possible. Thanks! /opt/cell/bin/ppu-gcc -O2 -Wall -I. -fPIC -I./include -fexceptions -std=gnu99 -Wformat -Wdisabled-optimization -Wundef -Wshadow -Wcast-align -Wwrite-strings -Wmissing-prototypes -Wmissing-declarations -Wmissing-noreturn -Wredundant-decls -Wnested-externs -Winline -Wformat -m32 -mabi=altivec -I./spebase -I./speevent -c -o libspe2.o libspe2.c make: /opt/cell/bin/ppu-gcc: Command not found Well, /opt/cell/bin/ppu-gcc doesn't seem to be part of Fedora/RHEL at all. Can't we use the regular gcc delivered with Fedora for that? If we need a special gcc, we have to get this one into Fedora first. Comments? It looks like you are trying to build on a non ppc system. /opt/cell/bin/ppu-gcc would refer to the cross-compiler when building on x86. The make.defines evaluates that with the following command: X86 = $(shell if ! uname -m | grep ppc ; then echo 1 ; fi) So if X86 contains one /opt/cell/bin/ppu-gcc is used instead of the native system gcc compiler. The latest version can be found at: Spec URL: ftp://testcase.software.ibm.com/fromibm/linux/libspe2.spec SRPM URL: ftp://testcase.software.ibm.com/fromibm/linux/libspe2-2.2.80.95-2.src.rpm Thanks to the extensive help from Robert. At least one point is open for discussion. We are used to put include files for our spu compiler into /usr/spu/include. Many tools (e.g. our Cell SDK3 make files) have this path hardcoded. The question now is if we can continue to use the path /usr/spu/include for SPU related include files. Thanks for your help. (In reply to comment #12) > We are used to put include files for > our spu compiler into /usr/spu/include. Many tools (e.g. our Cell SDK3 make > files) have this path hardcoded. > > The question now is if we can continue to use the path /usr/spu/include > for SPU related include files. Are these headers part of a cross-toolchain targetting bare-metal (no OS, no libc), spu-cpus (In GNU terms, canonicalization tripple == "spu")? If yes, then this would match existing practice of installing cross-GCC, even though this strictly speaking, would violate the FHS. If no, then this issue requires further discussion. For native headers, this location is non-acceptable. To use this package a version of gcc-spu is required. Is there one upstream in Fedora? I've been hacking on my own gcc-spu and this would help me a lot. I have a binutils package at the following location: http://blogs.linux.ie/balor/2008/06/02/fedora-9-cell-processor-packages/ (In reply to comment #13) > Are these headers part of a cross-toolchain targetting bare-metal (no OS, no > libc), spu-cpus (In GNU terms, canonicalization tripple == "spu")? Yes, exactly, that is the case. I just had a look at the libspe packages in gentoo and ubuntu. Ubuntu put them in /usr/spu/include and gentoo in /usr/spu-elf/include. (In reply to comment #14) > To use this package a version of gcc-spu is required. Is there one upstream in > Fedora? It is not necessarily required but very useful. We have plans to maintain spu-gcc, spu-binutils and spu-newlib in Fedora. (In reply to comment #12) It seems strange that elfspe is built only for ppc (%ifarch ppc) and that adabinding-devel is built only for ppc64 (%ifarch ppc64). I think they should be built regardless of target architecture. Could you compare your libspe2.spec with SF.net subversion? (In reply to comment #17) > It seems strange that elfspe is built only for ppc (%ifarch ppc) and that > adabinding-devel is built only for ppc64 (%ifarch ppc64). I think they should be > built regardless of target architecture. Could you compare your libspe2.spec > with SF.net subversion? I'll investigate that together with Dirk. Thanks. (In reply to comment #18) > I'll investigate that together with Dirk. Thanks. I talked with Gerhard about this before. He said: elfspe: There is no benefit seen in having a 64 bit elfspe binary. But I think that Fedora has ppc64 repository, so we should have 64 bit elfspe. libspe-adabinding: Because ppu-gnat creates 64bit binaries. It seems to be an IBM Cell SDK specific reason. (In reply to comment #12) No static library is included in the devel package. Gerhard said that it is because of IBM's internal users' request, not because of technical reason. I suggest including the static library in the package. Well, static libraries are not really wished in Fedora anyway. (In reply to comment #21) > Well, static libraries are not really wished in Fedora anyway. Correct. They require exceptions from FESCo, unless the package _only_ provides a static library. (In reply to comment #19) > (In reply to comment #18) > > I'll investigate that together with Dirk. Thanks. > > I talked with Gerhard about this before. He said: > > elfspe: There is no benefit seen in having a 64 bit elfspe binary. But I think > that Fedora has ppc64 repository, so we should have 64 bit elfspe. As long there is the understanding that only one elfspe binary, either 32bit or 64bit, can be installed, I don't think there is a real issue. > libspe-adabinding: Because ppu-gnat creates 64bit binaries. It seems to be an > IBM Cell SDK specific reason. My understanding is, that the ADA compiler can produce either 32bit or 64bit binaries depending on how the ADA compiler itself was built. From that perspective, choosing the 64bit seems the better choice. But I don't have neither a strong opinion nor a strong background in this area. (In reply to comment #22) > (In reply to comment #21) > > Well, static libraries are not really wished in Fedora anyway. > > Correct. They require exceptions from FESCo, unless the package _only_ provides > a static library. Ah, I see. That is mentioned in the packaging guideline. (In reply to comment #23) > As long there is the understanding that only one elfspe binary, either 32bit or > 64bit, can be installed, I don't think there is a real issue. It looks like the same situation as other executable packages, such as bash, coreutils, ppc64-utils, etc. The ppc64 repository (development/ppc64/, releases/9/Everything/ppc64/, etc) have only ppc64 packages, so I thought that the ppc64 package should be built for ppc64 targets. Is it unnecessary to care about this case? > My understanding is, that the ADA compiler can produce either 32bit or 64bit > binaries depending on how the ADA compiler itself was built. > From that perspective, choosing the 64bit seems the better choice. But I don't > have neither a strong opinion nor a strong background in this area. Hmm, the Fedora's ppc repository provides only 32bit version of libgnat, and the ppc64 repository provides no libgnat, though the gnat compiler seems to be able to generate both of 32bit and 64bit object files... (In reply to comment #25) > It looks like the same situation as other executable packages, such as bash, > coreutils, ppc64-utils, etc. The ppc64 repository (development/ppc64/, > releases/9/Everything/ppc64/, etc) have only ppc64 packages, so I thought that > the ppc64 package should be built for ppc64 targets. Is it unnecessary to care > about this case? Yes, I think so because elfspe is just a wrapper which calls starts SPU binaries. And SPU binaries are executed in 32bit mode only. There can be only one elfspe binaries which is either ppc or ppc64. And as mentioned above ppc would make more sense. (In reply to comment #26) > (In reply to comment #25) > > It looks like the same situation as other executable packages, such as bash, > > coreutils, ppc64-utils, etc. The ppc64 repository (development/ppc64/, > > releases/9/Everything/ppc64/, etc) have only ppc64 packages, so I thought that > > the ppc64 package should be built for ppc64 targets. Is it unnecessary to care > > about this case? > > Yes, I think so because elfspe is just a wrapper which calls starts SPU > binaries. And SPU binaries are executed in 32bit mode only. I can't understand that. If we use the 64bit elfspe to run an SPU program, it will be executed in 64bit mode. > There can be only one elfspe binaries which is either ppc or ppc64. And as > mentioned above ppc would make more sense. I thought that the ppc64 repository consisted of only the ppc64 packages for 64 bit only userland, while the ppc repository contained 32bit packages for default runtime environment and additional ppc64 packages for 64bit user programs. Is it my misunderstanding? Don't we need to consider the 64bit only environment? > I can't understand that. If we use the 64bit elfspe to run an SPU program, it
> will be executed in 64bit mode.
Only the 64bit elfspe will run in 64bit. The SPE application will always run in
32bit.
So I suggest to use one of the two following possibilities for elfspe:
1. Build a ppc and a ppc64 package of elfspe.
- the problem here would be that if one installs both packages on one system the
elfspe binary will be overwritten.
2. Build the ppc64 package only.
- Binaries in Fedora should be 64bit on a 64bit architecture.
The libspe2-adabinding-devel package contains only include files for developing
ada based SPE applications. Therefore I'd say that we should use either build a
ppc or a ppc64 package only.
As libgnat is available for ppc only it might make sense to build the
libspe2-adabinding-devel package for ppc as well.
Are there any opinions, suggestions? What is the correct way of handling this in
Fedora?
(In reply to comment #28) > > I can't understand that. If we use the 64bit elfspe to run an SPU program, it > > will be executed in 64bit mode. > > Only the 64bit elfspe will run in 64bit. The SPE application will always run in > 32bit. That is not correct. Your term "32bit" just means "SPE uses LP32 program model and 32bit ABI", I think. But if an SPE program is run by the 64bit mode PPE process, for example, the SPE can access whole of the process's 64bit effective address space as well as the PPE. That is the meaning of my term "64bit mode". Anyway, that is not a reason why I suggest building the 64bit elfspe. I'm just saying to follow the same manner as other executable packages, as below. > > So I suggest to use one of the two following possibilities for elfspe: > > 1. Build a ppc and a ppc64 package of elfspe. > - the problem here would be that if one installs both packages on one system the > elfspe binary will be overwritten. I don't think there is such a problem if we follow the same manner as other executable packages. That is, the ppc repository has only the 32bit elfspe package and the ppc64 repository has only the 64bit elfspe package. See the development/ppc directory and the development/ppc64 directory. The ppc directory has both of 32bit libraries and 64bit libraries, however only 32bit executables. The ppc64 directory has only 64bit binaries. > > 2. Build the ppc64 package only. > - Binaries in Fedora should be 64bit on a 64bit architecture. > > > The libspe2-adabinding-devel package contains only include files for developing > ada based SPE applications. Therefore I'd say that we should use either build a > ppc or a ppc64 package only. > As libgnat is available for ppc only it might make sense to build the > libspe2-adabinding-devel package for ppc as well. I don't know why libgnat is available only for 32bit ppc... is it because of a technical reason? > Are there any opinions, suggestions? What is the correct way of handling this in > Fedora? I'd like to know that, too. The reason why libgnat is only available for ppc and not ppc64 can be seen here: https://bugzilla.redhat.com/show_bug.cgi?id=241098 Concerning the include directory: I would go for /usr/include/spu. That seems to make the most sense for me. For now. Right now this is a ppc/ppc64 only package and has nothing to do with cross compiling in the traditional meaning. Including it that way would enable Fedora installations on PS3 to easily have libspe2 available. To make the complete spu buildchain available for other platforms is probably a much bigger discussion which does not really belong here in the review request for libspe2 on ppc/ppc64. If there is no one against this I would be willing to sponsor Jochen (after a few more little changes to the spec file). (In reply to comment #30) > If there is no one against this I would be willing to sponsor Jochen (after a > few more little changes to the spec file). OK, it looks like there is nobody against that proposal. How shall be continue? Please change to: %define spuinclude %{_includedir}/spu %{_sysconfdir}/rc.d/init.d/ == %{_initrddir} I have done a 32bit and 64bit test build and it looks pretty good. There are still a few things which should be fixed: elfspe2.ppc: W: service-default-enabled /etc/rc.d/init.d/elfspe2 elfspe2.ppc: E: incoherent-subsys /etc/rc.d/init.d/elfspe2 elfspe Thanks for your help Adrian. I changed what you asked for and fixed the errors and warnings. Furthermore I found some dependency problems which I resolved as well. You'll find the latest version here: Spec URL: ftp://testcase.software.ibm.com/fromibm/linux/libspe2.spec SRPM URL: ftp://testcase.software.ibm.com/fromibm/linux/libspe2-2.2.80-95-3.src.rpm Adrian, sorry for blaiming you, but from latest /usr/lib/rpm/macros (rpm 4.5.90): %_initddir %{_sysconfdir}/rc.d/init.d # Deprecated misspelling, present for backwards compatibility. %_initrddir %{_initddir} So %{_initrddir} shouldn't be used. Either the other macro of what Jochen already did. Jochen, were you able to solve "incoherent-subsys" or do you need help there? Adrian, AFAIK the warning "service-default-enabled" can be ignored, because libspe2 doesn't make sense when it's not enabled, thus I would suggest to leave it simply on per default. (In reply to comment #34) > Adrian, sorry for blaiming you, but from latest /usr/lib/rpm/macros (rpm 4.5.90): > > %_initddir %{_sysconfdir}/rc.d/init.d > # Deprecated misspelling, present for backwards compatibility. > %_initrddir %{_initddir} > > So %{_initrddir} shouldn't be used. Either the other macro of what Jochen already > did. I remember once we discussed this, however current Fedora packaging guidelines suggests to use %_initrddir : https://fedoraproject.org/wiki/Packaging/SysVInitScript#Initscripts_on_the_filesystem Very good. Another reviewer. The problem with %_initddir is that it only works with the rpm version (4.5.90). It does not exist for F8/F9. Makes sense to use the right one for rawhide. Jochen, you just need to remember to change it for the other branches. I am okay with "service-default-enabled". Seems to make sense. (In reply to comment #34) > Jochen, were you able to solve "incoherent-subsys" or do you need help there? Yes, I was able to solve this issue. Thanks. (In reply to comment #36) > I am okay with "service-default-enabled". Seems to make sense. libspe2 can be used without the elfspe2 init script. But so called "spulets" which are standalone spu programs can not run without this service. I changed the init script to be disabled per default. But as you don't mind I'll change it back to be enabled per default. Thanks. The latest available version is here: Spec URL: ftp://testcase.software.ibm.com/fromibm/linux/libspe2.spec SRPM URL: ftp://testcase.software.ibm.com/fromibm/linux/libspe2-2.2.80-95-4.src.rpm I changed the elfspe2 init script to be enabled per default. Sorry I posted the wrong URL for the SRPM! Here is the correct one: SRPM URL: ftp://testcase.software.ibm.com/fromibm/linux/libspe2-2.2.80.95-4.src.rpm rpmlint for mock-built binaries on ppc: elfspe2.ppc: W: service-default-enabled /etc/rc.d/init.d/elfspe2 can be ignored. Was decided to be more useful if automatically started as it is no service but just registers SPE binaries. rpmlint for mock-built binaries on ppc64 is happy clean installation and removal spec file looks good I am a bit unsure if we need to open ExcludeArch bugs for x86 and x86_64 for this package. Maybe somebody else knows what needs to be done in the case of ExclusiveArch. APPROVED New Package CVS Request ======================= Package Name: libspe2 Short Description: SPE Runtime Management Library Owners: jroth Branches: F-9 EL-5 InitialCC: adrian Cvsextras Commits: yes cvs done. Jochen, you may want to close this bug with CLOSED/NEXTRELEASE or similar. |