Bug 991400 - Review Request: myrulib - E-Book Library Manager
Review Request: myrulib - E-Book Library Manager
Status: CLOSED WONTFIX
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Igor Gnatenko
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2013-08-02 07:01 EDT by Vasiliy Glazov
Modified: 2013-12-11 02:00 EST (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-12-11 01:58:34 EST
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
ignatenko: fedora‑review?


Attachments (Terms of Use)

  None (edit)
Description Vasiliy Glazov 2013-08-02 07:01:22 EDT
Spec URL: https://raw.github.com/RussianFedora/myrulib/master/myrulib.spec
SRPM URL: http://koji.russianfedora.ru/koji/getfile?taskID=82983&name=myrulib-0.29.14-2.fc20.R.src.rpm

Description: MyRuLib is an application for organizing your own collection of e-books.

Import and organizing fb2, epub books.
Export books.
Work with net libraries.
Fast and simple GUI.
Comment 1 Michael Schwendt 2013-08-05 12:46:11 EDT
> --with-strip

Does that do what I think it does? And if so, are you aware of its consequences?
Comment 2 Vasiliy Glazov 2013-08-06 04:49:18 EDT
Spec URL: https://raw.github.com/RussianFedora/myrulib/master/myrulib.spec
SRPM URL: http://koji.russianfedora.ru/koji/getfile?taskID=83205&name=myrulib-0.29.14-3.fc20.R.src.rpm

With --with-strip debuginfo subpackage was empty. I change it to --without-strip.
Comment 3 Antonio Trande 2013-08-10 06:12:39 EDT
Hi Vasiliy.

> myrulib-0.29.14-3.fc20.R.src.rpm

*.R.* ? Not in Fedora, please.

> %{_datadir}/applications/%{name}.desktop
.desktop file validate is missing
http://fedoraproject.org/wiki/Packaging:Guidelines#desktop-file-install_usage

> Multilicensing

Compilation is performed from source files with different licenses:

/3rdparty/bzip2/blocksort.c (BSD)
/3rdparty/bin2c/bin2c.c     (GPLv2+)
/3rdparty/bzip2/*         --> LICENSE file (GPLv3)

See http://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Mixed_Source_Licensing_Scenario
Comment 4 Christopher Meng 2013-08-10 06:53:01 EDT
Please remove bundled libraries. 

Antonio's example indicates that you should use bzip2-devel but not bundled one.
Comment 5 Vasiliy Glazov 2013-08-12 03:27:56 EDT
Spec URL: https://raw.github.com/RussianFedora/myrulib/master/myrulib.spec
SRPM URL: http://koji.russianfedora.ru/koji/getfile?taskID=83453&name=myrulib-0.29.14-4.fc20.R.src.rpm

All bundled libraries removed.
.desktop file validated.
Suffix .R present only in my srpm file. If you rebuild it .R will be removed automattically.
Since I remove bundled libraries this files not compiled ant it's licenses not used.
Comment 6 Antonio Trande 2013-08-12 08:01:16 EDT
(In reply to Vasiliy Glazov from comment #5)
> Spec URL: https://raw.github.com/RussianFedora/myrulib/master/myrulib.spec
> SRPM URL:
> http://koji.russianfedora.ru/koji/getfile?taskID=83453&name=myrulib-0.29.14-
> 4.fc20.R.src.rpm
> 
> All bundled libraries removed.
...
> Since I remove bundled libraries this files not compiled ant it's licenses
> not used.

Which have been removed ?
I don't see any removal command in your .spec file.

> Suffix .R present only in my srpm file. If you rebuild it .R will be removed
> automattically.

Read http://fedoraproject.org/wiki/Package_Review_Process#Contributor:
"As a Contributor, you should have already made a package which adheres to the Package Naming Guidelines and Packaging Guidelines."
Comment 7 Vasiliy Glazov 2013-08-12 08:13:14 EDT
I add BuildRequires:  bzip2-devel, and configure --without-icu --without-sqlite
In buildlog http://koji.russianfedora.ru/koji/getfile?taskID=83453&name=build.log
you can see 
  Use Libxml2 parser library (default)?         no
  Use Expat XML parser instead of Libxml2?      yes
  Use FAXPP (Fast XML Pull Parser) library?     no
  Use builtin BZip2 library?                    no
  Use builtin SQLite3 library?                  no
  Use ICU for unicode collation?                no
  Use Cool Reader Engine?                       no
  Use system logger: syslog?                    no
  Include locale files into executable?         no
  Include links to the online collections?      yes
  Strip the executable file?                    no
  Link wxWidgets as a static library?           no

Bundled Bzip2 and SQLite3 removed from build process. Or I must remove them from sources? 

And I dont think that suffix .R in preliminary srpm change anything. You can take renamed srpm here https://dl.dropboxusercontent.com/u/1776041/myrulib-0.29.14-4.fc20.src.rpm
Comment 8 Antonio Trande 2013-08-12 09:30:36 EDT
(In reply to Vasiliy Glazov from comment #7)
> I add BuildRequires:  bzip2-devel, and configure --without-icu
> --without-sqlite
> In buildlog
> http://koji.russianfedora.ru/koji/getfile?taskID=83453&name=build.log
> you can see 
>   Use Libxml2 parser library (default)?         no
>   Use Expat XML parser instead of Libxml2?      yes
>   Use FAXPP (Fast XML Pull Parser) library?     no
>   Use builtin BZip2 library?                    no
>   Use builtin SQLite3 library?                  no
>   Use ICU for unicode collation?                no
>   Use Cool Reader Engine?                       no
>   Use system logger: syslog?                    no
>   Include locale files into executable?         no
>   Include links to the online collections?      yes
>   Strip the executable file?                    no
>   Link wxWidgets as a static library?           no
> 
> Bundled Bzip2 and SQLite3 removed from build process. Or I must remove them
> from sources? 
> 

http://fedoraproject.org/wiki/Packaging:Treatment_Of_Bundled_Libraries :)
Comment 9 Michael Schwendt 2013-08-12 10:59:54 EDT
Hint: it can be helpful to "damage" the bundled sources, so if they are built with accidentally, the build would fail. "Damaging" can be anything, such as deleting a file or inserting something into it that would make the compiler fail.
Comment 10 Vasiliy Glazov 2013-08-13 01:29:19 EDT
OK, I already done that and contact upstream developer to solve this issue.
https://github.com/lintest/myrulib/issues/6
(In russian).
Comment 11 Christopher Meng 2013-11-13 22:20:24 EST
0.29.16 is out.
Comment 12 Igor Gnatenko 2013-12-10 12:02:29 EST
Taken
Comment 13 Igor Gnatenko 2013-12-10 14:12:45 EST
are you sure that you want to maintain this pkg? Seems like this pkg is SHI~.

if want, you should do:
* Add to F coolreader (crengine subpackage)
* Add to F wxBZipStream
* Add "BuildRequires:  libxml2-devel" to spec
* Add "BuildRequires:  wxsqlite3-devel = 3.0.5" to spec
* %configure options should be
  --without-icu \
  --with-expat \
  --with-reader \
  --without-bzip2 \
  --without-sqlite \
  --without-wxsqlite \
  --without-strip
* Add to %prep section rm -rf 3rdparty/
* Add to %build section autoreconf -vfi
* Apply patch
--- configure.in.orig	2013-09-03 22:30:43.000000000 +0400
+++ configure.in	2013-12-10 22:37:57.274923790 +0400
@@ -197,6 +197,10 @@
     USE_WXSQL="builtin"
 fi
 
+if test "x$USE_WXSQL" != "xbuiltin" ; then 
+    PKG_CHECK_MODULES(WXSQL, wxsqlite-3.0.5 = 3.0.5, , USE_WXSQL="builtin")
+fi
+
 AC_MSG_CHECKING([for --with-locale])
 AC_ARG_WITH([locale], [AS_HELP_STRING([--with-locale], [Include locale files into executable])], USE_LOCALE="$withval", USE_LOCALE="no")
 AC_MSG_RESULT([$USE_LOCALE])
@@ -280,7 +284,7 @@
   WX_CXXFLAGS="-I\$(srcdir)/3rdparty/wxsqlite3/include $WX_CXXFLAGS"
   WX_LIBS="-lmrl_wxsqlite3 $WX_LIBS"
 else
-  WX_LIBS="-lwxsqlite3-2.8 $WX_LIBS"
+  WX_LIBS="$WXSQL_LIBS $WX_LIBS"
 fi
 
 dnl ==================
* Write the same patches for other libs (which have pkgfile) and for sure fix crengine in configure.in

This is first part.
--------------------------------------------------------------------------------

After this we could continue review ;)

And I ask you again: are you sure want to maintain this SHI~ SHI~ SHI~ pkg
Comment 14 Vasiliy Glazov 2013-12-11 01:58:34 EST
No.
I can't solve all this problems.

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