Bug 877403

Summary: Review Request: svnkit - Pure Java Subversion client library
Product: [Fedora] Fedora Reporter: Ismael Olea <ismael>
Component: Package ReviewAssignee: Brendan Jones <brendan.jones.it>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: brendan.jones.it, mizdebsk, notting, package-review, sochotni
Target Milestone: ---Flags: brendan.jones.it: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2013-01-07 21:22:24 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: 873738, 873749    
Bug Blocks: 858578    
Attachments:
Description Flags
JNA patch getFieldOrder none

Description Ismael Olea 2012-11-16 12:37:26 UTC
Spec URL: http://olea.org/tmp/omegat-fedora-feature/svnkit.spec
SRPM URL: http://olea.org/tmp/omegat-fedora-feature/svnkit-1.7.5-4.fc18.src.rpm

Description: 
SVNKit is a pure java Subversion client library. You would like to use SVNKit
when you need to access or modify Subversion repository from your Java
application, as a standalone program and plugin or web application. Being a
pure java program, SVNKit doesn't need any additional configuration or native
binaries to work on any OS that runs java.

Fedora Account System Username: olea

Comment 1 Mikolaj Izdebski 2012-11-16 12:51:53 UTC
Invalid spec file URL
HTTP 404: Not Found

Comment 2 Ismael Olea 2012-11-16 13:04:01 UTC
It's online now.

O:-)

Comment 3 Mikolaj Izdebski 2012-11-16 13:10:26 UTC
You should clean it up a bit before submiting for review.
* rm -rf %{buildroot}
* %defattr
* fully versioned dependenncies
* comment sources

Comment 4 Ismael Olea 2012-11-16 13:35:22 UTC
It's a back to life package. I've changed/removed just the minimal I need for OmegaT requirements but keeping the rest of the configuration for other needs out of my scope, for example the RHEL < 6 things.

This is why you'll see some sub-packages comented too.

What problem do you see in the current deps versions? I specified the main concern I've checked which is sqljet. Sequence-library is new in Fedora so I didn't see necessary to specify the version too.

Comment 5 Mikolaj Izdebski 2012-11-16 13:50:12 UTC
Let's take for example dependency of subpackage javahl.

Packaging guidelines [1] says "When a subpackage requires the base package, it MUST do so using a fully versioned arch-specific". Your package doesn't have such dependency, so it doesn't meet the Packaging Guidelines.

One of review blockers included in Review Guidelines [2] is that "The package MUST meet the Packaging Guidelines". Because "No package with blockers can be approved on a review" your package can't be approved unless you add fully versioned dependency.

Other points are simillar.

[1] https://fedoraproject.org/wiki/Packaging:Guidelines
[2] https://fedoraproject.org/wiki/Packaging:ReviewGuidelines

Comment 6 Ismael Olea 2012-11-20 16:03:26 UTC
Last revision:

Spec URL: http://olea.org/tmp/omegat-fedora-feature/svnkit.spec
SRPM URL: http://olea.org/tmp/omegat-fedora-feature/svnkit-1.7.5-5.fc17.src.rpm


For building you'll need the next steps:

mock init 
mock --install sequence-library-1.0.2-2.fc17.noarch.rpm [1]
mock --install sqljet-1.1.4-3.fc17.noarch.rpm [2]
mock --no-clean svnkit-1.7.5-5.fc17.src.rpm                          


[1] http://olea.org/tmp/omegat-fedora-feature/sequence-library-1.0.2-2.fc17.noarch.rpm
[2] http://olea.org/tmp/omegat-fedora-feature/sqljet-1.1.4-3.fc17.noarch.rpm

Comment 7 Ismael Olea 2012-11-21 22:38:25 UTC
Now sequence-library and sqljet are in rawhide so the previous mock steps are not needed anymore.

Comment 8 Brendan Jones 2012-11-25 16:40:23 UTC
I'll take this review on.

