Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: dragonegg - GCC plugin to use LLVM optimizers and code generators
https://bugzilla.redhat.com/show_bug.cgi?id=759712
Summary: Review Request: dragonegg - GCC plugin to use LLVM optimizers and code generators Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: eric@brouhaha.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, package-review@lists.fedoraproject.org Classification: Fedora Story Points: --- Type: ---
Spec URL: http://fedorapeople.org/~brouhaha/dragonegg/dragonegg.spec SRPM URL: http://fedorapeople.org/~brouhaha/dragonegg/dragonegg-3.0-0.1.rc3.fc16.src.r... Description: DragonEgg is a GCC plugin that replaces GCC's optimizers and code generators with those from the LLVM project.
Note that this package is for DragonEgg 3.0.rc3 rather than 3.0 final. This is because the LLVM package in rawhide is currently 3.0.rc3. I have done private builds of both LLVM 3.0 and DragonEgg 3.0 on Fedora 16, and verified that both work. I will update the DragonEgg spec for 3.0 final once the rawhide LLVM package is updated.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=759712
--- Comment #1 from Eric Smith eric@brouhaha.com 2011-12-03 06:13:32 EST --- I've found that DragonEgg 3.0 seems to work fine with the LLVM 3.0.rc3 in rawhide, so I loosened the buildrequires and updated the spec.
Spec URL: http://fedorapeople.org/~brouhaha/dragonegg/dragonegg.spec SRPM URL: http://fedorapeople.org/~brouhaha/dragonegg/dragonegg-3.0-1.fc16.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=759712
--- Comment #2 from Jerry James loganjerry@gmail.com 2011-12-15 17:55:56 EST --- I tried to build this on a rawhide machine, and got this: ... + make -j3 Compiling utils/TargetInfo.cpp g++: error: directory": No such file or directory make: *** [TargetInfo.o] Error 1 error: Bad exit status from /var/tmp/rpm-tmp.itjbTr (%build)
Building with "make VERBOSE=1" shows that the g++ invocation includes this: -DREVISION="Unversioned directory". The shell sees that as two words: -DREVISION="Unversioned directory"
REVISION is set to that value by svnversion. So this package cannot be built on a machine where svnversion is installed.
In mock, on the other hand, the build succeeds, since nothing pulls in svnversion, but I see this in the logs: make: svnversion: Command not found
It looks like REVISION is only used in one source file, src/Backend.cpp, and that the code that uses it is only compiled if IDENT_ASM_OP is defined, and that nothing defines IDENT_ASM_OP. So I did this in %prep:
sed -i "s/^REVISION:=.*/REVISION:=%{version}/" Makefile
What do you think?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=759712
Jerry James loganjerry@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@fedoraproject.org |loganjerry@gmail.com Flag| |fedora-review?
--- Comment #3 from Jerry James loganjerry@gmail.com 2011-12-15 19:14:33 EST --- I'll take this review. Quick note: the %defattr in %files is no longer needed.
Also, I had some kind of problem with the gcc versioning. I tried to install after building: # rpm -i dragonegg-3.0-1.fc17.x86_64.rpm error: Failed dependencies: gcc = 4.6.2-1.fc17 is needed by dragonegg-3.0-1.fc17.x86_64 # rpm -q gcc gcc-4.6.2-1.fc17.1.x86_64
Legend: +: OK -: must be fixed =: should be fixed (at your discretion) N: not applicable
MUST: [+] rpmlint output: dragonegg.x86_64: W: spelling-error Summary(en_US) optimizers -> optimizer, optimizes, optimize rs dragonegg.x86_64: W: spelling-error %description -l en_US optimizers -> optimizer, optimizes, optimize rs dragonegg.spec:12: W: macro-in-comment %{version} dragonegg.spec:12: W: macro-in-comment %{release} dragonegg.spec:19: W: macro-in-comment %{gcc_version} dragonegg.spec:20: W: macro-in-comment %{gcc_release} dragonegg.spec:22: W: macro-in-comment %{version} dragonegg.spec:22: W: macro-in-comment %{gcc_release} dragonegg.spec:76: W: macro-in-comment %{optflags} 2 packages and 1 specfiles checked; 0 errors, 9 warnings.
Those macros in comments need doubled % signs. [+] follows package naming guidelines [+] spec file base name matches package name [+] package meets the packaging guidelines [+] package uses a Fedora approved license [+] license field matches the actual license [+] license file is included in %doc [+] spec file is in American English [+] spec file is legible [+] sources match upstream: md5sum is 3704d215fb4343040eaff66a7a87c63a for both [+] package builds on at least one primary arch (tried x86_64) [N] appropriate use of ExcludeArch: Question: is llvm available on all arches? [+] all build requirements in BuildRequires [N] spec file handles locales properly [N] ldconfig in %post and %postun [+] no bundled copies of system libraries [N] no relocatable packages [+] package owns all directories that it creates [+] no files listed twice in %files [+] proper permissions on files [+] consistent use of macros [+] code or permissible content [N] large documentation in -doc [+] no runtime dependencies in %doc [N] header files in -devel [N] static libraries in -static [N] .so in -devel: this .so is a plugin, and is in exactly the right place [N] -devel requires main package [+] package contains no libtool archives [N] package contains a desktop file, uses desktop-file-install [+] package does not own files/dirs owned by other packages [+] all filenames in UTF-8
SHOULD: [N] query upstream for license text [N] description and summary contains available translations [+] package builds in mock: tried fedora-rawhide-i386 [+] package builds on all supported arches: tried i386 and x86_64 [-] package functions as described: could not test because I could not install; see above [+] sane scriptlets [N] subpackages require the main package [N] placement of pkgconfig files [+] file dependencies versus package dependencies [N] package contains man pages for binaries/scripts
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=759712
--- Comment #4 from Eric Smith eric@brouhaha.com 2011-12-15 19:38:53 EST --- Thanks for reviewing this. I agree with your patch for REVISION, will include it in the next spec, and will report a bug upstream if there isn't one already.
I'll look into the gcc dependency issue with rawhide. I didn't have that problem when I built the llvm-3.0rc3 and dragonegg packages on F16. I'll install rawhide in a VM for testing and report back when I have an updated spec and SRPM that fixes it.
I respectfully disagree with changing the macro-in-comment warnings as they are in comments copied directly from the existing gcc-python-plugin spec, and doubled percent signs might be confusing to anyone trying to actually understand the comments. I'd rather have the rpmlint warnings. However, if you really feel strongly that those warnings have to be fixed, let me know, and I'll do it.
LLVM is supported on i386 and x86_64, which are the current Fedora Primary Architectures. The Fedora llvm package spec excludes use of ocaml on s390, s390x, and sparc64, but does not exclude those or any other architectures. While I have a hard time believing that LLVM will work on all of the Fedora Secondary Architectures, if the LLVM spec doesn't exclude them I don't think the DragonEgg spec needs to either.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=759712
--- Comment #5 from Jerry James loganjerry@gmail.com 2011-12-16 16:02:36 EST --- (In reply to comment #4)
I respectfully disagree with changing the macro-in-comment warnings as they are in comments copied directly from the existing gcc-python-plugin spec, and doubled percent signs might be confusing to anyone trying to actually understand the comments. I'd rather have the rpmlint warnings. However, if you really feel strongly that those warnings have to be fixed, let me know, and I'll do it.
What I feel strongly about is that the rpm developers should fix their code so that rpm doesn't expand macros in comments. :-) As long as those macros don't cause any actual problems (and I did not notice any), then I am fine with leaving the comments as they are.
LLVM is supported on i386 and x86_64, which are the current Fedora Primary Architectures. The Fedora llvm package spec excludes use of ocaml on s390, s390x, and sparc64, but does not exclude those or any other architectures. While I have a hard time believing that LLVM will work on all of the Fedora Secondary Architectures, if the LLVM spec doesn't exclude them I don't think the DragonEgg spec needs to either.
That's fine. The DragonEgg spec should definitely follow the llvm spec in this regard. I just wasn't sure if the llvm spec had any ExcludeArch tags, and was too lazy to go look.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=759712
--- Comment #6 from Eric Smith eric@brouhaha.com 2012-02-06 06:42:45 EST --- The reason you had the dependency problem was that your gcc package ended in .fc17.1.x86_64 but should have been .fc17.x86_64. I have no idea where that extraneous .1 came from, but what I see in rawhide now is gcc-4.7.0-0.10.fc17.x86_64, which doesn't have that problem.
I *finally* have a working rawhide install. However, it appears that DragonEgg does not support gcc 4.7 yet, with build failures in src/Backend.cpp. It looks like this will have to remain on hold until that and any other gcc 4.7 incompatibilities are fixed.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=759712
--- Comment #7 from Jerry James loganjerry@gmail.com 2012-02-06 15:01:43 EST --- The RPM name probably came from somebody doing this: https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Minor_release_bump.... While the current Rawhide GCC doesn't have such a trailing number, this could happen again in the future. You'll need a strategy for dealing with it.
Good luck with gcc 4.7! I had to fix several of my packages, too.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=759712
--- Comment #8 from Eric Smith eric@brouhaha.com 2012-02-06 15:18:49 EST --- OK, I hadn't noticed the "Minor release bumps for old branches" in the naming guidelines before. I'm a little baffled about how that would come about in a rawhide package, but certainly my spec will need to address it.
https://bugzilla.redhat.com/show_bug.cgi?id=759712
--- Comment #9 from Eric Smith eric@brouhaha.com --- Rawhide now has LLVM 3.1, and DragonEgg 3.1 mostly supports GCC 4.7 (see the DragonEgg release notes for details), so it's time to try this again.
I have been unable to find any automatic way to deal with "minor release bumps for old branches" for GCC. There is apparently no way to get the required information at build time in Koji, for reasons stated at the top of the spec file. Note that this mechanism is copied from the gcc-python-plugin package, which thus has the same problem. If this actually occurs, it can be dealt with by manually changing the package spec, either turning off the with_hard_gcc_version_requirement flag, or tweaking the value of gcc_vr.
Spec URL: http://fedorapeople.org/~brouhaha/dragonegg/dragonegg.spec SRPM URL: http://fedorapeople.org/~brouhaha/dragonegg/dragonegg-3.1-1.fc18.src.rpm Koji scratch build for Rawhide: http://koji.fedoraproject.org/koji/taskinfo?taskID=4135197
https://bugzilla.redhat.com/show_bug.cgi?id=759712
Jerry James loganjerry@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+
--- Comment #10 from Jerry James loganjerry@gmail.com --- Okay, it looks great. Bummer about the minor release bumps, thing, but I don't see how to deal with it automatically either.
This package is APPROVED.
https://bugzilla.redhat.com/show_bug.cgi?id=759712
Eric Smith eric@brouhaha.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-cvs?
--- Comment #11 from Eric Smith eric@brouhaha.com --- New Package CVS Request ======================= Package Name: mingw-llvm Short Description: GCC plugin to use LLVM optimizers and code generators Owners: brouhaha Branches: InitialCC:
https://bugzilla.redhat.com/show_bug.cgi?id=759712
--- Comment #12 from Jon Ciesla limburgher@gmail.com --- Summary and SCM request package manes don't match, please correct.
https://bugzilla.redhat.com/show_bug.cgi?id=759712
--- Comment #13 from Eric Smith eric@brouhaha.com --- New Package CVS Request ======================= Package Name: dragonegg Short Description: GCC plugin to use LLVM optimizers and code generators Owners: brouhaha Branches: InitialCC:
Sorry about the cut-and-paste error!
https://bugzilla.redhat.com/show_bug.cgi?id=759712
Eric Smith eric@brouhaha.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-cvs?
https://bugzilla.redhat.com/show_bug.cgi?id=759712
--- Comment #14 from Jon Ciesla limburgher@gmail.com --- Git done (by process-git-requests).
https://bugzilla.redhat.com/show_bug.cgi?id=759712
Eric Smith eric@brouhaha.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution|--- |NEXTRELEASE Last Closed| |2012-06-10 14:23:55
https://bugzilla.redhat.com/show_bug.cgi?id=759712
--- Comment #15 from Fedora Update System updates@fedoraproject.org --- dragonegg-3.4-1.el7.2 has been submitted as an update for Fedora EPEL 7. https://admin.fedoraproject.org/updates/dragonegg-3.4-1.el7.2
https://bugzilla.redhat.com/show_bug.cgi?id=759712
--- Comment #16 from Fedora Update System updates@fedoraproject.org --- dragonegg-3.4-2.el7 has been submitted as an update for Fedora EPEL 7. https://admin.fedoraproject.org/updates/dragonegg-3.4-2.el7
https://bugzilla.redhat.com/show_bug.cgi?id=759712
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed In Version| |dragonegg-3.4-2.el7 Resolution|NEXTRELEASE |ERRATA
--- Comment #17 from Fedora Update System updates@fedoraproject.org --- dragonegg-3.4-2.el7 has been pushed to the Fedora EPEL 7 stable repository.
package-review@lists.fedoraproject.org