Bug 712579 - Review Request: disper - on-the-fly display switch utility
Review Request: disper - on-the-fly display switch utility
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
unspecified Severity medium
: ---
: ---
Assigned To: Matthias Runge
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-11 06:40 EDT by Mario Santagiuliana
Modified: 2012-01-26 17:57 EST (History)
7 users (show)

See Also:
Fixed In Version: disper-0.3.0-5.fc15
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-01-17 16:28:20 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mrunge: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Mario Santagiuliana 2011-06-11 06:40:00 EDT
Spec URL: http://marionline.fedorapeople.org/packages/SPECS/disper.spec
SRPM URL: http://marionline.fedorapeople.org/packages/SRPMS/disper-0.3.0-1.fc14.src.rpm
Description:
Disper is an on-the-fly display switch utility. It is intended to be used just before giving a presentation with a laptop, when all one wants is that the beamer, which has just been connected, is able to show whatever you prepared.

$ rpmlint rpmbuild/SPECS/disper.spec rpmbuild/RPMS/noarch/disper-0.3.0-1.fc14.noarch.rpm 
disper.noarch: W: spelling-error %description -l en_US beamer -> beamier, bearer, reamer
disper.noarch: W: spelling-error %description -l en_US heighest -> highest, heighten, height
disper.noarch: W: spelling-error %description -l en_US nVidia -> avidin
disper.noarch: W: spelling-error %description -l en_US backend -> backed, backbend, back end
1 packages and 1 specfiles checked; 0 errors, 4 warnings.

Additional info:
First disper packages for Fedora
http://fetzig.org/2009/07/07/nvidia-and-display-cloning/

Old review request in rpmfusion bugzilla
https://bugzilla.rpmfusion.org/show_bug.cgi?id=1486
Comment 1 Mario Santagiuliana 2011-06-21 14:31:30 EDT
Update dependencies. New packages provide. Please let me know if my spec file is wrong.
Comment 2 Didier 2011-06-24 03:14:54 EDT
Perhaps you should have a look at autorandr too : "Auto-detect the connect display hardware and load the appropiate X11 setup using xrandr or disper"

https://github.com/wertarbyte/autorandr
Comment 3 Mario Santagiuliana 2011-06-24 09:47:01 EDT
Thank you, I just create my own bash script to use disper quickly using short keys.
I need a reviewer now, could you help me?
Comment 4 Didier 2011-06-24 10:26:41 EDT
I wouldn't mind, but I'm not a reviewer ...
Thanks for the .spec, works flawlessly (F15 x86_64).
Comment 5 Aioanei Rares 2011-07-18 14:03:37 EDT
Your description section says "Disper gives you the option to either clone the all detected displays, or extend the desktop to them"; I'd say you either use "the detected displays" or "all detected displays" .

"For
cloning, the highest resolution supported by all displays devices is chosen;"
s/displays/display

"for extending every display device gets its heighest supported resolution."
It would be "highest" ,plus this section is a little unclear (at least to me).
Comment 6 Mario Santagiuliana 2011-07-19 07:42:57 EDT
I update the description using upstream website description:
http://willem.engen.nl/projects/disper/

New files:
SPEC: http://marionline.fedorapeople.org/packages/SPECS/disper.spec
SRPM: http://marionline.fedorapeople.org/packages/SRPMS/disper-0.3.0-3.fc14.src.rpm

Description:
Disper is an on-the-fly display switch utility. It is intended to be used just
before giving a presentation with a laptop, when all one wants is that the
beamer, which has just been connected, is able to show whatever you prepared.

Disper gives you the option to either clone all detected displays, or extend
the desktop to them. Resolutions are automatically detected. For cloning, the
highest common resolution supported by all displays is chosen; for extending
every display device gets its highest supported resolution. For special setups
requiring more detailed control, one can still use the standard display
configuration utilites.

At the moment nVidia cards are supported, and a basic XRandR backend is in
place.
Comment 7 Aioanei Rares 2011-07-19 10:18:07 EDT
Looks fine by me.
Comment 8 Mario Santagiuliana 2011-07-19 10:38:35 EDT
Thank you for your comment. If you are a reviewer can you change the fedora-review flag of this review request?
Comment 9 Aioanei Rares 2011-07-19 14:39:20 EDT
I am not a reviewer, I just do QA, but if you don't get any answers in a few days, try on the mailing list. Best of luck.
Comment 10 Mario Santagiuliana 2012-01-10 10:38:43 EST
I update the spec file to follow the package guidelines

