Bug 1338140 (guayadeque) - Review Request: guayadeque - Music player
Summary: Review Request: guayadeque - Music player
Keywords:
Status: CLOSED NEXTRELEASE
Alias: guayadeque
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Raphael Groner
QA Contact: Fedora Extras Quality Assurance
URL: https://github.com/anonbeat/guayadeque
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2016-05-21 17:34 UTC by MartinKG
Modified: 2016-06-09 19:34 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-06-09 19:34:43 UTC
Type: ---
Embargoed:
projects.rg: fedora-review+


Attachments (Terms of Use)
licensecheck.txt for 35561f6 (29.79 KB, text/plain)
2016-05-24 15:42 UTC, Raphael Groner
no flags Details

Description MartinKG 2016-05-21 17:34:49 UTC
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/guayadeque.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/guayadeque-0.4.1-0.1.beta1git65f759c.fc23.src.rpm

Description: Un-retired pymssql and returned to the state before it was orphaned.
Fedora Account System Username: martinkg

guayadeque was retired from git, because the developer of guayadeque has resumed development after a year of abstinence.

Comment 1 MartinKG 2016-05-21 17:38:50 UTC
I need access for f24 and rawhide branch again.

Comment 2 MartinKG 2016-05-21 17:45:04 UTC
$ koji list-pkgs  --show-blocked --tag f24 --package guayadeque
Package                 Tag                     Extra Arches     Owner          
----------------------- ----------------------- ---------------- ---------------
guayadeque              f24                                      martinkg        [BLOCKED]

$ koji list-pkgs  --show-blocked --tag rawhide --package guayadeque
Package                 Tag                     Extra Arches     Owner          
----------------------- ----------------------- ---------------- ---------------
guayadeque              f25                                      martinkg        [BLOCKED]

Comment 3 MartinKG 2016-05-22 13:30:39 UTC
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/guayadeque.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/guayadeque-0.4.1-0.2.beta1gitd2c0281.fc24.src.rpm

%changelog
* Sun May 22 2016 Martin Gansser <martinkg> - 0.4.1-0.2.beta1gitd2c0281
- Update to 0.4.1-0.2.beta1gitd2c0281
- Mark license files as %%license where available
- Cleanup spec file

rpmlint -i guayadeque.spec /home/martin/rpmbuild/SRPMS/guayadeque-0.4.1-0.2.beta1gitd2c0281.fc24.src.rpm /home/martin/rpmbuild/RPMS/x86_64/guayadeque-0.4.1-0.2.beta1gitd2c0281.fc24.x86_64.rpm /home/martin/rpmbuild/RPMS/x86_64/guayadeque-debuginfo-0.4.1-0.2.beta1gitd2c0281.fc24.x86_64.rpm
guayadeque.spec:29: W: unversioned-explicit-provides bundled(md5-polstra)
The specfile contains an unversioned Provides: token, which will match all
older, equal, and newer versions of the provided thing.  This may cause update
problems and will make versioned dependencies, obsoletions and conflicts on
the provided thing useless -- make the Provides versioned if possible.

guayadeque.src:29: W: unversioned-explicit-provides bundled(md5-polstra)
The specfile contains an unversioned Provides: token, which will match all
older, equal, and newer versions of the provided thing.  This may cause update
problems and will make versioned dependencies, obsoletions and conflicts on
the provided thing useless -- make the Provides versioned if possible.

guayadeque.x86_64: W: no-manual-page-for-binary guayadeque
Each executable in standard binary directories should have a man page.

3 packages and 1 specfiles checked; 0 errors, 3 warnings.

Comment 4 Raphael Groner 2016-05-22 13:48:39 UTC
My suggestion to run a complete new package review. Some general advice:

- Are you sure about all BuildRequires? Why subversion-devel?

- You can remove Group tag, it's obsolete.

- Please provide a proper patch for the desktop file. It makes it easier for
  further updates.
> # deleting Unity parts in guayadeque.desktop files
> sed -i '18,38d' runtime/guayadeque.desktop

- Are all of the explicit arguments to %cmake needed? Can you build with debug 
  flags instead of release?
> -DCMAKE_BUILD_TYPE='Release'
> -DCMAKE_INSTALL_PREFIX='%{_prefix}' 
...

