Bug 695058 - Review Request: transgui - An App to remotely control a Transmission Bit-Torrent client
Summary: Review Request: transgui - An App to remotely control a Transmission Bit-Torr...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Martin Gieseking
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-04-10 09:29 UTC by Praveen Kumar
Modified: 2012-03-08 04:55 UTC (History)
5 users (show)

Fixed In Version: transgui-3.2-5.fc16
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-03-08 03:57:07 UTC
Type: ---
martin.gieseking: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)
patch to enable the generation of debug information (14.18 KB, patch)
2011-11-17 18:41 UTC, Martin Gieseking
no flags Details | Diff

Description Praveen Kumar 2011-04-10 09:29:54 UTC
Spec URL: http://kumarpraveen.fedorapeople.org/transgui/transgui.spec
SRPM URL: http://kumarpraveen.fedorapeople.org/transgui/transgui-3.1-1.fc14.src.rpm
Description: Transmission Remote GUI is a feature rich cross platform front-end to remotely control a Transmission Bit-Torrent client daemon via its RPC protocol. It is faster and has more functionality than the built-in Transmission web interface.

rpmlint output:
#rpmlint ../SRPMS/transgui-3.1-1.fc14.src.rpm ../RPMS/i686/transgui-3.1-1.fc14.i686.rpm 
transgui.src: W: invalid-url Source0: transgui-3.1svn604.tar.gz
transgui.i686: W: no-manual-page-for-binary transgui
2 packages and 0 specfiles checked; 0 errors, 2 warnings.

Comment 1 Fabian Affolter 2011-04-11 17:05:24 UTC
Please see [1] because according to your spec file you are working with a snapshot.

[1] https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Release_Tag

Comment 2 Praveen Kumar 2011-04-11 18:36:39 UTC
Thanks for reminding, updated spec and srpm url :-
Spec URL: http://kumarpraveen.fedorapeople.org/transgui/transgui.spec
SRPM URL:
http://kumarpraveen.fedorapeople.org/transgui/transgui-3.1-2.20110410svn604.fc14.src.rpm

Comment 3 Fabian Affolter 2011-04-12 19:47:50 UTC
There is an empty debuginfo package warning. It's it possible to generate some debug info?

[fab@laptop023 x86_64]$ rpmlint transgui*
transgui.x86_64: W: no-manual-page-for-binary transgui
transgui-debuginfo.x86_64: E: empty-debuginfo-package
2 packages and 0 specfiles checked; 1 errors, 1 warnings.

