Bug 225653
Summary: | Merge Review: concurrent | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Nobody's working on this, feel free to take it <nobody> |
Component: | Package Review | Assignee: | Matt Wringe <mwringe> |
Status: | CLOSED RAWHIDE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | pcheung |
Target Milestone: | --- | Flags: | mwringe:
fedora-review+
|
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2007-04-05 18:44:52 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
Nobody's working on this, feel free to take it
2007-01-31 17:51:51 UTC
Please review updated spec file and srpm at: https://pcheung.108.redhat.com/files/documents/174/357/concurrent.spec https://pcheung.108.redhat.com/files/documents/174/358/concurrent-1.3.4-5jpp.1.fc7.src.rpm MUST: * package is named appropriately - match upstream tarball or project name - try to match previous incarnations in other distributions/packagers for consistency - specfile should be %{name}.spec - non-numeric characters should only be used in Release (ie. cvs or something) - for non-numerics (pre-release, CVS snapshots, etc.), see http://fedoraproject.org/wiki/Packaging/NamingGuidelines#PackageRelease - if case sensitivity is requested by upstream or you feel it should be not just lowercase, do so; otherwise, use all lower case for the name OK * is it legal for Fedora to distribute this? - OSI-approved - not a kernel module - not shareware - is it covered by patents? - it *probably* shouldn't be an emulator - no binary firmware X I don't know if we can just distribute this. The project claims to be in the public domain but sections of it are covered by a Technology License from Sun Microsystems Inc. (http://gee.cs.oswego.edu/dl/classes/EDU/oswego/cs/dl/util/sun-u.c.license.pdf) * license field matches the actual license. X the license field does not mention the Technology License * license is open source-compatible. - use acronyms for licences where common X I don't know if the Technology License is open source-compatible * specfile name matches %{name} OK * verify source and patches (md5sum matches upstream, know what the patches do) - if upstream doesn't release source drops, put *clear* instructions on how to generate the the source drop; ie. # svn export blah/tag blah # tar cjf blah-version-src.tar.bz2 blah OK, md5sum matches * skim the summary and description for typos, etc. OK * correct buildroot - should be: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) OK * if %{?dist} is used, it should be in that form (note the ? and % locations) OK * license text included in package and marked with %doc X The source does not include a specific license file, but it does mention the terms of the license in the intro.html file included. This file has a broken link to the Sun Technology license which should be patched. * keep old changelog entries; use judgement when removing (too old? useless?) * packages meets FHS (http://www.pathname.com/fhs/) * rpmlint on <this package>.srpm gives no output rpmlint concurrent-1.3.4-5jpp.1.fc7.src.rpm W: concurrent non-standard-group Development/Libraries/Java W: concurrent strange-permission concurrent.tar.gz 0660 W: concurrent strange-permission concurrent-1.3.4.build.xml 0660 W: concurrent strange-permission concurrent.spec 0640 X please fix these permission issues * changelog should be in a proper format OK * Packager tag should not be used OK * Vendor tag should not be used OK * Distribution tag should not be used OK * use License and not Copyright OK * Summary tag should not end in a period OK * if possible, replace PreReq with Requires(pre) and/or Requires(post) OK, does not use PreReq * specfile is legible OK * package successfully compiles and builds on at least x86 X package fails in mock. I will continue the review once the package can be built properly and the licensing issues are resolved. Error in mock build: cp: cannot stat `intro.html': No such file or directory (In reply to comment #2) > * is it legal for Fedora to distribute this? > - OSI-approved > - not a kernel module > - not shareware > - is it covered by patents? > - it *probably* shouldn't be an emulator > - no binary firmware > X I don't know if we can just distribute this. The project claims to be in the > public domain but sections of it are covered by a Technology License from Sun > Microsystems Inc. > (http://gee.cs.oswego.edu/dl/classes/EDU/oswego/cs/dl/util/sun-u.c.license.pdf) > This is OK as Public Domain, please see https://www.redhat.com/archives/fedora-packaging/2007-March/msg00142.html > * license field matches the actual license. > X the license field does not mention the Technology License As noted in the message on the mailing list > > * license is open source-compatible. > - use acronyms for licences where common > X I don't know if the Technology License is open source-compatible > Same as above. > * license text included in package and marked with %doc > X The source does not include a specific license file, but it does mention the > terms of the license in the intro.html file included. This file has a broken > link to the Sun Technology license which should be patched. > No license file as it is Public Domain, and I fixed the link in intro.html > * rpmlint on <this package>.srpm gives no output > rpmlint concurrent-1.3.4-5jpp.1.fc7.src.rpm > W: concurrent non-standard-group Development/Libraries/Java > W: concurrent strange-permission concurrent.tar.gz 0660 > W: concurrent strange-permission concurrent-1.3.4.build.xml 0660 > W: concurrent strange-permission concurrent.spec 0640 > > X please fix these permission issues Fixed. > X package fails in mock. > I will continue the review once the package can be built properly and the > licensing issues are resolved. > > Error in mock build: > cp: cannot stat `intro.html': No such file or directory Fixed. Updated spec and srpm at the same location. Thanks (In reply to comment #3) > (In reply to comment #2) > > * is it legal for Fedora to distribute this? > > - OSI-approved > > - not a kernel module > > - not shareware > > - is it covered by patents? > > - it *probably* shouldn't be an emulator > > - no binary firmware > > X I don't know if we can just distribute this. The project claims to be in the > > public domain but sections of it are covered by a Technology License from Sun > > Microsystems Inc. > > (http://gee.cs.oswego.edu/dl/classes/EDU/oswego/cs/dl/util/sun-u.c.license.pdf) > > > This is OK as Public Domain, please see > https://www.redhat.com/archives/fedora-packaging/2007-March/msg00142.html Right, I know public domain is acceptable, but is this project really public domain if it has that clause in there? > > * license field matches the actual license. > > X the license field does not mention the Technology License > As noted in the message on the mailing list > > > > * license is open source-compatible. > > - use acronyms for licences where common > > X I don't know if the Technology License is open source-compatible > > > Same as above. > > * license text included in package and marked with %doc > > X The source does not include a specific license file, but it does mention the > > terms of the license in the intro.html file included. This file has a broken > > link to the Sun Technology license which should be patched. > > > No license file as it is Public Domain, and I fixed the link in intro.html > > > * rpmlint on <this package>.srpm gives no output > > rpmlint concurrent-1.3.4-5jpp.1.fc7.src.rpm > > W: concurrent non-standard-group Development/Libraries/Java > > W: concurrent strange-permission concurrent.tar.gz 0660 > > W: concurrent strange-permission concurrent-1.3.4.build.xml 0660 > > W: concurrent strange-permission concurrent.spec 0640 > > > > X please fix these permission issues > Fixed. > > > X package fails in mock. > > I will continue the review once the package can be built properly and the > > licensing issues are resolved. > > > > Error in mock build: > > cp: cannot stat `intro.html': No such file or directory > Fixed. > > Updated spec and srpm at the same location. Thanks > Rest of review since the package now builds: * BuildRequires are proper - builds in mock will flush out problems here OK, build fine in mock - the following packages don't need to be listed in BuildRequires: bash bzip2 coreutils cpio diffutils fedora-release (and/or redhat-release) gcc gcc-c++ gzip make patch perl redhat-rpm-config rpm-build sed tar unzip which OK * summary should be a short and concise description of the package OK * description expands upon summary (don't include installation instructions) X Do we want to be advertising for that book? * make sure description lines are <= 80 characters OK * specfile written in American English OK * make a -doc sub-package if necessary OK, contains a javadoc subpackage * use macros appropriately and consistently - ie. %{buildroot} and %{optflags} vs. $RPM_BUILD_ROOT and $RPM_OPT_FLAGS OK * install section must begin with rm -rf $RPM_BUILD_ROOT or %{buildroot} OK * consider using cp -p to preserve timestamps X missing a -p on the first cp in %prep * split Requires(pre,post) into two separate lines OK * package contains code - see http://fedoraproject.org/wiki/Packaging/Guidelines#CodeVsContent - in general, there should be no offensive content OK * package should own all directories and files OK * there should be no %files duplicates OK * file permissions should be okay; %defattrs should be present OK * %clean should be present OK * %doc files should not affect runtime * if it is a web apps, it should be in /usr/share/%{name} and *not* /var/www * verify the final provides and requires of the binary RPMs rpm -qp --provides concurrent-1.3.4-5jpp.1.fc7.x86_64.rpm concurrent-1.3.4.jar.so()(64bit) concurrent = 0:1.3.4-5jpp.1.fc7 rpm -qp --requires concurrent-1.3.4-5jpp.1.fc7.x86_64.rpm /bin/sh /bin/sh java-gcj-compat java-gcj-compat libc.so.6()(64bit) libc.so.6(GLIBC_2.2.5)(64bit) libdl.so.2()(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libgcj_bc.so.1()(64bit) libm.so.6()(64bit) libpthread.so.0()(64bit) librt.so.1()(64bit) libz.so.1()(64bit) rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 rtld(GNU_HASH) rpm -qp --provides concurrent-javadoc-1.3.4-5jpp.1.fc7.x86_64.rpm concurrent-javadoc = 0:1.3.4-5jpp.1.fc7 rpm -qp --requires concurrent-javadoc-1.3.4-5jpp.1.fc7.x86_64.rpm /bin/ln /bin/rm /bin/rm X it should not need a requires on these * run rpmlint on the binary RPMs rpmlint concurrent-1.3.4-5jpp.1.fc7.x86_64.rpm W: concurrent non-standard-group Development/Libraries/Java rpmlint concurrent-javadoc-1.3.4-5jpp.1.fc7.x86_64.rpm W: concurrent-javadoc non-standard-group Development/Documentation rpmlint concurrent-debuginfo-1.3.4-5jpp.1.fc7.x86_64.rpm OK, the group warnings can be ignored SHOULD: * package should include license text in the package and mark it with %doc * package should build on i386 OK * package should build in mock OK (In reply to comment #4) ... > > > > > This is OK as Public Domain, please see > > https://www.redhat.com/archives/fedora-packaging/2007-March/msg00142.html > Right, I know public domain is acceptable, but is this project really public > domain if it has that clause in there? I've listed the Sun License and a link to the pdf in the original mail, and it was described as Public Domain, feel free to further discuss that in that discussion thread if you see fit. ... > X Do we want to be advertising for that book? Good catch, got rid of it. ... > * consider using cp -p to preserve timestamps > X missing a -p on the first cp in %prep Fixed. > rpm -qp --requires concurrent-javadoc-1.3.4-5jpp.1.fc7.x86_64.rpm > /bin/ln > /bin/rm > /bin/rm > > X it should not need a requires on these Fixed. New spec file and srpm uploaded at the same location. (In reply to comment #5) > (In reply to comment #4) > ... > > > > > > > This is OK as Public Domain, please see > > > https://www.redhat.com/archives/fedora-packaging/2007-March/msg00142.html > > Right, I know public domain is acceptable, but is this project really public > > domain if it has that clause in there? > > I've listed the Sun License and a link to the pdf in the original mail, and it > was described as Public Domain, feel free to further discuss that in that > discussion thread if you see fit. Ok, its seems to be ok https://www.redhat.com/archives/fedora-packaging/2007-April/msg00014.html Everything looks good APPROVED Package built in brew. |