Bug 205912 - Review Request: Thunar - Thunar File Manager
Review Request: Thunar - Thunar File Manager
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Patrice Dumas
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-09-09 23:20 EDT by Kevin Fenzi
Modified: 2007-11-30 17:11 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-09-17 19:17:36 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Kevin Fenzi 2006-09-09 23:20:22 EDT
Spec URL: http://www.scrye.com/~kevin/extras/Thunar/Thunar.spec
SRPM URL: http://www.scrye.com/~kevin/extras/Thunar/Thunar-0.4.0-0.1.rc1.fc6.src.rpm
Description: 

Thunar is a new modern file manager for the Xfce Desktop Environment.
It has been designed from the ground up to be fast and easy-to-use.
Its user interface is clean and intuitive, and does not include any
confusing or useless options. Thunar is fast and responsive with a
good start up time and directory load time.

NOTE: Thunar will be the file manager in the next Xfce release, but it should compile and work with the existing stable version as well.
Comment 1 Patrice Dumas 2006-09-11 04:43:20 EDT
Some BR missing:

freetype-devel
libxslt or %{_bindir}/xsltproc

this shouldn't be used, but is safer (assorted with --enable-xml2po):
gnome-doc-utils or %{_bindir}/xml2po

A 
BuildRequires: pkgconfig
could also be there, although it should be allready needed 
by some devel dependencies.


The trash panel applet isn't built on devel, I think it is because
there is no libxfce4panel-1.0.pc providing version 4.3.90, but instead
xfce4-panel-1.0.pc with 4.2.x. This seems to be a dependency for the
xfce beta repo? At the same time it seems that the latest exo isn't
in the beta repo, so Thunar becomes a bit hard to test...

In the mean time, maybe it is better not to build  the tpa plugin, and
enable it only when xfce4-panel is updated.

There is also, for -devel, a missing
Requires: pkgconfig

in files
%{_sysconfdir}/xdg/Thunar
should be
%dir %{_sysconfdir}/xdg/Thunar

It is not completly obvious whether Gconf is usefull or not. If
it is not usefull anymore it should be removed from  
configure.ac and thunar-vfs/Makefile.am, other wise there
should certainly be a BuildRequires.

I think that it would be nice to have the gtk-doc documentation
in -devel with --enable-gtk-doc and
BuildRequires: gtk-doc

As a side note, maybe you could consider packaging 
xfce4-dev-tools
from xfce4 beta, even before xfce4 is out, since some autoconf 
macros used by Thunar are in this packages (and not in thunar).
Comment 2 Kevin Fenzi 2006-09-12 14:32:30 EDT
ok, added: 
BuildRequires: freetype-devel
BuildRequires: pkgconfig
BuildRequires: libxslt-devel
BuildRequires: GConf2-devel
BuildRequires: gtk-doc

I'll take a look at not building the trash panel applet. 

Fixed %{_sysconfdir}/xdg/Thunar to be %dir %{_sysconfdir}/xdg/Thunar

The beta/rc repo should now be updated to 4.4rc1 versions in both the fc5 and 
devel branches. 

I'll look at the xfce4-dev-tools, but I would expect them to fix things so 
thats not required for the final release, since that package is supposed to 
only be needed for developers. 

