Spec URL: https://raw.github.com/Ignotus/fedora-packages/6779dd2061c252ccf26f525197ece392f38c5cbd/chicken/chicken.spec SRPM URL: http://kojipkgs.fedoraproject.org//work/tasks/1447/4671447/chicken-4.7.0.6-1.fc17.src.rpm Description: A compiler for the Scheme programming language Fedora Account System Username: minh
Oh, I'm sorry. It's a duplicate of the #819919
*** This bug has been marked as a duplicate of bug 819919 ***
Minh Ngo: Are you still interested in getting chicken-scheme into fedora? The duplicate 819919 has just been closed, non-responsive submitter. I can review if you like.
Yes, I'm
OK, I'll review this one.
OK, here we go! I'm aware that compilers are tricky, so please enlighten me if I'm just wrong... Some initial remarks: - The srpm link seems dead (error 404 for me). I rebuilt from spec for now. - I cannot find and GPL or MIT licensing in the sources at a quick glance. Could you comment on why the License: is "GPLv2 and BSD and MIT"? (the tests are not part of the binary RPM:s, so test licenses does not apply AFAIK). - Here is a -static subpackage. Could you comment on why this is indeed necessary? [5] - Are the %{optflags} really in effect as required in [6] ? - We have have rpmlint rpath warnings need to be dealt with [1]. - We have some header files under /usr/include. Shouldn't these go to a - -devel pkg as well as /usr/lib/libchicken.so? [2] - The package Provides: a lot of private libs under /usr/lib/chicken/6 which need to be filtered, see [3] and [4]. - The package chicken-doc is arched and will thus be duplicated in repos. Shouldn't it be noarch? - Use a wildcard as manpage extension (not .gz) as this might change. - rpmlint no-manual-page-for-binary can be ignored. - The private so-files files are generating rmplint devel-file-in-non-devel-package. These can be ignored. [1] http://fedoraproject.org/wiki/Packaging:Guidelines#Beware_of_Rpath [2] http://fedoraproject.org/wiki/Packaging:Guidelines#Devel_Packages [3] https://fedoraproject.org/wiki/Packaging:AutoProvidesAndRequiresFiltering [4] https://fedorahosted.org/fpc/ticket/189 [5] http://fedoraproject.org/wiki/Packaging:Guidelines#Packaging_Static_Libraries [6] http://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags
Hi! To get this review rolling I need you to reply to my remarks! If anything is unclear (some of the remarks are admittedly somewhat vague) just let me know, and we'll sort it out together.
To Alec: [ignotus@laptop chicken]$ wget http://code.call-cc.org/stability/4.7.0/chicken-4.7.0.6.tar.gz --2013-01-14 20:37:10-- http://code.call-cc.org/stability/4.7.0/chicken-4.7.0.6.tar.gz Resolving code.call-cc.org (code.call-cc.org)... 209.172.33.78 Connecting to code.call-cc.org (code.call-cc.org)|209.172.33.78|:80... connected. HTTP request sent, awaiting response... 200 OK Length: 3425028 (3.3M) [application/x-gzip] Saving to: `chicken-4.7.0.6.tar.gz' I can download an archive by this URL. Please check on your side again
Oh, I'm sorry, You have told about SRPM archive :)
What about licenses. Look in the LICENSE file. Line 239 - GPL Line 1 - BSD Line 12 - Seems like MIT license
have removed static lib have created a devel package SPEC: https://raw.github.com/Ignotus/fedora-packages/80fa8eb7ac1d5cfd75396b885652d07614563aec/chicken/chicken.spec SRPM: https://dl.dropbox.com/s/72m0w26j72nl9jp/chicken-4.7.0.6-2.fc17.src.rpm?dl=1
Hi Ngo! Nice to hear from you (back from New Year celebrations?) We have settled the first issue, the links. As for the LICENSE, the fact that it's mentioned in the LICENSE file isn't enough to get it included; e. g., licenses for test files which are not part of package should not be included the in License: tag. Furthermore, if multiple licenses are indeed used as you propose you must somehow provide a license break-down. [1] The static archive and package is removed. Fine with me, I assume you have checked that it wasn't essential for using the thing. Although the changelog states that you have coped with %{optflags} I cannot see that it's used. On the contrary, an example compilaton in the log is like gcc -L. -shared -Wl,-R"/usr/lib" srfi-13.import.o -o srfi-13.import.so \ -lchicken \ -lm -ldl Here you have at least two problems: the %{optflags} are missing, and here is also the reason for the bad rpath (see below). As a starter, you could try adding export CFLAGS="%{optflags}" before the 'make'. It might be enough to include %{optflags}. Use "rpm --eval %optflags" to see what optflags really is, and compare with the build logs to see that they are in effect. The rpath is trickier, you will need to patch the build script to get rid of that. Run rpmlint yourself, and look for lines like "chicken.i686: E: binary-or-shlib-defines-rpath /usr/bin/csc ['/usr/lib']". You must get rid of those, period. The -devel package solves the libchicken.so and /usr/include files nicely. The -doc package is still arched i. e., it's missing Arch: noarch tag. Just add that to the subpackage (to avoid generating different docs for i386/x86_64/arm). Please change %doc %{_mandir}/man1/cs?.1.gz to %doc %{_mandir}/man1/cs?.1.* - rumor is that gz some day will be replaced w something else. If you make something like rpm -q --provides -p <your binary rpm> you will see (excerpt) chicken: chicken chicken(x86-32) chicken.import.so csi.import.so data-structures.import.so extras.import.so files.import.so foreign.import.so irregex.import.so libchicken.so.6 The problem here is that almost all of these libraries are private and not meant to be used by others. But since they are in the Provides: list, other packages might (and thus eventually will) link to them causing all sorts of troubles e. g. when upgrading. To avoid this you must filter these Provides: as described in links in comment #6. This is actually a mess. Perhaps example in [2] could be useful, don't know. Summary: - Fix the rpath - Filter the Provides: - Either provide a license break-down if needed, or use a single license if this is the outcome of your break-down work. - Fix noarch docs, - Fix.* manpage in %files, Don't hesitate to ask! I'm here to help you :) [1] http://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Multiple_Licensing_Scenarios [2] http://leamas.fedorapeople.org/spotify/0.8.8/spotify-client.spec
PS. I forgot 'fix %{optflags}' in the summary. I bet you didn't make the same miss, but just in case... DS
Some more random remarks on documentation: - The LICENSE, NEWS* and README should all be in %doc, preferably in the base package (for LICENSE, this is a must). - The manpages can go in the base package as well, they are small and needed. - The build produces a nice html manual. Put that that into the -doc subpackage, it's a shame not packaging it!
Hi again. I'm sorry because of the late answer. I have also celebrated the Vietnamese New Year :). Seems most of your remarks have been fixed. SRPM: https://dl.dropbox.com/s/k4ctmvsgp2m1dx3/chicken-4.7.0.6-3.el6.src.rpm?token_hash=AAEObhnw7gQqGbPQMZai2c4x27m58_qiTKdc1t_oEn_aPA&dl=1 SPEC: https://raw.github.com/Ignotus/fedora-packages/3e350102bdc2bdefa738ce0fd744963b760551be/chicken/chicken.spec Please write me if I have forgotten something. - Adding LICENSE NEWS* README* files to the base package - Moving man pages to the base package - Adding html manuals into the doc package - Adding library filter - Moving include files to the devel package - Removing the static lib I know about the problem with %{optflags}. I'm going to fix it today but later.
Hi! Nice to hear from you, but I guessed it was the new year. Welcome back! License: If you insist on multiple licenses, you must provide a license break-down (links in comment #12). I'm not convinced this is necessary though. rpaths: have you really got rid of those and checked with rpmlint? The macros in %check also looks *very* strange, what's going on here? Static lib: Isn't it the buildflag STATICBUILD which enables this? Can't you just disable that instead of building and removing the .a lib? Just a guess from my side. Please check the provides (build binary rpms and run rpm -q --provide -p <your binary package> and verify that all looks sound. Also verify that you can install the package, since this often becomes problematic when filtering) Have you checked the build log and verified that %optflags is used? BuildArch: noarch is still missing for the docs package.
Ping?! This goes real slow... is it that you don't really have time for this, or is it something in the remarks which causes difficulties?
(In reply to comment #17) > Ping?! This goes real slow... is it that you don't really have time for > this, or is it something in the remarks which causes difficulties? I can continue working on this now that I have internet again, if need be.
Which is nice, thanks. Still, this bug (874980) is submitted by Minh Ngo, and he's the one which eventually need to submit the package. That said, it might be that Minh would appreciate a helping hand now during the review and perhaps also later as a co-maintainer. May I suggest that you approach him by a private email if you you are interested in this? Also, while writing: there are new filtering guidelines under way at [1]. That draft is voted on right now in the FPC. Given that current filtering guidelines are completely outdated, I think it's a better idea to use this draft to make a working filter setup. [1] https://fedoraproject.org/wiki/User:Toshio/AutoProvidesAndRequiresFilteringDraft
Minh: It's now a full month since your last message, so I invoke the stalled review policy [1]. Please state your intentions ASAP. [1] http://fedoraproject.org/wiki/Policy_for_stalled_package_reviews
Closing bug: unresponsive submitter as of policy in comment #20.