Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: k4dirstat - KDE4 version of kdirstat
https://bugzilla.redhat.com/show_bug.cgi?id=570017
Summary: Review Request: k4dirstat - KDE4 version of kdirstat Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: laurent.rineau__fedora@normalesup.org QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora
Spec URL: http://www.rineau.fr/k4dirstat.spec SRPM URL: http://www.rineau.fr/k4dirstat-0-0.2.20100223gitd3b530af3.fc12.src.rpm Description: KDirStat (KDE Directory Statistics) is a utility program that sums up disk usage for directory trees - very much like the Unix 'du' command. It can also help you clean up used space.
K4DirStat is the port to KDE4.
--------------- For the moment k4dirstat is quite new. But it is usable. See http://grumpypenguin.org/index.php?/archives/3-KDirStat-coming-to-KDE4.html
rpmlint says: /home/lrineau/RPM/SPECS/k4dirstat.spec: W: invalid-url Source0: k4dirstat-20100223gitd3b530af3.tar.gz k4dirstat.src: W: spelling-error %description -l en_US du -> dew, doe, Du k4dirstat.src: W: invalid-url Source0: k4dirstat-20100223gitd3b530af3.tar.gz k4dirstat.i686: W: spelling-error %description -l en_US du -> dew, doe, Du 2 packages and 1 specfiles checked; 0 errors, 4 warnings.
As far as I understand, that is OK.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=570017
--- Comment #1 from Laurent Rineau laurent.rineau__fedora@normalesup.org 2010-03-06 09:25:51 EST --- New release. The upstream author had made changes, after my remarks.
Spec URL: http://www.rineau.fr/k4dirstat.spec SRPM URL: http://www.rineau.fr/k4dirstat-0-0.3.20100304gitec01dd42.fc12.src.rpm
* Sat Mar 6 2010 Laurent.Rineau__fedora@normalesup.org - 0-0.3.20100304gitec01dd42%{?dist} - New upstream version. - New doc files (added upstream). Among them: COPYING, README, AUTHORS. - Patch0 is merged upstream.
I have also made a mock build successfully. And verified that the package does not conflict with kdirstat (the version for kDE3).
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=570017
Kalev Lember kalev@smartlink.ee changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |kalev@smartlink.ee AssignedTo|nobody@fedoraproject.org |kalev@smartlink.ee Flag| |fedora-review?
--- Comment #2 from Kalev Lember kalev@smartlink.ee 2010-03-11 05:39:05 EST --- Taking for review.
Build for current rawhide fails with the following error: /usr/bin/ld: CMakeFiles/k4dirstat.dir/kdirtreecache.o: undefined reference to symbol 'gzclose' /usr/bin/ld: note: 'gzclose' is defined in DSO /lib/libz.so.1 so try adding it to the linker command line /lib/libz.so.1: could not read symbols: Invalid operation collect2: ld returned 1 exit status
This is caused by a change in the default linker behaviour, see http://fedoraproject.org/wiki/Features/ChangeInImplicitDSOLinking for details.
You might want to use globs in %files section to make the package easier to maintain. It's entirely up to you, tough, if you want to use globs in there. For example, %{_kde4_iconsdir}/hicolor/16x16/apps/k4dirstat.png %{_kde4_iconsdir}/hicolor/32x32/apps/k4dirstat.png %{_kde4_iconsdir}/hicolor/48x48/apps/k4dirstat.png
could be replaced by: %{_kde4_iconsdir}/hicolor/*/apps/k4dirstat.png
and %{_kde4_docdir}/HTML/en/k4dirstat/common %{_kde4_docdir}/HTML/en/k4dirstat/index.cache.bz2 %{_kde4_docdir}/HTML/en/k4dirstat/index.docbook
could be replaced by: %{_kde4_docdir}/HTML/en/k4dirstat/*
Fix %changelog: don't use %{?dist} macro in there, specify your name and use a valid email address. * Tue Mar 2 2010 Laurent.Rineau__fedora@normale - 0-0.2.20100223gitd3b530af3%{?dist} should be changed to: * Tue Mar 2 2010 Laurent Rineau laurent.rineau__fedora@normalesup.org - 0-0.2.20100223gitd3b530af3
Use %global instead of %define as per https://fedoraproject.org/wiki/Packaging/Guidelines#.25global_preferred_over...
Source0: %{name}-%{preversion}.tar.gz
You'll need to add a comment how to generate the git snapshot tarball. See https://fedoraproject.org/wiki/Packaging:SourceURL#Using_Revision_Control
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=570017
Kalev Lember kalev@smartlink.ee changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |needinfo?(laurent.rineau__f | |edora@normalesup.org)
--- Comment #3 from Kalev Lember kalev@smartlink.ee 2010-03-23 17:01:56 EDT --- ping?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=570017
--- Comment #4 from Kalev Lember kalev@smartlink.ee 2010-04-13 15:41:25 EDT --- ping
Are you still interested in getting this package included in Fedora?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=570017
Laurent Rineau laurent.rineau__fedora@normalesup.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|needinfo?(laurent.rineau__f | |edora@normalesup.org) |
--- Comment #5 from Laurent Rineau laurent.rineau__fedora@normalesup.org 2010-04-13 16:04:59 EDT --- Yes. But my daily job is not friendly with me for one month. Maybe I should postpone that package submission for a while.
What is more, I do not know how to deal with the new link behavior of F-13. I am not used to it.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=570017
--- Comment #6 from Laurent Rineau laurent.rineau__fedora@normalesup.org 2010-06-08 18:00:03 EDT --- Spec URL: http://www.rineau.fr/k4dirstat.spec SRPM URL: http://www.rineau.fr/k4dirstat-0-0.4.20100304gitec01dd42.fc13.src.rpm
Changelog: * Tue Jun 8 2010 Laurent.Rineau__fedora@normalesup.org - 0-0.6.20100304gitec01dd42 - %%{_kde4_docdir}/HTML/en/k4dirstat/ must be owned. - use %%global instead of %%define
* Tue Jun 8 2010 Laurent.Rineau__fedora@normalesup.org - 0-0.5.20100304gitec01dd42 - Fix the changelog (bad email and bad use of %%{?dist}).
* Tue Jun 8 2010 Laurent.Rineau__fedora@normalesup.org - 0-0.4.20100304gitec01dd42 - Patch0 for F-13: link explicitly with zlib. - Add a comment on Source0.
(In reply to comment #2)
Taking for review.
Build for current rawhide fails with the following error: /usr/bin/ld: CMakeFiles/k4dirstat.dir/kdirtreecache.o: undefined reference to symbol 'gzclose'
My machines now run F-13. I have been able to test the issue, and patched k4dirstat so that it links with zlib explicitly.
You might want to use globs in %files section to make the package easier to maintain. It's entirely up to you, tough, if you want to use globs in there. For example, %{_kde4_iconsdir}/hicolor/16x16/apps/k4dirstat.png %{_kde4_iconsdir}/hicolor/32x32/apps/k4dirstat.png %{_kde4_iconsdir}/hicolor/48x48/apps/k4dirstat.png
could be replaced by: %{_kde4_iconsdir}/hicolor/*/apps/k4dirstat.png
and %{_kde4_docdir}/HTML/en/k4dirstat/common %{_kde4_docdir}/HTML/en/k4dirstat/index.cache.bz2 %{_kde4_docdir}/HTML/en/k4dirstat/index.docbook
could be replaced by: %{_kde4_docdir}/HTML/en/k4dirstat/*
I prefer not to use globbing to much, to avoid surprises when I update the spec file with a new upstream version. Is it required by the guidelines to minimize the size of %file? Anyway, %{_kde4_docdir}/HTML/en/k4dirstat/ must be owned, and my previous %file did not own it. It is fixed now. As for the icons, I prefer to specify the files list.
Use %global instead of %define as per https://fedoraproject.org/wiki/Packaging/Guidelines#.25global_preferred_over...
In this context, it is the same. But I agree that %global should be used blindly, unless the specific semantic of %define is wanted. Fixed.
Source0: %{name}-%{preversion}.tar.gz
You'll need to add a comment how to generate the git snapshot tarball. See https://fedoraproject.org/wiki/Packaging:SourceURL#Using_Revision_Control
I have added such a comment. Strangely, the md5sum of the tarball has changed, but not its content. Maybe I used non-default compression level for gzip in March.
Note that the upstream Git repository has not evolved since April, and the used Git revision is the same.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=570017
--- Comment #7 from Laurent Rineau laurent.rineau__fedora@normalesup.org 2010-06-08 18:01:52 EDT --- (In reply to comment #6)
SRPM URL: http://www.rineau.fr/k4dirstat-0-0.4.20100304gitec01dd42.fc13.src.rpm
Ooops! The right one is this one:
SRPM URL: http://www.rineau.fr/k4dirstat-0-0.6.20100304gitec01dd42.fc13.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=570017
Kalev Lember kalev@smartlink.ee changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED
--- Comment #8 from Kalev Lember kalev@smartlink.ee 2010-06-15 21:25:56 EDT --- (In reply to comment #6)
- Tue Jun 8 2010 Laurent.Rineau__fedora@normalesup.org - 0-0.6.20100304gitec01dd42
Sorry for picking on the changelog format so much, but it's supposed to be * date name <email> - version You are missing the name.
- Patch0 for F-13: link explicitly with zlib.
Did you submit the patch for upstream inclusion too?
I prefer not to use globbing to much, to avoid surprises when I update the spec file with a new upstream version. Is it required by the guidelines to minimize the size of %file?
Certainly not, it's up to you to use globbing if you think it makes your life easier.
Anyway, %{_kde4_docdir}/HTML/en/k4dirstat/ must be owned, and my previous %file did not own it.
{_kde4_appsdir}/k4dirstat/ isn't owned either.
I have added such a comment. Strangely, the md5sum of the tarball has changed, but not its content. Maybe I used non-default compression level for gzip in March.
As long as there is a way to regenerate tarball's content, it's all good.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=570017
--- Comment #9 from Kalev Lember kalev@smartlink.ee 2010-06-15 21:54:05 EDT --- How did you determine this is GPLv2? The source files say either:
License: GPL - See file COPYING for details.
or
License: LGPL - See file COPYING.LIB for details.
According to http://fedoraproject.org/wiki/Licensing , it should be GPL+ if the sources or the accompanying documentation doesn't specify a version.
I'd suggest to ask the upstream author for clarification and suggest him to use proper GPL/LGPL header in every source file, as explained in http://www.gnu.org/licenses/old-licenses/gpl-2.0.html#SEC4
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=570017
--- Comment #10 from Kalev Lember kalev@smartlink.ee 2010-06-15 22:00:22 EDT ---
Version: 0
Why 0? In http://grumpypenguin.org/index.php?/archives/3-KDirStat-coming-to-KDE4.html, the author comments "As you can see in the about dialog, it currently is set at version 2.7.0 /.../"
Also, in main.cpp there's a line: static const char version[] = "2.7.0";
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=570017
--- Comment #11 from Kalev Lember kalev@smartlink.ee 2010-08-04 06:49:17 EDT --- Laurent, are you still interested in finishing this review?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=570017
--- Comment #12 from Kalev Lember kalev@smartlink.ee 2010-08-16 17:19:27 EDT --- ping?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=570017
Kalev Lember kalev@smartlink.ee changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |needinfo?(laurent.rineau__f | |edora@normalesup.org)
--- Comment #13 from Kalev Lember kalev@smartlink.ee 2010-08-28 09:58:18 EDT --- Laurent, could you please respond?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=570017
--- Comment #14 from Kalev Lember kalev@smartlink.ee 2010-09-25 17:03:38 EDT --- This review appears to be stalled. As per http://fedoraproject.org/wiki/Policy_for_stalled_package_reviews I am going to close this ticket in one week if the submitter doesn't respond.
The intention is to close the ticket so that the package can be submitted by someone else in a separate ticket.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=570017
Dmitrij S. Kryzhevich krege@land.ru changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |krege@land.ru
--- Comment #15 from Dmitrij S. Kryzhevich krege@land.ru 2010-10-10 03:09:34 EDT --- I could (and want to) open new review on k4dirstat, but I need sponsor (I try to get it in BR #640889).
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=570017
Kalev Lember kalev@smartlink.ee changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Blocks| |201449(FE-DEADREVIEW) Resolution| |NOTABUG Flag|fedora-review?, | |needinfo?(laurent.rineau__f | |edora@normalesup.org) | Last Closed| |2010-10-10 04:43:28
--- Comment #16 from Kalev Lember kalev@smartlink.ee 2010-10-10 04:43:28 EDT --- Sure, go ahead. I'm closing this ticket so you can submit a new review request yourself.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=570017
--- Comment #17 from Laurent Rineau laurent.rineau__fedora@normalesup.org 2010-10-10 05:36:03 EDT --- Thanks guys. I am sorry I could not give more. My daily life is too busy to get time for Fedora, for the moment.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=570017
--- Comment #18 from Dmitrij S. Kryzhevich krege@land.ru 2010-10-10 06:33:29 EDT --- Opend here: https://bugzilla.redhat.com/show_bug.cgi?id=641690
package-review@lists.fedoraproject.org