Bug 1312015 - Review Request: javadocofflinesearch - Tool for offline searching in your docs via browser
Review Request: javadocofflinesearch - Tool for offline searching in your doc...
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Raphael Groner
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2016-02-25 09:21 EST by jiri vanek
Modified: 2016-04-15 03:21 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2016-04-14 23:19:58 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
projects.rg: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description jiri vanek 2016-02-25 09:21:37 EST
Spec URL: https://jvanek.fedorapeople.org/offlinesearch/v1/javadocofflinesearch.spec
SRPM URL: https://jvanek.fedorapeople.org/offlinesearch/v1/javadocofflinesearch-2.2-1.fc23.src.rpm
Description: Tool for offline searching in your docs via browser
Fedora Account System Username: jvanek

For convenience - https://jvanek.fedorapeople.org/offlinesearch/v1/noarch/ builds
Tested on f23+f24
Comment 1 Raphael Groner 2016-03-06 12:13:49 EST
Hi reporter,
are you interested in a review swap? I'm requesting some java packages, please feel free to look into them: bug #1305650 or bug #1305365
Comment 2 jiri vanek 2016-03-15 08:52:36 EDT
I'm reviewing 1305650. Feel free to - but dont feel forced to - to revenge yourself there :)
Comment 3 Raphael Groner 2016-03-24 09:28:41 EDT
Let give me some general advice before I can do a run of fedora-review tool. Please fix at least a., d. and j. (MUST).

a. License is GPLv3+, you can not downgrade version. MUST
https://github.com/judovana/JavadocOfflineSearch/blob/master/LICENSE

b. Improve URL and Source0 consistently with macros. RECOMMENDED ¹
URL:            https://github.com/judovana/%{srcname}
Source0:        %{url}/releases/tag/%{srcname}-%{version}/%{srcname}-%{srcname}-%{version}.tar.gz

c. Please add a comment what your patch does. SHOULD
> Patch1:         javadocFixes.patch
https://fedoraproject.org/wiki/Packaging:Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment

d. BuildRequires vs. Requires. MUST
Why do you need everything listed in BR explicitly also in Requires?
Please test to build without any listed Requires and add only those who do not get added automatically via rpmbuild. I think you can definitely remove:
- BuildRequires:  jpackage-utils
- BuildRequires:  java-devel
- Requires:  jpackage-utils
- Requires:  java-devel
https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires
https://fedoraproject.org/wiki/Packaging:Java#BuildRequires_and_Requires

e. Description is same as Summary. Alternatively, use text got from upstream documentation. To improve general readability, insert an empty line before %description. SHOULD
#Suggestion:
%description
%{summary}
#or:
%description
The goal of this project was to make searching in your (java)docs
as easy and comfortable as when you are browsing them online.
https://fedoraproject.org/wiki/Packaging:Guidelines#Summary_and_description

f. Improve removal of binaries. RECOMMENDED
- find -name '*.class' -exec rm -f '{}' \;
- find -name '*.jar' -exec rm -f '{}' \;
+ find -print -delete -name '*.class' -or -name '*.jar'

