Spec URL: http://www.seketeli.org/dodji/rpms/cloog/cloog.spec SRPM URL: http://www.seketeli.org/dodji/rpms/cloog/cloog-0.15-0.1.git95753.fc9.src.rpm Description: CLooG is a software which generates loops for scanning Z-polyhedra. That is, CLooG finds the code or pseudo-code where each integral point of one or more parametrized polyhedron or parametrized polyhedra union is reached. CLooG is designed to avoid control overhead and to produce a very efficient code.
Created attachment 317609 [details] My version of srpm I and Dodji made a srpm each. Lack of communication :( I did not look at Dodji's, so do not take it as a "fixed" or "better" version. Just uploading mine so that all work in one place.
To rebuild the srpm, you need the ppl library package, version 10pre30 at least. The problem is that it's not packaged in Fedora yet. I have updated the spec and srpm of the ppl package and filed a request in bug #463742. So you can go to that bug to get the srpm and build it so that you can build the cloog package.
Just noting that the maintainer of ppl on Fedora is also the upstream developer of ppl, so he actually knows pre31 (currently) is released. It may be that he has some reason he doesn't want to import pre tarball into Fedora.
@Mamoru: Yes, I know that. I just needed to have a packaged version of ppl 10pre<something> to actually be able to _build_ cloog, and maybe ease the process of review of cloog for those who might be willing to review of me. Cloog needs those versions of ppl. And I need a clood to be able to build/package gcc from trunk. It was not my intent to rush the ppl maintainer.
Some notes for 0.15-0.1: * SourceURL - When the source is created from git repository, please write in the spec file as comments how you created the source tarball: https://fedoraproject.org/wiki/Packaging/SourceURL#Using_Revision_Control * BuildRequires - As ppl-devel 0.10 is available on dist-f11, please modify BuildRequires. * Requires - Please check if all required packages are to be correctly installed for -devel subpackage. ! For example, %_includedir/%name/ppl_backend.h contains: -------------------------------------------------------------- 33 34 #include <ppl_c.h> 35 #include <gmp.h> 36 #include <stdlib.h> 37 #include <string.h> -------------------------------------------------------------- This means that -devel subpackage must have "Requires: ppl-devel gmp-devel" (here I am not saying about BuildRequires) * Group - Usually the main packge has "Group: System Environment/Libraries" and -devel subpackage has "Group: Development/Libraries". ! You can make your spec file based on the skeleton spec file created by $ rpmdev-newspec -t lib cloog * Dependency between main/subpackages - Usually the dependency between main package and subpackages must be EVR (Epoch:Version:Release) specific (i.e. -devel subpackage must have "Requires: %{name} = %{version}-%{release}) * Shipping static archives - Please explain why you want to ship static archives even if shared library is available. This must be avoided unless there is some specific reason. c.f. https://fedoraproject.org/wiki/Packaging/Guidelines#Exclusion_of_Static_Libraries * cflags - When compiling this software the option "-fomit-frame-pointer" is used. This option makes debugging very difficult, so this option must be removed. * Timestamps - Please consider to use -------------------------------------------------------------------- make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p" -------------------------------------------------------------------- to keep timestamps on installed files as much as possible. This method usually works for Makefiles generated by recent autotools. * %_infodir/dir file - Rebuild fails without -------------------------------------------------------------------- rm -f $RPM_BUILD_ROOT%{_infodir}/dir -------------------------------------------------------------------- http://koji.fedoraproject.org/koji/taskinfo?taskID=921939 * Info files - Files under %_infodir are automatically regarded as %doc. - Please follow https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Texinfo * %changelog format - For how to write %changelog, please follow https://fedoraproject.org/wiki/Packaging/Guidelines#Changelogs * Documents - Please add the files below to %doc of main package. -------------------------------------------------------------------- README -------------------------------------------------------------------- ! Note Please change the release number every time you modify your spec/srpm to avoid confusion.
ping?
Pong, Thank you very much for the review. I am going to go through it and fix things as requested. Cheers.
So I have updated the spec file and srpm following your recommendations. The new spec file is at http://www.seketeli.org/dodji/rpms/cloog/cloog.spec-0.15-0.2.git The new srpm is at http://www.seketeli.org/dodji/rpms/cloog/cloog-0.15-0.2.git57a0bc.fc10.src.rpm. Thanks again for your review.
For -0.2 ! First of all, you can try to check your package by rpmlint (in rpmlint package). rpmlint detects some general packaging issues. Then: * Requires ----------------------------------------------------------- Requires: ppl-devel >= 0.10, gmp-devel >= 4.1.3 ----------------------------------------------------------- - This should be for cloog-devel package, not for cloog package * Group - Group for -devel subpackage should be "Development/Libraries", not "Development/Library" * %_infodir/dir - This file must not be installed. Overwriting this file by this rpm breaks system "info" information. * Please remove this file at %install (not %clean) and remove this file from %files * And restore the previous %clean * Shipping static archives - What I meant is that unless some specific reason static archive libfoo.a must be removed (as well as libtool .la file) * calling ldconfig on scriptlet - When using "%post -p /sbin/ldconfig" (i.e. using /sbin/ldconfig directly instead of calling bash and executing ldconfig in the shell script), no other additional scriptlets are allowed. i.e. if there is some other scriptlets than /sbin/ldconfig, you have to write like: ---------------------------------------------------------- %post /sbin/ldconfig /sbin/install-info %{_infodir}/%{name}.info %{_infodir}/dir || : ---------------------------------------------------------- * %changelog - git revision differs between %release and %change.log ? Some header files design flaw - Well, for example the head of %_includedir/cloog/cloog.h says: ---------------------------------------------------------- 40 #ifndef CLOOG_H 41 #define CLOOG_H 42 43 #ifdef CLOOG_PPL_BACKEND 44 # define GNUMP 45 # include<cloog/ppl_backend.h> 46 #else 47 # include <polylib/missing.h> 48 # include<cloog/polylib_backend.h> 49 #endif ---------------------------------------------------------- However, where can we tell if CLOOG_PPL_BACKEND is defined or not (when this package was built) (i.e whether this header file includes ppl_backend.h or polylib_backend.h)? build.log shows that when rebuilding this package -DCLOOG_PPL_BACKEND is used, however installed header files does not save such information....
ping again?
Sorry for my late reply. The updated spec file is at http://www.seketeli.org/dodji/rpms/cloog/cloog.spec.0.15-0.2.git57a0bc. and the updated srpm is at http://www.seketeli.org/dodji/rpms/cloog/cloog-0.15-0.2.git57a0bc.fc10.src.rpm. Please find below my answers to your review. Thanks. >For -0.2 > >! First of all, you can try to check your package by rpmlint > (in rpmlint package). rpmlint detects some general packaging > issues. Done. I am still getting those two errors, but I am not sure how to fix them: cloog.x86_64: W: shared-lib-calls-exit /usr/lib64/libcloog.so.0.0.0 exit.5 cloog-devel.x86_64: W: no-documentation I'd appreciate any help there. > >Then: >* Requires >----------------------------------------------------------- >Requires: ppl-devel >= 0.10, gmp-devel >= 4.1.3 >----------------------------------------------------------- > - This should be for cloog-devel package, not for cloog package Fixed. > >* Group > - Group for -devel subpackage should be "Development/Libraries", > not "Development/Library" Fixed. >* %_infodir/dir > - This file must not be installed. Overwriting this file > by this rpm breaks system "info" information. > > * Please remove this file at %install (not %clean) > and remove this file from %files Done. > * And restore the previous %clean Done. >* Shipping static archives > - What I meant is that unless some specific reason static > archive libfoo.a must be removed (as well as libtool .la > file) I guess you meant that for the -devel package. I added an exclude for the .a library as well as for the .la files. > >* calling ldconfig on scriptlet > - When using "%post -p /sbin/ldconfig" (i.e. using /sbin/ldconfig > directly instead of calling bash and executing ldconfig in the > shell script), no other additional scriptlets are allowed. > > i.e. if there is some other scriptlets than /sbin/ldconfig, > you have to write like: >---------------------------------------------------------- >%post >/sbin/ldconfig >/sbin/install-info %{_infodir}/%{name}.info %{_infodir}/dir || : >---------------------------------------------------------- Done. > >* %changelog > - git revision differs between %release and %change.log Fixed. > >? Some header files design flaw > - Well, for example the head of %_includedir/cloog/cloog.h > says: >---------------------------------------------------------- > 40 #ifndef CLOOG_H > 41 #define CLOOG_H > 42 > 43 #ifdef CLOOG_PPL_BACKEND > 44 # define GNUMP > 45 # include<cloog/ppl_backend.h> > 46 #else > 47 # include <polylib/missing.h> > 48 # include<cloog/polylib_backend.h> > 49 #endif >---------------------------------------------------------- > However, where can we tell if CLOOG_PPL_BACKEND is defined > or not (when this package was built) (i.e whether this header > file includes ppl_backend.h or polylib_backend.h)? > build.log shows that when rebuilding this package -DCLOOG_PPL_BACKEND > is used, however installed header files does not save such > information.... How can I fix this ? I mean this is an upstream problem. Do you mean I should append a patch to the package ? I could as well ship the package as is, and submit a patch upstream to fix it ?
Well, would you change the release number every time you modify your spec file? (In reply to comment #12) > cloog.x86_64: W: shared-lib-calls-exit /usr/lib64/libcloog.so.0.0.0 > exit.5 - This library actually calls exit() in the library (e.g. source/names.c). This is unusual situation. Usually when some unexpected behavior happens in a function in a library, the function should return a value which tells the error or so and should call exit() ($ rpmlint -I shared-lib-calls-exit shows the explanation). Would you contact upstream? > cloog-devel.x86_64: W: no-documentation - This warning can be ignored. > I'd appreciate any help there. > >? Some header files design flaw > > - Well, for example the head of %_includedir/cloog/cloog.h > > says: > >---------------------------------------------------------- > > 40 #ifndef CLOOG_H > > 41 #define CLOOG_H > > 42 > > 43 #ifdef CLOOG_PPL_BACKEND > > 44 # define GNUMP > > 45 # include<cloog/ppl_backend.h> > > 46 #else > > 47 # include <polylib/missing.h> > > 48 # include<cloog/polylib_backend.h> > > 49 #endif > >---------------------------------------------------------- > > However, where can we tell if CLOOG_PPL_BACKEND is defined > > or not (when this package was built) (i.e whether this header > > file includes ppl_backend.h or polylib_backend.h)? > > build.log shows that when rebuilding this package -DCLOOG_PPL_BACKEND > > is used, however installed header files does not save such > > information.... > > How can I fix this ? I mean this is an upstream problem. Do you mean I should > append a patch to the package ? > I could as well ship the package as is, and submit a patch upstream to fix it ? - For this issue, I don't think this issue can be unresolved (I won't approve this package unless this is fixed). At least a patch should be appended or so and also this must be fixed upstream.
Again ping?
Created attachment 331051 [details] Adds a cloog-config.h file to the distribution This patch creates a cloog-config.h file that contains the configuration macros for this package. It then removes the need to have -DFOO configuration macros passed at compile time. I guess it should fix the problem you raised with the cloog/cloog.h header as the user can know how the cloog library was configured. If this approach is okay with you I will forward the patch to upstream.
Created attachment 331052 [details] pass make distcheck This patch removes some files left in builddir after make clean. Those where preventing make distcheck to pass. I am stacking this here because I'd like to forward it to upstream. At least it won't be lost if it's here.
> This library actually calls exit() in the library (e.g. > source/names.c). This is unusual situation. Usually when some unexpected > behavior happens in a function in a library, the function should > return a value which tells the error or so and should call > exit() ($ rpmlint -I shared-lib-calls-exit shows the explanation). > Would you contact upstream? Yes, sure. Though is this a blocker for the package to be sponsored ? I mean this is typically something that can be fixed in a subsequent upstream release. The reason why I am being a bit dense on this is that there are lots of exit() calls in the library and changing those would require some real API design changes. Otherwise, I would have provided a patch myself already. I will batch forward this issue to upstream with the patch https://bugzilla.redhat.com/attachment.cgi?id=331052 when you approve it. Thanks for your time.
(In reply to comment #17) > Created an attachment (id=331051) [details] > Adds a cloog-config.h file to the distribution Seems good. (In reply to comment #19) > > This library actually calls exit() in the library (e.g. > > source/names.c). This is unusual situation. Usually when some unexpected > > behavior happens in a function in a library, the function should > > return a value which tells the error or so and should call > > exit() ($ rpmlint -I shared-lib-calls-exit shows the explanation). > > > Would you contact upstream? > > Yes, sure. Though is this a blocker for the package to be sponsored ? Not a blocker.
I sent the patches to upstream. I am waiting for comments.
Upstream accepted the patches.They should properly appear in the upstream git soon.
Hey Mamoru, I am thinking that maybe I should change the name of the package from cloog to cloog-ppl, as this branch of the package is really intended to be compiled with the ppl backend only. When compiled with say the polylib backend, it becomes another library as include/cloog/polylib_backend.h is then included by cloog.h and induces a new API. In that case a new package cloog-polylib would then be generated. Also, cloog-ppl (that is at http://repo.or.cz/w/cloog-ppl.git) really is a special branch of the cloog tree (that is at http://repo.or.cz/w/cloog.git) where the ppl backend branch work is happening. It will be later merged back into the cloog main tree, after gcc 4.4 is released. So gcc 4.4 will be based on cloog-ppl, whereas gcc 4.5 is likely to be based on cloog, after cloog-ppl is merged back into the main tree. What do you think about this ?
Well, do you want to "re"submit a new review request for ppl once cloog-ppl is merged to ppl main tree? When the srpm name is to be renamed, we have to submit another review request for it. If not, I think the less troublesome way is to create cloog-ppl(-devel) binary rpms from ppl srpm.
(In reply to comment #24) > If not, I think the less troublesome way is to create > cloog-ppl(-devel) binary rpms from ppl srpm. Of course "from cloog srpm", sorry...
Okay, So I tried to do what you proposed. I now generate cloog-ppl and cloog-ppl-devel packages from the ppl srpm. spec: http://people.redhat.com/dseketel/rpms/cloog/cloog.spec.0.15-0.3.gitad322 srpm: http://people.redhat.com/dseketel/rpms/cloog/cloog-0.15-0.3.gitad322.fc10.src.rpm
Well: * Not a blocker, however "rpm -ivv cloog-ppl" bears some warning on info file: --------------------------------------------------------- D: adding 4 entries to Filedigests index. D: install: %post(cloog-ppl-0.15-0.3.gitad322.fc11.i386) synchronous scriptlet start D: install: %post(cloog-ppl-0.15-0.3.gitad322.fc11.i386) execv(/bin/sh) pid 5361 + /sbin/ldconfig + /sbin/install-info /usr/share/info/cloog.info /usr/share/info/dir install-info: warning: no info dir entry in `/usr/share/info/cloog.info' --------------------------------------------------------- * Usually for a packager needing sponsorship, I request him/her to either - submit another review request - or do a (at least one) pre-review of other person's review request as written on http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored . Do you have another review request of yours or review requests on which you did some pre-review? Note: Fedora package collection review requests which are waiting for someone to review can be checked on my wiki page: http://fedoraproject.org/wiki/User:Mtasaka#B._Review_request_tickets (Check "No one is reviewing")
For the info problem, I will submit a bug report to upstream. As for the second review request, I have just filed a new one at https://bugzilla.redhat.com/show_bug.cgi?id=485496.
Now approving. -------------------------------------------------------- This package (cloog) is APPROVED by mtasaka --------------------------------------------------------
Thanks.
New Package CVS Request ======================= Package Name: cloog Short Description: The Chunky Loop Generator Owners: dodji Branches: F-10 InitialCC: dodji
cvs done.
By the way, can this package be rebuilt on F-10? F-10 ppl is still 0.9-25.fc10. If you won't build this package on F-10, please close this bug as NEXTRELEASE.
No, it can't. It depends on ppl 0.10 that is available on the coming F-11 only. The only envisionned consumer of cloog-ppl is gcc 4.4 for now anyway.