Bug 494726 - Review Request: Gnote - Note Taking Application
Review Request: Gnote - Note Taking Application
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Christoph Wickert
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-04-07 17:51 EDT by Rahul Sundaram
Modified: 2013-03-13 01:44 EDT (History)
7 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-06-21 19:37:24 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
cwickert: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Rahul Sundaram 2009-04-07 17:51:42 EDT
Spec URL: http://sundaram.fedorapeople.org/packages/gnote.spec
SRPM URL: http://sundaram.fedorapeople.org/packages/gnote-0.1.1-1.fc10.src.rpm

Description: 

Gnote is a desktop note-taking application which is simple and easy to use.
It lets you organise your notes intelligently by allowing you to easily link
ideas together with Wiki style interconnects. It is a port of Tomboy to C++ 
and consumes less resources

rpmlint throws up:

gnote.i386: W: unstripped-binary-or-object /usr/bin/gnote
gnote.i386: W: non-conffile-in-etc /etc/gconf/schemas/gnote.schemas

First one, I welcome help in fixing it. Second warning should just be ignored. Do review the gconf schemas related post/pre scripts to make sure I got it right.
Comment 1 Matthias Clasen 2009-04-07 18:01:45 EDT
You seem to be missing a %pre script for the schemas, but %post/%postun look fine.
Comment 2 Rahul Sundaram 2009-04-07 18:37:54 EDT
Included the %pre section as per guidelines. The binary was not stripped
earlier because I hadn't installed redhat-rpm-config. A local issue resolved
thanks to the tip from hadess.

http://sundaram.fedorapeople.org/packages/gnote.spec
http://sundaram.fedorapeople.org/packages/gnote-0.1.1-2.fc10.src.rpm

This one should be good to go, now. Please review. Thanks
Comment 3 Christoph Wickert 2009-04-07 20:19:07 EDT
Wait a little, I'm going to review this now.
Comment 4 Christoph Wickert 2009-04-07 21:05:20 EDT
REVIEW for d7b9eb994e87725741b728a686490131  gnote-0.1.1-2.fc10.src.rpm


OK - MUST: rpmlint must be run on every package:
rpmlint /var/lib/mock/fedora-rawhide-i386/result/gnote-*
gnote.i386: W: non-conffile-in-etc /etc/gconf/schemas/gnote.schemas
3 packages and 0 specfiles checked; 0 errors, 1 warnings.

OK - MUST: The package is named according to the Package Naming Guidelines.
OK - MUST: The spec file name matches the base package %{name}, in the format %{name}.spec.
OK - MUST: The package meets the Packaging Guidelines.
OK - MUST: The package is licensed with a Fedora approved license and meets the Licensing Guidelines. (GPLv3+)
OK - MUST: The License field in the package spec file matches the actual license.
OK - MUST: The license file from the source package is included in %doc.
OK - MUST: The spec file is in American English.
OK - MUST: The spec file for the package is legible.
OK - MUST: The sources used to build the package match the upstream source by MD5 2a2578cc69df41bdb07c3d754c5593e6
OK - MUST: The package successfully compiles and builds into binary rpms on i386
N/A - MUST: If the package does not successfully compile, build or work on an architecture, then those architectures should be listed in the spec in ExcludeArch.
FAIL - MUST: Not all build dependencies are listed in BuildRequires: dbus-devel and desktop-file-utils are missing
N/A - MUST: The spec file handles locales properly with the %find_lang macro.
N/A - MUST: Every binary RPM package (or subpackage) which stores shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun.
N/A - MUST: If the package is designed to be relocatable, the packager must state this fact in the request for review, along with the rationalization for relocation of that specific package.
OK - MUST: The package owns all directories that it creates.
OK - MUST: The package does not contain any duplicate files in the %files listing.
OK - MUST: Permissions on files are set properly. Every %files section includes a %defattr(...) line.
OK - MUST: The package has a %clean section, which contains rm -rf $RPM_BUILD_ROOT.
OK - MUST: The package consistently uses macros, as described in the macros section of Packaging Guidelines.
OK - MUST: The package contains code, or permissable content.
N/A - MUST: Large documentation files should go in a -doc subpackage.
OK - MUST: Files included as %doc do not affect the runtime of the application.
N/A - MUST: Header files must be in a -devel package.
N/A - MUST: Static libraries must be in a -static package.
N/A - MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'.
N/A - 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.
N/A - MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release}
OK - MUST: The package does not contain any .la libtool archives.
OK - MUST: The package contains a GUI application and includes a %{name}.desktop file, and that file is properly installed with desktop-file-install in the %install section.
FAIL - MUST: The packages owns files or directories already owned by other packages: %{_datadir}/icons/hicolor/*/apps/
OK - MUST: At the beginning of %install, the package runs rm -rf $RPM_BUILD_ROOT.
OK - MUST: All filenames in rpm packages are valid UTF-8.


SHOULD Items:
N/A - 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.
N/A - SHOULD: The description and summary sections in the package spec file should contain translations for supported Non-English languages, if available.
FAIL - SHOULD: The the package does not build in mock due to the missing BuildRequires mentioned above.
OK - SHOULD: The package should compile and build into binary rpms on all supported architectures.
OK - SHOULD: The package functions as described.
OK - SHOULD: Scriptlets are used, those scriptlets must be sane.
N/A - SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency.
N/A - SHOULD: The placement of pkgconfig(.pc) files depends on their usecase, and this is usually for development purposes, so should be placed in a -devel pkg.
N/A - 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.

Issues:
- %files: you are owning all the apps folders inside %{_datadir}/icons/hicolor
- BuildRequires: missing dbus-devel and desktop-file-utils
- Requires: desktop-file-utils is not a requirement but a BuildReqiures
- Requires: gtkmm24 is not necessary since gnote already requires libgdkmm-2.4.so.1 which is provided by that package
- Gconf2 is missing for the scriptles:
   Requires(pre): GConf2
   Requires(post): GConf2
   Requires(preun): GConf2


Notes:
- BuildRequires: libxml2-devel is pretty redundant since it is already required by libxml++-devel GConf2-devel and a lot more.
- Consider using wildcards: %{_datadir}/icons/hicolor/*/apps/gnote.png
- Desktop file: remove --vendor=""
Comment 5 Christoph Wickert 2009-04-07 21:16:35 EDT
How about adding gnote to the autostart?