Comment 4 Praveen Kumar 2011-04-13 02:51:15 UTC
(In reply to comment #3)
> There is an empty debuginfo package warning. It's it possible to generate some
> debug info?
> 
> [fab@laptop023 x86_64]$ rpmlint transgui*
> transgui.x86_64: W: no-manual-page-for-binary transgui
> transgui-debuginfo.x86_64: E: empty-debuginfo-package
> 2 packages and 0 specfiles checked; 1 errors, 1 warnings.

Sorry but I am not getting that type of error during rpmlint, here is output
#rpmlint transgui.spec ../SRPMS/transgui-3.1-2.20110410svn604.fc14.src.rpm ../RPMS/i686/transgui-3.1-2.20110410svn604.fc14.i686.rpm 
transgui.spec: W: invalid-url Source0: transgui-3.1svn604.tar.gz
transgui.src: W: invalid-url Source0: transgui-3.1svn604.tar.gz
transgui.i686: W: no-manual-page-for-binary transgui
2 packages and 1 specfiles checked; 0 errors, 3 warnings.

Comment 5 Ben Thompson 2011-10-27 23:35:37 UTC
Are you still looking at getting this reviewed?

From rpmlint all I'm getting is invalid-url warning for Source0 so that seems ok.

Also note that I does not build:

Compiling vargrid.pas
vargrid.pas(441,41) Error: Wrong number of parameters specified for call to "DrawGridCheckboxBitmaps"
vargrid.pas(540,47) Error: Wrong number of parameters specified for call to "DrawGridCheckboxBitmaps"
vargrid.pas(1002) Fatal: There were 2 errors compiling module, stopping
Fatal: Compilation aborted
make: *** [transgui] Error 1

Comment 6 Praveen Kumar 2011-10-28 02:04:07 UTC
(In reply to comment #5)
> Are you still looking at getting this reviewed?
Yes, it's good if package is included in fedora repo.

> From rpmlint all I'm getting is invalid-url warning for Source0 so that seems
> ok.
> 
> Also note that I does not build:
> 
> Compiling vargrid.pas
> vargrid.pas(441,41) Error: Wrong number of parameters specified for call to
> "DrawGridCheckboxBitmaps"
> vargrid.pas(540,47) Error: Wrong number of parameters specified for call to
> "DrawGridCheckboxBitmaps"
> vargrid.pas(1002) Fatal: There were 2 errors compiling module, stopping
> Fatal: Compilation aborted
> make: *** [transgui] Error 1
Sorry I am unaware of vargrid, but it's build successful using koji and local machine.

Comment 7 Ben Thompson 2011-10-28 11:57:02 UTC
At (https://code.google.com/p/transmisson-remote-gui/wiki/Building) the errors previously stated are mentioned. Advising that the svn snapshot may require Lazarus version 0.9.30.

Comment 8 Ben Thompson 2011-10-28 13:23:33 UTC
Tried with lazarus 0.9.28.2 & 0.9.30 and fpc 2.4.2 and no luck. Can you update to the latest revision: 638. That one compiles for me.

Comment 10 Ben Thompson 2011-10-29 13:06:19 UTC
Builds fine, thanks.

Though you have forgotten to update the version of the latest entry in your spec file's changelog section (3.1-3)

And rpmlint on the src rpm gives:
transgui.src: W: strange-permission transgui.spec 0600L
transgui.src: W: strange-permission transgui.desktop 0600L

Comment 11 Praveen Kumar 2011-10-29 14:23:40 UTC
Thanks for pointing out, done now, please take a look

SPEC : http://kumarpraveen.fedorapeople.org/transgui/transgui.spec
SRPM :
http://kumarpraveen.fedorapeople.org/transgui/transgui-3.1-3.20111028svn638.fc16.src.rpm

Comment 12 Ben Thompson 2011-10-29 17:18:16 UTC
I'd also contact upstream, and read the following advice about an executable stack vulnerability (transgui.i686: W: executable-stack /usr/bin/transgui):

https://fedoraproject.org/wiki/Packaging_tricks#Executable_stack

Comment 14 Ben Thompson 2011-10-30 13:51:47 UTC
Looks good thanks.

Comment 15 Ben Thompson 2011-10-30 15:42:44 UTC
Maybe you could also add the man page.

Comment 16 Rahul Sundaram 2011-10-31 02:11:51 UTC
When doing a review, especially initially, you could use a checklist. Review helper tool provides one partially.

Comment 17 Praveen Kumar 2011-11-01 02:21:42 UTC
(In reply to comment #15)
> Maybe you could also add the man page.
Source does't contain man pages so I am not able to include those.

Comment 18 Praveen Kumar 2011-11-04 10:51:58 UTC
Ping ? Is anybody want to take it and do further review (mostly done) and approve it.

Comment 19 Martin Gieseking 2011-11-12 07:53:35 UTC
Would you like to update the package to the latest upstream release? There's also a source tarball available now. 

Just two quick comments:
- Currently, the debuginfo package is empty. It's probably because of the fpc 
  flag -g- applied in the Makefiles. You should patch them appropriately. 

- Also, drop the Encoding entry from the .desktop file as it's deprecated.

Comment 20 Praveen Kumar 2011-11-13 06:11:55 UTC
(In reply to comment #19)
> Would you like to update the package to the latest upstream release? There's
> also a source tarball available now. 
done
> Just two quick comments:
> - Currently, the debuginfo package is empty. It's probably because of the fpc 
>   flag -g- applied in the Makefiles. You should patch them appropriately.
rpmlint is not throwing any error about empty debuginfo.
> - Also, drop the Encoding entry from the .desktop file as it's deprecated.
done


[daredevil@localhost SPECS]$ rpmlint -i transgui.spec ../SRPMS/transgui-3.2-4.fc16.src.rpm ../RPMS/i686/transgui-3.2-4.fc16.i686.rpm 
transgui.spec: W: invalid-url Source0: https://transmisson-remote-gui.googlecode.com/files/transgui-3.2-src.zip HTTP Error 404: Not Found
The value should be a valid, public HTTP, HTTPS, or FTP URL.

transgui.src: W: invalid-url Source0: https://transmisson-remote-gui.googlecode.com/files/transgui-3.2-src.zip HTTP Error 404: Not Found
The value should be a valid, public HTTP, HTTPS, or FTP URL.

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

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

Updated SPEC and SRPM
SPEC: http://kumarpraveen.fedorapeople.org/transgui/transgui.spec
SRPM: http://kumarpraveen.fedorapeople.org/transgui/transgui-3.2-4.fc16.src.rpm

Comment 21 Martin Gieseking 2011-11-13 19:44:35 UTC
The package looks almost fine. You should have reset the Release number to 1, though. ;)

- The debuginfo package is still empty (see rpmlint output below). You can 
  also check the one from this koji build:
  http://koji.fedoraproject.org/koji/taskinfo?taskID=3511426

- If you want to add a manpage to your package, you can use this one from 
  Ubuntu: http://manpages.ubuntu.com/manpages/natty/man1/transgui.1.html

- As you probably don't want to build the package for EPEL < 6, you can drop
  the %defattr line in %files. It's no longer required.


$ rpmlint ./transgui-*.rpm
transgui.src: W: invalid-url Source0: https://transmisson-remote-gui.googlecode.com/files/transgui-3.2-src.zip HTTP Error 404: Not Found
transgui.x86_64: W: no-manual-page-for-binary transgui
transgui-debuginfo.x86_64: E: empty-debuginfo-package
3 packages and 0 specfiles checked; 1 errors, 2 warnings.


---------------------------------
key:

[+] OK
[.] OK, not applicable
[X] needs work
---------------------------------

[+] MUST: The package must be named according to the Package Naming Guidelines.
[+] MUST: The spec file name must match the base package %{name}.
[+] MUST: The package must meet the Packaging Guidelines.
[+] MUST: The package must be licensed with a Fedora approved license.
[+] MUST: The License field in the package spec file must match the actual license.
[+] MUST: The file containing the text of the license(s) for the package must be included in %doc.
[+] MUST: The spec file must be written in American English.
[+] MUST: The spec file for the package MUST be legible.
[+] MUST: The sources used to build the package must match the upstream source.
    $ md5sum transgui-3.2-src.zip*
    39904b86e8772060758e0fae9215f777  transgui-3.2-src.zip
    39904b86e8772060758e0fae9215f777  transgui-3.2-src.zip.upstream

[+] MUST: The package MUST successfully compile and build into binary rpms on at least one primary architecture.
[.] MUST: If the package does not successfully compile, ...
[+] MUST: All build dependencies must be listed in BuildRequires.
[.] MUST: The spec file MUST handle locales properly.
[.] MUST: Packages storing shared library files (not just symlinks) must call ldconfig in %post and %postun.
[+] MUST: Packages must NOT bundle copies of system libraries.
[.] MUST: If the package is designed to be relocatable, ...
[+] MUST: A package must own all directories that it creates. 
[+] MUST: A Fedora package must not list a file more than once in %files.
[+] MUST: Permissions on files must be set properly.
[+] MUST: Each package must consistently use macros.
[+] MUST: The package must contain code, or permissable content.
[.] MUST: Large documentation files must go in a -doc subpackage.
[+] MUST: Files in %doc must not affect the runtime of the application.
[.] MUST: Header files must be in a -devel package.
[.] MUST: Static libraries must be in a -static package.
[.] MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1), then library files that end in .so (without suffix) must go in a -devel package.
[.] MUST: devel packages must require the base package using a fully versioned dependency.
[+] MUST: Packages must NOT contain any .la libtool archives.
[+] MUST: Packages containing GUI applications must include a %{name}.desktop file.
[+] MUST: .desktop files must be properly installed with desktop-file-install in the %install section. 
[+] MUST: Packages must not own files or directories already owned by other packages.
[+] MUST: All filenames in rpm packages must be valid UTF-8.

EPEL <= 5 only:
[X] MUST: The spec file must contain a valid BuildRoot field.
[X] MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot}.
[X] MUST: Each package must have a %clean section, which contains rm -rf %{buildroot}.
[.] MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'

[.] SHOULD: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it.
[+] SHOULD: The reviewer should test that the package builds in mock.
[+] SHOULD: The package should compile and build into binary rpms on all supported architectures.
[+] SHOULD: The reviewer should test that the package functions as described.
[.] SHOULD: If scriptlets are used, those scriptlets must be sane.
[.] SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency.
[.] SHOULD: pkgconfig(.pc) files should be placed in a -devel pkg.
[.] SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself.
[X] SHOULD: Your package should contain man pages for binaries/scripts.

Comment 22 Praveen Kumar 2011-11-16 16:22:47 UTC
Sent mail to upstream about debuginfo, waiting for reply.Will update spec soon according to review.

Comment 23 Praveen Kumar 2011-11-17 15:28:10 UTC
Added manual, disabled debuginfo package because it's not generate those debuginfo for which rpm is looking for.

SPEC : http://kumarpraveen.fedorapeople.org/transgui/transgui.spec
SRPM : http://kumarpraveen.fedorapeople.org/transgui/transgui-3.2-5.fc16.src.rpm

Comment 24 Martin Gieseking 2011-11-17 18:41:19 UTC
Created attachment 534288 [details]
patch to enable the generation of debug information

Comment 25 Martin Gieseking 2011-11-17 18:41:55 UTC
(In reply to comment #23)
> Added manual, disabled debuginfo package because it's not generate those
> debuginfo for which rpm is looking for.

Disabling the generation of the debuginfo package is not a good idea. You can easily enable the generation of the debug data by patching the Makefiles. Just replace the options "-g-" with "-g" (see attached patch).

Also, don't compress the manpage manually. Instead, add the uncompressed file with Source2, and replace %{_mandir}/man1/%{name}.1.gz with %{_mandir}/man1/%{name}.1* in %files. rpmbuild automatically chooses a compression format and applies it.

Comment 26 Praveen Kumar 2011-11-19 07:05:14 UTC
(In reply to comment #25)
> (In reply to comment #23)
> > Added manual, disabled debuginfo package because it's not generate those
> > debuginfo for which rpm is looking for.
> 
> Disabling the generation of the debuginfo package is not a good idea. You can
> easily enable the generation of the debug data by patching the Makefiles. Just
> replace the options "-g-" with "-g" (see attached patch).
I already tried that before disabling it from spec but it show error:
extracting debug info from /home/daredevil/rpmbuild/BUILDROOT/transgui-3.2-5.fc16.i386/usr/bin/transgui
Stabs debuginfo not supported: /home/daredevil/rpmbuild/BUILDROOT/transgui-3.2-5.fc16.i386/usr/bin/transgui

> 
> Also, don't compress the manpage manually. Instead, add the uncompressed file
> with Source2, and replace %{_mandir}/man1/%{name}.1.gz with
> %{_mandir}/man1/%{name}.1* in %files. rpmbuild automatically chooses a
> compression format and applies it.
done
SPEC : http://kumarpraveen.fedorapeople.org/transgui/transgui.spec
SRPM :
http://kumarpraveen.fedorapeople.org/transgui/transgui-3.2-5.fc16.src.rpm

Comment 27 Martin Gieseking 2011-11-19 12:47:29 UTC
(In reply to comment #26)
> I already tried that before disabling it from spec but it show error:
> extracting debug info from
> /home/daredevil/rpmbuild/BUILDROOT/transgui-3.2-5.fc16.i386/usr/bin/transgui
> Stabs debuginfo not supported:
> /home/daredevil/rpmbuild/BUILDROOT/transgui-3.2-5.fc16.i386/usr/bin/transgui

OK, sorry. I only built the package locally with mock on a x86_64 system. The generation of the debug information works fine there:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3525549

In contrast to x86_64, the i386 lazarus libraries (which are statically linked to transgui) contain stab sections and thus lead to the above error. This should probably be fixed on the lazarus side -- or rpmbuild should just ignore the stab sections if dwarf data is present as well. Until then, it's acceptable to disable the debuginfo package. Please add a short comment about this to the spec.

----------------
Package APPROVED
----------------

Comment 28 Praveen Kumar 2011-11-19 19:25:31 UTC
New Package SCM Request
=======================
Package Name: transgui
Short Description: An App to remotely control a Transmission Bit-Torrent client
Owners: kumarpraveen
Branches: f15 f16

Comment 29 Gwyn Ciesla 2011-11-19 20:19:54 UTC
Git done (by process-git-requests).

Comment 30 Fedora Update System 2011-11-20 16:21:34 UTC
transgui-3.2-5.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/transgui-3.2-5.fc16

Comment 31 Fedora Update System 2011-11-20 23:55:54 UTC
transgui-3.2-5.fc16 has been pushed to the Fedora 16 testing repository.

Comment 32 Fedora Update System 2012-03-08 03:57:07 UTC
transgui-3.2-5.fc16 has been pushed to the Fedora 16 stable repository.

Comment 33 Fedora Update System 2012-03-08 04:55:08 UTC
transgui-3.2-5.fc16 has been pushed to the Fedora 16 stable repository.


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