This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 928609 - Review Request: xpra - screen for X [NEEDINFO]
Review Request: xpra - screen for X
Status: CLOSED WONTFIX
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
:
: 651591 (view as bug list)
Depends On: 953699 953701
Blocks: 990805 1198312 1243530
  Show dependency treegraph
 
Reported: 2013-03-28 00:20 EDT by T.C. Hollingsworth
Modified: 2015-07-15 12:48 EDT (History)
19 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
: 1198312 (view as bug list)
Environment:
Last Closed: 2015-03-03 14:18:42 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
jonathan.underwood: needinfo? (tchollingsworth)


Attachments (Terms of Use)

  None (edit)
Description T.C. Hollingsworth 2013-03-28 00:20:43 EDT
Spec: http://patches.fedorapeople.org/xpra/xpra.spec
SRPM: http://patches.fedorapeople.org/xpra/xpra-0.8.8-2.fc18.src.rpm
Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=5183160
FAS Username: patches
Description:
Xpra is "screen for X": it allows you to run X programs, usually on a remote
host, direct their display to your local machine, and then to disconnect from
these programs and reconnect from the same or another machine, without losing
any state. It gives you remote access to individual applications.

Xpra is "rootless" or "seamless": programs you run under it show up on your
desktop as regular programs, managed by your regular window manager.
Sessions can be accessed over SSH, or password protected over plain TCP
sockets.
Xpra is usable over reasonably slow links and does its best to adapt to
changing
network bandwidth constraints.

$ rpmlint SRPMS/xpra-0.8.8-2.fc18.src.rpm
RPMS/x86_64/xpra-0.8.8-2.fc18.x86_64.rpm
RPMS/x86_64/python-wimpiggy-0.8.8-2.fc18.x86_64.rpm
RPMS/x86_64/parti-0.8.8-2.fc18.x86_64.rpm 
xpra.src: W: summary-not-capitalized C screen for X
xpra.x86_64: W: summary-not-capitalized C screen for X

GNU screen is not generally capitalized.

xpra.x86_64: W: no-manual-page-for-binary xpra_Xdummy

This doesn't have a manpage since it's just a slightly modified version of the
xpra binary that uses the Xdummy driver instead.

python-wimpiggy.x86_64: W: spelling-error %description -l en_US compositing ->
composting, com positing, com-positing

False positive.

parti.x86_64: W: no-manual-page-for-binary parti-repl

This one could probably use a manpage, will look into this later.

