Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 181777
Summary: | Review Request: CCfits A C++ interface for cfitsio | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Sergio Pascual <sergio.pasra> |
Component: | Package Review | Assignee: | Ed Hill <ed> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | ||
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2006-03-08 10:36:13 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: | |||
Bug Depends On: | |||
Bug Blocks: | 163779 |
Description
Sergio Pascual
2006-02-16 15:00:56 UTC
Hi Sergio, I started to do a review of this package and got this far: good: + specfile is legible + builds on FC4 (will try in mock later) + dir ownership looks good + *.la files removed needswork: - Something is the matter with the upstream server. All my connections to it are timing out so I can't get a copy of the upstream code to compare. Hopefully, it'll get fixed soon and we'll be able to proceed! - builds on FC4 and rpmlint returns the following: W: CCfits summary-ended-with-dot A C++ interface for cfitsio (FITS File Subroutine Library). W: CCfits invalid-license GPL compatible (see License.txt) E: CCfits binary-or-shlib-defines-rpath /usr/lib/libCCfits.so.0.0.0 ['/usr/lib'] W: CCfits-devel summary-ended-with-dot Headers for developing programs that will use CCfits. W: CCfits-devel invalid-license GPL compatible (see License.txt) W: CCfits-docs invalid-license GPL compatible (see License.txt) E: CCfits-docs script-without-shellbang /usr/share/doc/CCfits-docs-1.4/html/support_subs.pl E: CCfits-docs wrong-script-interpreter /usr/share/doc/CCfits-docs-1.4/html/ccfitschange_sff.pl "/usr1/local/bin/perl5" W: CCfits-docs doc-file-dependency /usr/share/doc/CCfits-docs-1.4/html/ccfitschange_sff.pl /usr1/local/bin/perl5 - The license is essentially BSD without the advertisement clause, so please list it as BSD (which is GPL-compatible) - Please change the main Summary: to "A C++ interface for cfitsio" and remove the trailing "."-s in the others - the "rpath" mentioned above will probably need to be fixed And I'm sorry this isn't a full review -- am going to need to get a copy from upstream to complete it. Hi Ed, I have fixed the points you mentioned: * Changed license type to BSD. The license for CCfits is exactly the same as the license of cfitsio, but in that package the license field of the rpm is GPL! * Removed some perl files in the html directory * Removed the trailing dots in Summary and * Removed rpath in the shared library compile instruction. The new spec and srpm are here: http://t-rex.fis.ucm.es/~spr/CCfits.spec http://t-rex.fis.ucm.es/~spr/CCfits-1.4-1.fc4.src.rpm Hi Sergio, please take a look at bug # 172042 where Matt points out that BSD code linked against GPL code becomes GPL-ed. I think thats correct but IANAL. And I'll try to finish the review later this week or the coming weekend. Hi Sergio, the 1.4-1 SRPM and its clearly an improvement: + no more errors or warnings from rpmlint + builds in mock on FC4 One blocker remains, though. The source copy in your 1.4-1 SRPM does not match the upstream copy. I untarred both and a diff shows that upstream has apparently removed the "CCfits/License.txt" file. And I don't see any mention of the license terms on the upstream web site, but I may have somehow missed it (please point it out if I did!). Ultimately, I think the license is fine since works made by the US government for research purposes are frequently "distributable" and usable by all -- unless otherwise noted. So, could you please create an updated SRPM that removes the need for the "CCfits/License.txt" file and I'll take a quick (and hopefully last!) one look? Great! I looked for the license on the web site also and, as I didn't find it, I contacted with the upstream mantainer. He sent me the License.txt file and also told me that he has planned to include the license in the next upstream release. The updated (and I hope final) spec and srpm without License.txt are here: http://t-rex.fis.ucm.es/~spr/CCfits.spec http://t-rex.fis.ucm.es/~spr/CCfits-1.4-2.fc4.src.rpm OK, I'm glad that the license will be included in future releases. Thats good because it clarifies things. However, that does not mean that you should create your own tar-file (as you have done in the last two SRPMs) that include the license file. What you should do is keep the source pristine (use an *exact* copy of the upstream tar file) and add the license file as a separate patch. So we still have a blocker here and its the same as before: the souce does not match upstream! Please fix it by either: - including the licence file as a aeparate patch, or - leaving out the license file (for now) and including it in a later version -- once the upstream folks start shipping it in the official sources The choice is yours. But the current form of the SRPM is not acceptable. Please check that http://t-rex.fis.ucm.es/~spr/CCfits-1.4-2.fc4.src.rpm does not include License.txt Hi Sergio, the tar-file contained in your SRPM: 84e8387ee5c399c7bf0888e14576bc2b CCfits-1.4-2.fc4.src.rpm still does not match the upstream tar file: 707777558c46f153fe0b1d994bab4a52 CCfits-1.4.tar.gz 230edb39de6a1e3dc6e0adda65d0676d CCfits-1.4.tar.gz.upstream and this is a blocker. And, yes, it does appear that your tar file does not contain the License.txt file. So, its possible that I confused the tar file from your SRPM and the one from upstream in the previous review when I was trying to see why they differed. If so, I apologize for any confusion. In any case, the point is that you should ship an exact copy of the upstream sources. So please create a CCfits-1.4-3.fc4.src.rpm that has an exact copy and I'll approve it. Hi Ed. The problem was that the upstream source was changed without changing the version number and I didn't notice it. The source now includes the license and that confused us. Here are the new spec and srpm: http://t-rex.fis.ucm.es/~spr/CCfits-1.4-3.fc4.src.rpm http://t-rex.fis.ucm.es/~spr/CCfits.spec OK, I don't see any remaining blockers: a37b2dadbb4ca6791bab70ac31950a9e CCfits-1.4-3.fc4.src.rpm APPROVED. The BuildRequires: gcc-c++ is not necessary. Yes, I see gcc-c++ is listed as a exception in http://fedoraproject.org/wiki/Packaging/Guidelines I will change it. |