Comment 9 Brendan Jones 2012-11-25 17:00:59 UTC
Rpmlint output. I'd remove the extraneous commented sections  (why do you need them?) Using %% for your macros in comments will supress this warning.

Also the devel-dependancy error.

svnkit.src:25: W: macro-in-comment %{name}
svnkit.src:25: W: macro-in-comment %{name}
svnkit.src:25: W: macro-in-comment %{versionr}
svnkit.src:25: W: macro-in-comment %{versionr}
svnkit.src:86: W: macro-in-comment %package
svnkit.src:86: W: macro-in-comment %{name}
svnkit.src:89: W: macro-in-comment %{name}
svnkit.src:89: W: macro-in-comment %{version}
svnkit.src:89: W: macro-in-comment %{release}
svnkit.src:92: W: macro-in-comment %description
svnkit.src:92: W: macro-in-comment %{name}
svnkit.src:144: W: macro-in-comment %{_bindir}
svnkit.src:151: W: macro-in-comment %files
svnkit.src:151: W: macro-in-comment %{name}
svnkit.src:152: W: macro-in-comment %{install_loc}
svnkit.src:152: W: macro-in-comment %{name}
svnkit.src:153: W: macro-in-comment %doc
svnkit.noarch: E: devel-dependency java-devel
svnkit-cli.noarch: W: spelling-error Summary(en_US) Jsvn -> Sven
svnkit-cli.noarch: W: spelling-error %description -l en_US jsvn -> Sven
svnkit-javahl.noarch: W: spelling-error %description -l en_US tigris -> Tigris
5 packages and 0 specfiles checked; 1 errors, 20 warnings.

Comment 10 Ismael Olea 2012-11-25 18:17:16 UTC
Grrrr: The src was a wrong one. Please get this http://olea.org//tmp/omegat-fedora-feature/svnkit-1.7.5-5.olea.src.rpm

Don't worry about the olea suffix, it's not in the spec.

The issue with the commented macros is explained bc this is a revived dead package and while I didn't checked the commented bits, they are not relevant for my OmegaT support goals, I want them to be easely recovered when someone take interest on them.

Comment 11 Brendan Jones 2012-11-26 07:11:22 UTC
I noticed you removed the nailgun jar from the source. Given that nailgun is a package in Fedora, I'd remove the whole source directory. Is it required for anything other than the tests, and if are there any automated tests here that can be run in the buildroot?

Comment 12 Brendan Jones 2012-11-26 10:54:07 UTC
I'm running into some build errors in fedora-rawhide-x86_64

Some logs for you here: http://bsjones.fedorapeople.org/reviews/svnkit/

