Bug 669911 - (guestfs-browser) Review Request: guestfs-browser - Guest filesystem browser
Review Request: guestfs-browser - Guest filesystem browser
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Ankur Sinha (FranciscoD)
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2011-01-15 13:30 EST by Richard W.M. Jones
Modified: 2011-08-22 10:57 EDT (History)
4 users (show)

See Also:
Fixed In Version: guestfs-browser-0.2.1-1.fc16.2
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2011-08-11 19:29:12 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
sanjay.ankur: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)
Fedora 15 x86_64 build log (13.05 KB, text/plain)
2011-05-16 19:21 EDT, Julian Sikorski
no flags Details

  None (edit)
Description Richard W.M. Jones 2011-01-15 13:30:58 EST
Spec URL: http://oirase.annexia.org/reviews/guestfs-browser/guestfs-browser.spec
SRPM URL: http://oirase.annexia.org/reviews/guestfs-browser/guestfs-browser-0.1.5-1.fc14.src.rpm
Description: Guest filesystem browser

The Guest Filesystem Browser lets you browse inside the filesystems of
virtual machines and disk images using a simple graphical interface.

This package delivers some of the features of libguestfs and the
guestfish scripting tool to users who don't want to use the command
line.

Upstream, screenshots etc:
http://people.redhat.com/~rjones/guestfs-browser/

NB: This will only build with ocaml-camomile >= 0.8.1 and that package
is only available on Fedora 15+.
Comment 1 Richard W.M. Jones 2011-01-15 13:32:08 EST
Koji scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2723472
Comment 2 Richard W.M. Jones 2011-01-15 13:33:18 EST
rpmlint says:

guestfs-browser.src: W: spelling-error Summary(en_US) filesystem -> file system, file-system, systematic
guestfs-browser.src: W: spelling-error %description -l en_US filesystems -> file systems, file-systems, ecosystems
guestfs-browser.src: W: spelling-error %description -l en_US libguestfs -> guestrooms, guesthouse, guestroom
guestfs-browser.src: W: spelling-error %description -l en_US guestfish -> guest fish, guest-fish, guesthouse
guestfs-browser.x86_64: W: spelling-error Summary(en_US) filesystem -> file system, file-system, systematic
guestfs-browser.x86_64: W: spelling-error %description -l en_US filesystems -> file systems, file-systems, ecosystems
guestfs-browser.x86_64: W: spelling-error %description -l en_US guestfish -> guest fish, guest-fish, guesthouse
guestfs-browser-debuginfo.x86_64: E: debuginfo-without-sources
3 packages and 0 specfiles checked; 1 errors, 7 warnings.