SPEC: http://marionline.fedorapeople.org/packages/SPECS/disper.spec
SRPM:
http://marionline.fedorapeople.org/packages/SRPMS/disper-0.3.0-4.fc16.src.rpm
Comment 11 Dennis Appelon Nielsen 2012-01-11 06:36:06 EST
I have tried to give a shout-out for this to be pushed to testing
Comment 12 Mario Santagiuliana 2012-01-11 06:58:52 EST
Thank you Dennis for you comment.
For me the package works correctly. I need a reviewer to help me understand if the spec file is correct for Fedora repository.
Comment 13 Matthias Runge 2012-01-12 07:47:25 EST
disper.noarch: E: explicit-lib-dependency libX11
disper.noarch: E: explicit-lib-dependency libXrandr
disper.noarch: E: explicit-lib-dependency libnotify
Comment 14 Matthias Runge 2012-01-12 08:00:31 EST
To make my earlier post more clear:
[mrunge@mrungexp SPECS]$ rpmlint -i /home/mrunge/rpmbuild/RPMS/noarch/disper-0.3.0-4.fc16.noarch.rpm
disper.noarch: E: explicit-lib-dependency libX11
You must let rpm find the library dependencies by itself. Do not put unneeded
explicit Requires: tags.

disper.noarch: E: explicit-lib-dependency libXrandr
You must let rpm find the library dependencies by itself. Do not put unneeded
explicit Requires: tags.

disper.noarch: E: explicit-lib-dependency libnotify
You must let rpm find the library dependencies by itself. Do not put unneeded
explicit Requires: tags.

Just remove the Requires: -lines
Comment 15 Mario Santagiuliana 2012-01-12 08:45:00 EST
I know this but these three packages are required to disper to work and they don't require to build disper. For example disper script call notify-send that is include in libnotify package.
Comment 16 Matthias Runge 2012-01-13 02:36:32 EST
Then please add a comment to each requires:, why this is needed. 

Normally, rpmbuild should be smart enough to figure out requires to libs. notifiy-send is a binary included in libnotify-package. This should be an exception, I think. 

A general rule of thumb is, that rpmlint must be silent, when run over the package, especially it shouldn't complain on errors.
Comment 17 Mario Santagiuliana 2012-01-13 05:30:56 EST
Thank you Matthias, I  add comment in the spec file:
SPEC: http://marionline.fedorapeople.org/packages/SPECS/disper.spec
SRPM:
http://marionline.fedorapeople.org/packages/SRPMS/disper-0.3.0-5.fc16.src.rpm
Comment 18 Matthias Runge 2012-01-17 05:21:39 EST
Package Review
==============

Key:
- = N/A
x = Pass
! = Fail
? = Not evaluated



==== Generic ====
[x]: MUST Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: MUST Package successfully compiles and builds into binary rpms on at
     least one supported primary architecture.
[x]: MUST All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[x]: MUST Buildroot is not present
     Note: Unless packager wants to package for EPEL5 this is fine
[x]: MUST Package contains no bundled libraries.
[x]: MUST Changelog in prescribed format.
[x]: MUST Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
     Note: Clean would be needed if support for EPEL is required
[x]: MUST Sources contain only permissible code or content.
[x]: MUST Each %files section contains %defattr if rpm < 4.4
     Note: Note: defattr macros not found. They would be needed for EPEL5
[x]: MUST Macros in Summary, %description expandable at SRPM build time.
[x]: MUST Package requires other packages for directories it uses.
[x]: MUST Package uses nothing in %doc for runtime.
[x]: MUST Package is not known to require ExcludeArch.
[x]: MUST Permissions on files are set properly.
[x]: MUST Package does not contain duplicates in %files.
[x]: MUST Spec file lacks Packager, Vendor, PreReq tags.
[x]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf would be needed if support for EPEL5 is required
[x]: MUST If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %doc.
[x]: MUST License field in the package spec file matches the actual license.
[x]: MUST Package consistently uses macros (instead of hard-coded directory
     names).
[ ]: MUST Package meets the Packaging Guidelines.
[x]: MUST Package is named according to the Package Naming Guidelines.
[x]: MUST Package does not generates any conflict.
[x]: MUST Package obeys FHS, except libexecdir and /usr/target.
[x]: MUST Package must own all directories that it creates.
[x]: MUST Package does not own files or directories owned by other packages.
[x]: MUST Package installs properly.
[ ]: MUST Requires correct, justified where necessary.
[!]: MUST Rpmlint output is silent.

rpmlint disper-0.3.0-5.fc17.src.rpm

disper.src: W: spelling-error %description -l en_US beamer -> bearer, reamer, beater
disper.src: W: spelling-error %description -l en_US nVidia -> Dravidian
disper.src: W: spelling-error %description -l en_US backend -> backed, back end, back-end
1 packages and 0 specfiles checked; 0 errors, 3 warnings.


rpmlint disper-0.3.0-5.fc17.noarch.rpm

disper.noarch: E: explicit-lib-dependency libX11
disper.noarch: E: explicit-lib-dependency libXrandr
disper.noarch: E: explicit-lib-dependency libnotify
disper.noarch: W: spelling-error %description -l en_US beamer -> bearer, reamer, beater
disper.noarch: W: spelling-error %description -l en_US nVidia -> Dravidian
disper.noarch: W: spelling-error %description -l en_US backend -> backed, back end, back-end
1 packages and 0 specfiles checked; 3 errors, 3 warnings.


[x]: MUST Sources used to build the package match the upstream source, as
     provided in the spec URL.