Comment 13 Ismael Olea 2012-11-27 13:15:32 UTC
(In reply to comment #12)
> I'm running into some build errors in fedora-rawhide-x86_64
> 
> Some logs for you here: http://bsjones.fedorapeople.org/reviews/svnkit/

Damn, jna has been updated to 3.5.0 and is broking this: https://lists.fedoraproject.org/pipermail/java-devel/2012-November/004572.html

Seems there is a new release upstream but not checked yet if fix this :-/

Comment 14 Ismael Olea 2012-11-27 15:19:56 UTC
1.7.6 fails too :-/

Reported at upstream: http://issues.tmatesoft.com/issue/SVNKIT-329

Comment 15 Brendan Jones 2012-11-28 07:42:35 UTC
I haven't tested this - but perhaps you could work it out. Here's a suggested patch.

Comment 16 Brendan Jones 2012-11-28 07:44:21 UTC
Created attachment 653320 [details]
JNA patch getFieldOrder

Its at least a starting point of discussion with upstream. I've simply listed the fields in the order they are in the class.

I haven't run the package.

Comment 17 Brendan Jones 2012-11-28 07:47:02 UTC
There's a typo in the patch. Change "USerLength" to "UserLength"

Comment 19 Brendan Jones 2012-12-08 13:14:16 UTC
I haven't forgotten about this. I'll try and finsish this off later tonight or tomorrow

Comment 20 Brendan Jones 2012-12-09 12:45:35 UTC
Things I'd like to see fixed: 

 - please remove all unused variables. 
   %global svn_revision, %eclipse_name , core_plug*, version_r, jan_plugin* are a few examples. 
 - Remove all the commented eclipse plugin sections. References of these should be available in fedora before the package was orphaned so nothing will be lost

Previously this packaged removed all JARs from upstream and repackaged a clean source. This included removing the template.jars as well.

This leads me to two questions [1]:
 - are the template.jar files required for the function to package function. 
 - I'm assuming the previous maintainer had a good reason to repack the source so that no jar's are shipped in the src.rpm and I'd just like you to justify why you are not doing the same? 
 
[1] http://fedoraproject.org/wiki/Packaging:Java#Pre-built_JAR_files_.2F_Other_bundled_software

Comment 21 Ismael Olea 2012-12-10 12:47:20 UTC
(In reply to comment #20)
> Things I'd like to see fixed: 
> 
>  - please remove all unused variables. 
>    %global svn_revision, %eclipse_name , core_plug*, version_r, jan_plugin*
> are a few examples. 
>  - Remove all the commented eclipse plugin sections. References of these
> should be available in fedora before the package was orphaned so nothing
> will be lost

Ok...

Here they are:

http://olea.org/tmp/omegat-fedora-feature/svnkit.spec
http://olea.org//tmp/omegat-fedora-feature/svnkit-1.7.6-2.fc19.src.rpm

 
> Previously this packaged removed all JARs from upstream and repackaged a
> clean source. This included removing the template.jars as well.
> 
> This leads me to two questions [1]:
>  - are the template.jar files required for the function to package function. 
>  - I'm assuming the previous maintainer had a good reason to repack the
> source so that no jar's are shipped in the src.rpm and I'd just like you to
> justify why you are not doing the same? 

It's not a bundled external dependency but an upstream jar. So I see no reason to remove while loyal with policy.

Comment 22 Brendan Jones 2012-12-10 13:22:26 UTC
You could also lose 

%global versionr 1.7.5-v1

(In reply to comment #20)
> Previously this packaged removed all JARs from upstream and repackaged a
> clean source. This included removing the template.jars as well.
> 
> This leads me to two questions [1]:
>  - are the template.jar files required for the function to package function. 

But are they required for the package to function? (I can see one referenced in the source but I was hoping you'd tell me that)

grep -nir template.jar *
svnkit/src/main/java/org/tmatesoft/svn/core/io/SVNRepositoryFactory.java:110:    private static final String REPOSITORY_TEMPLATE_PATH = "/org/tmatesoft/svn/core/io/repository/template.jar";

>  - I'm assuming the previous maintainer had a good reason to repack the
> source so that no jar's are shipped in the src.rpm and I'd just like you to
> justify why you are not doing the same? 

The previous maintainer repacked the source such that the nailgun.jar was _not even shipped_ with the SRPM. I'm asking the why you have changed this policy. This is a very valid question.

I'd like to see your %prep section at least remove all *.jars and *.class files (with the exception of template.jar) above to ensure upstream doesn't slip something in at a later release. This is good policy and is mentioned in the java packaging guidelines.

The rest of the package is looking really good.

Comment 23 Stanislav Ochotnicky 2012-12-10 15:23:19 UTC
template.jar is OK, since it's just a tarball with some source templates. However (future) maintainer should make sure with each new upstream release that upstream didn't slip in some bundled binary code.

as for nailgun.jar: having it in SRPM *looks* OK, but in that case you need to include ASL 2.0 license text to SRPM Sources (point 4.1 of ASL states you need to distribute license text together with sources/binaries). No need to install ASL 2.0 license since binary rpms don't contain any ASL 2.0 code it seems. That said, the jar should definitely be removed during %prep. Using binaries during build is strictly prohibited unless it's being used for bootstrapping (and you'd need FESCO exception even for that). More about bundling: http://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries

Comment 24 Ismael Olea 2012-12-12 14:02:41 UTC
(In reply to comment #22)
> You could also lose 
> 
> %global versionr 1.7.5-v1

done
  
> But are they required for the package to function? (I can see one referenced
> in the source but I was hoping you'd tell me that)

As a packager, while it obeys Fedora guidelines I don't want to inquire if every bit from upstream is really used or not. That could be a interesting effort as a code contributor but it's not my interest here. 

OTOH, it seems to bootstrap a subversion server so, yes, it's part of sw feature.
 
> The previous maintainer repacked the source such that the nailgun.jar was
> _not even shipped_ with the SRPM. I'm asking the why you have changed this
> policy. This is a very valid question.

For the license concerns reported by @Stanislav I added a ASL 2.0 license inside SRPM
 
> I'd like to see your %prep section at least remove all *.jars and *.class
> files (with the exception of template.jar) above to ensure upstream doesn't
> slip something in at a later release. This is good policy and is mentioned
> in the java packaging guidelines.

My work process is to explicit remove each one jar file. It's completely expresive and helps package maintenance, ie: detecting new revisions on dependencies or new dependencies by itself.

And not all recomendations on guidelines are about deleting but detecting. Check de the example snippet at https://fedoraproject.org/wiki/Packaging:Java#Pre-built_JAR_files_.2F_Other_bundled_software

Anyhow, I added a find command to visualize any potential issue.

> The rest of the package is looking really good.

thanks

here it is the new release:

http://olea.org/tmp/omegat-fedora-feature/svnkit.spec
http://olea.org/tmp/omegat-fedora-feature/svnkit-1.7.6-3.fc19.src.rpm

Comment 25 Stanislav Ochotnicky 2012-12-12 17:32:16 UTC
(In reply to comment #24)
> > I'd like to see your %prep section at least remove all *.jars and *.class
> > files (with the exception of template.jar) above to ensure upstream doesn't
> > slip something in at a later release. This is good policy and is mentioned
> > in the java packaging guidelines.
> 
> My work process is to explicit remove each one jar file. It's completely
> expresive and helps package maintenance, ie: detecting new revisions on
> dependencies or new dependencies by itself.
> 
> And not all recomendations on guidelines are about deleting but detecting.
> Check de the example snippet at
> https://fedoraproject.org/wiki/Packaging:Java#Pre-built_JAR_files_.
> 2F_Other_bundled_software

If you read the snippet closely you'll notice it calls exit 1 if it finds any binary files. Current svnkit spec does:
find -name *jar -or -name *.class|grep -v class

Which rougly translates to "look for jar files and class files and print just jar". 

You or other packager *will* miss new binary jar addition this way. Something which would be much better in my opinion:

# delete every jar/class except whitelisted
JAR_WHITELIST="\(template.jar\)"
find . ! -regex ".*$JAR_WHITELIST.*" -and \
       \( -name '*.jar' -or -name '*.class' \) \
       -delete

Comment 26 Brendan Jones 2012-12-13 07:23:38 UTC
> For the license concerns reported by @Stanislav I added a ASL 2.0 license
> inside SRPM

Personally, I would rather include a script file for cleaning the source, rather than adding an extra license file for an unused jar file in the SRPM, but that's a matter of personal preference.


Full review below. All fedora-review detected issues can be ignored:

The requires warning is due to fedora-review not handling the %{?is_a} macro. 
Being noarch, you could lose the %{?is_a} macro but it doesn't hurt. 

Other MUST issues:

 - exit 1 in %prep when unxepected jars or class files found (as per Stanislav' comment)
 - You only need to install the license file once if the sub-package requires the base-package where it is present. 
 - A simply comment explaining the license breakdown
 - Your commented link to the maven pom is wrong. IF you haven't edited the pm file there's no reason why you can't just use the direct link for Source1:

Source1 http://search.maven.org/remotecontent?filepath=org/tmatesoft/%{name}/%{name}/%{version}/%{name}-%{version}.pom

Should:
 - remove bindir comment in %files section

Address the must issues and I'll approve  the review



Package Review
==============

Key:
[x] = Pass
[!] = Fail
[-] = Not applicable
[?] = Not evaluated
[x] = Manual review needed


Issues:
=======
[!]: Package installs properly.
     Note: Installation errors (see attachment)
See: https://fedoraproject.org/wiki/Packaging:Guidelines
[!]: Fully versioned dependency in subpackages, if present.
     Note: Missing: 'Requires: %%{name} =' in: %package cli, %package javahl
See: http://fedoraproject.org/wiki/Packaging/Guidelines#RequiringBasePackage
[!]: Javadoc subpackages have Requires: jpackage-utils
See: https://fedoraproject.org/wiki/Packaging:Java
[!]: Package do not use a name that already exist
     Note: A package already exist with this name, please check
     https://admin.fedoraproject.org/pkgdb/acls/name/svnkit
See: https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Conflicting_Package_Names
[!]: Bundled jar/class files should be removed before build
     Note: Jar files in source (see attachment)
See: http://fedoraproject.org/wiki/Packaging:Java#Pre-built_JAR_files_.2F_Other_bundled_software'


===== MUST items =====

Generic:
[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]: Package successfully compiles and builds into binary rpms on at least one
     supported primary architecture.
[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: All build dependencies are listed in BuildRequires, except for any that
     are listed in the exceptions section of Packaging Guidelines.
[x]: Package contains no bundled libraries.
[x]: Changelog in prescribed format.
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Sources contain only permissible code or content.
[x]: Each %files section contains %defattr if rpm < 4.4
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package contains desktop file if it is a GUI application.
[x]: Development files must be in a -devel package
[x]: Package requires other packages for directories it uses.
[x]: Package uses nothing in %doc for runtime.
[x]: Package is not known to require ExcludeArch.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Fully versioned dependency in subpackages, if present.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in %package
     javadoc, %package cli, %package javahl
[x]: Package complies to the Packaging Guidelines
[x]: Spec file lacks Packager, Vendor, PreReq tags.
[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.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "Apache (v2.0)", "Unknown or generated". 2 files have unknown license.
     Detailed output of licensecheck in /home/bsjones/rpmbuild/SRPMS/review-
     svnkit/licensecheck.txt
[!]: License file installed when any subpackage combination is installed.
*** dont need to specify the license multiple times when requiring the 
main package 
[x]: Package consistently uses macro is (instead of hard-coded directory
     names).
[!]: If the package is under multiple licenses, the licensing breakdown must
     be documented in the spec.
[x]: Package is named using only allowed ASCII characters.
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
     Note: Package contains no Conflicts: tag(s)
[!]: Package do not use a name that already exist
     Note: A package already exist with this name, please check
     https://admin.fedoraproject.org/pkgdb/acls/name/svnkit
[x]: Package obeys FHS, except libexecdir and /usr/target.
[x]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[!]: Package installs properly.
     Note: Installation errors (see attachment)
[x]: Package is not relocatable.
[-]: Requires correct, justified where necessary.
[x]: CheckResultdir
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: Sources used to build the package match the upstream source, as provided
     in the spec URL.
[x]: Spec file is legible and written in American English.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[-]: Package contains systemd file(s) if in need.
[x]: File names are valid UTF-8.
[x]: Large documentation must go in a -doc subpackage.
     Note: Documentation size is 61440 bytes in 6 files.
[x]: Packages must not store files under /srv, /opt or /usr/local

Java:
[x]: Packages have proper BuildRequires/Requires on jpackage-utils
[!]: Fully versioned dependency in subpackages, if present.
     Note: Missing: 'Requires: %%{name} =' in: %package cli, %package javahl
[x]: Javadoc documentation files are generated and included in -javadoc
     subpackage
[!]: Javadoc subpackages have Requires: jpackage-utils
[x]: Javadocs are placed in %{_javadocdir}/%{name} (no -%{version} symlink)
[!]: Bundled jar/class files should be removed before build
     Note: Jar files in source (see attachment)

Maven:
[x]: Pom files have correct add_maven_depmap call
     Note: Some add_maven_depmap calls found. Please check if they are correct
[x]: Old add_to_maven_depmap macro is not being used
[x]: Packages DOES NOT have Requires(post) and Requires(postun) on jpackage-
     utils for %update_maven_depmap macro
[x]: If package contains pom.xml files install it (including depmaps) even
     when building with ant
[x]: Package DOES NOT use %update_maven_depmap in %post/%postun
[x]: Packages use %{_mavenpomdir} instead of %{_datadir}/maven2/poms

===== SHOULD items =====

Generic:
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[-]: If the source package does not include license text(s) as a separate file
     from upstream, the packager SHOULD query upstream to include it.
[x]: Dist tag is present.
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[-]: Final provides and requires are sane (rpm -q --provides and rpm -q
     --requires).
[?]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: Patches link to upstream bugs/comments/lists or are otherwise justified.
[x]: The placement of pkgconfig(.pc) files are correct.
[x]: SourceX tarball generation or download is documented.
[!]: SourceX / PatchY prefixed with %{name}.
     Note: Source3 (LICENSE-2.0.txt) Source0 (org.tmatesoft.svn_1.7.6.src.zip)
[x]: SourceX is a working URL.
[ ]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[ ]: %check is present and all tests pass.
[ ]: Packages should try to preserve timestamps of original installed files.
[x]: Spec use %global instead of %define.

Java:
[x]: Package has BuildArch: noarch (if possible)
[x]: Package uses upstream build method (ant/maven/etc.)

===== EXTRA items =====

Generic:
[!]: Rpmlint is run on all installed packages.
     Note: Mock build failed
[x]: Spec file according to URL is the same as in SRPM.
[x]: Large data in /usr/share should live in a noarch subpackage if package is
     arched.


Installation errors
-------------------
INFO: mock.py version 1.1.28 starting...
Start: init plugins
INFO: selinux disabled
Finish: init plugins
Start: run
Mock Version: 1.1.28
INFO: Mock Version: 1.1.28
Start: lock buildroot
INFO: installing package(s): /home/bsjones/rpmbuild/SRPMS/review-svnkit/results/svnkit-cli-1.7.6-3.fc19.noarch.rpm /home/bsjones/rpmbuild/SRPMS/review-svnkit/results/svnkit-javadoc-1.7.6-3.fc19.noarch.rpm /home/bsjones/rpmbuild/SRPMS/review-svnkit/results/svnkit-1.7.6-3.fc19.noarch.rpm /home/bsjones/rpmbuild/SRPMS/review-svnkit/results/svnkit-javahl-1.7.6-3.fc19.noarch.rpm
ERROR: Command failed: 
 # ['/usr/bin/yum', '--installroot', '/var/lib/mock/fedora-rawhide-x86_64/root/', 'install', '/home/bsjones/rpmbuild/SRPMS/review-svnkit/results/svnkit-cli-1.7.6-3.fc19.noarch.rpm', '/home/bsjones/rpmbuild/SRPMS/review-svnkit/results/svnkit-javadoc-1.7.6-3.fc19.noarch.rpm', '/home/bsjones/rpmbuild/SRPMS/review-svnkit/results/svnkit-1.7.6-3.fc19.noarch.rpm', '/home/bsjones/rpmbuild/SRPMS/review-svnkit/results/svnkit-javahl-1.7.6-3.fc19.noarch.rpm']
Error: Package: svnkit-javahl-1.7.6-3.fc19.noarch (/svnkit-javahl-1.7.6-3.fc19.noarch)
           Requires: svnkit(x86-64) = 1.7.6-3.fc19
 You could try using --skip-broken to work around the problem
Error: Package: svnkit-cli-1.7.6-3.fc19.noarch (/svnkit-cli-1.7.6-3.fc19.noarch)
           Requires: svnkit(x86-64) = 1.7.6-3.fc19
 You could try running: rpm -Va --nofiles --nodigest



Rpmlint
-------
Checking: svnkit-cli-1.7.6-3.fc19.noarch.rpm
          svnkit-javadoc-1.7.6-3.fc19.noarch.rpm
          svnkit-1.7.6-3.fc19.src.rpm
          svnkit-1.7.6-3.fc19.noarch.rpm
          svnkit-javahl-1.7.6-3.fc19.noarch.rpm
svnkit-cli.noarch: W: spelling-error Summary(en_US) Jsvn -> Sven
svnkit-cli.noarch: W: spelling-error %description -l en_US jsvn -> Sven
svnkit.src:11: W: macro-in-comment %{name}
svnkit.src:11: W: macro-in-comment %{name}
svnkit.src:11: W: macro-in-comment %{version}
svnkit.src:11: W: macro-in-comment %{version}
svnkit.src:129: W: macro-in-comment %{_bindir}
svnkit-javahl.noarch: W: spelling-error %description -l en_US tigris -> Tigris
5 packages and 0 specfiles checked; 0 errors, 8 warnings.




Jar and class files in source
-----------------------------
./svnkit-1.7.6/svnkit/src/main/java/org/tmatesoft/svn/core/io/repository/template.jar
./svnkit-1.7.6/svnkit/src/main/resources/org/tmatesoft/svn/core/io/repository/template.jar


Requires
--------
svnkit-cli-1.7.6-3.fc19.noarch.rpm (rpmlib, GLIBC filtered):
    
    svnkit(x86-64) = 1.7.6-3.fc19

svnkit-javadoc-1.7.6-3.fc19.noarch.rpm (rpmlib, GLIBC filtered):
    
    jpackage-utils

svnkit-1.7.6-3.fc19.noarch.rpm (rpmlib, GLIBC filtered):
    
    jna >= 3.0
    jpackage-utils
    jpackage-utils >= 0:1.6
    sequence-library
    sqljet >= 1.1.4
    subversion-javahl >= 1.5
    tomcat-servlet-3.0-api >= 7.0.0
    trilead-ssh2 >= 213

svnkit-javahl-1.7.6-3.fc19.noarch.rpm (rpmlib, GLIBC filtered):
    
    subversion-javahl >= 1.5
    svnkit(x86-64) = 1.7.6-3.fc19



Provides
--------
svnkit-cli-1.7.6-3.fc19.noarch.rpm:
    
    svnkit-cli = 1.7.6-3.fc19

svnkit-javadoc-1.7.6-3.fc19.noarch.rpm:
    
    svnkit-javadoc = 1.7.6-3.fc19

svnkit-1.7.6-3.fc19.noarch.rpm:
    
    mvn(org.tmatesoft.svnkit:svnkit) = 1.7.6
    svnkit = 1.7.6-3.fc19

svnkit-javahl-1.7.6-3.fc19.noarch.rpm:
    
    svnkit-javahl = 1.7.6-3.fc19



MD5-sum check
-------------
http://www.svnkit.com/org.tmatesoft.svn_1.7.6.src.zip :
  CHECKSUM(SHA256) this package     : 55aa1f530cd3675e5dc4435a869787f4f33466c9c221cccedc822bb071647ae0
  CHECKSUM(SHA256) upstream package : 55aa1f530cd3675e5dc4435a869787f4f33466c9c221cccedc822bb071647ae0


Generated by fedora-review 0.3.1 (b71abc1) last change: 2012-10-16
Buildroot used: fedora-rawhide-x86_64
Command line :/usr/bin/fedora-review -m fedora-rawhide-x86_64 -n svnkit

Comment 27 Stanislav Ochotnicky 2012-12-13 09:27:21 UTC
(In reply to comment #26)
>  - Your commented link to the maven pom is wrong. IF you haven't edited the
> pm file there's no reason why you can't just use the direct link for Source1:
> 
> Source1
> http://search.maven.org/remotecontent?filepath=org/tmatesoft/%{name}/%{name}/
> %{version}/%{name}-%{version}.pom

This is actually much better:
http://repo1.maven.org/maven2/org/tmatesoft/svnkit/svnkit/1.7.6/svnkit-1.7.6.pom

Comment 28 Brendan Jones 2012-12-13 13:04:56 UTC
Thanks Stanislav - wasn't sure where a direct link would reside

Comment 29 Ismael Olea 2012-12-17 13:38:18 UTC
(In reply to comment #26)
> 
> Other MUST issues:
> 
>  - exit 1 in %prep when unxepected jars or class files found (as per
> Stanislav' comment)

fixed

>  - You only need to install the license file once if the sub-package
> requires the base-package where it is present. 

fixed

>  - A simply comment explaining the license breakdown

I did it with:

# just in SRPM due to nailgun.jar comes included in svnkit sources:
Source3:        LICENSE-2.0.txt

is it not clear enough? :-m

>  - Your commented link to the maven pom is wrong. IF you haven't edited the
> pm file there's no reason why you can't just use the direct link for Source1:

It's edited, but fixed the URL

>  - remove bindir comment in %files section

fixed


http://olea.org/tmp/omegat-fedora-feature/svnkit.spec
http://olea.org/tmp/omegat-fedora-feature/svnkit-1.7.6-4.fc17.src.rpm

Comment 30 Brendan Jones 2012-12-17 14:11:25 UTC
(In reply to comment #29)
> >  - A simply comment explaining the license breakdown
> 
> I did it with:
> 
> # just in SRPM due to nailgun.jar comes included in svnkit sources:
> Source3:        LICENSE-2.0.txt
> 
> is it not clear enough? :-m

I was referring to the Tmate AND ASL 1.1 dual licensing. 

It was not apparent to me why the ASL.1.1 was required.

Comment 31 Ismael Olea 2012-12-17 16:31:36 UTC
(In reply to comment #30)
 
> I was referring to the Tmate AND ASL 1.1 dual licensing. 
> 
> It was not apparent to me why the ASL.1.1 was required.

I see. I just reused from the previous spec.

Doing a license auditory I get all ASL licesed code is related with nailgun code included at svnkit-test/ which I'm not packaging in any way. It can checked with:

 find -name *java -exec egrep -H "(Version 1.1|Version 2.0)" {} \;          

and comparing to:

 find -name *java -exec egrep -H "(Version 1.1|Version 2.0)" {} \;|grep -v svnkit-test

So definitely I left the TMate license and added both ASL 1.1 and 2.2 as included in SRPM only

http://olea.org/tmp/omegat-fedora-feature/svnkit.spec (r. 1.7.6-5)

Comment 32 Ismael Olea 2012-12-27 18:36:39 UTC
Ping

Comment 33 Brendan Jones 2012-12-28 13:18:49 UTC
All looks good now. This package is APPROVED.

Comment 34 Ismael Olea 2012-12-28 13:27:28 UTC
New Package SCM Request
=======================
Package Name: svnkit
Short Description:  Pure java Subversion client library
Owners: olea
Branches:

Comment 35 Kevin Fenzi 2012-12-30 20:27:09 UTC
This isn't a New Package. ;) 

I've unretired it in pkgdb, so you should now be able to claim ownership. 

Note that you still need to file a ticket with rel-eng to unblock it. (refer to this review in that ticket)

Comment 36 Ismael Olea 2013-01-07 19:21:34 UTC
Done: https://fedorahosted.org/rel-eng/ticket/5437

Thanks.

Comment 37 Ismael Olea 2013-01-07 21:22:24 UTC
svnkit is now in rawhide: http://koji.fedoraproject.org/koji/taskinfo?taskID=4846905

Thanks all.