Bug 448201 - Review Request: pyvnc2swf - Vnc screen recorder
Summary: Review Request: pyvnc2swf - Vnc screen recorder
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Michel Lind
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-05-24 11:08 UTC by David Timms
Modified: 2008-10-01 06:43 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-09-28 00:41:52 UTC
Type: ---
Embargoed:
michel: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)
attempt at wrapper script (1.31 KB, text/plain)
2008-06-07 02:54 UTC, David Timms
no flags Details
diff between last .spec and this (2.66 KB, text/plain)
2008-07-15 23:14 UTC, David Timms
no flags Details
Updated spec fixing absolute symlinks (3.76 KB, text/plain)
2008-08-15 00:01 UTC, Paul Howarth
no flags Details

Description David Timms 2008-05-24 11:08:50 UTC
Spec URL: http://members.iinet.net.au/~timmsy/pyvnc2swf/pyvnc2swf.spec
SRPM URL: 
http://members.iinet.net.au/~timmsy/pyvnc2swf/pyvnc2swf-0.9.3-0.1.fc9.src.rpm
Description: 
Vnc2swf is a cross-platform screen recording tool creating ShockWave Flash
(swf) format media.

Comment 1 David Timms 2008-05-24 11:22:05 UTC
1. I'm not sure about, during rpmbuild:
+ desktop-file-install --vendor fedora --dir
/var/tmp/pyvnc2swf-0.9.3-0.1.fc9-root-davidt/usr/share/applications
/home/davidt/src/redhat/SOURCES/pyvnc2swf.desktop
+ /usr/lib/rpm/find-debuginfo.sh /home/davidt/src/redhat/BUILD/pyvnc2swf-0.9.3
find: debug: No such file or directory
+ /usr/lib/rpm/check-buildroot
  is that a problem, that I can do something about ?

2. The packages includes 2 other command line tools, but I haven't wrapped them
or anything else, and hence they aren't easily acceptable. I asked a while ago:
http://www.redhat.com/archives/rhl-devel-list/2008-May/msg00351.html
  and saw:
http://www.redhat.com/archives/rhl-devel-list/2006-December/msg00356.html
  but wondered if there is a clear guideline on the best way to handle the app
not being on he normal path ?

Comment 2 David Timms 2008-05-25 02:43:56 UTC
Spec URL: http://members.iinet.net.au/~timmsy/pyvnc2swf/pyvnc2swf.spec
SRPM URL: 
http://members.iinet.net.au/~timmsy/pyvnc2swf/pyvnc2swf-0.9.3-0.2.fc9.src.rpm

Updated package to fix missing BR: desktop-file-utils scratch build now succeeds.