g. Installation of jar file is wrong and needs fix. MUST
- cp -r  dist/javadoc  $RPM_BUILD_ROOT/%{_javadocdir}/%{name}
+ install -m0644 assembly/*.jar -D $RPM_BUILD_ROOT/%{_javadir}/%{name}.jar
https://fedoraproject.org/wiki/Packaging:Java#Installation_directory

h. Send your start script as a patch to upstream. SHOULD
https://fedoraproject.org/wiki/Packaging:Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment

i. Obsolete lines. Please remove. SHOULD
> Group:          Documentation
> %defattr(-,root,root,-)
> %defattr(-,root,root,-)
(two times %defattr)

j. Use %license. MUST
- %doc LICENSE
+ %license LICENSE
https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text

¹ RECOMMENDED means here it's a personal preference and not (clearly) in guidelines, it's up to you to decide.
Comment 4 jiri vanek 2016-03-24 09:56:19 EDT
(In reply to Raphael Groner from comment #3)
> Let give me some general advice before I can do a run of fedora-review tool.
> Please fix at least a., d. and j. (MUST).
> 
> a. License is GPLv3+, you can not downgrade version. MUST
> https://github.com/judovana/JavadocOfflineSearch/blob/master/LICENSE
> 
Crap. Thats (nearly) the same typo you had :) How could I!
Sure, will be fixed.
I'm really happy that I have the same bug in my spec I was so nasty to you :)

> b. Improve URL and Source0 consistently with macros. RECOMMENDED ¹
> URL:            https://github.com/judovana/%{srcname}
> Source0:       
> %{url}/releases/tag/%{srcname}-%{version}/%{srcname}-%{srcname}-%{version}.
> tar.gz

Right!

> 
> c. Please add a comment what your patch does. SHOULD
> > Patch1:         javadocFixes.patch
> https://fedoraproject.org/wiki/Packaging:
> Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment
> 

yes, I already fixed this upstream. So I will add link.
> d. BuildRequires vs. Requires. MUST
> Why do you need everything listed in BR explicitly also in Requires?
> Please test to build without any listed Requires and add only those who do
> not get added automatically via rpmbuild. I think you can definitely remove:
> - BuildRequires:  jpackage-utils
> - BuildRequires:  java-devel
> - Requires:  jpackage-utils
> - Requires:  java-devel
> https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires
> https://fedoraproject.org/wiki/Packaging:Java#BuildRequires_and_Requires

Hm. Not sure about that. First of all I need to fix
-Requires:       java 
+Requires:       java-headless

You are probably right with:
 - BuildRequires:  jpackage-utils
 - BuildRequires:  java-devel

And I really can safely remove them.

BUt I disagree with 
 - Requires:  jpackage-utils
 - Requires:  java-devel

First of all, I dont see `Requires:  java-devel` :)
And jpackage utils... hm.. I think Its safer to include them, But I can remove them if you insists.


> test to build without any listed Requires and add only those who do
> not get added automatically via rpmbuild.

You may see that I have sme recommends in there. But hte thers? I wsa to often surprised by missing generated dependence that I'm little bit unwiling to follow this advice :( 

> 
> e. Description is same as Summary. Alternatively, use text got from upstream
> documentation. To improve general readability, insert an empty line before

Sure!

> %description. SHOULD
> #Suggestion:
> %description
> %{summary}
> #or:
> %description
> The goal of this project was to make searching in your (java)docs
> as easy and comfortable as when you are browsing them online.

Ok. Sopunds right.

> https://fedoraproject.org/wiki/Packaging:Guidelines#Summary_and_description
> 
> f. Improve removal of binaries. RECOMMENDED
> - find -name '*.class' -exec rm -f '{}' \;
> - find -name '*.jar' -exec rm -f '{}' \;
> + find -print -delete -name '*.class' -or -name '*.jar'

Sure!

> 
> g. Installation of jar file is wrong and needs fix. MUST
> - cp -r  dist/javadoc  $RPM_BUILD_ROOT/%{_javadocdir}/%{name}
> + install -m0644 assembly/*.jar -D $RPM_BUILD_ROOT/%{_javadir}/%{name}.jar
> https://fedoraproject.org/wiki/Packaging:Java#Installation_directory
> 

This advice do not sound right
I'm packing the lone jar to %{_javadir}
  cp dist/%{name}.jar $RPM_BUILD_ROOT/%{_javadir}
exactly as the guidelines says. Have I missed osmthing more?
Of ocuorse I canuse install insted of cp for this file.

Same for javadoc - it is placed right. And Wihtout that manual copy, no javadoc appeared.

As fr "assembly" in your sugestion. Where did you get that? The ant run by default do not run assembly target in build file.
Also this generated file would be against all guidelines (it is containing budled lucene pdfbox .. and all deps)

So I would really stay with dist.



> h. Send your start script as a patch to upstream. SHOULD
> https://fedoraproject.org/wiki/Packaging:
> Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment

Upstream do not need this launcher. Its released jar have mainclass. So you just java -jar it.

> 
> i. Obsolete lines. Please remove. SHOULD
> > Group:          Documentation
> > %defattr(-,root,root,-)
> > %defattr(-,root,root,-)
> (two times %defattr)
They an really miss? ok then.

> 
> j. Use %license. MUST
> - %doc LICENSE
> + %license LICENSE

>Oh! Sure!

> https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text
> 
> ¹ RECOMMENDED means here it's a personal preference and not (clearly) in
> guidelines, it's up to you to decide.

Not much to not-follow. the advices are good except the few I think I'm right.

Thanx!
Comment 5 Raphael Groner 2016-03-24 10:30:18 EDT
(In reply to jiri vanek from comment #4)
…
> This advice do not sound right
> I'm packing the lone jar to %{_javadir}
>   cp dist/%{name}.jar $RPM_BUILD_ROOT/%{_javadir}
> exactly as the guidelines says. Have I missed osmthing more?
> Of ocuorse I canuse install insted of cp for this file.

Look into build.xml about build of jar file.
https://github.com/judovana/JavadocOfflineSearch/blob/master/build.xml#L103

> Same for javadoc - it is placed right. And Wihtout that manual copy, no
> javadoc appeared.

Javadoc is not the topic.

> As fr "assembly" in your sugestion. Where did you get that? The ant run by
> default do not run assembly target in build file.
> Also this generated file would be against all guidelines (it is containing
> budled lucene pdfbox .. and all deps)

I fail to see where any jar files get bundled.

> So I would really stay with dist.

I would not recommend to use a completely different build logic. Guidelines clearly state to use build process of upstream, whereever possible. I agree about missing part for installation at upstream, so we obviously have to patch that but not seamlessly the whole build.
Comment 6 jiri vanek 2016-03-24 10:41:42 EDT
That is only top level build script.
It calls https://github.com/judovana/JavadocOfflineSearch/blob/master/nbproject/build-impl.xml

Which is the one who build.  I call "ant" and then I pack the classes with fresh manifest by java -jar.  What better can i do?


That is standard netbeans-project build. I doubt it is handed differently somewhere else. And if it is, then all NB projects I ever packed, Are wrong.
Comment 7 Raphael Groner 2016-03-24 11:02:43 EDT
(In reply to jiri vanek from comment #6)
…
> Which is the one who build.

"Java packages MUST BuildRequire their respective build system" and that means for me that we also have to use it *correctly*, what else sense would that sentence have?
https://fedoraproject.org/wiki/Packaging:Java#BuildRequires_and_Requires

> I call "ant" and then I pack the classes with
> fresh manifest by java -jar.  What better can i do?

Sure. But the ant run does the "pack" for you inclusively. So why do it a second time and how to handle future upstream changes? It shouldn't be of any relevance if you are upstream as well or not. Hmm okay, it's becoming more a philosophical question on that level of discussion.

> That is standard netbeans-project build. I doubt it is handed differently
> somewhere else. And if it is, then all NB projects I ever packed, Are wrong.

No idea what you did in past and general internals of netbeans build process. It does not matter for a right review, we've to concentrate on guidelines and those chnage over time as well.
Comment 8 jiri vanek 2016-03-24 11:44:03 EDT
(In reply to Raphael Groner from comment #7)
> (In reply to jiri vanek from comment #6)
> …
> > Which is the one who build.
> 
> "Java packages MUST BuildRequire their respective build system" and that
> means for me that we also have to use it *correctly*, what else sense would
> that sentence have?
> https://fedoraproject.org/wiki/Packaging:Java#BuildRequires_and_Requires

Thats exactly what I'm doing. Using ant. And "ant" called as it is in this project generates classes.
If I would call other possible target - as "ant assembly" then it will generate jar, but ant with assembled and inlcuded all depndencies.

I think there is also ant dist, which generates jar. But that jar have in-manifest classpath which is hard to get rid of.

AFAIK this step,as I did it, is really ok with guidelines.
> 
> > I call "ant" and then I pack the classes with
> > fresh manifest by java -jar.  What better can i do?
> 
> Sure. But the ant run does the "pack" for you inclusively. So why do it a
> second time and how to handle future upstream changes? It shouldn't be of

Explained above.

> any relevance if you are upstream as well or not. Hmm okay, it's becoming
> more a philosophical question on that level of discussion.
> 
> > That is standard netbeans-project build. I doubt it is handed differently
> > somewhere else. And if it is, then all NB projects I ever packed, Are wrong.
> 
> No idea what you did in past and general internals of netbeans build
> process. It does not matter for a right review, we've to concentrate on
> guidelines and those chnage over time as well.

I still hope it is explained above.

*however*  I'm far from being  I'm all knowing. If you know/find the better way how to build NB project generated build files from command-line, then please, share!

But I'm not aware, nor was it found in previous review I encountered, nor I myself consider it bad.


Spec URL: https://jvanek.fedorapeople.org/offlinesearch/v1/javadocofflinesearch.spec
SRPM URL: https://jvanek.fedorapeople.org/offlinesearch/v1/javadocofflinesearch-2.2-1.fc23.src.rpm
Description: Tool for offline searching in your docs via browser
Fedora Account System Username: jvanek


If fixed all I agreed above. Except install of javadoc. I tried, but install command is somehow clumsy in directories.

Also I had to fix the find command of yours. Position of -delete  meters a lot!-)
Comment 9 jiri vanek 2016-03-24 11:45:35 EDT
Oh and the requires - I'm afraid I need all of them as they are settled. I tried to remove, and always something failed... :( Sorry.

On your question why all from build is in runtime. THe project have no tests. So it gave sense.
Comment 10 jiri vanek 2016-03-30 05:43:18 EDT
Sorry, I posted wrong links. Noted no sooner then today.

Spec URL: https://jvanek.fedorapeople.org/offlinesearch/v2/javadocofflinesearch.spec
SRPM URL: https://jvanek.fedorapeople.org/offlinesearch/v2/javadocofflinesearch-2.2-1.fc23.src.rpm
Description: Tool for offline searching in your docs via browser
Fedora Account System Username: jvanek
Comment 11 Raphael Groner 2016-03-30 05:57:36 EDT
I fail to see any fixes.
Comment 12 jiri vanek 2016-03-30 06:01:14 EDT
--- /home/jvanek/git/javadocofflinesearch/javadocofflinesearch.spec-old
+++ /home/jvanek/git/javadocofflinesearch/javadocofflinesearch.spec
@@ -6,18 +6,17 @@
 Summary:        Tool for offline searching in your docs via browser
 BuildArch:      noarch
 
-License:        GPLv2
-URL:            https://github.com/judovana/JavadocOfflineSearch
-Source0:        https://github.com/judovana/JavadocOfflineSearch/releases/tag/%{srcname}-2.2/%{srcname}-%{srcname}-%{version}.tar.gz
+License:        GPLv3
+URL:            https://github.com/judovana/%{srcname}
+Source0:        https://github.com/judovana/%{srcname}/releases/tag/%{srcname}-2.2/%{srcname}-%{srcname}-%{version}.tar.gz
+# already fixed in upstream https://github.com/judovana/JavadocOfflineSearch/commit/853285f3105506b860c762f534e1a7c2733a2c61
 Patch1:         javadocFixes.patch
 
-BuildRequires:  jpackage-utils
-BuildRequires:  java-devel
 BuildRequires:  ant
 BuildRequires:  tagsoup
 BuildRequires:  lucene-core
 #BuildRequires:  lucene-analyzers-common hidden in
-BuildRequires:   lucene-analysis
+BuildRequires:  lucene-analysis
 BuildRequires:  lucene-queries
 BuildRequires:  lucene-queryparser
 BuildRequires:  apache-commons-cli
@@ -26,12 +25,12 @@
 BuildRequires:  apache-commons-logging
 BuildRequires:  fontbox
 
-Requires:       jpackage-utils
-Requires:       java
+Requires:  jpackage-utils
+Requires:  java-headless
 Requires:  tagsoup
 Requires:  lucene-core
 #Requires:  lucene-analyzers-common hidden in
-Requires:   lucene-analysis
+Requires:  lucene-analysis
 Requires:  lucene-queries
 Requires:  lucene-queryparser
 Requires:  apache-commons-cli
@@ -39,8 +38,10 @@
 #Recommends:  apache-commons-logging-api included in:
 Recommends:  apache-commons-logging
 Recommends:  fontbox 
+
 %description
-Tool for offline searching in your docs via browser
+The goal of this project was to make searching in your (java)docs as easy and
+ comfortable as when you are browsing them online.
 
 %package javadoc
 Summary:        Javadocs for %{name}
@@ -54,8 +55,7 @@
 
 %prep
 %setup -q -n %{srcname}-%{srcname}-%{version}
-find -name '*.class' -exec rm -f '{}' \;
-find -name '*.jar' -exec rm -f '{}' \;
+find -print -name '*.class' -or -name '*.jar'  -delete
 %patch1 -p1
 
 
@@ -68,11 +68,12 @@
 popd
 
 %install
-mkdir -p $RPM_BUILD_ROOT/%{_javadir}
-mkdir -p $RPM_BUILD_ROOT/%{_javadocdir}/
+install -m0644  dist/%{name}.jar -D $RPM_BUILD_ROOT/%{_javadir}/%{name}.jar
+
+mkdir -p $RPM_BUILD_ROOT/%{_javadocdir}
+cp -r  dist/javadoc  $RPM_BUILD_ROOT/%{_javadocdir}/%{name}
+
 mkdir -p $RPM_BUILD_ROOT/%{_bindir}
-cp dist/%{name}.jar $RPM_BUILD_ROOT/%{_javadir}
-cp -r  dist/javadoc  $RPM_BUILD_ROOT/%{_javadocdir}/%{name}
 
 cat <<EOF > $RPM_BUILD_ROOT/%{_bindir}/%{name}
 #!/bin/bash
@@ -99,16 +100,14 @@
 
 
 %files
-%defattr(-,root,root,-)
 %{_javadir}/*
-%doc LICENSE
+%license LICENSE
 %attr(755,root,root) %{_bindir}/%{name}
 
 
 %files javadoc
-%defattr(-,root,root,-)
 %{_javadocdir}/%{name}
-%doc LICENSE
+%license LICENSE
 
 
 %changelog
Comment 13 Raphael Groner 2016-03-30 06:51:07 EDT
aa. Please use GPLv3+ (mind the plus, instead of just GPLv3 that forbids any improvement) to not run into potential further trouble about license upgrades.
Though as you're also upstream, I think this is a minor issue with license.
https://fedoraproject.org/wiki/Licensing:FAQ?rd=Licensing/FAQ#GNU_General_Public_License

bb. I still think you're doing it wrong with the "pack manually", see comment #7 and comment #8. Can you provide the custom build script for ant (incl. all the Fedora unbundling) also in the upstream sources?

cc. BUILD FAILED
/builddir/build/BUILD/JavadocOfflineSearch-JavadocOfflineSearch-2.2/nbproject/build-impl.xml:923: The following error occurred while executing this line:
/builddir/build/BUILD/JavadocOfflineSearch-JavadocOfflineSearch-2.2/nbproject/build-impl.xml:263: Unable to find a javac compiler;
com.sun.tools.javac.Main is not on the classpath.
Perhaps JAVA_HOME does not point to the JDK.
It is currently set to "/usr/lib/jvm/java-1.8.0-openjdk-1.8.0.77-1.b03.fc25.x86_64/jre"

You must follow the java guideline and use 'BuildRequires: java-devel'. Just depending on ant only seems not to be enough.
https://fedoraproject.org/wiki/Packaging:Java#BuildRequires_and_Requires
Comment 14 jiri vanek 2016-03-30 09:14:14 EDT
Spec URL: https://jvanek.fedorapeople.org/offlinesearch/v3/javadocofflinesearch.spec
SRPM URL: https://jvanek.fedorapeople.org/offlinesearch/v3/javadocofflinesearch-2.2-1.fc23.src.rpm

(also bin directory with build binaries..)

License fixed, java-devel added.
TYVM!

The packing:
This idea really crossed my mind. Issue is that netbeans generated (and from time to time regenerated) build script is easy to extend, but clumsy to modify. So I have basically three options:
 1-  patch (in spec) generated buildscript to exclude manifest generation and call ant dist. Then this jar may be reused
 2 - create new upstream ant target, which pack the built classes.  This may sound like best idea but the target is completely useless out of fedora world.
 3 - go as now - use generated target to build classes and pack manually


From those (3) seems like smallest evil based on my experience on packaging netbeasn projects.

IMHO - build IS  *.java -> *.class. As you can equally run directory full of classes or ziparchive full of classes (jar). Fedora have infrastructure to help with jars. So result of build - the classes - are JARed.

When you look to release on github - the preferred way out of distribution package is assembled jar with all dependences.
Comment 15 jiri vanek 2016-03-30 09:18:31 EDT
And! -  Thank you for review. Although I disagree, I appreciate your comments.
Comment 16 Raphael Groner 2016-03-30 09:44:10 EDT
Well, what you write is common issue with Netbeans' internal ant.
Comment 17 Raphael Groner 2016-03-30 10:12:46 EDT
GPL (v2)
--------
JavadocOfflineSearch-JavadocOfflineSearch-2.2/src/javadocofflinesearch/server/ServerLauncher.java
JavadocOfflineSearch-JavadocOfflineSearch-2.2/src/javadocofflinesearch/server/TinyHttpdImpl.java

=> Please fix, we now use GPLv3+. All other files are without license header, so LICENSE applies generally.


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

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


Issues:
=======
- Packages have proper BuildRequires/Requires on jpackage-utils
- Sources used to build the package match the upstream source, as provided
  in the spec URL.
  Note: Upstream MD5sum check error, diff is in /home/builder/fedora-
  review/1312015-javadocofflinesearch/diff.txt
  See: http://fedoraproject.org/wiki/Packaging/SourceURL
- Javadoc subpackages should not have Requires: jpackage-utils
  Note: jpackage-utils requires are automatically generated by the
  buildsystem
  See: https://fedoraproject.org/wiki/Packaging:Java


===== 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.
[!]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "GPL (v2)", "Unknown or generated". 29 files have unknown
     license. Detailed output of licensecheck in /home/builder/fedora-
     review/1312015-javadocofflinesearch/licensecheck.txt
=> Two source files have license header of GPLv2+.

[x]: License file installed when any subpackage combination is installed.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[!]: Requires correct, justified where necessary.
=> See above.

[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Package is not known to require an ExcludeArch tag.
[!]: Package complies to the Packaging Guidelines
=> Please fix license of source files, and Requires.

[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[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 %license.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 0 bytes in 0 files.
[x]: Packages must not store files under /srv, /opt or /usr/local

Java:
[x]: Bundled jar/class files should be removed before build
[x]: Javadoc documentation files are generated and included in -javadoc
     subpackage
[x]: Javadocs are placed in %{_javadocdir}/%{name} (no -%{version} symlink)

Maven:
[-]: If package contains pom.xml files install it (including metadata) even
     when building with ant
=> Not available.

[x]: Old add_to_maven_depmap macro is not being used

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

Generic:
[-]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[!]: Final provides and requires are sane (see attachments).
=> See above.

[?]: 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.
[-]: 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.
=> Task info: http://koji.fedoraproject.org/koji/taskinfo?taskID=13508089

[-]: %check is present and all tests pass.
[!]: Packages should try to preserve timestamps of original installed
     files.
=> Use 'install -p' or 'cp -p'.

[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[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)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Fully versioned dependency in subpackages if applicable.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

Java:
[?]: Package uses upstream build method (ant/maven/etc.)
=> See the comments about discrepancy with Netbeans.

[x]: Packages are noarch unless they use JNI

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

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: javadocofflinesearch-2.2-1.fc25.noarch.rpm
          javadocofflinesearch-javadoc-2.2-1.fc25.noarch.rpm
          javadocofflinesearch-2.2-1.fc25.src.rpm
javadocofflinesearch.noarch: W: no-documentation
=> You could add '%doc README.md' if of any help for users.

=> IGNORE all below:
javadocofflinesearch.noarch: W: no-manual-page-for-binary javadocofflinesearch
javadocofflinesearch-javadoc.noarch: W: spelling-error Summary(en_US) Javadocs -> Java docs, Java-docs, Avocados
javadocofflinesearch.src: W: strange-permission JavadocOfflineSearch-JavadocOfflineSearch-2.2.tar.gz 640
3 packages and 0 specfiles checked; 0 errors, 4 warnings.




Rpmlint (installed packages)
----------------------------
sh: /usr/bin/python: No such file or directory
javadocofflinesearch.noarch: W: no-documentation
javadocofflinesearch.noarch: W: no-manual-page-for-binary javadocofflinesearch
javadocofflinesearch-javadoc.noarch: W: spelling-error Summary(en_US) Javadocs -> Java docs, Java-docs, Avocados
2 packages and 0 specfiles checked; 0 errors, 3 warnings.
=> IGNORE all


Requires
--------
javadocofflinesearch (rpmlib, GLIBC filtered):
    apache-commons-cli
    java-headless
    jpackage-utils
    lucene-analysis
    lucene-core
    lucene-queries
    lucene-queryparser
    tagsoup

javadocofflinesearch-javadoc (rpmlib, GLIBC filtered):
    javadocofflinesearch
    javapackages-tools
    jpackage-utils



Provides
--------
javadocofflinesearch:
    javadocofflinesearch

javadocofflinesearch-javadoc:
    javadocofflinesearch-javadoc



Source checksums
----------------
https://github.com/judovana/JavadocOfflineSearch/releases/tag/JavadocOfflineSearch-2.2/JavadocOfflineSearch-JavadocOfflineSearch-2.2.tar.gz :
  CHECKSUM(SHA256) this package     : 72a6314fa9146dfe5b3908fc9dcf06670aa4dcba48744ea0b60d64ecf5719ceb
  CHECKSUM(SHA256) upstream package : 38134ec817f64cabb5222cc8380d5cab7593c8649f448110a96fd84d3d4dfb14
diff -r also reports differences


Generated by fedora-review 0.6.0 (3c5c9d7) last change: 2015-05-20
Command line :/usr/bin/fedora-review -v -m fedora-rawhide-x86_64 -b 1312015
Buildroot used: fedora-rawhide-x86_64
Active plugins: Generic, Shell-api, Java
Disabled plugins: C/C++, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP, Ruby
Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6
Comment 18 Raphael Groner 2016-03-30 10:14:38 EDT
> cat 1312015-javadocofflinesearch/diff.txt 
Nur in /home/builder/fedora-review/1312015-javadocofflinesearch/srpm-unpacked/JavadocOfflineSearch-JavadocOfflineSearch-2.2.tar.gz-extract: JavadocOfflineSearch-JavadocOfflineSearch-2.2.
Nur in /home/builder/fedora-review/1312015-javadocofflinesearch/upstream-unpacked/Source0: JavadocOfflineSearch-JavadocOfflineSearch-2.2.tar.gz.
Comment 20 jiri vanek 2016-03-31 10:24:03 EDT
 wget https://github.com/judovana/JavadocOfflineSearch/archive/JavadocOfflineSearch-2.2.tar.gz

sha256sum JavadocOfflineSearch-2.2.tar.gz 
72a6314fa9146dfe5b3908fc9dcf06670aa4dcba48744ea0b60d64ecf5719ceb  JavadocOfflineSearch-2.2.tar.gz


This already happened to me with github. not sure whats wrong:(
Comment 21 jiri vanek 2016-03-31 10:50:31 EDT
Spec URL: https://jvanek.fedorapeople.org/offlinesearch/v3/javadocofflinesearch.spec
SRPM URL:  https://jvanek.fedorapeople.org/offlinesearch/v3/javadocofflinesearch-2.2-1.fc23.src.rpm

and ... https://jvanek.fedorapeople.org/offlinesearch/v3/bins/

rpmlint  RPMS/noarch/javadocofflinesearch-* SRPMS/javadocofflinesearch-2.2-1.fc23.src.rpm  SPECS/javadocofflinesearch.spec 
javadocofflinesearch.noarch: W: no-documentation
javadocofflinesearch.noarch: W: no-manual-page-for-binary javadocofflinesearch
javadocofflinesearch-javadoc.noarch: W: spelling-error Summary(en_US) Javadocs -> Java docs, Java-docs, Avocados
3 packages and 1 specfiles checked; 0 errors, 3 warnings.


--- /home/jvanek/git/javadocofflinesearch/javadocofflinesearch.spec-old
+++ /home/jvanek/git/javadocofflinesearch/javadocofflinesearch.spec
@@ -8,10 +8,12 @@
 
 License:        GPLv3+
 URL:            https://github.com/judovana/%{srcname}
-Source0:        https://github.com/judovana/%{srcname}/releases/tag/%{srcname}-2.2/%{srcname}-%{srcname}-%{version}.tar.gz
+Source0:        https://github.com/judovana/%{srcname}/releases/tag/%{srcname}-2.2/%{srcname}-%{version}.tar.gz
 # already fixed in upstream https://github.com/judovana/JavadocOfflineSearch/commit/853285f3105506b860c762f534e1a7c2733a2c61
 Patch1:         javadocFixes.patch
-
+# fixed license https://github.com/judovana/JavadocOfflineSearch/commit/7d0c410d9ef215499f4fa4fb67e9a105f0a95ba7
+Patch2:         7d0c410d9ef215499f4fa4fb67e9a105f0a95ba7.diff
+ 
 BuildRequires:  ant
 BuildRequires:  java-devel
 BuildRequires:  tagsoup
@@ -48,7 +50,6 @@
 Summary:        Javadocs for %{name}
 Group:          Documentation
 Requires:       %{name} = %{version}-%{release}
-Requires:       jpackage-utils
 
 %description javadoc
 This package contains the API documentation for %{name}.
@@ -58,6 +59,7 @@
 %setup -q -n %{srcname}-%{srcname}-%{version}
 find -print -name '*.class' -or -name '*.jar'  -delete
 %patch1 -p1
+%patch2 -p1
 
 
 %build



Fixed - license and source tarball.


The only not fixed issue remains the requires you complain about. But I really dont know how to make them better. Whatever I remove, always leads to failure.

So far so god!
  thanx!
Comment 22 Raphael Groner 2016-03-31 12:08:15 EDT
Okay, we're getting closer to an approval. Thanks for your patience.

. We need 'BuildRequires: ant java-devel jpackage-utils'. But do not add something of that as Requires in javadoc subpackage, means remove 'Requires: jpackage-utils' and let rpmbuild care about automatically generated dependencies. We'll see success in the f-r log.
https://fedoraproject.org/wiki/Packaging:Java#BuildRequires_and_Requires

> - Packages have proper BuildRequires/Requires on jpackage-utils> - Javadoc subpackages should not have Requires: jpackage-utils
>  Note: jpackage-utils requires are automatically generated by the
>  buildsystem
>  See: https://fedoraproject.org/wiki/Packaging:Java


. Hint: You can use a special feature of GitHub to generate patches, in combination with rpm macro expansion. Also use %autosetup.
https://fedoraproject.org/wiki/How_to_create_an_RPM_package#.25prep_section:_.25autosetup_command

URL:            https://github.com/judovana/%{srcname}
Source0:        %{url}/releases/tag/%{srcname}-2.2/%{srcname}-%{srcname}-%{version}.tar.gz
# javadocFixes
Patch0:         %{url}/commit/853285f3105506b860c762f534e1a7c2733a2c61.patch
# fixed license
Patch1:         %{url}/commit/7d0c410d9ef215499f4fa4fb67e9a105f0a95ba7.patch

%prep
# use autosetup to untar source and apply above patches
%autosetup -p1 -n%{srcname}-%{srcname}-%{version}
# remove all %patch lines


Please upload an updated spec file. I get an old one for v3.
Comment 23 jiri vanek 2016-04-01 06:45:15 EDT
> Please upload an updated spec file. I get an old one for v3.


My focus is really bad. Sorry The fixed stuff was in v4, together with bins

Spec URL: https://jvanek.fedorapeople.org/offlinesearch/v4/javadocofflinesearch.spec
SRPM URL:  https://jvanek.fedorapeople.org/offlinesearch/v4/javadocofflinesearch-2.2-1.fc23.src.rpm
Comment 24 jiri vanek 2016-04-01 07:07:50 EDT
(In reply to Raphael Groner from comment #22)
> Okay, we're getting closer to an approval. Thanks for your patience.

Thanx for YOURS patience.

> 
> . We need 'BuildRequires: ant java-devel jpackage-utils'. But do not add
> something of that as Requires in javadoc subpackage, means remove 'Requires:
> jpackage-utils' and let rpmbuild care about automatically generated
> dependencies. We'll see success in the f-r log.
> https://fedoraproject.org/wiki/Packaging:Java#BuildRequires_and_Requires


Oh. Sure. This is/was already fixed in v4.
> 
> > - Packages have proper BuildRequires/Requires on jpackage-utils
> …
> > - Javadoc subpackages should not have Requires: jpackage-utils
> >  Note: jpackage-utils requires are automatically generated by the
> >  buildsystem
> >  See: https://fedoraproject.org/wiki/Packaging:Java
> 

same.
> 
> . Hint: You can use a special feature of GitHub to generate patches, in
> combination with rpm macro expansion. Also use %autosetup.
> https://fedoraproject.org/wiki/How_to_create_an_RPM_package#.25prep_section:
> _.25autosetup_command

As you wish, done.
> 
> URL:            https://github.com/judovana/%{srcname}
> Source0:       
> %{url}/releases/tag/%{srcname}-2.2/%{srcname}-%{srcname}-%{version}.tar.gz
> # javadocFixes
> Patch0:         %{url}/commit/853285f3105506b860c762f534e1a7c2733a2c61.patch
> # fixed license
> Patch1:         %{url}/commit/7d0c410d9ef215499f4fa4fb67e9a105f0a95ba7.patch
> 
> %prep
> # use autosetup to untar source and apply above patches
> %autosetup -p1 -n%{srcname}-%{srcname}-%{version}
> # remove all %patch lines
> 
> 
> Please upload an updated spec file. I get an old one for v3.


Spec URL: https://jvanek.fedorapeople.org/offlinesearch/v5/javadocofflinesearch.spec
SRPM URL:  https://jvanek.fedorapeople.org/offlinesearch/v5/javadocofflinesearch-2.2-1.fc23.src.rpm

and ... https://jvanek.fedorapeople.org/offlinesearch/v5/bins/
Comment 25 Raphael Groner 2016-04-01 07:26:17 EDT
APPROVED

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

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


Issues:
=======
- Packages have proper BuildRequires/Requires on jpackage-utils
=> Discussed below.

- Sources used to build the package match the upstream source, as provided
  in the spec URL.
  Note: Upstream MD5sum check error, diff is in /home/builder/fedora-
  review/1312015-javadocofflinesearch/diff.txt
  See: http://fedoraproject.org/wiki/Packaging/SourceURL
=> Discussed below, known issue with GitHub.

- No desktop file. That makes start somehow harder for unexperienced users.


===== 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]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "Unknown or generated". 31 files have unknown license. Detailed
     output of licensecheck in /home/builder/fedora-
     review/1312015-javadocofflinesearch/licensecheck.txt
=> GPLv3+ generally.
[x]: License file installed when any subpackage combination is installed.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[?]: Package contains desktop file if it is a GUI application.
=> Can you provide one?

[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Package is not known to require an ExcludeArch tag.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[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 %license.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 0 bytes in 0 files.
[x]: Packages must not store files under /srv, /opt or /usr/local

Java:
[x]: Bundled jar/class files should be removed before build
[x]: Javadoc documentation files are generated and included in -javadoc
     subpackage
[x]: Javadoc subpackages should not have Requires: jpackage-utils
[x]: Javadocs are placed in %{_javadocdir}/%{name} (no -%{version} symlink)

Maven:
[-]: If package contains pom.xml files install it (including metadata) even
     when building with ant
=> Not available.

[x]: Old add_to_maven_depmap macro is not being used

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

Generic:
[-]: 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]: Final provides and requires are sane (see attachments).
[?]: 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.
[-]: 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.
=> Please use 'install -p' or 'cp -p'.

[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[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)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Fully versioned dependency in subpackages if applicable.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

Java:
[?]: Package uses upstream build method (ant/maven/etc.)
=> See discussion below about (commonly known) discrepancy with Netbeans.

[x]: Packages are noarch unless they use JNI

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

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: javadocofflinesearch-2.2-1.fc25.noarch.rpm
          javadocofflinesearch-javadoc-2.2-1.fc25.noarch.rpm
          javadocofflinesearch-2.2-1.fc25.src.rpm
javadocofflinesearch.noarch: W: no-documentation
javadocofflinesearch.noarch: W: no-manual-page-for-binary javadocofflinesearch
javadocofflinesearch-javadoc.noarch: W: spelling-error Summary(en_US) Javadocs -> Java docs, Java-docs, Avocados
3 packages and 0 specfiles checked; 0 errors, 3 warnings.




Rpmlint (installed packages)
----------------------------
sh: /usr/bin/python: No such file or directory
javadocofflinesearch.noarch: W: no-documentation
javadocofflinesearch.noarch: W: no-manual-page-for-binary javadocofflinesearch
javadocofflinesearch-javadoc.noarch: W: spelling-error Summary(en_US) Javadocs -> Java docs, Java-docs, Avocados
2 packages and 0 specfiles checked; 0 errors, 3 warnings.



Requires
--------
javadocofflinesearch (rpmlib, GLIBC filtered):
    apache-commons-cli
    java-headless
    jpackage-utils
    lucene-analysis
    lucene-core
    lucene-queries
    lucene-queryparser
    tagsoup

javadocofflinesearch-javadoc (rpmlib, GLIBC filtered):
    javadocofflinesearch
    javapackages-tools



Provides
--------
javadocofflinesearch:
    javadocofflinesearch

javadocofflinesearch-javadoc:
    javadocofflinesearch-javadoc



Source checksums
----------------
https://github.com/judovana/JavadocOfflineSearch/releases/tag/JavadocOfflineSearch-2.2/JavadocOfflineSearch-2.2.tar.gz :
  CHECKSUM(SHA256) this package     : 72a6314fa9146dfe5b3908fc9dcf06670aa4dcba48744ea0b60d64ecf5719ceb
  CHECKSUM(SHA256) upstream package : f8d873cd5675dd04b0bf019398aee6c0c8ddeffc9d7f6717c139716643d47858
diff -r also reports differences


Generated by fedora-review 0.6.0 (3c5c9d7) last change: 2015-05-20
Command line :/usr/bin/fedora-review -v -m fedora-rawhide-x86_64 -b 1312015
Buildroot used: fedora-rawhide-x86_64
Active plugins: Generic, Shell-api, Java
Disabled plugins: C/C++, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP, Ruby
Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6
Comment 26 Gwyn Ciesla 2016-04-01 14:07:43 EDT
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/javadocofflinesearch
Comment 27 Fedora Update System 2016-04-02 14:36:13 EDT
javadocofflinesearch-2.2-1.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-cfa83b8982
Comment 28 Fedora Update System 2016-04-02 14:37:02 EDT
javadocofflinesearch-2.2-1.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2016-d447bd309b
Comment 29 Fedora Update System 2016-04-03 18:50:44 EDT
javadocofflinesearch-2.2-1.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-cfa83b8982
Comment 30 Fedora Update System 2016-04-05 04:55:57 EDT
javadocofflinesearch-2.2-1.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-d447bd309b
Comment 31 Fedora Update System 2016-04-14 23:19:55 EDT
javadocofflinesearch-2.2-1.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report.
Comment 32 Fedora Update System 2016-04-15 03:21:03 EDT
javadocofflinesearch-2.2-1.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.

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