Bug 506581 - Review Request: xscope - X Window Protocol Viewer
Review Request: xscope - X Window Protocol Viewer
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Christian Krause
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-06-17 16:15 EDT by Yanko Kaneti
Modified: 2009-07-01 01:38 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-07-01 01:38:59 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
chkr: fedora‑review+
tibbs: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Yanko Kaneti 2009-06-17 16:15:35 EDT
Spec URL: http://www.declera.com/~yaneti/xscope/xscope.spec
SRPM URL: http://www.declera.com/~yaneti/xscope/xscope-1.1-1.gitfccbbd6.fc12.src.rpm
Description: xscope sits in-between an X11 client and an X11 server and prints the contents of each request, reply, error, or event that is communicated between them. This information can be useful in debugging and performance tuning of X11 servers and clients.
Comment 1 Christian Krause 2009-06-26 12:07:50 EDT
Hi,

I've reviewed the package and it looks quite good. There are only minor TODOs:
- clarification about the license (I've sent a mail to fedora-legal.)
- description formatting
- functional test
- if possible it would be great if the package would compile in F10, too...

Here is the detailed review:

* rpmlint:  OK
rpmlint SPECS/xscope.spec RPMS/i586/xscope-* SRPMS/xscope-1.1-1.gitfccbbd6.fc11.src.rpm
3 packages and 1 specfiles checked; 0 errors, 0 warnings.

* naming: OK
- name matches upstream
- spec file name matches package name
- snapshot release tag OK according:
http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Snapshot_packages

* License: TODO
- Although the meaning of the license seems to match the BSD license, I'm not 100% sure its a real BSD license since the wording is different. Just to confirm I've asked on the fedora-legal list for confirmation.
- COPYING file packaged

* specfile in American English and legible: OK

* %description: TODO (minor)
- I would reformat the last sentence in the %description section to use up the available space of 80 charaters.

* Sources: OK
- Source0 URL ok
- spectool -g xscope.spec works
- sources matches upstream - md5sum:
c37ec177b56d5909584c1672b6beabd5  xscope-1.1.tar.bz2

* Patch0: OK
- patch file can be regenerated by the supplied git diff command
- upstream status of the patch is obvious
fce6e9df7881061013f4acd43742585b  xscope-1.1-diff_to_git.patch

* Compilation: OK
- mock build works
- package builds correctly in koji for F12 and F11, but failed for F10
- RPMOPTFLAGS used
- parallel build supported via _smp_mflags

* debuginfo sub-package: OK
- non-empty
- debuginfo file works together with gdb

* BuildRequires: OK

* Locales handling: OK (n/a)

* shared/static libs, pkgconfig/header/*.la files: OK (n/a)

* packages must own all directories: OK

* files not listed twice: OK

* permissions of files: OK
- %defattr used
- final file permissions OK

* %clean section: OK

* macro usage: OK

* code vs. content: OK (only code)

* large documentation into subpackage: OK (n/a)

* GUI application needs %{name}.desktop: OK (n/a)

* no directories owned which are already owned by other packages: OK

* rm -rf %{buildroot} at the beginning of %{install}: OK

* all filenames UTF8: OK

* functional test: TODO
- running it on the same machine as the X server did not work well:
  - started "xscope -v1"
  - started "DISPLAY=:1 gedit" in another console
  - xscope displays some X11 protocol communication
  - but gedit doesn't start up completly, X server + xscope + gedit hangs, it is necessary to kill xscope by logging in via ssh...
- probably it is just wrong usage - any hints welcome ;-)


Best regards,
Christian
Comment 2 Yanko Kaneti 2009-06-27 06:41:07 EDT
(In reply to comment #1)
> I've reviewed the package and it looks quite good. There are only minor TODOs:
> - clarification about the license (I've sent a mail to fedora-legal.)
> * License: TODO
> - Although the meaning of the license seems to match the BSD license, I'm not
> 100% sure its a real BSD license since the wording is different. Just to
> confirm I've asked on the fedora-legal list for confirmation.

Thanks. I've only looked cursory on the licence because frankly licenses make me queasy. If its not really an acceptable BSD like license then so be it.


> - description formatting
> * %description: TODO (minor)
> - I would reformat the last sentence in the %description section to use up the
> available space of 80 charaters.

done that


> - functional test
> * functional test: TODO
> - running it on the same machine as the X server did not work well:
>   - started "xscope -v1"
>   - started "DISPLAY=:1 gedit" in another console
>   - xscope displays some X11 protocol communication
>   - but gedit doesn't start up completly, X server + xscope + gedit hangs, it
> is necessary to kill xscope by logging in via ssh...
> - probably it is just wrong usage - any hints welcome ;-)

Indeed the lockup you describe happens here too. In fact every time when the output of xscope itself is running over a terminal on the same server. Don't really know what to make of it. :/ . The program itslef is in a half baked state, and I am not sure it even understands the whole range of todays X server traffic but at least I've managed to succesfully use the package by redirecting the output to file or running vie another machine.  It was all an attempt to debug some X slownes over ssh links. I've got the needed capture, don't know enough of X to say if its useful or not.


> - package builds correctly in koji for F12 and F11, but failed for F10

I've added a patch to lower the xorg-macros requirement a bit, I don't think it has any detrimental effect on the F10 build. This fixes the build on F10.



* Sun Jun 27 2009 Yanko Kaneti <yaneti@declera.com> 1.1-2.gitfccbbd6
- Implement review feedback
- Patch to build on Fedora 10

SPEC: http://www.declera.com/~yaneti/xscope/xscope.spec
SRPM: http://www.declera.com/~yaneti/xscope/xscope-1.1-2.gitfccbbd6.fc12.src.rpm
Comment 3 Yanko Kaneti 2009-06-28 03:26:48 EDT
* Sun Jun 28 2009 Yanko Kaneti <yaneti@declera.com> 1.1-3.gitfccbbd6
- The software has a MIT not BSD license

SPEC: http://www.declera.com/~yaneti/xscope/xscope.spec
SRPM:
http://www.declera.com/~yaneti/xscope/xscope-1.1-3.gitfccbbd6.fc12.src.rpm

Sorry about that.
Comment 4 Christian Krause 2009-06-29 19:21:30 EDT
(In reply to comment #3)
> * Sun Jun 28 2009 Yanko Kaneti <yaneti@declera.com> 1.1-3.gitfccbbd6
> - The software has a MIT not BSD license
> 
> SPEC: http://www.declera.com/~yaneti/xscope/xscope.spec
> SRPM:
> http://www.declera.com/~yaneti/xscope/xscope-1.1-3.gitfccbbd6.fc12.src.rpm

Thank you very much for the update.

All mentioned issues were addressed:

* License: OK
- got clarification from the fedora-legal mailing list: https://www.redhat.com/archives/fedora-legal-list/2009-June/msg00045.html
- License is MIT according to the COPYING file
- License of source files matches actual license (I've roughly scanned over the sources...)
- MIT acceptable for Fedora
- COPYING file packaged

* %description: OK

* compilation: OK
- package builds now fine in F10

The small problem that it is not possible let the program put its output in an xterm running on the same X-server which is used to connect the clients I would not consider as a major problem. This tool is for debugging anyway and since e.g. logging into a file works fine on the same machine this issue will not hold back the approval.

-> APPROVED
Comment 5 Yanko Kaneti 2009-06-29 19:34:34 EDT
New Package CVS Request
=======================
Package Name: xscope
Short Description: X Window Protocol Viewer
Owners: yaneti
Branches: F-10 F-11
InitialCC:
Comment 6 Jason Tibbitts 2009-06-30 22:36:18 EDT
CVS done.
Comment 7 Yanko Kaneti 2009-07-01 01:38:59 EDT
Imported. Builds done. Bodhi updates pending.
Thanks again for the review.

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