4 packages and 0 specfiles checked; 0 errors, 5 warnings.
Comment 1 T.C. Hollingsworth 2013-03-28 00:28:55 EDT
*** Bug 651591 has been marked as a duplicate of this bug. ***
Comment 2 Dominik 'Rathann' Mierzejewski 2013-03-28 09:25:12 EDT
I think you can enable WebM/VPX. libvpx is available in Fedora.
Comment 3 T.C. Hollingsworth 2013-03-28 18:01:57 EDT
Unfortunately the vpx codec requires libswscale, which is part of ffmpeg. :-(
Comment 4 Karel Volný 2013-04-11 09:44:31 EDT
a few complaints from the fedora-review tool:

- install fails because of dependency on gstreamer-plugins-ugly, this belongs to the rpmfusion part
- gtk-update-icon-cache and update-desktop-database not called in post scripts

in addition, it reports more rpmlint problems not discussed above - strange permissions:

xpra.x86_64: E: non-standard-executable-perm /usr/lib64/python2.7/site-packages/xpra/wait_for_x_server.so 0775L
xpra.x86_64: E: non-standard-executable-perm /usr/lib64/python2.7/site-packages/xpra/xor/cyxor.so 0775L
xpra.x86_64: E: non-standard-executable-perm /usr/lib64/python2.7/site-packages/xpra/stats/cymaths.so 0775L
xpra.x86_64: E: non-standard-executable-perm /usr/lib64/python2.7/site-packages/xpra/rencode/_rencode.so 0775L
python-wimpiggy.x86_64: E: non-standard-executable-perm /usr/lib64/python2.7/site-packages/wimpiggy/lowlevel/bindings.so 0775L
python-wimpiggy.x86_64: E: non-standard-executable-perm /usr/lib64/python2.7/site-packages/wimpiggy/gdk/gdk_atoms.so 0775L

I believe these should be 755

also the list corresponds to the list of unversioned libraries:

xpra: /usr/lib64/python2.7/site-packages/xpra/rencode/_rencode.so
xpra: /usr/lib64/python2.7/site-packages/xpra/stats/cymaths.so
xpra: /usr/lib64/python2.7/site-packages/xpra/wait_for_x_server.so
xpra: /usr/lib64/python2.7/site-packages/xpra/xor/cyxor.so
python-wimpiggy: /usr/lib64/python2.7/site-packages/wimpiggy/gdk/gdk_atoms.so
python-wimpiggy: /usr/lib64/python2.7/site-packages/wimpiggy/lowlevel/bindings.so

I believe this fits under the exception mentioned in https://fedoraproject.org/wiki/Packaging:Guidelines#Devel_Packages

"As an additional complication, some software generates unversioned shared objects which are not intended to be used as system libraries. These files are usually plugins or modular functionality specific to an application, and are not located in the ld library paths or cache. This means that they are not located directly in /usr/lib or /usr/lib64, or in a directory listed as a library path in /etc/ld.so.conf (or an /etc/ld.so.conf.d/config file). Usually, these unversioned shared objects can be found in a dedicated subdirectory under /usr/lib or /usr/lib64 (e.g. /usr/lib/purple-2/ is the plugin directory used for libpurple applications). In these cases, the unversioned shared objects do not need to be placed in a -devel package."

... right?
Comment 5 T.C. Hollingsworth 2013-04-11 16:55:26 EDT
Thanks for the review!

(In reply to comment #4)
> a few complaints from the fedora-review tool:
> 
> - install fails because of dependency on gstreamer-plugins-ugly, this
> belongs to the rpmfusion part
> - gtk-update-icon-cache and update-desktop-database not called in post
> scripts

I fixed both of these.
 
> in addition, it reports more rpmlint problems not discussed above - strange
> permissions:
<snip list> 
> I believe these should be 755

I'm not sure why rpmlint doesn't complain about these on my machine, but I nonetheless ensured chmod was run on these files in the spec.

> also the list corresponds to the list of unversioned libraries:
<snip list>
> 
> I believe this fits under the exception mentioned in
> https://fedoraproject.org/wiki/Packaging:Guidelines#Devel_Packages
<snip quote>
> 
> ... right?

Yes.  These unversioned shared objects are C extensions to the Python interpreter, so thus fit the spirit and letter of the quoted exception.

--

Spec: http://patches.fedorapeople.org/xpra/xpra.spec
SRPM: http://patches.fedorapeople.org/xpra/xpra-0.8.8-3.fc18.src.rpm
Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=5243685

* Thu Apr 11 2013 T.C. Hollingsworth <tchollingsworth@gmail.com> - 0.8.8-3
- drop unmet dependency on gstreamer-plugins-ugly
- fix permissions on shared objects
- add scriptlets necessary for icon/desktop file
Comment 6 Karel Volný 2013-04-16 04:58:47 EDT
(In reply to comment #5)
> > - install fails because of dependency on gstreamer-plugins-ugly, this
> > belongs to the rpmfusion part
> > - gtk-update-icon-cache and update-desktop-database not called in post
> > scripts
> 
> I fixed both of these.

thanks

> > in addition, it reports more rpmlint problems not discussed above - strange
> > permissions:
> <snip list> 
> > I believe these should be 755
> 
> I'm not sure why rpmlint doesn't complain about these on my machine, but I
> nonetheless ensured chmod was run on these files in the spec.

probably it has something to do with the way mock sets up the environment ...

so, trying with the new version, stay tuned :-)
Comment 7 Karel Volný 2013-04-16 06:38:55 EDT
Package Review
==============

Key:
[x] = Pass
[!] = Fail
[-] = Not applicable
[?] = Not evaluated
[ ] = Manual review needed

"^" my notes
[+] reviewed, okay


Issues:
=======
- update-desktop-database is invoked when required
  Note: desktop file(s) in xpra
  See: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache

^ seems like a false positive - filed bug #952593


===== MUST items =====

C/C++:
[+]: Package does not contain kernel modules.
[+]: Package contains no static executables.
^ find rpms-unpacked/ | xargs file | grep static
[+]: Development (unversioned) .so files in -devel subpackage, if present.
     Note: Unversioned so-files in private %_libdir subdirectory (see
     attachment). Verify they are not in ld path.
^ see comments #4/#5
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.

Generic:
[+]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
^ GPLv2+
[+]: %build honors applicable compiler flags or justifies otherwise.
[!]: Package contains no bundled libraries without FPC exception.
^ there is rencode.pyx - http://code.google.com/p/rencode/
  and python-webm - http://code.google.com/p/python-webm/
  and a part of toonplayer - https://bitbucket.org/aalex/toonplayer
    gl-hello.py => gl_texture_bind_test.py
^ this influences the license breakdown, rencode and toonplayer are GPLv3+ and webm is BSD-like
[+]: Changelog in prescribed format.
[!]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf %{buildroot} present but not required
[+]: Sources contain only permissible code or content.
[-]: Development files must be in a -devel package
[+]: Package requires other packages for directories it uses.
[+]: Package uses nothing in %doc for runtime.
[+]: Package is not known to require ExcludeArch.
[!]: Fully versioned dependency in subpackages, if present.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in parti ,
     python-wimpiggy
^ python-wimpiggy is the base package, it doesn't need to depend on anything
  both xpra and parti depend od python-wimpiggy, I believe that {?_isa} is not mandatory in this case
  the problem is the "==" syntax, I cannot find this documented as valid, single "=" should be used
[!]: Package complies to the Packaging Guidelines
[!]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "BSD (2 clause)", "*No copyright* GPL (v3 or later)", "GPL (v3 or
     later)", "Unknown or generated". 4 files have unknown license. Detailed
     output of licensecheck in
     /home/kvolny/Projects/928609-xpra/licensecheck.txt
^ see bundled code discussion     
[+]: License file installed when any subpackage combination is installed.
^ it is in wimpiggy which is required by all
[+]: Package consistently uses macro is (instead of hard-coded directory
     names).
[+]: Package is named according to the Package Naming Guidelines.
[+]: Package does not generate any conflict.
[+]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[+]: Package must own all directories that it creates.
[+]: Package does not own files or directories owned by other packages.
[+]: Requires correct, justified where necessary.
[+]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[+]: gtk-update-icon-cache is invoked when required
     Note: icons in xpra
[+]: Useful -debuginfo package or justification otherwise.
[+]: Large documentation must go in a -doc subpackage.
     Note: Documentation size is 194560 bytes in 8 files.
^ note that the file NEWS is the same for all packages, could we have it symlinked?
[x]: All build dependencies are listed in BuildRequires, except for any that
     are listed in the exceptions section of Packaging Guidelines.
[x]: %config files are marked noreplace or the reason is justified.
[x]: Each %files section contains %defattr if rpm < 4.4
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package contains desktop file if it is a GUI application.
[x]: Package installs a %{name}.desktop using desktop-file-install if there is
     such a file.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Spec file lacks Packager, Vendor, PreReq tags.
[x]: 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]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: No %config files under /usr.
[x]: Package do not use a name that already exist
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as provided
     in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local