mkdir -p %{buildroot}/%{_sysconfdir}/autostart
desktop-file-install                          \
  --dir=%{buildroot}/%{_sysconfdir}/autostart \
  data/gnote.desktop
Comment 6 Christoph Wickert 2009-04-07 21:18:06 EDT
Oops, needs to be %{buildroot}/%{_sysconfdir}/xdg/autostart of course
Comment 7 Matthias Clasen 2009-04-07 23:11:40 EDT
I don't think installing the package should autostart the application. 
That might make sense in some cases, but this is not one of them...
Comment 8 Christoph Wickert 2009-04-08 03:11:17 EDT
Why not? The application resides in the tray normally and IMO tray apps should be autostarted. But I leave this up to Rahul.
Comment 9 Michael Schwendt 2009-04-08 04:58:03 EDT
> OK - MUST: The spec file is in American English.

The %description is British English. ;-)


> FAIL - MUST: Not all build dependencies are listed in
> BuildRequires: dbus-devel and desktop-file-utils are missing

This is the infamous Fedora pkg-config problem, where Requires.private in the pkgconfig file is evaluated and suffers from a missing dependency on dbus-devel (for dbus-1.pc). Normally, BR GConf2-devel would suffice.


--- gnote.spec.orig     2009-04-08 10:48:55.000000000 +0200
+++ gnote.spec  2009-04-08 10:50:09.000000000 +0200
@@ -24,7 +24,7 @@
 
 %build
 %configure --disable-schemas-install
-make %{?_smp_mflags}
+V=1 make %{?_smp_mflags}
 
 %install
 rm -rf $RPM_BUILD_ROOT
Comment 10 Rahul Sundaram 2009-04-08 08:08:49 EDT
I didn't do autostart but fixed rest of the issues. 

