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=225888 --- Comment #3 from Alexander Kurtakov <akurtako@xxxxxxxxxx> 2011-01-24 05:19:52 EST --- (In reply to comment #2) > === REQUIRED ITEMS === > [!] Rpmlint output: > hsqldb.noarch: W: name-repeated-in-summary C Hsqldb > * I guess make it a bit different :-) Fixed. > hsqldb.noarch: W: spelling-error %description -l en_US servlets -> servants, > serviettes, serviette > * no problem > hsqldb.noarch: W: non-standard-group Development/Java > * Applications/Databases I guess? Fixed. > hsqldb.noarch: W: non-conffile-in-etc /etc/maven/fragments/hsqldb > * no problem > hsqldb.noarch: E: non-readable /var/lib/hsqldb/sqltool.rc 0600L > * I guess this is supposed to be like that to allow only hsqldb user to do > things. A bit of verification woul be great though Yep, you're right it's this way to prevent random user reading a password from the file. > hsqldb.noarch: W: non-conffile-in-etc /etc/sysconfig/hsqldb > * Make it into %(config) Fixed. > hsqldb.noarch: W: no-manual-page-for-binary hsqldbRunUtil > * Would be great to have it... > hsqldb.noarch: W: dangerous-command-in-%post rm > hsqldb.noarch: W: dangerous-command-in-%preun rm > * I prefer to have dangling symlinks, but it's up to you Done. > hsqldb.noarch: E: postin-without-chkconfig /etc/rc.d/init.d/hsqldb > hsqldb.noarch: E: preun-without-chkconfig /etc/rc.d/init.d/hsqldb > hsqldb.noarch: W: service-default-enabled /etc/rc.d/init.d/hsqldb > hsqldb.noarch: W: service-default-enabled /etc/rc.d/init.d/hsqldb > hsqldb.noarch: W: no-reload-entry /etc/rc.d/init.d/hsqldb > hsqldb.noarch: E: subsys-not-used /etc/rc.d/init.d/hsqldb > > * Whole init system needs looking into. Perhaps even creating systemd units > etc. Fixed as much as my knowledge allows me. > > hsqldb.src: W: strange-permission hsqldb-1.8.0-standard.cfg 0755L > * should probably be 644 Fixed. > > hsqldb.src:214: W: macro-in-comment %{_sbindir} > hsqldb.src:214: W: macro-in-comment %{name} > hsqldb.src:215: W: macro-in-comment %{_sbindir} > hsqldb.src:215: W: macro-in-comment %{name} > > * remove commented code, we have git history to deal with things like that now > :-) Fixed by escaping. > hsqldb.src:47: W: mixed-use-of-spaces-and-tabs (spaces: line 47, tab: line 31) > * choose one and stick with it Fixed. > hsqldb.src: W: invalid-url Source0: hsqldb_1_8_0_10.zip > * I believe this should be proper URL as is in the comment above Source0 Fixed. > hsqldb-demo.noarch: W: non-standard-group Development/Java > * Documentation ? Fixed. > hsqldb-demo.noarch: W: no-documentation > * no problem > hsqldb-javadoc.noarch: W: non-standard-group Development/Java > hsqldb-manual.noarch: W: non-standard-group Development/Java > * Documentation Fixed. > hsqldb-javadoc.noarch: W: wrong-file-end-of-line-encoding > /usr/share/javadoc/hsqldb-1.8.0.10/hsqldbstylesheet.css > hsqldb-manual.noarch: W: wrong-file-end-of-line-encoding > /usr/share/doc/hsqldb-1.8.0.10/changelist_1_7_2.txt > hsqldb-manual.noarch: W: wrong-file-end-of-line-encoding > /usr/share/doc/hsqldb-1.8.0.10/changelog_1_7_1.txt > hsqldb-manual.noarch: W: wrong-file-end-of-line-encoding > /usr/share/doc/hsqldb-1.8.0.10/changelist_1_7_3.txt > hsqldb-manual.noarch: W: wrong-file-end-of-line-encoding > /usr/share/doc/hsqldb-1.8.0.10/hypersonic_lic.txt > hsqldb-manual.noarch: W: wrong-file-end-of-line-encoding > /usr/share/doc/hsqldb-1.8.0.10/changelist_1_8_0.txt > hsqldb-manual.noarch: W: wrong-file-end-of-line-encoding > /usr/share/doc/hsqldb-1.8.0.10/readme-docauthors.txt > hsqldb-manual.noarch: W: wrong-file-end-of-line-encoding > /usr/share/doc/hsqldb-1.8.0.10/databaseinjar.txt > hsqldb-manual.noarch: W: wrong-file-end-of-line-encoding > /usr/share/doc/hsqldb-1.8.0.10/changelog_1_8_0.txt > hsqldb-manual.noarch: W: wrong-file-end-of-line-encoding > /usr/share/doc/hsqldb-1.8.0.10/changelog_1_7_2.txt > Should be fixed with sed in prep Hmm, I don't see thsi with version 1.8.1. > > 5 packages and 0 specfiles checked; 4 errors, 35 warnings. > > > [x] Package is named according to the Package Naming Guidelines[1]. > [x] Spec file name must match the base package name, in the format > %{name}.spec. > [x] Package meets the Packaging Guidelines[2]. > [x] Package successfully compiles and builds into binary rpms. > [x] Buildroot definition is not present > [x] Package is licensed with an open-source compatible license and meets other > legal requirements as defined in the legal section of Packaging > Guidelines[3,4]. > [x] License field in the package spec file matches the actual license. > License type: BSD > [x] If (and only if) the source package includes the text of the license(s) in > its own file, then that file, containing the text of the license(s) for the > package is included in %doc. > [!] All independent sub-packages have license of their own Fixed. > Javadoc subpackage should include license as should manual subpackage. > [x] Spec file is legible and written in American English. > [x] Sources used to build the package matches the upstream source, as provided > in the spec URL. > MD5SUM this package : 17410483b5b5f267aa18b7e00b65e6e0 > [!] All build dependencies are listed in BuildRequires, except for any that > are listed in the exceptions section of Packaging Guidelines[5]. > I see no reason to have coreutils/shadow-utils in Requires. Also if you decide > to move symlink generation from post/postun to build phase you can remove > Requires(post) on servlet25 Removed. > [x] Package must own all directories that it creates. > [x] Package requires other packages for directories it uses. > [x] Package does not contain duplicates in %files. > [!] Permissions on files are set properly. > %attr(0755,hsqldb,hsqldb) %{_localstatedir}/lib/%{name}/data > - this world readable setting seems like a unnecessary security "risk" at first > glance. > Fixed. > Some of other custom attrs can be also removed (644 ones). > > [!] Package does NOT have a %clean section which contains rm -rf %{buildroot} > (or $RPM_BUILD_ROOT). (not needed anymore) Fixed. > * can be removed > [!] Package consistently uses macros (no %{buildroot} and $RPM_BUILD_ROOT > mixing) Fixed. > * please use one or the other (maven metadata has buildroot macro) > [!] Package contains code, or permissable content. > [!] Fully versioned dependency in subpackages, if present. Are there problems with these 2? > [-] Package contains a properly installed %{name}.desktop file if it is a GUI > application. > [x] Package does not own files or directories owned by other packages. > [x] Javadoc documentation files are generated and included in -javadoc > subpackage > [!] Javadocs are placed in %{_javadocdir}/%{name} (no -%{version} symlinks) > [x] Packages have proper BuildRequires/Requires on jpackage-utils > [!] Javadoc subpackages have Require: jpackage-utils > Please add it Fixed. > [!] Package uses %global not %define Fixed. > [-] If package uses tarball from VCS include comment how to re-create that > tarball (svn export URL, git clone URL, ...) > [x] If source tarball includes bundled jar/class files these need to be > removed prior to building > "-exec rm -f {} \;" can be replaced with "-delete" though. > [x] All filenames in rpm packages must be valid UTF-8. > [!] Jar files are installed to %{_javadir}/%{name}.jar (see [6] for details) Fixed. > [x] If package contains pom.xml files install it (including depmaps) even when > building with ant > [x] pom files has correct add_to_maven_depmap call which resolves to the pom > file (use "JPP." and "JPP-" correctly) > > === Maven === > [x] Use %{_mavenpomdir} macro for placing pom files instead of > %{_datadir}/maven2/poms > [-] If package uses "-Dmaven.test.skip=true" explain why it was needed in a > comment > [-] If package uses custom depmap "-Dmaven2.jpp.depmap.file=*" explain why > it's needed in a comment > [x] Package uses %update_maven_depmap in %post/%postun > [x] Packages have Requires(post) and Requires(postun) on jpackage-utils (for > %update_maven_depmap macro) > > === Other suggestions === > [x] If possible use upstream build method (maven/ant/javac) > [x] Avoid having BuildRequires on exact NVR unless necessary > [x] Package has BuildArch: noarch (if possible) > [!] Latest version is packaged. > 1.8.1.3 is latest 1.8.x upstream version (there is 2.0 too) Updated to 1.8.1.3. Version 2.x are incompatible and this will break LibreOffice/OpenOffice. > [x] Reviewer should test that the package builds in mock. > Tested on: fedora-x86_64-rawhide > > > === Other Issues === > 1. I don't understand need for manually unzipping in beginning of %prep > 2. Patches should have comments about reasons and upstreamability > > > [1] https://fedoraproject.org/wiki/Packaging:NamingGuidelines > [2] https://fedoraproject.org/wiki/Packaging:Guidelines > [3] https://fedoraproject.org/wiki/Packaging:LicensingGuidelines > [4] https://fedoraproject.org/wiki/Licensing:Main > [5] https://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2 > [6] https://fedoraproject.org/wiki/Packaging:Java#Filenames As this review was pretty bad one :). I'm asking you to do a new review pointing the parts that still need work. I tried to fix everything mentioned but it appears to be too much for me to handle it properly. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review