Comment 3 David Timms 2008-05-25 02:45:30 UTC
(In reply to comment #1)
> or anything else, and hence they aren't easily acceptable. I asked a while ago:
                                                 accessible.

Comment 4 Michel Lind 2008-06-05 20:51:33 UTC
Requires: on python unnecessary (automatically picked up by RPM, in a stricter
form: python(abi) = 2.5

Missing Requires: on pygame (used in image.py and play.py, listed on website)

The debug lines are harmless. debuginfo packages are only automatically
generated for C/C++ applications.

In %changelog, you mean to say using desktop-file-install to install desktop
file, not to install an icon, right?

Looks like your other questions have been answered on fedora-devel, but do ask
if you have any other question. Awaiting updated SRPM.

Comment 5 David Timms 2008-06-07 02:54:17 UTC
Created attachment 308584 [details]
attempt at wrapper script

(In reply to comment #4)
> Requires: on python unnecessary (automatically picked up by RPM, in a
stricter
> form: python(abi) = 2.5
Thanks, done.

> Missing Requires: on pygame (used in image.py and play.py, listed on website)

Done. 
The optional pymedia would seem to use mpeg libraries, and hence isn't in
fedora, and can't be required.

> The debug lines are harmless. debuginfo packages are only automatically
> generated for C/C++ applications.
OK.

> In %changelog, you mean to say using desktop-file-install to install desktop
> file, not to install an icon, right?
Yes, I see what you mean and updated that changelog line.

> Looks like your other questions have been answered on fedora-devel, but do
ask
> if you have any other question.
For my purposes, the .desktop pointing to the site-packages path starts the gui
app fine. None of the 3x tool work from the command line:
    * vnc2swf.py - Recorder
    * edit.py - Movie editor (This is NOT a general SWF file editor. It only
supports movies generated by vnc2swf.)
    * play.py - Simple movie viewer 

Which method is considered most appropriate for making these tools easily
usable ?:
- bash script - exec python the tool path
- python script - to retrieve the correct 32/64 bit path, and invoke.
- adding *.pth to the site-packages directory
- moving these three .py scripts to /usr/bin/
I have attached my attempt, but this is unsuccessful; since the pymedia is not
found the wrapper bombs out with:
./vnc2swf
libpath=/usr/lib/python2.5/site-packages/pyvnc2swf
Traceback (most recent call last):
  File "./vnc2swf", line 35, in <module>
    execfile(get_python_lib() + "/pyvnc2swf/vnc2swf.py")
  File "/usr/lib/python2.5/site-packages/pyvnc2swf/vnc2swf.py", line 32, in
<module>
    from movie import SWFInfo
ImportError: No module named movie

Any ideas on how to ignore the import - and why when vnc2swf.py is called
directly, that it works, even without pymedia installed ?

Comment 6 David Timms 2008-06-13 11:19:48 UTC
(In reply to comment #5)
> Which method is considered most appropriate for making these tools easily
> usable ?:
> - bash script - exec python the tool path
I asked on fedora-list, and received a tip that I can use a bash script to
invoke python to get the site-lib directory, and then exec the required .py
within that path.

Updated: 
http://members.iinet.net.au/~timmsy/pyvnc2swf/pyvnc2swf.spec
http://members.iinet.net.au/~timmsy/pyvnc2swf/pyvnc2swf-0.9.3-1.fc9.src.rpm

That hopefully covers all the reviewer notes so far.

Comment 7 Michel Lind 2008-06-21 06:01:49 UTC
The Bash trick should really be unnecessary: your package is noarch, so even on
64-bit systems the files will be in /usr/lib/pythonX.Y/... , never
/usr/lib64/pythonX.Y/... . The invocation you're using actually returns
/usr/lib64 on 64-bit systems, which is not where your files end up installed.

See
http://fedoraproject.org/wiki/Packaging/Python:

It might be a good idea to generate the wrapper script during the packaging
process, though:

Put this at the top of the specfile:

%{!?python_sitelib: %define python_sitelib %(%{__python} -c "from
distutils.sysconfig import get_python_lib; print get_python_lib()")}

(note: not get_python_lib(1) which gives you the arch-specific directory)

%install
...
cat > $RPM_BUILD_ROOT%{_bindir}/vnc2swf-edit << EOF
exec python %{python_sitelib}/%{name}/edit.py "$@"
EOF

But do you really need to do this? edit.py works just fine if you symlink it.
Likewise with vnc2swf itself. I'd suggest just symlinking them all to /usr/bin.

Could not reproduce the ImportError you're getting -- movie comes from pyvnc2swf
so I'm not sure why you're getting that. Is that from *after* you install the
RPM or before?

Note that the recordwin script has, hardbaked into it, x11vnc and xwininfo.
x11vnc is part of libvncserver, but apparently it's not shipped in the Fedora
package at all. You might want to file a bug asking for it to be included, and
meanwhile, %exclude %{_bindir}/recordwin.sh from the %files section?

edit.py throws some exceptions if called without the right arguments; you might
want to fix the code path so that if any argument is missing, it exits after
displaying the error messages rather than attempting (futilely) to continue.



Comment 8 David Timms 2008-06-21 12:40:04 UTC
(In reply to comment #7)
> The Bash trick should really be unnecessary: your package is noarch, so even on
> 64-bit systems the files will be in /usr/lib/pythonX.Y/... , never
> /usr/lib64/pythonX.Y/... . The invocation you're using actually returns
OK, I reread the Packaging/Python info, and see where it's coming from: I
thought that %sitelib would be /usr/lib64 on x86_64, but as you mention this is
noarch.

> Put this at the top of the specfile:
> 
> %{!?python_sitelib: %define python_sitelib %(%{__python} -c "from
> distutils.sysconfig import get_python_lib; print get_python_lib()")}
Updated the sitelib assignment.

> But do you really need to do this? edit.py works just fine if you symlink it.
> Likewise with vnc2swf itself. I'd suggest just symlinking them all to /usr/bin.
OK. Without the need to determine the final arch specific location of the *.py,
yes, I can simply use a symlink, done for the 3x command line tools.

> Could not reproduce the ImportError you're getting -- movie comes from pyvnc2swf
> so I'm not sure why you're getting that. Is that from *after* you install the
> RPM or before?
Here is a test case:
1. vncserver :1
2. vnc2swf
3. options|server|localhost:1
4. start
5. stop
6. save as test.swf
7. exit
8. vnc2swf-play test.swf

result:
 vnc2swf-play ~/dev/pytest.swf 
Using pygame 1.7.1release
Input movie: version=5, size=1024x768, framerate=12fps, frames=187, duration=15.6s.
Output movie size: 1024x768
Scanning source swf file: /home/davidt/dev/pytest.swf...
Traceback (most recent call last):
  File "/usr/bin/vnc2swf-play", line 267, in <module>
    play(args, info, debug=debug)
  File "/usr/bin/vnc2swf-play", line 234, in play
    PygameMoviePlayer(movie, debug=debug).play()
  File "/usr/bin/vnc2swf-play", line 165, in play
    self.builder.start()
  File "/usr/lib/python2.5/site-packages/pyvnc2swf/output.py", line 942, in start
    self.stream.open()
  File "/usr/bin/vnc2swf-play", line 79, in open
    import pymedia
ImportError: No module named pymedia
===
Since fedora now has a flash player, I removed the play.py script and symlink.

> Note that the recordwin script has, hardbaked into it, x11vnc and xwininfo.
> x11vnc is part of libvncserver, but apparently it's not shipped in the Fedora
> package at all. You might want to file a bug asking for it to be included, and
> meanwhile, %exclude %{_bindir}/recordwin.sh from the %files section?
Yes, disabled recordwin for now. It looks to me like x11vnc is a separate part
of the libvncserver project:
http://sourceforge.net/project/showfiles.php?group_id=32584

> edit.py throws some exceptions if called without the right arguments; you might
> want to fix the code path so that if any argument is missing, it exits after
> displaying the error messages rather than attempting (futilely) to continue.
Created a patch that changes the return into a sys.exit. This seems to work well
to show the usage and exit at that point.

updated spec: http://members.iinet.net.au/~timmsy/pyvnc2swf/pyvnc2swf.spec
src.rpm: 
http://members.iinet.net.au/~timmsy/pyvnc2swf/pyvnc2swf-0.9.3-2.fc9.src.rpm

Comment 9 Michel Lind 2008-07-15 14:36:19 UTC
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

MUST, needs work:
• rpmlint: using rm in %preun. See below
• license field accurate: from the headers, this should be GPLv2+ not GPLv2
• own all directories: need to Require: hicolor-icon-theme for app icon

SHOULD, needs work:
• scriplets are sane: using rm in %preun. Does not seem to be necessary, as
  the files removed are listed in the package manifest (rpm -ql) and thus
  are removed automatically

MUST, OK:

• package name
• spec file name
• package guideline-compliant
• license complies with guidelines
• license file not deleted
• spec in US English
• spec legible
• source matches upstream: MD5 e871d134420867438cadd1f6320072f0
• builds under >= 1 archs, others excluded: noarch, builds
• build dependencies complete: yes
• no dupes in %files
• permission
• %clean RPM_BUILD_ROOT
• macros used consistently
• Package contains code
• doc not runtime dependent
• desktop file uses desktop-file-install
• clean buildroot before install
• filenames UTF-8

SHOULD, OK:
• package build in mock on all architectures:
  http://koji.fedoraproject.org/koji/taskinfo?taskID=717372
• package functioned as described: yes
• require package not files
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.9 (GNU/Linux)

iEYEARECAAYFAkh8tc8ACgkQWt0J3fd+7ZAI5QCfRvPiBOLdA+1yRY6yfetRFpJ9
fh4AnjCA5F8CTGVQCyrJBKaGZzyoM+Ki
=jkzQ
-----END PGP SIGNATURE-----


Comment 10 David Timms 2008-07-15 23:14:04 UTC
Created attachment 311894 [details]
diff between last .spec and this

updated spec: http://members.iinet.net.au/~timmsy/pyvnc2swf/pyvnc2swf.spec
src.rpm: 
http://members.iinet.net.au/~timmsy/pyvnc2swf/pyvnc2swf-0.9.3-3.fc9.src.rpm

- del preun symbolic link removes - already done by files section
- add Requires hicolor-icon-theme, so that icon dir is owned correctly
- fix license tag to GPLv2+ since sources say: or later version
- fix some comments spelling

$ rpmlint SRPMS/pyvnc2swf-0.9.3-2.fc9.src.rpm 1 packages and 0 specfiles
checked; 0 errors, 0 warnings.

$ rpmlint --info RPMS/noarch/pyvnc2swf-0.9.3-3.fc9.noarch.rpm
pyvnc2swf.noarch: W: symlink-should-be-relative /usr/bin/vnc2swf
/usr/lib/python2.5/site-packages/pyvnc2swf/vnc2swf.py
Absolute symlinks are problematic eg. when working with chroot environments.

pyvnc2swf.noarch: W: symlink-should-be-relative /usr/bin/vnc2swf-edit
/usr/lib/python2.5/site-packages/pyvnc2swf/edit.py
Absolute symlinks are problematic eg. when working with chroot environments.

1 packages and 0 specfiles checked; 0 errors, 2 warnings.

Michel: I did a bit of a search on absolute symlinks, and tried to make such
using symlinks -cs:
  http://rpmlint.zarb.org/cgi-bin/trac.cgi/ticket/25
, but that didn't resolve the rpm warning. Do you know if this is considered
normal to ignore, or can you point to a way to resolve it ?

Comment 11 Paul Howarth 2008-08-15 00:01:26 UTC
Created attachment 314363 [details]
Updated spec fixing absolute symlinks

The symlinks program is failing to convert your absolute symlink to a relative one because, although your absolute symlink is correctly formed in the package, it's dangling within the buildroot. If you're intending to use the symlinks program, you need to generate your absolute symlink such that the target is also within the buildroot.

Comment 12 David Timms 2008-08-16 03:25:44 UTC
(In reply to comment #11)
> Created an attachment (id=314363) [details]
> Updated spec fixing absolute symlinks
Thanks Paul. I wondered if there is a simpler way to implement this so that rpmlint is happy, and spot gave another suggestion.

In fact, it was what I originally implemented, and caused the lint error that I thought meant the method should not be used. Changing to cd to the link destination dir, then ln to the source{target} {although I understand would make the same link}, means that rpmlint has no qualms with the method, without the need to use symlinks.

Updated: http://members.iinet.net.au/~timmsy/pyvnc2swf/pyvnc2swf.spec
SRPM: 
http://members.iinet.net.au/~timmsy/pyvnc2swf/pyvnc2swf-0.9.3-4.fc9.src.rpm

- fix rpmlint W: symlink-should-be-relative
- mod to use file exclude for the recordwin scripts

rpmlint --info /home/davidt/rpmbuild/SRPMS/pyvnc2swf-0.9.3-4.fc9.src.rpm /home/davidt/rpmbuild/RPMS/noarch/pyvnc2swf-0.9.3-4.fc9.noarch.rpm pyvnc2swf.spec
2 packages and 1 specfiles checked; 0 errors, 0 warnings.

Comment 13 Michel Lind 2008-08-25 06:29:41 UTC
Everything looks clear here; approving. Sorry for the delay!

APPROVED

Comment 14 David Timms 2008-09-04 23:05:48 UTC
New Package CVS Request
=======================
Package Name: pyvnc2swf
Short Description: Vnc screen recorder
Owners: dtimms
Branches: F-8 F-9 EL-5
InitialCC:

Comment 15 Kevin Fenzi 2008-09-05 16:42:39 UTC
cvs done.

Comment 16 Fedora Update System 2008-09-07 14:16:06 UTC
pyvnc2swf-0.9.3-4.fc8 has been submitted as an update for Fedora 8.
http://admin.fedoraproject.org/updates/pyvnc2swf-0.9.3-4.fc8

Comment 17 Fedora Update System 2008-09-07 14:16:11 UTC
pyvnc2swf-0.9.3-4.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/pyvnc2swf-0.9.3-4.fc9

Comment 18 Fedora Update System 2008-09-11 17:03:32 UTC
pyvnc2swf-0.9.3-4.fc8 has been pushed to the Fedora 8 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 pyvnc2swf'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F8/FEDORA-2008-7874

Comment 19 Fedora Update System 2008-09-11 17:19:09 UTC
pyvnc2swf-0.9.3-4.fc9 has been pushed to the Fedora 9 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 pyvnc2swf'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F9/FEDORA-2008-7983

Comment 20 Fedora Update System 2008-10-01 06:35:31 UTC
pyvnc2swf-0.9.3-4.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 21 Fedora Update System 2008-10-01 06:43:06 UTC
pyvnc2swf-0.9.3-4.fc8 has been pushed to the Fedora 8 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.