I think "debuginfo-without-sources" is a bug in rpmlint.  GDB
doesn't understand OCaml source.  However you can still use GDB
to get stack traces, so the debuginfo *is* useful for debugging
problems in OCaml native code (for OCaml bytecode you can use
OCaml's own debugger).
Comment 3 Julian Sikorski 2011-05-16 19:21:20 EDT
Created attachment 499242 [details]
Fedora 15 x86_64 build log

The package does not build in mock for me.
Comment 4 Richard W.M. Jones 2011-05-17 03:21:17 EDT
We changed the OCaml API a bit.  It needs this patch
at least.

http://git.annexia.org/?p=guestfs-browser.git;a=commitdiff;h=3b2348f5f4b770e3e7f886d851eeadc59bbd2357
Comment 6 Richard W.M. Jones 2011-05-17 05:07:05 EDT
Koji scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3076231
Comment 7 Richard W.M. Jones 2011-07-26 08:02:25 EDT
Updated Koji scratch build in dist-f16:

http://koji.fedoraproject.org/koji/taskinfo?taskID=3230787
Comment 8 Ankur Sinha (FranciscoD) 2011-07-26 11:14:24 EDT
Review:

+ OK
- NA
? ISSUE

+ Package meets naming and packaging guidelines
+ Spec file matches base package name.
+ Spec has consistant macro usage.
+ Meets Packaging Guidelines.
+ License
+ License field in spec matches
+ License file included in package
+ Spec in American English
+ Spec is legible.
+ Sources match upstream md5sum:
[ankur@ankur SOURCES]$ md5sum ~/rpmbuild/SPECS/guestfs-browser-0.1.6.tar.gz guestfs-browser-0.1.6.tar.gz 
c5fde7bc4f4a72d40f675a228332c6d5  /home/ankur/rpmbuild/SPECS/guestfs-browser-0.1.6.tar.gz
c5fde7bc4f4a72d40f675a228332c6d5  guestfs-browser-0.1.6.tar.gz


- Package needs ExcludeArch
+ BuildRequires correct
- Spec handles locales/find_lang
- Package is relocatable and has a reason to be.
- Package has %defattr and permissions on files is good.
^^ defattr is not needed after rpm 4.4 iirc,

- Package has a correct %clean section.
^^ The %clean section is not required for F-13 and above. 

- Package has correct buildroot
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
^^ Fedora (as of F-10) does not require the presence of the BuildRoot

+ Package is code or permissible content.
- Doc subpackage needed/used.
+ Packages %doc files don't affect runtime.

- Headers/static libs in -devel subpackage.
- Spec has needed ldconfig in post and postun
- .pc files in -devel subpackage/requires pkgconfig
- .so files in -devel subpackage.
- -devel package Requires: %{name} = %{version}-%{release}
- .la files are removed.

? Package is a GUI app and has a .desktop file
^^ please include a desktop file

+ Package compiles and builds on at least one arch.
+ Package has no duplicate files in %files.
+ Package doesn't own any directories other packages own.
+ Package owns all the directories it creates.
? No rpmlint output.
[ankur@ankur SRPMS]$ rpmlint /var/lib/mock/fedora-rawhide-i386/result/*.rpm ../SPECS/guestfs-browser.spec guestfs-browser-0.1.6-1.fc15.src.rpm
guestfs-browser.i686: W: spelling-error Summary(en_US) filesystem -> file system, file-system, systemically
guestfs-browser.i686: W: spelling-error %description -l en_US filesystems -> file systems, file-systems, systematizes
guestfs-browser.i686: W: spelling-error %description -l en_US guestfish -> guest fish, guest-fish, guesthouse
guestfs-browser.src: W: spelling-error Summary(en_US) filesystem -> file system, file-system, systemically
guestfs-browser.src: W: spelling-error %description -l en_US filesystems -> file systems, file-systems, systematizes
guestfs-browser.src: W: spelling-error %description -l en_US libguestfs -> guesthouses
guestfs-browser.src: W: spelling-error %description -l en_US guestfish -> guest fish, guest-fish, guesthouse
guestfs-browser-debuginfo.i686: E: debuginfo-without-sources
guestfs-browser.src: W: spelling-error Summary(en_US) filesystem -> file system, file-system, systemically
guestfs-browser.src: W: spelling-error %description -l en_US filesystems -> file systems, file-systems, systematizes
guestfs-browser.src: W: spelling-error %description -l en_US libguestfs -> guesthouses
guestfs-browser.src: W: spelling-error %description -l en_US guestfish -> guest fish, guest-fish, guesthouse
4 packages and 1 specfiles checked; 1 errors, 11 warnings.
[ankur@ankur SRPMS]$

^^ The debuginfo package needs to be corrected. 

SHOULD Items:

+ Should build in mock.
+ Should build on all supported archs
^^ koji scratch build is okay

- Should function as described.
- Should have sane scriptlets.
- Should have subpackages require base package with fully versioned depend.
+ Should have dist tag
+ Should package latest version
- check for outstanding bugs on package. (For core merge reviews)

+ requires/provides look sane:
== guestfs-browser-0.1.6-1.fc16.i686.rpm ==
Provides:
guestfs-browser = 0.1.6-1.fc16
guestfs-browser(x86-32) = 0.1.6-1.fc16

Requires:
/usr/bin/gnome-open
/usr/bin/hivexregedit
libatk-1.0.so.0
libc.so.6
libc.so.6(GLIBC_2.0)
libc.so.6(GLIBC_2.1)
libc.so.6(GLIBC_2.1.2)
libc.so.6(GLIBC_2.11)
libc.so.6(GLIBC_2.2)
libc.so.6(GLIBC_2.3)
libc.so.6(GLIBC_2.3.2)
libc.so.6(GLIBC_2.3.4)
libc.so.6(GLIBC_2.4)
libc.so.6(GLIBC_2.7)
libcairo.so.2
libdl.so.2
libdl.so.2(GLIBC_2.0)
libdl.so.2(GLIBC_2.1)
libfontconfig.so.1
libfreetype.so.6
libgcc_s.so.1
libgcc_s.so.1(GCC_3.0)
libgcc_s.so.1(GCC_3.3.1)
libgdk-x11-2.0.so.0
libgdk_pixbuf-2.0.so.0
libgio-2.0.so.0
libglib-2.0.so.0
libgmodule-2.0.so.0
libgobject-2.0.so.0
libgthread-2.0.so.0
libgtk-x11-2.0.so.0
libguestfs >= 1.9.11
libguestfs.so.0
libhivex.so.0
libm.so.6
libm.so.6(GLIBC_2.0)
libm.so.6(GLIBC_2.1)
libpango-1.0.so.0
libpangocairo-1.0.so.0
libpangoft2-1.0.so.0
libpng12.so.0
libpthread.so.0
libpthread.so.0(GLIBC_2.0)
libpthread.so.0(GLIBC_2.1)
libpthread.so.0(GLIBC_2.2)
libpthread.so.0(GLIBC_2.3.2)
librt.so.1
libvirt.so.0
libvirt.so.0(LIBVIRT_0.0.3)
libvirt.so.0(LIBVIRT_0.0.5)
libvirt.so.0(LIBVIRT_0.1.0)
libvirt.so.0(LIBVIRT_0.1.1)
libvirt.so.0(LIBVIRT_0.1.4)
libvirt.so.0(LIBVIRT_0.1.5)
libvirt.so.0(LIBVIRT_0.1.9)
libvirt.so.0(LIBVIRT_0.2.0)
libvirt.so.0(LIBVIRT_0.2.1)
libvirt.so.0(LIBVIRT_0.2.3)
libvirt.so.0(LIBVIRT_0.3.0)
libvirt.so.0(LIBVIRT_0.3.2)
libvirt.so.0(LIBVIRT_0.3.3)
libvirt.so.0(LIBVIRT_0.4.1)
libvirt.so.0(LIBVIRT_0.4.2)
rtld(GNU_HASH)

== guestfs-browser-0.1.6-1.fc16.src.rpm ==
Provides:

Requires:
hivex-devel >= 1.2.4-3
libguestfs-devel >= 1.9.11
libvirt-devel
ocaml
ocaml-bitstring-devel
ocaml-camlp4-devel
ocaml-camomile-devel >= 0.8.1
ocaml-camomile-data
ocaml-extlib-devel
ocaml-findlib-devel
ocaml-hivex-devel
ocaml-lablgtk-devel
ocaml-libvirt-devel
ocaml-libguestfs-devel
ocaml-xml-light-devel
/usr/bin/pod2man
/usr/bin/pod2html

== guestfs-browser-debuginfo-0.1.6-1.fc16.i686.rpm ==
Provides:
guestfs-browser-debuginfo = 0.1.6-1.fc16
guestfs-browser-debuginfo(x86-32) = 0.1.6-1.fc16

Requires:

--------------------------------------------------------------------

Issues:

1. The defattr, buildroot, clean sections can be removed. Please let them be if you intend to build for EPEL etc. as per their guidelines. 

2. Please include a desktop file. Note that you will have to include some scriptlets when you add it.

3. Please look at the issue with the debuginfo.


Thanks,
Ankur
Comment 9 Richard W.M. Jones 2011-07-26 12:40:17 EDT
Thanks for the review.

The updated package is here:

http://oirase.annexia.org/reviews/guestfs-browser/guestfs-browser.spec
http://oirase.annexia.org/reviews/guestfs-browser/guestfs-browser-0.1.6-2.fc15.src.rpm

I've corrected everything, except the debuginfo issue which
(per comment 0) I don't think is a problem.  There is no source,
but gdb cannot use OCaml source anyway, and we can still
generate backtraces using the debuginfo information so it
seems useful.
Comment 10 Ankur Sinha (FranciscoD) 2011-07-26 13:06:20 EDT
Looks good! Issues have been fixed.

BTW: You do not need to run both desktop-file-install and desktop-file-validate. In this case desktop-file-install is enough. 

XXX APPROVED XXX
Comment 11 Richard W.M. Jones 2011-07-26 13:23:40 EDT
Thanks, the documentation on desktop-file-{install,validate} was
pretty unclear ...  I will fix this when I put it into git.

New Package SCM Request
=======================
Package Name: guestfs-browser
Short Description: Guest filesystem browser
Owners: rjones
Branches: f15
InitialCC:
Comment 12 Jon Ciesla 2011-07-26 13:39:51 EDT
Git done (by process-git-requests).
Comment 13 Richard W.M. Jones 2011-07-26 15:30:09 EDT
Rawhide build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3232244
Comment 14 Fedora Update System 2011-07-27 05:41:37 EDT
guestfs-browser-0.1.6-3.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/guestfs-browser-0.1.6-3.fc15
Comment 15 Fedora Update System 2011-07-30 23:48:33 EDT
guestfs-browser-0.1.6-3.fc15 has been pushed to the Fedora 15 testing repository.
Comment 16 Fedora Update System 2011-08-01 18:18:18 EDT
guestfs-browser-0.2.1-1.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/guestfs-browser-0.2.1-1.fc15
Comment 17 Josh Stone 2011-08-02 14:50:52 EDT
(In reply to comment #0)
> NB: This will only build with ocaml-camomile >= 0.8.1 and that package
> is only available on Fedora 15+.

Should there be a final Requires for this too?  I get "Fatal error: exception Not_found" when I run, and strace says:
> open("/usr/share/camomile/database/general_category.mar", O_RDONLY) = -1 ENOENT (No such file or directory)
That file is provided by ocaml-camomile-data, but I don't happen to have any ocaml* packages at all on this system.
Comment 18 Richard W.M. Jones 2011-08-02 15:12:51 EDT
(In reply to comment #17)
> (In reply to comment #0)
> > NB: This will only build with ocaml-camomile >= 0.8.1 and that package
> > is only available on Fedora 15+.
> 
> Should there be a final Requires for this too?  I get "Fatal error: exception
> Not_found" when I run, and strace says:
> > open("/usr/share/camomile/database/general_category.mar", O_RDONLY) = -1 ENOENT (No such file or directory)
> That file is provided by ocaml-camomile-data, but I don't happen to have any
> ocaml* packages at all on this system.

Ah ha, yes that is the problem.

I'll add the required runtime deps ...
Comment 19 Fedora Update System 2011-08-02 17:02:45 EDT
guestfs-browser-0.2.1-1.fc16.2 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/guestfs-browser-0.2.1-1.fc16.2
Comment 20 Fedora Update System 2011-08-02 17:02:53 EDT
guestfs-browser-0.2.1-1.fc15.1 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/guestfs-browser-0.2.1-1.fc15.1
Comment 21 Fedora Update System 2011-08-11 19:29:05 EDT
guestfs-browser-0.2.1-1.fc15.1 has been pushed to the Fedora 15 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 22 Fedora Update System 2011-08-22 10:57:17 EDT
guestfs-browser-0.2.1-1.fc16.2 has been pushed to the Fedora 16 stable repository.  If problems still persist, please make note of it in this bug report.

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