[x]: Package successfully compiles and builds into binary rpms on at least one
     supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).

Python:
[+]: Python eggs must not download any dependencies during the build process.
[+]: A package which is used by another package via an egg interface should
     provide egg info.
^ note that the egg-infos don't look 100% correct, e.g. parti-all has License: UNKNOWN
[+]: Package meets the Packaging Guidelines::Python
[x]: Package contains BR: python2-devel or python3-devel
[x]: Binary eggs must be removed in %prep

===== SHOULD items =====

Generic:
[-]: If the source package does not include license text(s) as a separate file
     from upstream, the packager SHOULD query upstream to include it.
[+]: Final provides and requires are sane (see attachments).
[+]: Package functions as described.
[+]: Latest version is packaged.
[+]: Package does not include license text files separate from upstream.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
^ no translations at all
[?]: Package should compile and build into binary rpms on all supported
     architectures.
[+]: %check is present and all tests pass.
[+]: Packages should try to preserve timestamps of original installed files.
[x]: Sources can be downloaded from URI in Source: tag
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: Dist tag is present.
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: SourceX tarball generation or download is documented.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define.

===== EXTRA items =====

Generic:
[x]: Large data in /usr/share should live in a noarch subpackage if package is
     arched.
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: xpra-0.8.8-3.fc18.x86_64.rpm
          parti-0.8.8-3.fc18.x86_64.rpm
          python-wimpiggy-0.8.8-3.fc18.x86_64.rpm
xpra.x86_64: W: summary-not-capitalized C screen for X
xpra.x86_64: W: no-manual-page-for-binary xpra_Xdummy
parti.x86_64: W: no-manual-page-for-binary parti-repl
python-wimpiggy.x86_64: W: spelling-error %description -l en_US compositing -> composting, com positing, com-positing
3 packages and 0 specfiles checked; 0 errors, 4 warnings.

^ see the initial comment


Rpmlint (installed packages)
----------------------------
# rpmlint parti xpra python-wimpiggy
parti.x86_64: W: no-manual-page-for-binary parti-repl
xpra.x86_64: W: summary-not-capitalized C screen for X
xpra.x86_64: W: no-manual-page-for-binary xpra_Xdummy
python-wimpiggy.x86_64: W: spelling-error %description -l en_US compositing -> composting, com positing, com-positing
3 packages and 0 specfiles checked; 0 errors, 4 warnings.
# echo 'rpmlint-done:'



Requires
--------
parti (rpmlib, GLIBC filtered):
    /usr/bin/python
    dbus-python
    python(abi)
    python-wimpiggy

xpra (rpmlib, GLIBC filtered):
    /bin/sh
    /usr/bin/python
    PyOpenGL
    config(xpra)
    dbus-python
    gstreamer
    gstreamer-plugins-base
    gstreamer-plugins-good
    gstreamer-python
    libX11.so.6()(64bit)
    libc.so.6()(64bit)
    libpthread.so.0()(64bit)
    libpython2.7.so.1.0()(64bit)
    numpy
    pulseaudio
    pulseaudio-utils
    pygtkglext
    python(abi)
    python-imaging
    python-numeric
    python-wimpiggy
    rtld(GNU_HASH)
    xorg-x11-drv-dummy
    xorg-x11-drv-void
    xorg-x11-server-Xvfb
    xorg-x11-server-utils

