Bug 574575 - Review Request: log5j - Simple java logging library
Summary: Review Request: log5j - Simple java logging library
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Marek Mahut
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-03-17 20:55 UTC by Justin Sherrill
Modified: 2017-07-04 17:18 UTC (History)
4 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2017-07-04 17:18:35 UTC
Type: ---
Embargoed:
mmahut: fedora-review+


Attachments (Terms of Use)

Description Justin Sherrill 2010-03-17 20:55:15 UTC
Spec URL: http://jlsherrill.fedorapeople.org/log5j/log5j.spec
SRPM URL: http://jlsherrill.fedorapeople.org/log5j/log5j-1.2-1.fc12.src.rpm
Description: The log5j package supports a 'modernized' interface on top of the class Log4j API usage.

Comment 1 Justin Sherrill 2010-03-18 15:39:16 UTC
This is my first fedora package submission, so I need a sponsor :}

Comment 2 Alex Orlandi 2010-03-21 00:55:22 UTC
Informal review

rpmlint on spec returns 1 warning:

$ rpmlint log5j.spec
log5j.spec: W: invalid-url Source0: log5j-1.2.tar.gz
0 packages and 1 specfiles checked; 0 errors, 1 warnings

you should specify the upstream download url in Source0.


After the build, rpmlint returns 1 error and 10 warning:

$rpmlint log5j.spec ../RPMS/noarch/log5j-*

log5j.spec:4: W: non-standard-group Application/Development
log5j.spec:23: W: non-standard-group Development/Documentation

   - "Group" needs to be a pre-existing group, like "Applications/Engineering"; run "less /usr/share/doc/rpm-*/GROUPS" to see the complete list. If you create a sub-package "...-doc" with documentation, use the group "Documentation".

log5j.spec: W: invalid-url Source0: log5j-1.2.tar.gz
   - see above

log5j.noarch: W: spelling-error Summary(en_US) Amodern -> Modern, A modern, Moderate
 - I think should be "A modern"  in the summary

log5j.noarch: W: spelling-error %description -l en_US printf -> print, prints, print f
   - it is correct; this Warning can be ignored