/home/mrunge/712579/disper_0.3.0.tar.gz :
  MD5SUM this package     : aff6ca9266eecfc3f646c0de573eca91
  MD5SUM upstream package : aff6ca9266eecfc3f646c0de573eca91

[x]: MUST Spec file is legible and written in American English.
[x]: MUST Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[-]: MUST Package contains a SysV-style init script if in need of one.
[x]: MUST File names are valid UTF-8.
[x]: SHOULD Reviewer should test that the package builds in mock.
[x]: 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.
[x]: SHOULD Dist tag is present.
[x]: SHOULD No file requires outside of /etc, /bin, /sbin, /usr/bin,
     /usr/sbin.
[ ]: SHOULD Final provides and requires are sane (rpm -q --provides and rpm -q
     --requires).
[x]: SHOULD Package functions as described.
[x]: SHOULD Package does not include license text files separate from
     upstream.
[x]: SHOULD SourceX is a working URL.
[-]: SHOULD Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: SHOULD Package should compile and build into binary rpms on all supported
     architectures.
[-]: SHOULD %check is present and all tests pass.
[x]: SHOULD Packages should try to preserve timestamps of original installed
     files.
[x]: SHOULD Spec use %global instead of %define.

Issues:
[!]: MUST Rpmlint output is silent.

rpmlint disper-0.3.0-5.fc17.src.rpm

disper.src: W: spelling-error %description -l en_US beamer -> bearer, reamer, beater
disper.src: W: spelling-error %description -l en_US nVidia -> Dravidian
disper.src: W: spelling-error %description -l en_US backend -> backed, back end, back-end
1 packages and 0 specfiles checked; 0 errors, 3 warnings.


rpmlint disper-0.3.0-5.fc17.noarch.rpm

disper.noarch: E: explicit-lib-dependency libX11
disper.noarch: E: explicit-lib-dependency libXrandr
disper.noarch: E: explicit-lib-dependency libnotify
disper.noarch: W: spelling-error %description -l en_US beamer -> bearer, reamer, beater
disper.noarch: W: spelling-error %description -l en_US nVidia -> Dravidian
disper.noarch: W: spelling-error %description -l en_US backend -> backed, back end, back-end
1 packages and 0 specfiles checked; 3 errors, 3 warnings.


OK, I see those error. They may be false positives. I understand, it uses notify-send from libnotify.

[mrunge@mrungexp ~]$ rpm -ql libXrandr
/usr/lib64/libXrandr.so.2
/usr/lib64/libXrandr.so.2.2.0
/usr/share/doc/libXrandr-1.3.1
/usr/share/doc/libXrandr-1.3.1/AUTHORS
/usr/share/doc/libXrandr-1.3.1/COPYING
/usr/share/doc/libXrandr-1.3.1/ChangeLog

Which file from libXrandr is used?

Which one from libX11?
[mrunge@mrungexp ~]$ rpm -ql libX11
/usr/lib64/libX11-xcb.so.1
/usr/lib64/libX11-xcb.so.1.0.0
/usr/lib64/libX11.so.6
/usr/lib64/libX11.so.6.3.0

Could you please check this? 

I guess, binary notify-send from libnotify may be a false positive.

If those requires are corrected, I would approve this package.
Comment 19 Mario Santagiuliana 2012-01-17 05:39:41 EST
disper load libXrandr.so.2 and libX11.so.6 using python, from src/xrandr/core.py:

[...cut...]
 51
 52 xlib = cdll.LoadLibrary("libX11.so.6")
 53 rr = cdll.LoadLibrary("libXrandr.so.2")
 54
[...cut...]
Rpm doesn't show this loading...If I remove the two requires disper doesn't work properly. 

Thank you!
Comment 20 Mario Santagiuliana 2012-01-17 05:41:17 EST
Sorry for my English, rpm doesn't view this loading and doesn't include the requires packages automatically...
Comment 21 Matthias Runge 2012-01-17 05:43:36 EST
yeah, rpmbuild doesn't resolve this requirements proberly.

Package is finally

ACCEPTED
Comment 22 Mario Santagiuliana 2012-01-17 05:51:31 EST
Thank you Matthias for your review!
Comment 23 Mario Santagiuliana 2012-01-17 05:52:38 EST
New Package SCM Request
=======================
Package Name: disper
Short Description: on-the-fly display switch utility
Owners: marionline
Branches: f15 f16
InitialCC:
Comment 24 Gwyn Ciesla 2012-01-17 08:35:10 EST
Git done (by process-git-requests).
Comment 25 Fedora Update System 2012-01-17 16:30:09 EST
disper-0.3.0-5.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/disper-0.3.0-5.fc15
Comment 26 Fedora Update System 2012-01-17 16:33:05 EST
disper-0.3.0-5.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/disper-0.3.0-5.fc16
Comment 27 Fedora Update System 2012-01-26 17:53:49 EST
disper-0.3.0-5.fc16 has been pushed to the Fedora 16 stable repository.
Comment 28 Fedora Update System 2012-01-26 17:57:14 EST
disper-0.3.0-5.fc15 has been pushed to the Fedora 15 stable repository.

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