Bug 570017

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> - 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).

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> - 0-0.2.20100223gitd3b530af3


Use %global instead of %define as per https://fedoraproject.org/wiki/Packaging/Guidelines#.25global_preferred_over_.25define


> 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

ping?

ping

Are you still interested in getting this package included in Fedora?

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.

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> - 0-0.6.20100304gitec01dd42
- %%{_kde4_docdir}/HTML/en/k4dirstat/ must be owned.
- use %%global instead of %%define

* Tue Jun  8 2010  <Laurent.Rineau__fedora> - 0-0.5.20100304gitec01dd42
- Fix the changelog (bad email and bad use of %%{?dist}).

* Tue Jun  8 2010  <Laurent.Rineau__fedora> - 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_.25define

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.

(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

(In reply to comment #6)
> * Tue Jun  8 2010  <Laurent.Rineau__fedora> - 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.

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

> 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";

Laurent, are you still interested in finishing this review?

ping?

Laurent, could you please respond?

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.

I could (and want to) open new review on k4dirstat, but I need sponsor (I try to get it in BR #640889).

Sure, go ahead. I'm closing this ticket so you can submit a new review request yourself.

Thanks guys. I am sorry I could not give more. My daily life is too busy to get time for Fedora, for the moment.

Opend here: https://bugzilla.redhat.com/show_bug.cgi?id=641690