New files: 
Spec: http://www.scrye.com/~kevin/extras/Thunar/Thunar.spec
Srpm: http://www.scrye.com/~kevin/extras/Thunar/Thunar-0.4.0-0.2.rc1.fc6.src.rpm
Comment 3 Patrice Dumas 2006-09-12 16:21:28 EDT
(In reply to comment #2)
> ok, added: 
> BuildRequires: freetype-devel
> BuildRequires: pkgconfig
> BuildRequires: libxslt-devel

This seems wrong to me, since what is required is the xsltproc 
program, which is in libxslt.

> BuildRequires: GConf2-devel
> BuildRequires: gtk-doc
> 
> I'll take a look at not building the trash panel applet. 

It is automatically not built with the xfce-panel in -devel,
so it is allready not built, however it is in %files...

> I'll look at the xfce4-dev-tools, but I would expect them to fix things so 
> thats not required for the final release, since that package is supposed to 
> only be needed for developers. 

It is not required at all for this release. However I consider
it a good thing when all the packages required for developers
are there, even they are not needed at all (as buildrequires or 
requires) in the normal cases.

Indeed, it may be usefull in case somebody want to modify the
tarball itself, regenerate configure file, do tests, regenerate
documentation and so on. Not a blocker, just a suggestion.
Comment 4 Kevin Fenzi 2006-09-13 20:22:06 EDT
>> BuildRequires: libxslt-devel
>
>This seems wrong to me, since what is required is the xsltproc 
>program, which is in libxslt.

Oops. You are right. I will fix that. Only libxslt should be required there. 

>> I'll take a look at not building the trash panel applet. 
>
>It is automatically not built with the xfce-panel in -devel,
>so it is allready not built, however it is in %files...

Ah, I see the issue now. What do you suggest? This package is needed for the 
core Xfce 4.4 because xfdesktop needs to link to it to handle vfs/desktop 
issues. I guess I can disable the trash until it's approved? 
Or leave it in and see if any reviewers can build against the beta/rc1 repo? 

I'll see on xfce4-dev-tools... in any case I don't think that should hold up 
this review... 
Comment 5 Patrice Dumas 2006-09-14 05:28:58 EDT
(In reply to comment #4)

> Ah, I see the issue now. What do you suggest? This package is needed for the 
> core Xfce 4.4 because xfdesktop needs to link to it to handle vfs/desktop 
> issues. I guess I can disable the trash until it's approved? 

Are you sure? The files mising are
usr/libexec/xfce4/panel-plugins/thunar-tpa
and
/usr/share/xfce4/panel-plugins/thunar-tpa.desktop
Nothing can be linked against those files?

Anyway I think that the right thing to do for now is to remove the
tpa entries not found from %files.

> Or leave it in and see if any reviewers can build against the beta/rc1 repo? 

I'll try that (if you've updated exo in the beta repo ;-)
 
> I'll see on xfce4-dev-tools... in any case I don't think that should hold up 
> this review... 

It doesn't, it is just something to take care in the long term.
Comment 6 Patrice Dumas 2006-09-14 05:47:43 EDT
An unowned directories:
%{_datadir}/Thunar/

The files in %{_datadir}/doc/Thunar/ should better be directly in 
%docs
If you want to keep them in %{_datadir}/doc/Thunar/ they should
be flagged as %doc. (the %{_datadir}/doc/Thunar/html directory 
is right as is, it should not be in %doc, as it is used at runtime).

Not a blocker, but a suggestion, it seems to me that the man page 
are better with wildcards, such that they are picked even if 
not gzipped or bzip2ed..., so I propose:
%{_mandir}/man1/Thunar.1*
Comment 7 Patrice Dumas 2006-09-14 05:56:24 EDT
Also, I think it could be nice to have the examples packaged in 
-devel. For that I propose the following in %install:

make -C examples distclean

And the devel has a %doc:
%doc examples
Comment 8 Kevin Fenzi 2006-09-15 01:17:36 EDT
ok, thanks for the excellent feedback. 

I think I have addressed all your comments. 
I have left the tpa plugin/desktop removed/commented out. Comment in those 
lines if you build against the beta repo. 

Changelog: 

* Thu Sep 14 2006 Kevin Fenzi <kevin@tummy.com> - 0.4.0-0.3.rc1
- Cleaned up BuildRequires some more
- Disabled tpa plugin and desktop for now
- Moved some files from doc/Thunar to be %%doc
- Changed man to use wildcard in files
- Added examples to devel subpackage
- Made sure some examples are not executable.

Spec: http://www.scrye.com/~kevin/extras/Thunar/Thunar.spec
Srpm: http://www.scrye.com/~kevin/extras/Thunar/Thunar-0.4.0-0.3.rc1.fc6.src.rpm
Comment 9 Patrice Dumas 2006-09-15 04:32:15 EDT
We're getting near.

* There are dependencies for the devel package that are missing.
It may need more investigation, but looks like one need at least:

Requires: exo-devel >= 0.3.1.10

* doc files moved to %docs should be removed from %_datadir/doc/Thunar.

* some files are listed twice, becacuse of: 
%{_datadir}/Thunar/
%{_datadir}/Thunar/sendto/thunar-sendto-email.desktop

It should be
%dir %{_datadir}/Thunar/
%dir %{_datadir}/Thunar/sendto/
%{_datadir}/Thunar/sendto/thunar-sendto-email.desktop

or 
%{_datadir}/Thunar/  
Comment 10 Kevin Fenzi 2006-09-15 21:12:43 EDT
ok, new version: 

* Fri Sep 15 2006 Kevin Fenzi <kevin@tummy.com> - 0.4.0-0.4.rc1
- Added Requires: exo-devel >= 0.3.1.10 to devel. 
- exclude docs moved from datadir to docs
- Fixed datdir including files twice

Spec: http://www.scrye.com/~kevin/extras/Thunar/Thunar.spec
Srpm: http://www.scrye.com/~kevin/extras/Thunar/Thunar-0.4.0-0.4.rc1.fc6.src.rpm
Comment 11 Patrice Dumas 2006-09-16 06:58:17 EDT
There is still an issue, in %files
%{_datadir}/Thunar/sendto/thunar-sendto-email.desktop
is listed twice...

Otherwise
* rpmlint is silent
* free software licence, included
* named according to guidelines
* follow packaging guidelines
* specfile legible
* sane provides
Provides: config(Thunar) = 0.4.0-0.4.rc1 libthunar-vfs-1.so.2 libthunarx-1.so.2
thunar-apr.so thunar-sbr.so thunar-uca.so
* source match upstream
b07ae97a69964112e0af84ba2f63585d  Thunar-0.4.0rc1.tar.bz2
* libs packaged correctly
* docs not used at runtime
* desktop files present and following guidelines
* build and work in devel
* own all directories
* no file included twice
* %files right except for a typo

APPROVED
if you fix the file included twice.
Comment 12 Kevin Fenzi 2006-09-17 19:17:36 EDT
Thanks a lot for the review. 

I fixed the file included twice in the just imported version. 

Imported into devel and built. 

 17872 (Thunar): Build on target fedora-development-extras succeeded.
     Build logs may be found at http://buildsys.fedoraproject.org/logs/fedora-
development-extras/17872-Thunar-0.4.0-0.5.rc1.fc6/

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