http://sundaram.fedorapeople.org/packages/gnote.spec
http://sundaram.fedorapeople.org/packages/gnote-0.1.1-3.fc10.src.rpm
Comment 11 Michael Schwendt 2009-04-08 09:47:33 EDT
Still contains "Requires: gtkmm24" (see bottom of comment 4) and:
https://fedoraproject.org/wiki/Packaging:Guidelines#Requires
Comment 12 Christoph Wickert 2009-04-08 10:11:25 EDT
(In reply to comment #9)
> This is the infamous Fedora pkg-config problem, where Requires.private in the
> pkgconfig file is evaluated and suffers from a missing dependency on dbus-devel
> (for dbus-1.pc). Normally, BR GConf2-devel would suffice.

I have seen the error message too. Thanks a lot for your explanation and the hint, Michael.

(In reply to comment #10)
> I didn't do autostart but fixed rest of the issues. 

Ok.

78d037b7591dc016546bc11e34afebd1  gnote-0.1.1-3.fc10.src.rpm
fixes the outstanding issues:
OK - %files
OK - BuildRequires
OK - Requires: desktop-file-utils is not a requirement but a BuildReqiures
OK - Gconf2 for the scriptles:
OK - redundant BuildRequires: libxml2-devel dropped
OK - Using wildcards: %{_datadir}/icons/hicolor/*/apps/gnote.png
OK - Desktop file: --vendor="" removed

Todo:
- Please drop "Requires: gtkmm24" and let rpm pick up the libs automatically.
- %description not ending with a dot.
- If you use "V=1 make %{?_smp_mflags}" you don't need "BuildRequires: dbus-devel.

These issues are minor and can be fixed after import. This package is

APPROVED
Comment 13 Michael Schwendt 2009-04-08 10:34:41 EDT
> - If you use "V=1 make %{?_smp_mflags}" you don't need
> "BuildRequires: dbus-devel.

Not true. V=1 enables verbose output for the shave build tool and has nothing to do with the configure script check for GConf2.
Comment 14 Christoph Wickert 2009-04-08 11:17:51 EDT
Ok, I tested it in mock and it worked without buildrequiring dbus-devel. Tested again without V=1 and dbus-devel is now pulled in automatically. I don't see a GConf2 build in koji, so I have no idea what has changed between yesterday and today.
Comment 15 Michael Schwendt 2009-04-08 12:47:21 EDT
1) You pass $V to "make" not "configure", so it _cannot_ make a difference.

2) Fedora 10:

$ pkg-config --exists --print-errors gconf-2.0 && echo "true"
true
$ sudo rpm -e dbus-devel
$ pkg-config --exists --print-errors gconf-2.0 && echo "true"
Package dbus-1 was not found in the pkg-config search path.
Perhaps you should add the directory containing `dbus-1.pc'
to the PKG_CONFIG_PATH environment variable
Package 'dbus-1', required by 'gconf', not found
$ grep Req /usr/lib/pkgconfig/gconf-2.0.pc 
Requires: glib-2.0
Requires.private: ORBit-2.0 dbus-1

3) Rawhide:

$ rpm -qR GConf2-devel|grep dbus
pkgconfig(dbus-1)
Comment 16 Christoph Wickert 2009-04-08 13:41:39 EDT
All of what you write is correct, but this still doesn't explain why two rawhide mockbuilds failed yesterday while it works today.
Comment 17 Michael Schwendt 2009-04-08 14:34:35 EDT
Can't follow you with that. I only commented on $V which is unrelated to dbus-devel (comment 13) or %configure failures.

Whether the package failed to build on Rawhide before, I don't know. Build logs would tell. I don't see any obvious reason why BR dbus-devel would be needed for Rawhide, since there are automatic pkgconfig(foo) dependencies in the packages.
Comment 18 Rahul Sundaram 2009-04-09 03:08:35 EDT
Thanks for the review. I will use scratch builds to confirm the BR and fix the other minor issues before importing. 

New Package CVS Request
=======================
Package Name: gnote
Short Description: Note Taking Application 
Owners: sundaram
Branches: F-10 
InitialCC:
Comment 19 Kevin Fenzi 2009-04-09 20:17:20 EDT
cvs done.
Comment 20 Fedora Update System 2009-04-09 22:30:23 EDT
gnote-0.1.1-5.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/gnote-0.1.1-5.fc10
Comment 21 Fedora Update System 2009-04-13 15:30:35 EDT
gnote-0.1.1-5.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update gnote'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-3499
Comment 22 Fedora Update System 2009-04-16 03:12:41 EDT
gnote-0.1.2-2.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/gnote-0.1.2-2.fc10
Comment 23 Fedora Update System 2009-04-17 14:01:22 EDT
gnote-0.1.2-2.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update gnote'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-3713
Comment 24 Fedora Update System 2009-04-23 08:34:24 EDT
gnote-0.2.0-1.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/gnote-0.2.0-1.fc10
Comment 25 Christoph Wickert 2009-06-21 19:37:24 EDT
Gnote 0.5 is in Fedora 10 and 11 but the update system did not catch this. Closing.

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