log5j.noarch: E: description-line-too-long C Logger facade that supports printf style message format for both performance and ease of use.
   - max 79 char per line (see http://fedoraproject.org/wiki/Common_Rpmlint_issues#description-line-too-long)

log5j.noarch: W: non-standard-group Application/Development
log5j.noarch: W: invalid-license Apache Software License, v. 2.0
   - you should use "ASL 2.0" (see http://fedoraproject.org/wiki/Licensing#Good_Licenses for allowed shortname)

log5j.noarch: W: no-documentation
log5j-javadoc.noarch: W: non-standard-group Development/Documentation
   - see above
log5j-javadoc.noarch: W: invalid-license Apache Software License, v. 2.0
   - see above
2 packages and 1 specfiles checked; 1 errors, 10 warnings.

Comment 3 Alex Orlandi 2010-03-21 10:57:47 UTC
patch file name convention should be the following:
%{name}-%{version}-patch_purpose.patch

(some people omit -%{version})

So instead of
   remove-override.patch
you should use something like:
   log5j-1.2-remove_override.patch

Comment 4 Alex Orlandi 2010-03-21 11:20:24 UTC
(In reply to comment #2)
> Informal review
> 
> rpmlint on spec returns 1 warning:
> 
> $ rpmlint log5j.spec
> log5j.spec: W: invalid-url Source0: log5j-1.2.tar.gz
> 0 packages and 1 specfiles checked; 0 errors, 1 warnings
>

anyway the tar.gz contained in the src.rpm matches the one of the upstream (assuming http://log5j.googlecode.com/files/log5j-1.2.tar.gz):
md5sum is 2c9bfe631747e0943163998bea454db3

> you should specify the upstream download url in Source0.
> 
> [...]

Comment 5 Justin Sherrill 2010-04-07 20:20:24 UTC
I had run rpmlint, but evidently was running an older version and didn't know to run it on the src rpm as well. 

Now i just get:

log5j.spec: W: invalid-url Source0: http://log5j.googlecode.com/files/log5j-1.2.tar.gz HTTP Error 404: Not Found
log5j.noarch: W: spelling-error %description -l en_US printf -> print, prints, print f
log5j.noarch: W: no-documentation
2 packages and 0 specfiles checked; 0 errors, 2 warnings

I'm not sure about the 404 error, as the file exists just fine and I think the other ones are ok.


Spec URL: http://jlsherrill.fedorapeople.org/log5j/log5j.spec
SRPM URL: http://jlsherrill.fedorapeople.org/log5j/log5j-1.2-1.fc12.src.rpm

Comment 6 Mat Booth 2010-04-11 13:34:38 UTC
Hi Justin,

Thanks for putting the work in and submitting a package. :-)

When updating your SPEC file, please don't forget to add a changelog entry and bump the revision number. This will help anyone reviewing your package.

Comment 7 Miroslav Suchý 2010-04-12 09:30:08 UTC
==============

Key:
 - = N/A
 x = Check
 ! = Problem
 ? = Not evaluated

=== REQUIRED ITEMS ===
 [x] Package is named according to the Package Naming Guidelines.
 [x] Spec file name must match the base package %{name}, in the format
%{name}.spec.
 [x] Package meets the Packaging Guidelines including the Java specific items
 [x] Package successfully compiles and builds into binary rpms on at least one
supported architecture.
     Tested on: devel/x86_64
     http://koji.fedoraproject.org/koji/taskinfo?taskID=2109749
 [x] Rpmlint output:
     log5j.src: W: spelling-error %description -l en_US printf -> print, prints, print f
     log5j.src: W: invalid-url Source0: http://log5j.googlecode.com/files/log5j-1.2.tar.gz HTTP Error 404: Not Found
     first can be ignored, second is weird, as the file really reside on that url and wget download it without problem
 [x] Package is not relocatable.
 [x] Buildroot is correct
      %{_tmppath}/%{name}-%{version}-%{release}-buildroot
      may need to change, see notes on bottom
 [x] Package is licensed with an open-source compatible license and meets other
legal requirements as defined in the legal section of Packaging Guidelines.
 [x] License field in the package spec file matches the actual license.
     License type: ASL 2.0
 [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.
     source package do not have license text
 [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.
6a9ae267f3190aa83aa3af018678517f170c7a8f353c68421c89f2da74f5c4a6  log5j-1.2.tar.gz
6a9ae267f3190aa83aa3af018678517f170c7a8f353c68421c89f2da74f5c4a6  ../SOURCES/log5j-1.2.tar.gz
 [x] Package is not known to require ExcludeArch
 [x] All build dependencies are listed in BuildRequires, except for any that
are listed in the exceptions section of Packaging Guidelines.
 [-] The spec file handles locales properly.
 [-] ldconfig called in %post and %postun if required.
 [x] Package must own all directories that it creates.
 [x] Package requires other packages for directories it uses.
      /usr/share/java is owned by libgcj, which is required by ant, which is required by log5j
 [x] Package does not contain duplicates in %files.
 [x] Permissions on files are set properly.
 [x] Package has a %clean section, which contains rm -fR $RPM_BUILD_ROOT.
 [x] Package consistently uses macros.
 [x] Package contains code, or permissable content.
 [x] Large documentation files are in a -doc subpackage, if required.
   -javadoc package present
 [x] Package uses nothing in %doc for runtime.
 [-] Header files in -devel subpackage, if present.
 [-] Static libraries in -devel subpackage, if present.
 [-] Package requires pkgconfig, if .pc files are present.
 [-] Development .so files in -devel subpackage, if present.
 [-] Fully versioned dependency in subpackages, if present.
 [x] Package does not contain any libtool archives (.la).
 [-] 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.

=== SUGGESTED ITEMS ===
 [x] Latest version is packaged.
 1.2
 [x] Package does not include license text files separate from upstream.
 [-] Description and summary sections in the package spec file contains
translations for supported Non-English languages, if available.
 [x] Reviewer should test that the package builds in mock.
     Tested on: koji scratch build
 [x] Package should compile and build into binary rpms on all supported
architectures.
     Tested on:koji scratch build
 [?] Package functions as described.
 [-] Scriptlets must be sane, if used.
 [-] The placement of pkgconfig(.pc) files is correct.
 [-] File based requires are sane.
 [-] %check is present and the tests pass


================
*** APPROVED ***
================

Optionally, but recommended:
You have specified BuildRoot, this is not needed for current Fedora, but since I know you would like to build it for EPEL-5 you may want to use one of the more prefered value. E.g.:
%(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
See:
http://fedoraproject.org/wiki/EPEL/GuidelinesAndPolicies#BuildRoot_tag


Note, you will need to get sponsor before you request cvs branches. You may want to ping Dennis Gilmore, which can probably sponsor you.

Comment 8 Justin Sherrill 2010-04-12 17:58:07 UTC
Thanks Mirek.  I fixed the Build root and will talk with Dennis.

Comment 9 Alexander Kurtakov 2010-04-19 14:34:07 UTC
Justin, 
You have to make a cvs request as described here https://fedoraproject.org/wiki/CVS_admin_requests

Comment 10 Dennis Gilmore 2010-04-19 20:17:18 UTC
Ill take a look at reviewing the package

Comment 11 Dennis Gilmore 2010-04-19 20:23:51 UTC
 rpmlint /var/lib/mock/fedora-13-x86_64/result
log5j.noarch: W: spelling-error %description -l en_US printf -> print, prints, print f
log5j.noarch: W: no-documentation
log5j.src: W: spelling-error %description -l en_US printf -> print, prints, print f
log5j.src: W: invalid-url Source0: http://log5j.googlecode.com/files/log5j-1.2.tar.gz HTTP Error 404: Not Found
3 packages and 0 specfiles checked; 0 errors, 4 warnings.

you really should at the least provide a valid url for the source tarball

Comment 12 Justin Sherrill 2010-04-19 20:29:47 UTC
So the url is actually valid.  http://log5j.googlecode.com/files/log5j-1.2.tar.gz  is good url.    I mentioned this in comment #5 and still haven't figured out why rpmlint is complaining.   Any suggestions would be awesome :}

-Justin

Comment 13 Justin Sherrill 2010-07-09 14:05:12 UTC
I updated the spec file in accordance with https://fedoraproject.org/wiki/Packaging/SourceURL.  I never figured out why rpmlint failed on the URL, but according to that if it does (For a redirect for example), then you would simply use the tar file as Source0 and add a comment.  I have done this now:

Spec URL: http://jlsherrill.fedorapeople.org/log5j/log5j.spec
SRPM URL: http://jlsherrill.fedorapeople.org/log5j/log5j-1.2-1.fc12.src.rpm

Comment 14 Marek Mahut 2010-08-09 15:09:54 UTC
Looks sane to me. APPROVED.

Dennis, I will take care of Justin's sponsorship.

Comment 15 Alexander Kurtakov 2010-09-03 10:12:37 UTC
Justin, you have to create a git request as described here https://fedoraproject.org/wiki/PackageMaintainers/Git_Admin_Requests

Comment 16 Alexander Kurtakov 2010-11-19 11:21:45 UTC
Are there any plans to finish this review?

Comment 17 Upstream Release Monitoring 2015-12-14 10:40:48 UTC
jerboaa's scratch build of java-1.8.0-openjdk?#d28765c33d068af9ff432a92443b93beeef88a22 for git://pkgs.fedoraproject.org/java-1.8.0-openjdk?#d28765c33d068af9ff432a92443b93beeef88a22 and rawhide failed http://koji.fedoraproject.org/koji/taskinfo?taskID=12181621

Comment 18 Alexander Kurtakov 2017-07-04 17:18:35 UTC
Nothing will happen here it looks and packaging changed so much it's pointless to keep it around.


Note You need to log in before you can comment on or make changes to this bug.