Comment 5 MartinKG 2016-05-22 16:24:56 UTC
(In reply to Raphael Groner from comment #4)
> My suggestion to run a complete new package review. Some general advice:
> 
> - Are you sure about all BuildRequires? Why subversion-devel?
done

> 
> - You can remove Group tag, it's obsolete.
done

> 
> - Please provide a proper patch for the desktop file. It makes it easier for
>   further updates.
> > # deleting Unity parts in guayadeque.desktop files
> > sed -i '18,38d' runtime/guayadeque.desktop
done

> 
> - Are all of the explicit arguments to %cmake needed? Can you build with
> debug 
>   flags instead of release?
> > -DCMAKE_BUILD_TYPE='Release'
> > -DCMAKE_INSTALL_PREFIX='%{_prefix}' 
> ...
dropped a few Options that are already in %cmake macro

Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/guayadeque.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/guayadeque-0.4.1-0.3.beta1git35561f6.fc24.src.rpm

%changelog
* Sun May 22 2016 Martin Gansser <martinkg> - 0.4.1-0.3.beta1git35561f6
- Update to 0.4.1-0.3.beta1git35561f6
- Dropped BR subversion-devel
- Removed Group tag, it's obsolete
- Addes %%{name}-desktop.patch
- Dropped -DCMAKE_INSTALL_PREFIX='%%{_prefix}' because it's already in %%cmake macro
- Changed -DCMAKE_BUILD_TYPE='Release' to -DCMAKE_BUILD_TYPE='Debug'

Comment 6 Raphael Groner 2016-05-23 08:08:12 UTC
Review swap? Maybe can you take bug #1279087 or bug #1338553?

Comment 7 Raphael Groner 2016-05-24 15:39:51 UTC
Package Review
==============

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


Issues:
=======
- Package does not use a name that already exists.
  Note: A package with this name already exists. Please check
  https://admin.fedoraproject.org/pkgdb/package/guayadeque
  See:
  https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Conflicting_Package_Names
=> This new review is to unretire the package.

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

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: Header files in -devel subpackage, if present.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.

Generic:
[?]: 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: "BSD (3 clause)", "LGPL (v2.1 or later)", "GPL (v3 or later)",
     "Unknown or generated", "*No copyright* Public domain". 169 files have
     unknown license. Detailed output of licensecheck in /home/builder
     /fedora-review/1338140-guayadeque/licensecheck.txt
=> Please explain why those files with no license header.

[x]: License file installed when any subpackage combination is installed.
[!]: If the package is under multiple licenses, the licensing breakdown
     must be documented in the spec.
[x]: %build honors applicable compiler flags or justifies otherwise.
[?]: Package contains no bundled libraries without FPC exception.
=> Source files without license header, see above.

[x]: Changelog in prescribed format.
[?]: Sources contain only permissible code or content.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[-]: The spec file handles locales properly.
[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]: update-desktop-database is invoked in %post and %postun if package
     contains desktop file(s) with a MimeType: entry.
     Note: desktop file(s) with MimeType entry in guayadeque
[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
=> Task info: http://koji.fedoraproject.org/koji/taskinfo?taskID=14236630

[x]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 10240 bytes in 1 files.
[?]: Package complies to the Packaging Guidelines
=> see above, licensing/bundling?

[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]: Package contains desktop file if it is a GUI application.
[x]: Package installs a %{name}.desktop using desktop-file-install or
     desktop-file-validate if there is such a file.
[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 is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== 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).
[x]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in
     guayadeque-debuginfo
=> Ignore. debuginfo is automatically generated.

[?]: Package functions as described.
[?]: Latest version is packaged.
=> Latest commit is 13013ff, 2016-05-22. Maybe take this tarball.

[x]: Package does not include license text files separate from upstream.
[-]: 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=14236630

[ ]: %check is present and all tests pass.
[ ]: Packages should try to preserve timestamps of original installed
     files.
[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]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Uses parallel make %{?_smp_mflags} macro.
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

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

Generic:
[!]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.
     Note: Arch-ed rpms have a total of 1781760 bytes in /usr/share
=> Please split locales into a l10n subpackage.

[x]: Rpmlint is run on debuginfo package(s).
     Note: No rpmlint messages.
[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: guayadeque-0.4.1-0.3.beta1git35561f6.fc25.x86_64.rpm
          guayadeque-debuginfo-0.4.1-0.3.beta1git35561f6.fc25.x86_64.rpm
          guayadeque-0.4.1-0.3.beta1git35561f6.fc25.src.rpm
guayadeque.x86_64: W: no-manual-page-for-binary guayadeque
guayadeque.src:29: W: unversioned-explicit-provides bundled(md5-polstra)
3 packages and 0 specfiles checked; 0 errors, 2 warnings.




Rpmlint (debuginfo)
-------------------
Checking: guayadeque-debuginfo-0.4.1-0.3.beta1git35561f6.fc25.x86_64.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.





Rpmlint (installed packages)
----------------------------
sh: /usr/bin/python: No such file or directory
guayadeque.x86_64: W: no-manual-page-for-binary guayadeque
2 packages and 0 specfiles checked; 0 errors, 1 warnings.



Requires
--------
guayadeque (rpmlib, GLIBC filtered):
    /bin/sh
    libc.so.6()(64bit)
    libcurl.so.4()(64bit)
    libdbus-1.so.3()(64bit)
    libdbus-1.so.3(LIBDBUS_1_3)(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libgdk_pixbuf-2.0.so.0()(64bit)
    libgio-2.0.so.0()(64bit)
    libglib-2.0.so.0()(64bit)
    libgobject-2.0.so.0()(64bit)
    libgpod.so.4()(64bit)
    libgstreamer-1.0.so.0()(64bit)
    libm.so.6()(64bit)
    libpthread.so.0()(64bit)
    libsqlite3.so.0()(64bit)
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3)(64bit)
    libstdc++.so.6(CXXABI_1.3.8)(64bit)
    libtag.so.1()(64bit)
    libwx_baseu-3.0.so.0()(64bit)
    libwx_baseu-3.0.so.0(WXU_3.0)(64bit)
    libwx_baseu_net-3.0.so.0()(64bit)
    libwx_baseu_xml-3.0.so.0()(64bit)
    libwx_baseu_xml-3.0.so.0(WXU_3.0)(64bit)
    libwx_gtk3u_adv-3.0.so.0()(64bit)
    libwx_gtk3u_adv-3.0.so.0(WXU_3.0)(64bit)
    libwx_gtk3u_aui-3.0.so.0()(64bit)
    libwx_gtk3u_aui-3.0.so.0(WXU_3.0)(64bit)
    libwx_gtk3u_core-3.0.so.0()(64bit)
    libwx_gtk3u_core-3.0.so.0(WXU_3.0)(64bit)
    libwx_gtk3u_html-3.0.so.0()(64bit)
    libwx_gtk3u_html-3.0.so.0(WXU_3.0)(64bit)
    libwx_gtk3u_qa-3.0.so.0()(64bit)
    libwx_gtk3u_qa-3.0.so.0(WXU_3.0)(64bit)
    libwxcode_gtk3u_wxsqlite3-3.0.so.0()(64bit)
    rtld(GNU_HASH)

guayadeque-debuginfo (rpmlib, GLIBC filtered):



Provides
--------
guayadeque:
    application()
    application(guayadeque.desktop)
    bundled(md5-polstra)
    guayadeque
    guayadeque(x86-64)
    mimehandler(application/ogg)
    mimehandler(application/x-flac)
    mimehandler(audio/mp4)
    mimehandler(audio/mpeg)
    mimehandler(audio/mpegurl)
    mimehandler(audio/ogg)
    mimehandler(audio/x-ape)
    mimehandler(audio/x-flac)
    mimehandler(audio/x-m4a)
    mimehandler(audio/x-mod)
    mimehandler(audio/x-mp3)
    mimehandler(audio/x-mpeg)
    mimehandler(audio/x-mpegurl)
    mimehandler(audio/x-ms-asf)
    mimehandler(audio/x-ms-asx)
    mimehandler(audio/x-ms-wax)
    mimehandler(audio/x-ms-wma)

guayadeque-debuginfo:
    guayadeque-debuginfo
    guayadeque-debuginfo(x86-64)



Source checksums
----------------
https://github.com/anonbeat/guayadeque/archive/35561f6e8b6cef2307a3f589cd4ad096c604d51e/guayadeque-35561f6e8b6cef2307a3f589cd4ad096c604d51e.tar.gz#/guayadeque-35561f6.tar.gz :
  CHECKSUM(SHA256) this package     : 1e184c0428447e87536204ef0fd5920433ed1709096d0c34ff4a8b51900c29ef
  CHECKSUM(SHA256) upstream package : 1e184c0428447e87536204ef0fd5920433ed1709096d0c34ff4a8b51900c29ef


Generated by fedora-review 0.6.1 (f03e4e7) last change: 2016-05-02
Command line :/usr/bin/fedora-review -v -m fedora-rawhide-x86_64 -b 1338140
Buildroot used: fedora-rawhide-x86_64
Active plugins: Generic, Shell-api, C/C++
Disabled plugins: Java, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP
Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6

Comment 8 Raphael Groner 2016-05-24 15:42:11 UTC
Created attachment 1161083 [details]
licensecheck.txt for 35561f6

Comment 9 MartinKG 2016-05-25 08:37:02 UTC
(In reply to Raphael Groner from comment #7)
> Package Review
> ==============

> ===== MUST items =====
> Generic:
> [?]: 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: "BSD (3 clause)", "LGPL (v2.1 or later)", "GPL (v3 or later)",
>      "Unknown or generated", "*No copyright* Public domain". 169 files have
>      unknown license. Detailed output of licensecheck in /home/builder
>      /fedora-review/1338140-guayadeque/licensecheck.txt
> => Please explain why those files with no license header.

don't know, will ask upstream.

 
> [x]: License file installed when any subpackage combination is installed.
> [!]: If the package is under multiple licenses, the licensing breakdown
>      must be documented in the spec.
> [x]: %build honors applicable compiler flags or justifies otherwise.
> [?]: Package contains no bundled libraries without FPC exception.
> => Source files without license header, see above.

don't know, will ask upstream.

> 
> [x]: Large documentation must go in a -doc subpackage. Large could be size
>      (~1MB) or number of files.
>      Note: Documentation size is 10240 bytes in 1 files.
> [?]: Package complies to the Packaging Guidelines
> => see above, licensing/bundling?

> 
> ===== SHOULD items =====
> 
> [?]: Package functions as described.
> [?]: Latest version is packaged.
> => Latest commit is 13013ff, 2016-05-22. Maybe take this tarball.

next package will contains the recent version
 
> ===== EXTRA items =====
> 
> Generic:
> [!]: Large data in /usr/share should live in a noarch subpackage if package
>      is arched.
>      Note: Arch-ed rpms have a total of 1781760 bytes in /usr/share
> => Please split locales into a l10n subpackage.

don't know how to split in a l10n package, need assistance here.

Comment 10 MartinKG 2016-05-27 12:25:12 UTC
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/guayadeque.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/guayadeque-0.4.1-0.4.beta1git13013ff.fc24.src.rpm

%changelog
* Wed May 25 2016 Martin Gansser <martinkg> - 0.4.1-0.4.beta1git13013ff
- Update to 0.4.1-0.4.beta1git13013ff
- Split locales into a l10n subpackage

Comment 11 MartinKG 2016-05-30 14:42:12 UTC
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/guayadeque.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/guayadeque-0.4.1-0.5.beta1git26eaf8d.fc24.src.rpm

%changelog
* Mon May 30 2016 Martin Gansser <martinkg> - 0.4.1-0.5.beta1git26eaf8d
- Update to 0.4.1-0.5.beta1git26eaf8d

Comment 12 Raphael Groner 2016-05-31 18:50:32 UTC
> [!]: If the package is under multiple licenses, the licensing breakdown
>      must be documented in the spec.

Please do so. That's MUST.
https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Multiple_Licensing_Scenarios

wxCurl has wxWidgets Library License (two files in src/curl and two files in src/wx/curl), so append wxWidgets as short name to License tag.
https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#Good_Licenses


> [?]: Package contains no bundled libraries without FPC exception.
> => Source files without license header, see above.

Licensecheck script fails to detect the header, we overlooked that in first run. SHOULD ask upstream why wxCurl is bundled and try to unbundle.
https://github.com/anonbeat/guayadeque/issues/8#issuecomment-222497195
https://sourceforge.net/projects/wxcurl/


Subpackages for locales look nearly good (ignore the rpmlint warnings), except:
[!]: Package requires other packages for directories it uses.
     Note: No known owner of /usr/share/guayadeque,
     /usr/share/locale/sr_latin, /usr/share/locale/sr_latin/LC_MESSAGES
[!]: Package must own all directories that it creates.
     Note: Directories without known owners:
     /usr/share/locale/sr_latin/LC_MESSAGES, /usr/share/locale/sr_latin,
     /usr/share/guayadeque
=> Fix with:
%files
%dir %{_datadir}/%{name}

guayadeque-langpack-sr_latin.noarch: E: incorrect-locale-subdir /usr/share/locale/sr_latin/LC_MESSAGES/guayadeque.mo
guayadeque-langpack-sr_latin.noarch: E: invalid-lc-messages-dir /usr/share/locale/sr_latin/LC_MESSAGES/guayadeque.mo
=> sr_latin is not a valid locale identifier, change to sr@latin.
https://github.com/anonbeat/guayadeque/issues/6


(Sorry for the delay of my response, I'm quite busy currently.)

Comment 13 MartinKG 2016-06-04 10:50:02 UTC
(In reply to Raphael Groner from comment #12)
> 
> guayadeque-langpack-sr_latin.noarch: E: incorrect-locale-subdir
> /usr/share/locale/sr_latin/LC_MESSAGES/guayadeque.mo
> guayadeque-langpack-sr_latin.noarch: E: invalid-lc-messages-dir
> /usr/share/locale/sr_latin/LC_MESSAGES/guayadeque.mo
> => sr_latin is not a valid locale identifier, change to sr@latin.
> https://github.com/anonbeat/guayadeque/issues/6
> 

changed sr_latin to sr@latin in the spec file
...
%lang_subpkg sr@latin "Serbian (Latin)"
%lang_subpkg sv Swedish
%lang_subpkg th Thai
%lang_subpkg tr Turkish
%lang_subpkg uk Ukrainian

%prep
%setup -qn %{name}-%{commit0}
%patch0 -p0

#rm -rf src/curl src/wx/curl
mv po/sr-latin po/sr@latin
sed -i -e 's|ADD_SUBDIRECTORY( sr-latin )|ADD_SUBDIRECTORY( sr@latin )|' po/CMakeLists.txt
sed -i -e 's|sr_latin|sr@latin|' po/sr@latin/CMakeLists.txt
...

but @ was detected as Illegal character:

[martin@fc24 SPECS]$ rpmbuild -ba guayadeque.spec 
error: line 71: Illegal char '@' (0x40) in: %package        langpack-sr@latin

need more assistance !

Comment 14 Raphael Groner 2016-06-04 16:24:52 UTC
Maybe modify the lang_subpkg macro definition a little bit to include all variants into one subpackage for the same language code, mind the additional star character (*) in the last line.

%define         lang_subpkg() \
%package        langpack-%{1}\
Summary:        %{2} language data for %{name}\
BuildArch:      noarch \
Requires:       %{name} = %{version}-%{release}\
Supplements:    (%{name} = %{version}-%{release} and langpacks-%{1})\
\
%description    langpack-%{1}\
%{2} language data for %{name}.\
\
%files          langpack-%{1}\
%{_datadir}/locale/%{1}*/LC_MESSAGES/%{name}.mo

...
%lang_subpkg sr "Serbian (Cyrillic and Latin)"
%lang_subpkg sv Swedish
%lang_subpkg th Thai

https://www.gnu.org/software/gettext/manual/html_node/Locale-Names.html

Comment 15 MartinKG 2016-06-04 19:13:46 UTC
(In reply to Raphael Groner from comment #12)

> > [?]: Package contains no bundled libraries without FPC exception.
> > => Source files without license header, see above.
> 
> Licensecheck script fails to detect the header, we overlooked that in first
> run. SHOULD ask upstream why wxCurl is bundled and try to unbundle.
> https://github.com/anonbeat/guayadeque/issues/8#issuecomment-222497195
> https://sourceforge.net/projects/wxcurl/

the packaging of wxcurl is currently discussed on the FedoraForum:
http://forums.fedoraforum.org/showthread.php?t=310141

need more assistance !


> Subpackages for locales look nearly good (ignore the rpmlint warnings),
> except:
> [!]: Package requires other packages for directories it uses.
>      Note: No known owner of /usr/share/guayadeque,
>      /usr/share/locale/sr_latin, /usr/share/locale/sr_latin/LC_MESSAGES
> [!]: Package must own all directories that it creates.
>      Note: Directories without known owners:
>      /usr/share/locale/sr_latin/LC_MESSAGES, /usr/share/locale/sr_latin,
>      /usr/share/guayadeque
> => Fix with:
> %files
> %dir %{_datadir}/%{name}
> 
done

> guayadeque-langpack-sr_latin.noarch: E: incorrect-locale-subdir
> /usr/share/locale/sr_latin/LC_MESSAGES/guayadeque.mo
> guayadeque-langpack-sr_latin.noarch: E: invalid-lc-messages-dir
> /usr/share/locale/sr_latin/LC_MESSAGES/guayadeque.mo
> => sr_latin is not a valid locale identifier, change to sr@latin.
> https://github.com/anonbeat/guayadeque/issues/6
>
done

new rpm package:
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/guayadeque.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/guayadeque-0.4.1-0.6.beta1git79b6383.fc24.src.rpm

%changelog
* Sat Jun 04 2016 Martin Gansser <martinkg> - 0.4.1-0.6.beta1git79b6383
- Update to 0.4.1-0.6.beta1git79b6383
- Added wxWidgets to License tag
- Added %%dir %%{_datadir}/%%{name} because it's owned by the package
- modified macro for l10n subpackage

Comment 16 Raphael Groner 2016-06-05 08:52:15 UTC
Sorry but I still can not approve.

> [!]: If the package is under multiple licenses, the licensing breakdown
>      must be documented in the spec.

Please do so. That's MUST.
https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Multiple_Licensing_Scenarios

> [?]: Package contains no bundled libraries without FPC exception.
=> Report the (partly) bundled wxcurl to Packaging Committee and ask for help.
https://fedoraproject.org/wiki/Packaging:Guidelines#Bundling_and_Duplication_of_system_libraries
https://fedoraproject.org/wiki/Packaging_Committee

Comment 17 MartinKG 2016-06-05 16:37:01 UTC
(In reply to Raphael Groner from comment #16)
> Sorry but I still can not approve.
> 
> > [!]: If the package is under multiple licenses, the licensing breakdown
> >      must be documented in the spec.
> 
> Please do so. That's MUST.
> https://fedoraproject.org/wiki/Packaging:
> LicensingGuidelines#Multiple_Licensing_Scenarios
> 
is this ok for the licensing breakdown ?

SPEC file:
https://martinkg.fedorapeople.org/Packages/test/guayadeque.spec

due the bundling problem i will ask upstream and the Packaging Comittee.

Comment 18 MartinKG 2016-06-05 17:20:47 UTC
unbundle wxcurl library is discussed on the
Fedora Packaging list, Thread: partly bundled wxcurl in package guayadeque 

https://lists.fedoraproject.org/archives/list/packaging@lists.fedoraproject.org
/thread/JN34LTK5HVGEMCOJ45A4AADG2ADKAXCX/

Comment 19 Raphael Groner 2016-06-05 19:14:26 UTC
Thanks for the fixes and awareness about wxcurl bundling. APPROVED

Small correction for a typo in license breakdown: LGPv2+ should be LGPLv2+.

Comment 20 Dominik 'Rathann' Mierzejewski 2016-06-05 21:43:47 UTC
(In reply to MartinKG from comment #18)
> unbundle wxcurl library is discussed on the
> Fedora Packaging list, Thread: partly bundled wxcurl in package guayadeque 
> 
> https://lists.fedoraproject.org/archives/list/packaging@lists.fedoraproject.
> org
> /thread/JN34LTK5HVGEMCOJ45A4AADG2ADKAXCX/

Per current guidelines, it's not strictly necessary to unbundle anymore. However, I applaud your efforts raising the issue with upstream.

Note that you must add
Provides: bundled(wxcurl) = wxcurl_version
until bundling is resolved. Please also mention the github issue you opened in a comment above that line.

Comment 21 MartinKG 2016-06-06 07:42:26 UTC
@Raphael Thanks for the review.

new rpm package:
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/guayadeque.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/guayadeque-0.4.1-0.7.beta1git79b6383.fc24.src.rpm

%changelog
* Sun Jun 05 2016 Martin Gansser <martinkg> - 0.4.1-0.7.beta1git79b6383
- Documented licensing breakdown
- Added Provides: bundled(wxcurl) = wxcurl_version

Comment 22 MartinKG 2016-06-06 08:21:22 UTC
I have a problem with unretire the package
https://lists.fedoraproject.org/archives/list/packaging@lists.fedoraproject.org/thread/D5BNMGEGDFXAHXQAEB7AARBEKDMGBIYF/
any help ?

Comment 23 Raphael Groner 2016-06-06 09:19:03 UTC
Request that the Release Engineering team unblock the package for the releases that the package should be un-retired for via their trac instance. In this request, please post a link to the completed re-review and clearly specify which branches should be unblocked. 
https://fedorahosted.org/rel-eng/newticket

Comment 24 MartinKG 2016-06-09 19:34:43 UTC
package has been built successfully on fc24 and rawhide.


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