Bug 509310

rpmlint output:

rpmlint *.rpm
gpointing-device-settings.x86_64: W: non-conffile-in-etc /etc/gconf/schemas/gpointing-device-settings_gnome_settings_daemon.schemas
libgpds-devel.x86_64: W: no-documentation
5 packages and 0 specfiles checked; 0 errors, 2 warnings.

Could you add proper Provides/Obsoletes for gsynaptics?

(In reply to comment #2)
> Could you add proper Provides/Obsoletes for gsynaptics?  

Yeah, of course I could, provided the current maintainer of gsynaptics agrees...

Additionally, there's some chances this could be also obsoleted by some work on gnome-mouse-properties itself:

http://bugzilla.gnome.org/show_bug.cgi?id=154029

http://bugzilla.gnome.org/show_bug.cgi?id=578444

(In reply to comment #3)
> (In reply to comment #2)
> > Could you add proper Provides/Obsoletes for gsynaptics?  
> 
> Yeah, of course I could, provided the current maintainer of gsynaptics
> agrees...  

I *was* one of the maintainers, but Andreas Thienemann and me decides to orphan it. CC'ing Peter Hutterer for feedback.


(In reply to comment #4)
> Additionally, there's some chances this could be also obsoleted by some work on
> gnome-mouse-properties itself:

Nevertheless I'd like to see this package in Fedora because it is much smaller and has less dependencies than contril-center. Useful other desktops but Gnome.

Happy to obsolete gsynaptics, in fact we were about to mark it dead anyway.

What gconf keys does gpointing-device-settings use for mouse and touchpad configuration? If they are different than the ones the control-center uses then we're in for conflicts with control-center.

Here is the schema file

<?xml version="1.0"?>
<gconfschemafile>
  <schemalist>

    <schema>
      <key>/schemas/apps/gnome_settings_daemon/plugins/pointing-device/active</key>
      <applyto>/apps/gnome_settings_daemon/plugins/pointing-device/active</applyto>
      <owner>gnome-settings-daemon</owner>
      <type>bool</type>
      <default>TRUE</default>
      <locale name="C">
        <short>Enable pointing-device plugin</short>
        <long>Set to True to enable the plugin to manage GPointingDeviceSettings settings.</long>
      </locale>
    </schema>
    <schema>
      <key>/schemas/apps/gnome_settings_daemon/plugins/pointing-device/priority</key>
      <applyto>/apps/gnome_settings_daemon/plugins/pointing-device/priority</applyto>
      <owner>gnome-settings-daemon</owner>
      <type>int</type>
      <default>7</default>
      <locale name="C">
        <short></short>
        <long></long>
      </locale>
    </schema>
  </schemalist>
</gconfschemafile>

sorry, does not look like the right stuff, digging further...

Before I post the full review, here are some issues I've found so far:

* sources: TODO
- please fix Source0 according to the example in the guidelines:
http://fedoraproject.org/wiki/Packaging:SourceURL
(you should also verify that spectool -g works correctly and retrieves the source file

* subpackage for libgpds: TODO
- the naming is not consistent:
packagename: libgpds
library name: libgpds (incl. SONAME)
include path for header files: /usr/include/gpointing-device-settings
pkgconfig file: libgpointing-device-settings.pc
library flags in the pkgconfig file: -lgpointing-device-settings
- at least the last problem must be fixed - otherwise the pkgconfig file can't be used to link anything against this library
- preferably also the include path and the pkgconfig file name could be fixed

* BuildRequires: TODO
- if there is no specific reason, only gettext (and not gettext-devel) should
be a BR

* package owns all directories that it creates: TODO
- files are copied into /usr/lib/gnome-settings-daemon-2.0, but it is neither owned by this package nor is the owner of this directory required
- I suggest to add: Requires: gnome-settings-daemon

* the corresponding gsynaptics configuration dialog has a menu item in System -> Preferences -> Touchpad
- IMHO the same should be done for gpointing-device-settings - please create an appropriate *.desktop file

OK, applied some changes (same SPEC URL)

Added Obsoletes/Provides gsynaptics (please double check those)

(In reply to comment #9)
> Before I post the full review, here are some issues I've found so far:
> 
> * sources: TODO
> - please fix Source0 according to the example in the guidelines:
> http://fedoraproject.org/wiki/Packaging:SourceURL
> (you should also verify that spectool -g works correctly and retrieves the
> source file

Fixed, but unfortunately I can't use the common example with sf.jp

> 
> * subpackage for libgpds: TODO
> - the naming is not consistent:
> packagename: libgpds
> library name: libgpds (incl. SONAME)
> include path for header files: /usr/include/gpointing-device-settings
> pkgconfig file: libgpointing-device-settings.pc
> library flags in the pkgconfig file: -lgpointing-device-settings
> - at least the last problem must be fixed - otherwise the pkgconfig file can't
> be used to link anything against this library
> - preferably also the include path and the pkgconfig file name could be fixed
> 
I am not sure how should I fix that. any pointers?


> * BuildRequires: TODO
> - if there is no specific reason, only gettext (and not gettext-devel) should
> be a BR

Fixed

> 
> * package owns all directories that it creates: TODO
> - files are copied into /usr/lib/gnome-settings-daemon-2.0, but it is neither
> owned by this package nor is the owner of this directory required
> - I suggest to add: Requires: gnome-settings-daemon

good catch: added

> 
> * the corresponding gsynaptics configuration dialog has a menu item in System
> -> Preferences -> Touchpad
> - IMHO the same should be done for gpointing-device-settings - please create an
> appropriate *.desktop file  

As far as I understand it, that is something like a test program I'd rather not expose to end users in the menu

Here is my full review of the package. In general it looks ok, but there are some TODO items left. Once there are fixed, please provide again the spec file as well as a source rpm.

* rpmlint: OK
rpmlint SPECS/gpointing-device-settings.spec SRPMS/gpointing-device-settings-1.3.1-2.fc11.src.rpm RPMS/i586/libgpds-* RPMS/i586/gpointing-device-settings-*
libgpds-devel.i586: W: no-documentation
gpointing-device-settings.i586: W: non-conffile-in-etc /etc/gconf/schemas/gpointing-device-settings_gnome_settings_daemon.schemas
5 packages and 1 specfiles checked; 0 errors, 2 warnings.

both warnings are false positive:
- no-documentation: nothing can be done here
- non-conffile-in-etc for schema files can be ignored, schema files are no
config files

* naming / sub-packaging: TODO
- name matches upstream
- spec file name matches package name
- I suggest to avoid the splitting of the daemon plugin/settings dialog and the library into separate packages, because of the following reasons:
a) the language files seem to contain the translations for both - it would be quite strange if e.g. the libgpds package should be installed stand-alone and there would be no translations available - the other way around (put the translations into the libgpds package) won't be much better
b) I don't think that the library will be used by other tools for now.

IMHO having just two packages:
gpointing-device-settings
and 
gpointing-device-settings-devel
would be better.

* sources: OK
- spectool -g works
- md5sum 2b0a567739fb565364cdca8dfc72545c  gpointing-device-settings-1.3.1.tar.gz
- sources matches upstream

* License: OK
- License LGPLv3+ acceptable
- License in spec file matches the actual license 
- License file packaged

* spec file written in English and legible: OK

* compilation: OK
- supports parallel build
- RPM_OPT_FLAGS are correctly used
- builds in mock (F11)
- builds in koji for F11 and F12

* BuildRequires: OK

* locales handling: OK

* ldconfig in %post and %postun: OK

* package owns all directories that it creates: OK

* no files listed twice in %files: OK

* file permissions: OK
- %defattr used
- actual permissions in packages ok

* %clean section: OK

* macro usage: OK

* code vs. content: OK (only code)

* large documentation into sub-package: OK (n/a)

* header files in -devel sub-package: OK

* static libraries in -static package: OK (n/a)

* package containing *.pc files must "Requires: pkgconfig": OK

* pkgconfig file: TODO
- The pkgconfig file contains the following linker flags:
Libs: -L${libdir} -lgpointing-device-settings
which won't work at all, since the library's name is "libgpds.so". Please
substitute the line with something like this:
Libs: -L${libdir} -lgpds

* *.so link in -devel package: OK

* - devel package requires base package using fully versioned dependency: OK

* packages must not contain *.la files: OK

* GUI applications must provide *.desktop file: TODO
I'm quite sure that the gpointing-device-settings is the regular configuration dialog for the user to setup his mouse/touchpad preferences. Similar as it was done in the gsynaptics package it must be available in the Preferences menu:
e.g. in System -> Preferences -> Pointing Device Settings
Please add the according .desktop file.

* packages must not own files/dirs already owned by other packages: OK

* rm -rf $RPM_BUILD_ROOT at the beginning of %install: OK

* all file names UTF-8: OK

* functional test: OK
- settings dialog basically works
- changing of a couple of settings has the desired effect

* debuginfo sub-package: OK
- non-empty

* Obsoletes: TODO
- Obsoletes: the version must be greater than the version of the current package, otherwise the current gsynaptics pacakge would not be obsoleted
Obsoletes: gsynaptics < 0.9.16-2

* Scriptlets: TODO
- gconf: please adjust the GConf scriptlets according to http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#GConf

*ping*

There was no response since over a month. Please reply if you are still interested in this package.

sorry, this fell off my radar. I'll try to upload an updated spec/srpm later today.

Spec URL: 
http://giallu.fedorapeople.org/gpointing-device-settings.spec
SRPM URL: 
http://giallu.fedorapeople.org/gpointing-device-settings-1.3.1-3.fc11.src.rpm


(In reply to comment #11)
> IMHO having just two packages:
> gpointing-device-settings
> and 
> gpointing-device-settings-devel
> would be better.

DONE

> * pkgconfig file: TODO
> - The pkgconfig file contains the following linker flags:
> Libs: -L${libdir} -lgpointing-device-settings
> which won't work at all, since the library's name is "libgpds.so". Please
> substitute the line with something like this:
> Libs: -L${libdir} -lgpds
DONE

> 
> * GUI applications must provide *.desktop file: TODO
> I'm quite sure that the gpointing-device-settings is the regular configuration
> dialog for the user to setup his mouse/touchpad preferences. Similar as it was
> done in the gsynaptics package it must be available in the Preferences menu:
> e.g. in System -> Preferences -> Pointing Device Settings
> Please add the according .desktop file.

DONE (.desktop file adapted from gsynaptic)

> 
> * Obsoletes: TODO
> - Obsoletes: the version must be greater than the version of the current
> package, otherwise the current gsynaptics pacakge would not be obsoleted
> Obsoletes: gsynaptics < 0.9.16-2
> 
DONE



> * Scriptlets: TODO
> - gconf: please adjust the GConf scriptlets according to
> http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#GConf  
DONE

Thanks for the new package. Please see my comments below:

(In reply to comment #14)
> Spec URL: 
> http://giallu.fedorapeople.org/gpointing-device-settings.spec
> SRPM URL: 
> http://giallu.fedorapeople.org/gpointing-device-settings-1.3.1-3.fc11.src.rpm

Somehow the spec file differs from the spec file inside the src.rpm. I looks like that the standalone spec file contains some more fixes. Please make sure that both match in the next update. ;-)

> (In reply to comment #11)
> > IMHO having just two packages:
> > gpointing-device-settings
> > and 
> > gpointing-device-settings-devel
> > would be better.
> 
> DONE

Ok, good.

> > * pkgconfig file: TODO
> > - The pkgconfig file contains the following linker flags:
> > Libs: -L${libdir} -lgpointing-device-settings
> > which won't work at all, since the library's name is "libgpds.so". Please
> > substitute the line with something like this:
> > Libs: -L${libdir} -lgpds
> DONE

Ok.

> > * GUI applications must provide *.desktop file: TODO
> > I'm quite sure that the gpointing-device-settings is the regular configuration
> > dialog for the user to setup his mouse/touchpad preferences. Similar as it was
> > done in the gsynaptics package it must be available in the Preferences menu:
> > e.g. in System -> Preferences -> Pointing Device Settings
> > Please add the according .desktop file.
> 
> DONE (.desktop file adapted from gsynaptic)

Unfortunately there is an icon referenced ("touchpad") which was only provided by the gsynaptic package and so it is missing in the menu entry. Probably we can use either another icon provided by one of the base packages or we can add the icon from the gsynaptic package as a separate source to this package.

Additionally desktop-file-validate complains about some problems:
desktop-file-validate /usr/share/applications/gpointing-device-settings.desktop
/usr/share/applications/gpointing-device-settings.desktop: warning: key "Encoding" in group "Desktop Entry" is deprecated
/usr/share/applications/gpointing-device-settings.desktop: warning: value "GNOME;Application;Settings;" for key "Categories" in group "Desktop Entry" contains a deprecated value "Application"


> > * Obsoletes: TODO
> > - Obsoletes: the version must be greater than the version of the current
> > package, otherwise the current gsynaptics pacakge would not be obsoleted
> > Obsoletes: gsynaptics < 0.9.16-2
> > 
> DONE

You have used:
Obsoletes:      gsynaptics <= 0.9.17
IMHO this should be:
Obsoletes:      gsynaptics < 0.9.17
( http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Renaming.2Freplacing_existing_packages )

> > * Scriptlets: TODO
> > - gconf: please adjust the GConf scriptlets according to
> > http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#GConf  
> DONE  

OK.

The package looks now quite good and there are only some very minor issues left.

(In reply to comment #15)
> Somehow the spec file differs from the spec file inside the src.rpm. I looks
> like that the standalone spec file contains some more fixes. Please make sure
> that both match in the next update. ;-)

Yeah, I catched some more last minute issues and forgot to reupload the srpm.


> > 
> > DONE (.desktop file adapted from gsynaptic)
> 
> Unfortunately there is an icon referenced ("touchpad") which was only provided
> by the gsynaptic package and so it is missing in the menu entry. Probably we
> can use either another icon provided by one of the base packages or we can add
> the icon from the gsynaptic package as a separate source to this package.
>

icon added from gsynaptics

> Additionally desktop-file-validate complains about some problems:
> desktop-file-validate /usr/share/applications/gpointing-device-settings.desktop
> /usr/share/applications/gpointing-device-settings.desktop: warning: key
> "Encoding" in group "Desktop Entry" is deprecated
> /usr/share/applications/gpointing-device-settings.desktop: warning: value
> "GNOME;Application;Settings;" for key "Categories" in group "Desktop Entry"
> contains a deprecated value "Application"
> 
> 

Both should be OK now.


> You have used:
> Obsoletes:      gsynaptics <= 0.9.17
> IMHO this should be:
> Obsoletes:      gsynaptics < 0.9.17
> (
> http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Renaming.2Freplacing_existing_packages

Well, that's just an example, both will work just fine. Anyway, I removed it.

http://giallu.fedorapeople.org/gpointing-device-settings.spec
http://giallu.fedorapeople.org/gpointing-device-settings-1.3.1-4.fc11.src.rpm

Thanks for the new package. All problems are solved now besides one minor issue:


(In reply to comment #16)
> > You have used:
> > Obsoletes:      gsynaptics <= 0.9.17
> > IMHO this should be:
> > Obsoletes:      gsynaptics < 0.9.17
> > (
> > http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Renaming.2Freplacing_existing_packages
> 
> Well, that's just an example, both will work just fine. Anyway, I removed it.

You've used now:

Obsoletes:      gsynaptics = 0.9.17

which won't work since it will not obsolete the currently installed versions named like 0.9.16.x . Looks like this was just a typo. Please can you use

Obsoletes:      gsynaptics < 0.9.17

Once this minor change is done, the package can be approved.

Doh! I removed the wrong char (staying up late didn't help...).

I can't upload now the fixed spec, but if you can approve the review anyway I'll surely import in CVS the fixed version.

FWIW, I recommend adding the following patch to the rpm before distribution. Otherwise users may start to rely on wrong gconf keys.

http://git.gnome.org/cgit/gpointing-device-settings/commit/?id=d39cf65fceb7abb4c5db241bb261f4e0f9eeb8a8

Thanks Peter! applied and fixed the obsolete line in:

http://giallu.fedorapeople.org/gpointing-device-settings.spec
http://giallu.fedorapeople.org/gpointing-device-settings-1.3.1-5.fc11.src.rpm

New package looks good. All reported issues were fixed.

Only two minor hints:
- please send Patch0 upstream
- please add for Patch0: and Patch1: a comment regarding the upstream status

-> APPROVED

Thanks!

Peter, would comaintain this with me? (I'll probably need a hand from you anyway...)

New Package CVS Request
=======================
Package Name: gpointing-device-settings
Short Description: Configuration tool for pointing devices
Owners: giallu
Branches: F-11
InitialCC:

sure, works for me, I'll probably end up doing some bug fixes anyway :)

Ok. I hope this does not add confusion to the CVS admins ;)

New Package CVS Request
=======================
Package Name: gpointing-device-settings
Short Description: Configuration tool for pointing devices
Owners: giallu whot
Branches: F-11
InitialCC:

cvs done with F-12 added.

gpointing-device-settings-1.3.1-5.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/gpointing-device-settings-1.3.1-5.fc11

gpointing-device-settings-1.3.1-5.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.