python-wimpiggy (rpmlib, GLIBC filtered):
    libXcomposite.so.1()(64bit)
    libXdamage.so.1()(64bit)
    libXfixes.so.3()(64bit)
    libXrandr.so.2()(64bit)
    libXtst.so.6()(64bit)
    libatk-1.0.so.0()(64bit)
    libc.so.6()(64bit)
    libcairo.so.2()(64bit)
    libfontconfig.so.1()(64bit)
    libfreetype.so.6()(64bit)
    libgdk-x11-2.0.so.0()(64bit)
    libgdk_pixbuf-2.0.so.0()(64bit)
    libgio-2.0.so.0()(64bit)
    libglib-2.0.so.0()(64bit)
    libgobject-2.0.so.0()(64bit)
    libgtk-x11-2.0.so.0()(64bit)
    libpango-1.0.so.0()(64bit)
    libpangocairo-1.0.so.0()(64bit)
    libpangoft2-1.0.so.0()(64bit)
    libpthread.so.0()(64bit)
    libpython2.7.so.1.0()(64bit)
    pycairo
    pygobject2
    pygtk2
    python(abi)
    rtld(GNU_HASH)



Provides
--------
parti:
    parti
    parti(x86-64)

xpra:
    config(xpra)
    mimehandler(text/x-xpraconfig)
    xpra
    xpra(x86-64)

python-wimpiggy:
    python-wimpiggy
    python-wimpiggy(x86-64)



Unversioned so-files
--------------------
xpra: /usr/lib64/python2.7/site-packages/xpra/rencode/_rencode.so
xpra: /usr/lib64/python2.7/site-packages/xpra/stats/cymaths.so
xpra: /usr/lib64/python2.7/site-packages/xpra/wait_for_x_server.so
xpra: /usr/lib64/python2.7/site-packages/xpra/xor/cyxor.so
python-wimpiggy: /usr/lib64/python2.7/site-packages/wimpiggy/gdk/gdk_atoms.so
python-wimpiggy: /usr/lib64/python2.7/site-packages/wimpiggy/lowlevel/bindings.so

^ see comments #4/#5

MD5-sum check
-------------
http://xpra.org/src/xpra-0.8.8.tar.xz :
  CHECKSUM(SHA256) this package     : 0edc22f1512d2633f2d52047393c1bd7153b55c3dd7505190ed373420116f4f0
  CHECKSUM(SHA256) upstream package : 0edc22f1512d2633f2d52047393c1bd7153b55c3dd7505190ed373420116f4f0


Generated by fedora-review 0.4.0 (660ce56) last change: 2013-01-29
Buildroot used: fedora-18-x86_64
Command line :/bin/fedora-review -b 928609
Comment 8 Karel Volný 2013-04-16 07:14:07 EDT
to summarize:

* bundled code

 - the file gl_texture_bind_test.py is based on gl-hello.py from toonplayer

 this is irrelevant, as it doesn't get packaged; if it would be included, the only issue with this would be to get the licensing correct

 - rencode - http://code.google.com/p/rencode/

 hm, well, deluge seems to be using this too and it is not explicitly discussed in the review request (bug #221669) ...

 however, the deluge's version seems completely different, so it would be a big problem to share the code, so I would go for an exception here

 note that this adds GPLv3+ to licenses, and according to https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#Footnote3 I believe whole xpra would need to upgrade to GPLv3+ ...?

 - python-webm - http://code.google.com/p/python-webm/

 I believe this is a good candidate for unbundling, but I'm not opposing another exception

* rm -rf %{buildroot}

 we've dropped this long time ago ... I cannot find it anywhere as formal requirement not to have this, but I'd remove the line so the tool doesn't complain about that :-)

* versioned dependency operator

 according to: 
http://docs.fedoraproject.org/en-US/Fedora_Draft_Documentation/0.1/html/RPM_Guide/ch-advanced-packaging.html#id709435
 "==" is not valid operator, just "="

* large docs

 I'd suggest not to have three copies of NEWS ...
Comment 9 T.C. Hollingsworth 2013-04-18 20:56:47 EDT
It appears the version Deluge uses is included with rencode upstream, and provides the same interface in pure Python as the Cython version bundled with xpra and also included with rencode upstream.  Therefore, it seems prudent just to package python-rencode, whose review is in bug 953699.  I've notified the deluge maintainers and suggested using the python-rencode package in bug 953700.

I also unbundled python-webm, it's in bug 953701.  The other minor issues identified have also been fixed.

Spec: http://patches.fedorapeople.org/xpra/xpra.spec
SRPM: http://patches.fedorapeople.org/xpra/xpra-0.8.8-4.fc19.src.rpm
Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=5273541

* Thu Apr 18 2013 T.C. Hollingsworth <tchollingsworth@gmail.com> - 0.8.8-4
- unbundle rencode and webm
- fix equality operator in Requires
- drop unnecessary multiple copies of NEWS
- don't remove buildroot
Comment 10 Antoine Martin 2013-05-02 04:00:57 EDT
Hi there, xpra maintainer here.

Is there any particular reason why you didn't contact us at all about this?
It seems to me that:
* we could have benefited from the feedback
* our users could have had better packages
* you could/will avoid issues by discussing with us (esp with current trunk)

FYI: 0.10 will be substantially different when it comes to packaging.
Comment 11 Antoine Martin 2013-05-02 04:24:28 EDT
> [!]: Package contains no bundled libraries without FPC exception.
> ^ there is rencode.pyx - http://code.google.com/p/rencode/
FYI: xpra can be built without rencode: --without-rendode
But, the performance difference is very noticeable

> rencode vs upstream (...)
We have merged the latest changes from upstream, and added some python2.4/3.x compatibility fixes which have been sent upstream (but not heard back).

> and python-webm - http://code.google.com/p/python-webm/
FYI, as above: can be built without webp (aka webm): --without-webp

> and a part of toonplayer - https://bitbucket.org/aalex/toonplayer
Has been removed from the test tree.

> ^ python-wimpiggy is the base package...
FYI: as of this week's trunk:
parti has been removed completely: we don't maintain it or use it, and no-one uses it anyway (that we know of)
wimpiggy source has been merged into xpra

> ^ note that the file NEWS is the same for all packages, could we have it symlinked?
As per above: no longer an issue

> ^ note that the egg-infos don't look 100% correct, e.g. parti-all has License: UNKNOWN
As per above: no longer an issue
Comment 12 T.C. Hollingsworth 2013-05-02 04:53:12 EDT
(In reply to comment #10)
> Hi there, xpra maintainer here.
> 
> Is there any particular reason why you didn't contact us at all about this?
> It seems to me that:
> * we could have benefited from the feedback
> * our users could have had better packages
> * you could/will avoid issues by discussing with us (esp with current trunk)
> 
> FYI: 0.10 will be substantially different when it comes to packaging.

Hi, Antoine!  Sorry I didn't get in touch, this was just a little side project for me and I've been busy with other things.  I'm subscribed to your mailing list now so I can keep up with changes better.  Plus, TBH there have been no major issues that I felt the need to complain about.  ;-)

(In reply to comment #11)
> > [!]: Package contains no bundled libraries without FPC exception.
> > ^ there is rencode.pyx - http://code.google.com/p/rencode/
> FYI: xpra can be built without rencode: --without-rendode
> But, the performance difference is very noticeable

In Fedora we like to avoid bundling libraries wherever possible, so we need to ship rencode and webm separately.  But I understand both rencode and webm are small and it would be a pain for your users to require it as a dependency.  (However, if you'd be interested in doing it anyway or making it an option at build time, I'd be happy to send a patch your way.)

> > rencode vs upstream (...)
> We have merged the latest changes from upstream, and added some
> python2.4/3.x compatibility fixes which have been sent upstream (but not
> heard back).

In cases like this, we'd still want to keep rencode seperate, but apply your changes as a patch to that package so other consumers can benefit from your bugfixes in upstream's absence.

> > and python-webm - http://code.google.com/p/python-webm/
> FYI, as above: can be built without webp (aka webm): --without-webp

Again, we really want to keep this functionality, we just need to ship the package separately.
 
> > and a part of toonplayer - https://bitbucket.org/aalex/toonplayer
> Has been removed from the test tree.
> 
> > ^ python-wimpiggy is the base package...
> FYI: as of this week's trunk:
> parti has been removed completely: we don't maintain it or use it, and
> no-one uses it anyway (that we know of)
> wimpiggy source has been merged into xpra

Awesome!  I'll go ahead and drop parti from the 0.9.0 package I'm working on.  No sense in shipping it in Fedora if it's going to go away soon.

> > ^ note that the file NEWS is the same for all packages, could we have it symlinked?
> As per above: no longer an issue
> 
> > ^ note that the egg-infos don't look 100% correct, e.g. parti-all has License: UNKNOWN
> As per above: no longer an issue

Thanks for the heads up!
Comment 13 T.C. Hollingsworth 2013-05-06 14:46:39 EDT
Spec: http://patches.fedorapeople.org/xpra/xpra.spec
SRPM: http://patches.fedorapeople.org/xpra/xpra-0.9.0-1.fc19.src.rpm

* Thu May 02 2013 T.C. Hollingsworth <tchollingsworth@gmail.com> - 0.9.0-1
- new upstream release 0.9.0
  http://lists.devloop.org.uk/pipermail/shifter-users/2013-April/000479.html
- delete the bundled code in prep instead of inside the patches
- don't bother including parti; it's going away upstream soon
- merge python-wimpiggy into main xpra package; it won't be seperated upstream soon
Comment 14 T.C. Hollingsworth 2013-05-06 22:14:15 EDT
This update fixes a small glitch in the rencode unbundling.

Also, python-rencode is in F(17|18|19) updates-testing now, and python-webm is waiting on git.

--

Spec: http://patches.fedorapeople.org/xpra/xpra.spec
SRPM: http://patches.fedorapeople.org/xpra/xpra-0.9.0-2.fc19.src.rpm
Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=5338048

* Tue May 07 2013 T.C. Hollingsworth <tchollingsworth@gmail.com> - 0.9.0-2
- fix rencode __version__ importing
Comment 15 T.C. Hollingsworth 2013-05-08 13:28:23 EDT
python-webm is now in Rawhide and updates-testing for all active branches as well.
Comment 16 Karel Volný 2013-05-09 08:53:11 EDT
Hi, I'm sorry but I got buried in some other stuff, if anyone is able to pick up this feel free to do so, otherwise I'll take a look again not until the next week (which doesn't sound that far in the future, but I just don't want to leave you uninformed for such long time, it's three weeks since my last comment already ...)
Comment 17 T.C. Hollingsworth 2013-05-10 15:25:22 EDT
No worries, it took that three weeks to ready the deps anyway.  ;-)  Next week is fine.

In the meantime, here's a minor upstream bugfix update.

--

Spec: http://patches.fedorapeople.org/xpra/xpra.spec
SRPM: http://patches.fedorapeople.org/xpra/xpra-0.9.1-1.fc19.src.rpm
Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=5362810

* Fri May 10 2013 T.C. Hollingsworth <tchollingsworth@gmail.com> - 0.9.1-1
- new upstream release 0.9.1
  http://lists.devloop.org.uk/pipermail/shifter-users/2013-May/000522.html
Comment 18 T.C. Hollingsworth 2013-05-13 20:13:07 EDT
Upstream made some more minor bugfixes.

--

Spec: http://patches.fedorapeople.org/xpra/xpra.spec
SRPM: http://patches.fedorapeople.org/xpra/xpra-0.9.2-1.fc19.src.rpm
Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=5375605

* Mon May 13 2013 T.C. Hollingsworth <tchollingsworth@gmail.com> - 0.9.2-1
- new upstream release 0.9.2
  http://lists.devloop.org.uk/pipermail/shifter-users/2013-May/000525.html
Comment 19 Antoine Martin 2013-05-20 09:56:47 EDT
More bugfixes today (some more important ones this time around).

Just (wrongly) posted this on the rpmfusion ticket: I haven't tested systemd integration, but others have:
https://wiki.archlinux.org/index.php/Xpra#Server
Comment 20 Christopher Meng 2013-07-31 23:49:19 EDT
TC I think you can close the ticket on rpmfusion.

BTW what's the progress here?
Comment 21 T.C. Hollingsworth 2013-08-01 03:24:26 EDT
(In reply to Christopher Meng from comment #20)
> TC I think you can close the ticket on rpmfusion.

If you look more closely you'll notice that the only open review there is for an add-on package to this one.

Please keep discussions about third-party repositories in their own bug trackers.  They might involve issues that shouldn't be discussed here.  ;-)
 
> BTW what's the progress here?

Sorry, forgot to update this.

--

Spec: http://patches.fedorapeople.org/xpra/xpra.spec
SRPM: http://patches.fedorapeople.org/xpra/xpra-0.9.8-1.fc19.src.rpm
Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=5687291

* Thu Aug 01 2013 T.C. Hollingsworth <tchollingsworth@gmail.com> - 0.9.8-1
- new upstream release 0.9.8
  http://lists.devloop.org.uk/pipermail/shifter-users/2013-July/000615.html
- use HTTPS for URL and Source0
Comment 22 T.C. Hollingsworth 2013-08-06 20:45:04 EDT
Christopher:  in comment 16 Karel said he wouldn't mind if someone takes this over if he gets busy and forgets about this, so feel free to take this over if you want this to get in faster.
Comment 23 Christopher Meng 2013-08-06 22:12:04 EDT
I'm sorry for some reason I can't take this.

I'll leave it to nobody if you agree.
Comment 24 T.C. Hollingsworth 2013-08-06 22:18:00 EDT
(In reply to Christopher Meng from comment #23)
> I'm sorry for some reason I can't take this.

Maybe because the fedora-review flag was set?  Try it now.
Comment 25 Christopher Meng 2013-08-06 22:41:17 EDT
(In reply to T.C. Hollingsworth from comment #24)
> (In reply to Christopher Meng from comment #23)
> > I'm sorry for some reason I can't take this.
> 
> Maybe because the fedora-review flag was set?  Try it now.

No, I don't take it because I have no time now.

There is a swap in devel, I think you can consider about swapping...
Comment 26 Antoine Martin 2013-08-13 03:53:33 EDT
0.10 released today, please be aware of some packaging issues that *will* affect the way you were planning on doing things (unbundling libs):
http://lists.devloop.org.uk/pipermail/shifter-users/2013-August/000642.html
Comment 27 T.C. Hollingsworth 2013-10-07 18:07:10 EDT
Spec: http://patches.fedorapeople.org/xpra/xpra.spec
SRPM: http://patches.fedorapeople.org/xpra/xpra-0.10.4-1.fc19.src.rpm
Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=6033594

* Mon Oct 07 2013 T.C. Hollingsworth <tchollingsworth@gmail.com> - 0.10.4-1
- rebase to 0.10.4
- don't ship webm stuff that doesn't work without ffmpeg anyway
Comment 28 Antoine Martin 2013-10-08 00:26:40 EDT
> "don't ship webm stuff that doesn't work without ffmpeg anyway"
I don't know who told you that, this is wrong.
"webm" (aka "webp") has nothing to do with ffmpeg.

VPX, which is a distant cousin of webp, does require ffmpeg (for colourspace conversion via ffmpeg's "swscale"), this is a soft runtime dependency.
FYI: it should even be possible to enable client VPX support without swscale installed when rendering to accelerated GL windows - this isn't implemented yet.
Comment 29 T.C. Hollingsworth 2013-10-08 04:36:35 EDT
(In reply to Antoine Martin from comment #28)
> > "don't ship webm stuff that doesn't work without ffmpeg anyway"
> I don't know who told you that, this is wrong.
> "webm" (aka "webp") has nothing to do with ffmpeg.
>
> VPX, which is a distant cousin of webp, does require ffmpeg (for colourspace
> conversion via ffmpeg's "swscale"), this is a soft runtime dependency.
> FYI: it should even be possible to enable client VPX support without swscale
> installed when rendering to accelerated GL windows - this isn't implemented
> yet.

I thought webm == VP8 and wouldn't work without swscale.

If having the webm module around does do something even with vpx disabled, or if having libvpx enabled even with ffmpeg disabled is useful (or will be so in the future), I'd be happy to add them back.
Comment 30 Antoine Martin 2013-10-08 05:23:20 EDT
Although they are related, webm != vpx
webm (aka webp) is for single frames, vpx is for video streams.

Xpra's webp codec does not require anything beyond "python-webm", be aware though that the upstream project is unresponsive and that others (at least Debian for sure) are now using our more up to date version. (which you will need to support webp encoding of transparent windows)

Xpra's vpx codec requires swscale on the server for converting BGRA pixels from the X11 server into a YCbCr 420 planar. On the client side, vpx *could* work without swscale if we added code to enable vpx with the OpenGL codepath.

I hope this clarifies things.
Comment 31 T.C. Hollingsworth 2013-10-08 14:21:26 EDT
(In reply to Antoine Martin from comment #30)
> Although they are related, webm != vpx
> webm (aka webp) is for single frames, vpx is for video streams.

Ah, it does WebP not VP8. (And I should have remembered that since it uses libwebp and not libvpx, d'oh!)  It turns out it wasn't showing up in xpra_launcher with my package because of an unbundling fail.  :-/

> Xpra's webp codec does not require anything beyond "python-webm", be aware
> though that the upstream project is unresponsive and that others (at least
> Debian for sure) are now using our more up to date version. (which you will
> need to support webp encoding of transparent windows)

I'm applying your patches to our separate python-webm too, BTW.  :-)

> Xpra's vpx codec requires swscale on the server for converting BGRA pixels
> from the X11 server into a YCbCr 420 planar. On the client side, vpx *could*
> work without swscale if we added code to enable vpx with the OpenGL codepath.

Okay, if client-side libvpx-without-swscale becomes a reality one day, please let me know and I'll enable it.  In the meantime, I'd prefer not to drag in a libvpx dependency that won't do anything with the Fedora build (and might mislead people into thinking this hobbled build supports VP8 when it doesn't at all).

People who want the video-based codecs instead of the image ones (i.e. everybody) will just have to grab them from that other repo.  ;-)

> I hope this clarifies things.

Thanks a lot!
Comment 32 T.C. Hollingsworth 2013-10-08 14:32:28 EDT
Spec: http://patches.fedorapeople.org/xpra/xpra.spec
SRPM: http://patches.fedorapeople.org/xpra/xpra-0.10.4-2.fc19.src.rpm
Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=6037266

* Tue Oct 08 2013 T.C. Hollingsworth <tchollingsworth@gmail.com> - 0.10.4-2
- reenable webp support
- fix webm unbundling to support importing all modules in the webm package
- require latest python-webm so it matches what's bundled upstream
Comment 33 Nicolas Chauvet (kwizart) 2013-10-18 08:56:03 EDT
- starting review -
Comment 34 T.C. Hollingsworth 2013-10-18 18:47:11 EDT
Sorry, meant to update this to the latest upstream last night but I forgot.  :-(

--

Spec: http://patches.fedorapeople.org/xpra/xpra.spec
SRPM: http://patches.fedorapeople.org/xpra/xpra-0.10.6-1.fc19.src.rpm
Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=6077461

* Fri Oct 18 2013 T.C. Hollingsworth <tchollingsworth@gmail.com> - 0.10.6-1
- new upstream release 0.10.6
  http://lists.devloop.org.uk/pipermail/shifter-users/2013-October/000726.html
Comment 35 Sebastian Dyroff 2013-12-27 15:57:02 EST
Hey, what is the status of this package review? Are there some obvious problems which prevents inclusion into fedora?
Comment 36 Antoine Martin 2014-01-20 07:03:36 EST
Please note that as of version 0.11.0, released just now:
http://lists.devloop.org.uk/pipermail/shifter-users/2014-January/000792.html

A number of dependencies have been added or changed - which may require changes to the spec file, OTOH:
* python-lz4 is nice to have (much faster than zlib) and easy to package
* we have a Cython colourspace conversion fallback, so vpx support no longer requires swscale (ffmpeg/libav)
* we can take advantage of OpenCL or CUDA if present for colourspace conversion too (let's hope the OpenCL packaging gets sorted out)
* NVENC (xpra exclusive) and CUDA are unlikely to be packaged by Fedora, but maybe for RHEL?
* webp: found a memory leak in it, so this encoding is no longer used automatically (for lossless refresh/small regions) and very strongly discouraged - until I find the source of the leak. So the dependency can be dropped, at least for now.
Comment 37 Antoine Martin 2014-03-28 03:17:35 EDT
And now 0.12.0 is out:
http://xpra.org/trac/wiki/News#a2014-03-23

Just one packaging update in there: libfakeXinerama has been added as an optional dependency for making fullscreen applications work with the dummy X11 server.

Are there any hold ups? Anything I can help with?
(for many common workloads, xpra is much more efficient than the alternatives currently packaged in the repos, it's a bit sad to see Fedora people stuck using early 1990s technologies)
Comment 38 Antoine Martin 2014-03-30 05:58:58 EDT
Oh, and since we are the new effective upstream for python-webm, it is worth mentioning that we have now fixed the memory leak in there too.

AFAICT, this affects the version currently shipped in the repos for all Fedora releases. See here for details:
http://xpra.org/trac/ticket/491#comment:4

"python-pillow" also leaks memory, but we're not fixing that one, the bug has been reported upstream instead (see link above - a test case is included).
Comment 39 Stephen Gauthier 2014-05-08 00:22:44 EDT
I would also like to inquire as to the status of this review.

Xpra is currently at stable 0.12.5 so this also probably needs some update. I'd be willing to contribute if there is something I could do to help.
Comment 40 Nicolas Chauvet (kwizart) 2014-06-02 15:33:03 EDT
Ok, sorry but I was not the right person for review swap.
Comment 41 Antoine Martin 2014-08-17 10:03:40 EDT
0.14.0 is out and will be a LTS release (18 months+) tailor made for packaging into distributions. *Many* of the changes were in fact related to RPM packaging and security.
In no small part trying to deal with the rather disappointing update schedule of media libraries which aren't part of Fedora proper. This may be OK for some when used through a media player only (I think not, but that may just be me), a lot less so when used with a network facing tool like xpra.

Anyway, all this and more is discussed in greater detail here:
http://xpra.org/trac/wiki/News#a0.14.0Release

I'm still puzzled about the fact that this ticket has been stuck for well over a year. Especially considering that there are a number of Fedora users willing to help move things forward (myself included). What's the procedure for getting this unstuck?
Comment 42 Jonathan Underwood 2014-12-19 18:37:49 EST
@tchollingsworth: are you still proposing this package for review?
Comment 43 Karel Volný 2015-01-06 06:33:11 EST
(In reply to Antoine Martin from comment #41)
> I'm still puzzled about the fact that this ticket has been stuck for well over
> a year.

seems everyone doing some work on this has run out of energy or time for this ...

> What's the procedure for getting this unstuck?

if the original reporter won't answer within some reasonable timeframe (you can also consider trying to reach him via different channels), you can take over this review request

meanwhile, you can propose a patch for the latest spec (comment #34) to update it to the new upstream version

then a reviewer is needed ... you can take this role (or find someone else if you take over the request)

please see https://fedoraproject.org/wiki/Package_Review_Process
Comment 44 Jonathan Underwood 2015-03-03 14:18:42 EST
OK, I have updated the package and cloned this bug in order to take over the package submission process - see BZ#1198312. Reviewers very welcome.

If tchollingsworth returns and wants to pick the package up again that's totally fine, but in the meantime this bug probably needs closing.

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