Bug 1198312 - Review Request: xpra - Remote display server for applications and desktops
Summary: Review Request: xpra - Remote display server for applications and desktops
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: T.C. Hollingsworth
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Keywords:
Depends On: 928609 953699 953701 1199189
Blocks: 990805 1243530
TreeView+ depends on / blocked
 
Reported: 2015-03-03 19:12 UTC by Jonathan Underwood
Modified: 2015-07-15 16:48 UTC (History)
21 users (show)

(edit)
Clone Of: 928609
(edit)
Last Closed: 2015-04-21 18:49:49 UTC
tchollingsworth: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Jonathan Underwood 2015-03-03 19:12:05 UTC
+++ This bug was initially created as a clone of Bug #928609 +++

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.

--- Additional comment from T.C. Hollingsworth on 2013-03-28 00:28:55 EDT ---



--- Additional comment from Dominik 'Rathann' Mierzejewski on 2013-03-28 09:25:12 EDT ---

I think you can enable WebM/VPX. libvpx is available in Fedora.

--- Additional comment from T.C. Hollingsworth on 2013-03-28 18:01:57 EDT ---

Unfortunately the vpx codec requires libswscale, which is part of ffmpeg. :-(

--- Additional comment from Karel Volný on 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?

--- Additional comment from T.C. Hollingsworth on 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

--- Additional comment from Karel Volný on 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 :-)

--- Additional comment from Karel Volný on 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

--- Additional comment from Karel Volný on 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 ...

--- Additional comment from T.C. Hollingsworth on 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

--- Additional comment from Antoine Martin on 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.

--- Additional comment from Antoine Martin on 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

--- Additional comment from T.C. Hollingsworth on 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!

--- Additional comment from T.C. Hollingsworth on 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

--- Additional comment from T.C. Hollingsworth on 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

--- Additional comment from T.C. Hollingsworth on 2013-05-08 13:28:23 EDT ---

python-webm is now in Rawhide and updates-testing for all active branches as well.

--- Additional comment from Karel Volný on 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 ...)

--- Additional comment from T.C. Hollingsworth on 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

--- Additional comment from T.C. Hollingsworth on 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

--- Additional comment from Antoine Martin on 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

--- Additional comment from Christopher Meng on 2013-07-31 23:49:19 EDT ---

TC I think you can close the ticket on rpmfusion.

BTW what's the progress here?

--- Additional comment from T.C. Hollingsworth on 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

--- Additional comment from T.C. Hollingsworth on 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.

--- Additional comment from Christopher Meng on 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.

--- Additional comment from T.C. Hollingsworth on 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.

--- Additional comment from Christopher Meng on 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...

--- Additional comment from Antoine Martin on 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

--- Additional comment from T.C. Hollingsworth on 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

--- Additional comment from Antoine Martin on 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.

--- Additional comment from T.C. Hollingsworth on 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.

--- Additional comment from Antoine Martin on 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.

--- Additional comment from T.C. Hollingsworth on 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!

--- Additional comment from T.C. Hollingsworth on 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

--- Additional comment from Nicolas Chauvet (kwizart) on 2013-10-18 08:56:03 EDT ---

- starting review -

--- Additional comment from T.C. Hollingsworth on 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

--- Additional comment from Sebastian Dyroff on 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?

--- Additional comment from Antoine Martin on 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.

--- Additional comment from Antoine Martin on 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)

--- Additional comment from Antoine Martin on 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).

--- Additional comment from Stephen Gauthier on 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.

--- Additional comment from Nicolas Chauvet (kwizart) on 2014-06-02 15:33:03 EDT ---

Ok, sorry but I was not the right person for review swap.

--- Additional comment from Antoine Martin on 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?

--- Additional comment from Jonathan Underwood on 2014-12-19 18:37:49 EST ---

@tchollingsworth: are you still proposing this package for review?

--- Additional comment from Karel Volný on 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 1 Jonathan Underwood 2015-03-03 19:14:59 UTC
OK, picking this up in an effort to move it forward. If the original package submitter returns and wants to pick it up again, fine by me, but in the meantime, if someone wants to review this great.

SPEC: https://jgu.fedorapeople.org/xpra.spec
SRPM: https://jgu.fedorapeople.org/xpra-0.14.19-1.fc20.src.rpm

* Tue Mar  3 2015 Jonathan G. Underwood <jonathan.underwood@gmail.com> - 0.14.19-1
- Update to upstream 0.14.19
- Add BuildRequires for libxkbfile-devel
- No longer need to unbundle webp
- Rework and rename patch for unbundling of rencode

Comment 2 T.C. Hollingsworth 2015-03-04 09:01:28 UTC
Thanks, I'm happy for someone with more time to take this over.  Sorry I missed your earlier ping.   I'll review instead!  :-)

=====

Some issues:

----

> Requires:       python-webm >= 0.2.2-3

AFAICT, this isn't needed anymore.  Also, you're not building webp support, see below.

----

This is what the buildsystem reports as enabled/disabled when I built it here.  Some prohibited items are still built if the dependencies are around, and there appear to be some things that could possibly be enabled.

> * PIC                  : Y
> * Xdummy               : Auto
> * Xdummy_wrapper       : Auto
> * argb                 : Y
> * avcodec2_static      : N
> * avcodec_static       : N
> * bencode              : Y
> * bundle_tests         : N
> * client               : Y
> * clipboard            : Y
> * csc_cython           : Y
> * csc_opencl           : N

This should be enabled by default, but you are missing the necessary BuildRequires/Requires.

https://www.xpra.org/trac/wiki/CSC/OpenCL

> * csc_swscale          : N
> * cymaths              : Y
> * cython_bencode       : Y
> * cyxor                : Y
> * debug                : N
> * dec_avcodec          : N
> * dec_avcodec2         : Y

This is a prohibited item and should be disabled.

> * enc_x264             : N
> * enc_x265             : N
> * gtk2                 : Y
> * gtk3                 : N

I guess this is disabled because it is unstable, but please keep an eye out for this to change in future releases.

> * gtk_x11              : Y
> * html5                : Y
> * memoryview           : N
> * nvenc                : N
> * opengl               : Y
> * qt4                  : N

Same here.

> * rencode              : Y
> * server               : Y
> * shadow               : Y
> * sound                : Y
> * strict               : Y
> * swscale_static       : N
> * verbose              : N
> * vpx                  : N

Is it still necessary to disable this?

libswscale isn't strictly required these days:
https://www.xpra.org/trac/wiki/CSC

> * vpx_static           : N
> * warn                 : Y
> * webp                 : N

Why is this disabled?  If the upstream defaults have changed, it might still be nice to enable this, given we'll lack support for many other codecs in Fedora.  (OTOH if you get vpx working this could be left off.)

> * webp_static          : N
> * x11                  : Y
> * x264_static          : N
> * x265_static          : N

----

Some prohibited content is installed:

> drwxr-xr-x  /usr/lib64/python2.7/site-packages/xpra/codecs/dec_avcodec2
> -rw-r--r--  /usr/lib64/python2.7/site-packages/xpra/codecs/dec_avcodec2/__init__.py
> -rw-r--r--  /usr/lib64/python2.7/site-packages/xpra/codecs/dec_avcodec2/__init__.pyc
> -rw-r--r--  /usr/lib64/python2.7/site-packages/xpra/codecs/dec_avcodec2/__init__.pyo
> -rwxr-xr-x  /usr/lib64/python2.7/site-packages/xpra/codecs/dec_avcodec2/decoder.so

ffmpeg, see above

> -rw-r--r--  /usr/share/xpra/www/include/web-socket-js/WebSocketMain.swf

SWF files are prebuilt binary programs, please remove them from the package:
https://fedoraproject.org/wiki/Packaging:Guidelines#No_inclusion_of_pre-built_binaries_or_libraries

This code prefers to use native WebSocket functionality so you can safely delete the Flash file and the HTML5 client will still work fine in modern browsers.

----

Bundled JavaScript libraries:

> -rw-r--r--  /usr/share/xpra/www/include/jquery.min.js

Please add Requires: js-jquery and add `ln -sf ../../javascript/jquery/2/jquery.min.js %{buildroot}%{_datadir}/xpra/www/include/jquery.min.js` to %install to avoid bundling.

> -rw-r--r--  /usr/share/xpra/www/include/jqueryui.min.js

Not yet packaged.  Please add `Provides: bundled(js-jquery-ui) = 1.10.4` for tracking purposes.

> drwxr-xr-x  /usr/share/xpra/www/include/web-socket-js

Not yet packaged.  Please add `Provides: bundled(js-web-socket-js)` for tracking purposes.

----

Only one rpmlint warning of interest:

> xpra.x86_64: E: script-without-shebang /usr/share/xpra/www/include/web-socket-js/web_socket.js

Please make this file nonexecutable.

====

I think that's enough for now.  This package NEEDS WORK.

Comment 3 T.C. Hollingsworth 2015-03-04 13:28:31 UTC
Sorry, one mistake:

> Please add Requires: js-jquery and add `ln -sf ../../javascript/jquery/2/jquery.min.js %{buildroot}%{_datadir}/xpra/www/include/jquery.min.js` to %install to avoid bundling.

That needs a third "../" :-)

You could also use an absolute symlink to %{_jsdir}/jquery/2/jquery.min.js; the choice is yours:
https://fedoraproject.org/wiki/Packaging:Guidelines#Symlinks

Comment 4 Jonathan Underwood 2015-03-04 13:40:06 UTC
Hi, great thanks for reappearing on this, and the useful pre-review. There's a lot here to work on, not least of which is the need for a package of pyopencl. I'll start to look at these thing. Thanks again.

Comment 5 T.C. Hollingsworth 2015-03-04 13:53:49 UTC
> not least of which is the need for a package of pyopencl

Make that two mistakes, I thought we had it.  I wouldn't consider this a review blocking issue, then.

Neither is vpx BTW, if testing indicates it doesn't actually work.  But if all you have to do is delete one line from the spec, why not?  :-)

Comment 6 Jonathan Underwood 2015-03-04 14:24:29 UTC
(In reply to T.C. Hollingsworth from comment #2)
[snip]
> > * dec_avcodec          : N
> > * dec_avcodec2         : Y
> 
> This is a prohibited item and should be disabled.

My local builds (in mock) don't build dec_avcodec2 support - upstream default is to have this disabled. So, I'm slightly confused as to why when you build the package dec_avcodec2 is enabled.

Comment 7 Jonathan Underwood 2015-03-04 15:02:34 UTC
(In reply to T.C. Hollingsworth from comment #2)
> > * rencode              : Y
> > * server               : Y
> > * shadow               : Y
> > * sound                : Y
> > * strict               : Y
> > * swscale_static       : N
> > * verbose              : N
> > * vpx                  : N
> 
> Is it still necessary to disable this?
> 
> libswscale isn't strictly required these days:
> https://www.xpra.org/trac/wiki/CSC

I am a bit confused here as, as I think you're conflating two issues - vpx, and CSC, but perhaps I misunderstand.

Anyway, regarding CSC, upstream defaults are:

  --with-csc_opencl       or   --without-csc_opencl         (default: False)
  --with-csc_cython       or   --without-csc_cython         (default: True)

so, --without-csc_opencl and --with-csc_cython. Are you saying you'd like to see the package use --with-csc_opencl and --without-csc_cython instead? 


Regarding vpx support, I'll enable that and see what happens, but that seems independent of the CSC question, right? Have I missed something?

Comment 8 Jonathan Underwood 2015-03-04 16:52:32 UTC
Updated packages which build fine - haven't had chance to test their usage yet though.

SPEC: https://jgu.fedorapeople.org/xpra.spec
SRPM: https://jgu.fedorapeople.org/xpra-0.14.19-2.fc20.src.rpm

* Tue Mar  3 2015 Jonathan G. Underwood <jonathan.underwood@gmail.com> - 0.14.19-2
- Update Summary to be more descriptive of package
- Use packaged js-jquery
- Add provides for bundled(js-jquery-ui) and bundled(js-web-socket-js)
- Build with vpx and webp support enabled
- Remove any installed SWF files
- Remove executable flag for all .js files
- Remove Requires for python-webm

Comment 9 Jonathan Underwood 2015-03-04 17:49:05 UTC
Update again to work on F20. Minimally tested locally on F20 by starting session, running xterm, killing session.

SPEC: https://jgu.fedorapeople.org/xpra.spec
SRPM: https://jgu.fedorapeople.org/xpra-0.14.19-2.fc20.src.rpm


* Tue Mar  3 2015 Jonathan G. Underwood <jonathan.underwood@gmail.com> - 0.14.19-3
- Use js-jquery package only on F22 or later - not available on
  earlier distros

Comment 10 Jonathan Underwood 2015-03-04 18:38:50 UTC
For anyone wanting to test packages, I've created a COPR repository here:

https://copr.fedoraproject.org/coprs/jgu/xpra/

Comment 11 Jonathan Underwood 2015-03-04 22:45:18 UTC
SPEC: https://jgu.fedorapeople.org/xpra.spec
SRPM: https://jgu.fedorapeople.org/xpra-0.14.19-4.fc20.src.rpm

%changelog
* Tue Mar  3 2015 Jonathan G. Underwood <jonathan.underwood@gmail.com> - 0.14.19-4
- Add --with-Xdummy and --with-Xdummy_wrapper build options since Xorg
  not installed at build time so autodetection fails

Comment 12 T.C. Hollingsworth 2015-03-04 23:40:48 UTC
(In reply to Jonathan Underwood from comment #7)
> (In reply to T.C. Hollingsworth from comment #2)
> > > * rencode              : Y
> > > * server               : Y
> > > * shadow               : Y
> > > * sound                : Y
> > > * strict               : Y
> > > * swscale_static       : N
> > > * verbose              : N
> > > * vpx                  : N
> > 
> > Is it still necessary to disable this?
> > 
> > libswscale isn't strictly required these days:
> > https://www.xpra.org/trac/wiki/CSC
> 
> I am a bit confused here as, as I think you're conflating two issues - vpx,
> and CSC, but perhaps I misunderstand.

They're related issues.  All the video codecs require a CSC to perform pre-conversion except in certain limited circumstances.  In the earlier version of the code, there weren't any such limited circumstances and libswscale from ffmpeg was the only CSC.  That's why vpx was previously disabled.

See the comments from Antoine, an upstream developer, in the old bug for more details on all this.  Note also that he said improvements were on the way, and they came.  Now that there are multiple CSC implementations and at least one that works on Fedora, vpx and other future libre codecs should work fine.

> Anyway, regarding CSC, upstream defaults are:
> 
>   --with-csc_opencl       or   --without-csc_opencl         (default: False)
>   --with-csc_cython       or   --without-csc_cython         (default: True)
> 
> so, --without-csc_opencl and --with-csc_cython. Are you saying you'd like to
> see the package use --with-csc_opencl and --without-csc_cython instead? 

No, you can build more than one of these.  The wiki oddly says that opencl is enabled by default these days, but I guess the code is what counts so this can be skipped for now.

=====

It sounds like all the issues I've found so far have been addressed.  Let me put it through a few more paces and see if we can't get this approved.

Comment 13 Jonathan Underwood 2015-03-04 23:50:08 UTC
OK, thanks for the explanation - I'm all clear now. 

Just collecting a few things here for later:
1) Need to package js-jquery-ui and js-web-socket-js from upstream and unbundle from xpra - this probably needs to happen before package is approved..


Outstanding issues for post-approval:
1) Package pyopencl
2) Package PyOpenGL-accelerate (or work with PyOpenGL packagers to build the accelerate module at the same time as the main package - even though they're separate tarballs, they are version lock-stepped as far as I can see).
3) Enable --with-csc_opencl

Comment 14 Jonathan Underwood 2015-03-04 23:51:52 UTC
Hmm... I am finding that when forwarding individual app windows (as opposed to the whole desktop) window resizing doesn't work properly. From Googling I can't work out if this is expected behavior.

Comment 15 T.C. Hollingsworth 2015-03-05 00:11:26 UTC
(In reply to Jonathan Underwood from comment #13)
> Just collecting a few things here for later:
> 1) Need to package js-jquery-ui and js-web-socket-js from upstream and
> unbundle from xpra - this probably needs to happen before package is
> approved..

jquery things that are not yet packaged have a blanket exception from the FPC.  You're right about js-web-socket-js though, that really should get packaged.  It doesn't even require building, it's just a bunch of static files.

> Outstanding issues for post-approval:
> 1) Package pyopencl
> 2) Package PyOpenGL-accelerate (or work with PyOpenGL packagers to build the
> accelerate module at the same time as the main package - even though they're
> separate tarballs, they are version lock-stepped as far as I can see).

Glad you noticed this too, I just noticed it with testing also.

> 3) Enable --with-csc_opencl

(In reply to Jonathan Underwood from comment #14)
> Hmm... I am finding that when forwarding individual app windows (as opposed
> to the whole desktop) window resizing doesn't work properly. From Googling I
> can't work out if this is expected behavior.

Hmm, I never use xpra for a whole desktop, and resizing windows works just fine here with your package.

Comment 16 Jonathan Underwood 2015-03-05 00:20:58 UTC
(In reply to T.C. Hollingsworth from comment #15)
> (In reply to Jonathan Underwood from comment #14)
> > Hmm... I am finding that when forwarding individual app windows (as opposed
> > to the whole desktop) window resizing doesn't work properly. From Googling I
> > can't work out if this is expected behavior.
> 
> Hmm, I never use xpra for a whole desktop, and resizing windows works just
> fine here with your package.

Interesting. Which Fedora versions are you running on the client and server?

I am running F20 on the (remote) server, and F21 locally for the client, and when I resize the frame, the application itself doesn't resize properly. I tried this both with and without xdummy, and see the same thing. My client desktop is Gnome 3, and on the server I was simply running either xterm or gnome-terminal.

Earlier in the day I tested with F20 as both client and server (the same machine in fact), and windows resized just fine there.

Comment 17 T.C. Hollingsworth 2015-03-05 00:39:33 UTC
(In reply to Jonathan Underwood from comment #16)
> Interesting. Which Fedora versions are you running on the client and server?

I only did F20->F20 as well using your package as both the client and server. :-(

I did test your package as a client on F20 against several xpra servers in the wild running on various distributions without trouble though.

I could potentially install it on an F21 server and test it out in awhile but unless I get the wild idea to update my laptop from F20 while away from home I'm probably not going to be able to test an F21 client anytime soon.  :-/

Do the upstream RPMs work fine in the same configuration?  Can you reproduce it on F21 with a different window manager?

Comment 18 T.C. Hollingsworth 2015-03-05 00:50:04 UTC
Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated

===== ISSUES =====

[!]: Package does not contain bundled libraries.

Please package js-web-socket-js to avoid bundling as discussed earlier.

[!]: The summary in bugzilla does not match the summary in the spec.

Please correct these items for approval.  Other issues identified can be corrected in the course of normal development.

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

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[-]: 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.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "GPL (v2 or later)", "Unknown or generated", "MIT/X11 (BSD like)", "LGPL
     (v3 or later)", "BSD (2 clause)", "*No copyright* MIT/X11 (BSD like)".
     418 files have unknown license. Detailed output of licensecheck in
     /var/tmp/1198312-xpra/licensecheck.txt
[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: update-desktop-database is invoked in %post and %postun if package
     contains desktop file(s) with a MimeType: entry.
     Note: desktop file(s) with MimeType entry in xpra
[x]: gtk-update-icon-cache is invoked in %postun and %posttrans if package
     contains icons.
     Note: icons in xpra
[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[-]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 112640 bytes in 3 files.
[x]: Package complies to the Packaging Guidelines
[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).
[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 requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any that
     are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: %config files are marked noreplace or the reason is justified.
[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 or desktop-
     file-validate if there is such a file.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[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

Python:
[x]: Python eggs must not download any dependencies during the build process.
[x]: A package which is used by another package via an egg interface should
     provide egg info.
[x]: 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:
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
     Note: Found : Packager: T.C. Hollingsworth <tchollingsworth@gmail.com>
     See: http://fedoraproject.org/wiki/Packaging:Guidelines#Tags
[-]: 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]: Final provides and requires are sane (see attachments).
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: Patches link to upstream bugs/comments/lists or are otherwise justified.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[x]: 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 (not strictly required in GL).
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

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

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


Rpmlint
-------
Checking: xpra-0.14.19-4.fc20.x86_64.rpm
          xpra-0.14.19-4.fc20.src.rpm
xpra.x86_64: W: no-manual-page-for-binary xpra_Xdummy
xpra.src:37: W: unversioned-explicit-provides bundled(js-jquery)
xpra.src:41: W: unversioned-explicit-provides bundled(js-web-socket-js)
2 packages and 0 specfiles checked; 0 errors, 3 warnings.




Rpmlint (installed packages)
----------------------------
Cannot parse rpmlint output:


Requires
--------
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)
    libXcomposite.so.1()(64bit)
    libXdamage.so.1()(64bit)
    libXext.so.6()(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)
    libm.so.6()(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)
    libvpx.so.1()(64bit)
    libwebp.so.4()(64bit)
    libxkbfile.so.1()(64bit)
    numpy
    pulseaudio
    pulseaudio-utils
    pygtkglext
    python(abi)
    python-imaging
    python-numeric
    python-rencode
    rtld(GNU_HASH)
    xorg-x11-drv-dummy
    xorg-x11-drv-void
    xorg-x11-server-Xvfb
    xorg-x11-server-utils



Provides
--------
xpra:
    application()
    application(xpra.desktop)
    application(xpra_launcher.desktop)
    bundled(js-jquery)
    bundled(js-jquery-ui)
    bundled(js-web-socket-js)
    config(xpra)
    mimehandler(text/x-xpraconfig)
    mimehandler(x-scheme-handler/xpra)
    xpra
    xpra(x86-64)



Unversioned so-files
--------------------
xpra: /usr/lib64/python2.7/site-packages/xpra/codecs/argb/argb.so
xpra: /usr/lib64/python2.7/site-packages/xpra/codecs/csc_cython/colorspace_converter.so
xpra: /usr/lib64/python2.7/site-packages/xpra/codecs/vpx/decoder.so
xpra: /usr/lib64/python2.7/site-packages/xpra/codecs/vpx/encoder.so
xpra: /usr/lib64/python2.7/site-packages/xpra/codecs/webp/decode.so
xpra: /usr/lib64/python2.7/site-packages/xpra/codecs/webp/encode.so
xpra: /usr/lib64/python2.7/site-packages/xpra/codecs/xor/cyxor.so
xpra: /usr/lib64/python2.7/site-packages/xpra/gtk_common/gdk_atoms.so
xpra: /usr/lib64/python2.7/site-packages/xpra/net/bencode/cython_bencode.so
xpra: /usr/lib64/python2.7/site-packages/xpra/server/stats/cymaths.so
xpra: /usr/lib64/python2.7/site-packages/xpra/x11/bindings/core_bindings.so
xpra: /usr/lib64/python2.7/site-packages/xpra/x11/bindings/display_source.so
xpra: /usr/lib64/python2.7/site-packages/xpra/x11/bindings/keyboard_bindings.so
xpra: /usr/lib64/python2.7/site-packages/xpra/x11/bindings/randr_bindings.so
xpra: /usr/lib64/python2.7/site-packages/xpra/x11/bindings/wait_for_x_server.so
xpra: /usr/lib64/python2.7/site-packages/xpra/x11/bindings/window_bindings.so
xpra: /usr/lib64/python2.7/site-packages/xpra/x11/bindings/ximage.so
xpra: /usr/lib64/python2.7/site-packages/xpra/x11/gtk_x11/gdk_bindings.so
xpra: /usr/lib64/python2.7/site-packages/xpra/x11/gtk_x11/gdk_display_source.so

Source checksums
----------------
https://xpra.org/src/xpra-0.14.19.tar.xz :
  CHECKSUM(SHA256) this package     : b5ac3696d4a29090d57a996fedf55afe65012668efdb153fd8212ff2b5572e4a
  CHECKSUM(SHA256) upstream package : b5ac3696d4a29090d57a996fedf55afe65012668efdb153fd8212ff2b5572e4a


Generated by fedora-review 0.5.2 (63c24cb) last change: 2014-07-14
Command line :/usr/bin/fedora-review -b1198312
Buildroot used: fedora-20-x86_64
Active plugins: Python, Generic, Shell-api, C/C++
Disabled plugins: Java, SugarActivity, fonts, Haskell, Ocaml, Perl, R, PHP, Ruby
Disabled flags: EXARCH, EPEL5, BATCH, DISTTAG

Comment 19 Antoine Martin 2015-03-05 05:34:10 UTC
Upstream here. Sorry for replying late.
First, I would like to say that many of us are happy Fedora users and would love to help in getting xpra in Fedora proper. So if you need us to do anything, do let us know. Working with downstream distributions is always worth the effort, we have greatly benefited from the feedback we have received so far.

>> * gtk3                 : N
> I guess this is disabled because it is unstable, but please keep an eye out for this to change in future releases.
Indeed.
In 0.15 (which is running late), we have split the packages into:
* xpra-common
* xpra
* python3-xpra - which is the GTK3 version. Still not ready for prime time, but at least getting the packaging in order by splitting things.

>> * qt4                  : N
> Same here.
This one has been dropped completely in 0.15 because of a complete lack of maintenance.

Note: we're not going to unbundle jquery and websockets, because it would be too costly for us to make builds for all the platforms we support (centos etc). But if we can somehow make it easier, let us know.
The HTML5 client is making great progress in trunk.

Re CSC:
* you don't /really/ need pyopencl, we have the cython csc module as fallback - which is usually fast enough
* I have experienced issues with pyopencl and the AMD icd, causing hangs because of interference with the signal handlers... so we're unsure what to do with this one (and in most cases, swscale is just as fast..)

> The wiki oddly says that opencl is enabled by default these days, but I guess the code is what counts so this can be skipped for now.
It is when built.
We have a "csc-modules" command line switch to be able to enable/disable csc modules at runtime. I believe that what the wiki is meant to say is that it will be used by default when present.

> Hmm... I am finding that when forwarding individual app windows (as opposed to the whole desktop) window resizing doesn't work properly.
It is meant to work properly... BUT, there are lots of issues in this area, newer toolkits (especially GTK) seem to use server-side resizing. This is being addressed in 0.15, at least with *nix clients for now.
> I am running F20 on the (remote) server, and F21 locally for the client, and when I resize the frame, the application itself doesn't resize properly. I tried this both with and without xdummy, and see the same thing. My client desktop is Gnome 3, and on the server I was simply running either xterm or gnome-terminal.
Well, that's very odd. I test with xterm hundreds of times a day, and I use F21.. though no with Gnome 3.

> Package PyOpenGL-accelerate (or work with PyOpenGL packagers to build the
> accelerate module at the same time as the main package - even though they're
> separate tarballs, they are version lock-stepped as far as I can see).
Definitely MUST be in lockstep.
We have hit some very obscure bugs when those two packages had version mismatch issues. I ended up adding version checks to prevent us using zero copy upload in such cases (so it should now degrade well - at least as well as we can make it)

Comment 20 Jonathan Underwood 2015-03-05 12:01:22 UTC
Thanks for the input Antoine, much appreciated.

(In reply to Antoine Martin from comment #19)
[snip]
> Note: we're not going to unbundle jquery and websockets, because it would be
> too costly for us to make builds for all the platforms we support (centos
> etc). But if we can somehow make it easier, let us know.
> The HTML5 client is making great progress in trunk.
> 

It would be great if there were flags such as --use-system-websockets-js and --use-system-jquery for setup.py, but it's not a big deal to work around the unbundling without them.

> Re CSC:
> * you don't /really/ need pyopencl, we have the cython csc module as
> fallback - which is usually fast enough
> * I have experienced issues with pyopencl and the AMD icd, causing hangs
> because of interference with the signal handlers... so we're unsure what to
> do with this one (and in most cases, swscale is just as fast..)
> 

OK, for the time being we'll stick with just the cython csc module then.

[snip]

> > Hmm... I am finding that when forwarding individual app windows (as opposed to the whole desktop) window resizing doesn't work properly.
> It is meant to work properly... BUT, there are lots of issues in this area,
> newer toolkits (especially GTK) seem to use server-side resizing. This is
> being addressed in 0.15, at least with *nix clients for now.
> > I am running F20 on the (remote) server, and F21 locally for the client, and when I resize the frame, the application itself doesn't resize properly. I tried this both with and without xdummy, and see the same thing. My client desktop is Gnome 3, and on the server I was simply running either xterm or gnome-terminal.
> Well, that's very odd. I test with xterm hundreds of times a day, and I use
> F21.. though no with Gnome 3.
> 

If you had time, it would be very helpful to see if you could reproduce the problem I am seeing - I was using an F20 server, and an F21 client running Gnome. If you can see what happens with F21 server and client running Gnome, that would be a helpful data point (I am away from the F21 machine presently and can't enable ssh access to it).

> > Package PyOpenGL-accelerate (or work with PyOpenGL packagers to build the
> > accelerate module at the same time as the main package - even though they're
> > separate tarballs, they are version lock-stepped as far as I can see).
> Definitely MUST be in lockstep.
> We have hit some very obscure bugs when those two packages had version
> mismatch issues. I ended up adding version checks to prevent us using zero
> copy upload in such cases (so it should now degrade well - at least as well
> as we can make it)

Right, understood. I've filed an RFE with the PyOpenGL packager to include the accelerate module, but he may not like that option and we may have to package it separately and be careful about updates - pretty straightforward stuff - I am sure we'll eventually get it packaged, but fortunately it's not a blocker.

Comment 21 Jonathan Underwood 2015-03-05 15:19:19 UTC
BZ #1199189 is a review request for the unbundled js-web-socket-js package.

Comment 22 Jonathan Underwood 2015-03-18 19:39:59 UTC
SPEC: https://jgu.fedorapeople.org/xpra.spec
SRPM: https://jgu.fedorapeople.org/xpra-0.14.19-6.fc20.src.rpm



* Wed Mar 18 2015 Jonathan G. Underwood <jonathan.underwood@gmail.com> - 0.14.19-6
- Unbundle js-query even on Fedora 20

* Wed Mar 18 2015 Jonathan G. Underwood <jonathan.underwood@gmail.com> - 0.14.19-5
- Unbundle web-socket-js

Comment 23 Jonathan Underwood 2015-03-19 12:04:34 UTC
SPEC: https://jgu.fedorapeople.org/xpra.spec
SRPM: https://jgu.fedorapeople.org/xpra-0.14.21-1.fc20.src.rpm

* Wed Mar 18 2015 Jonathan G. Underwood <jonathan.underwood@gmail.com> - 0.14.21-1
- Update to 0.14.21

Comment 24 Jonathan Underwood 2015-03-24 13:45:45 UTC
SPEC: https://jgu.fedorapeople.org/xpra.spec
SRPM: https://jgu.fedorapeople.org/xpra-0.14.21-2.fc20.src.rpm


* Mon Mar 23 2015 Jonathan G. Underwood <jonathan.underwood@gmail.com> - 0.14.21-2
- Add conditionals for building with ffmpeg and x264 support, disabled
  by default


Doesn't affect the package for Fedora, but does make it to rebuild the package with support for the other codecs if one wanted to. Unintrusive change.

The js-web-socket-js package is now built in Fedora, and so the unbundling work is now complete, so hopefully this package can now be approved :).

Comment 25 Jonathan Underwood 2015-03-24 14:00:25 UTC
Actually, the way I added the conditionals is broken, so for now we'll go with this:

SRPM: https://jgu.fedorapeople.org/xpra-0.14.21-1.fc20.src.rpm

Comment 26 T.C. Hollingsworth 2015-03-24 14:37:54 UTC
Some remaining issues:

- I missed one more bundled library in the first pass, my apologies. Fortunately it is already packaged, so this is an easy fix.

This also depends on js-zlib [html5/include/(de|in)flate.min.js], please do the symlink/Requires dance for it too.  js-zlib will never be in Fedora <21 due to missing closure-compiler, so you'll need to do the conditional thing you did for jQuery.

- The "Provides:       bundled(js-web-socket-js)" is now unneeded.

- Please make sure the spec and SRPM are congruent and always link to both when you update the review, so automated tools like fedora-review don't get confused.

- The Summary as listed in Bugzilla doesn't match the Summary in the spec file.  Please correct this so the SCM request will be honored later.

Please correct these and make sure fedora-review/rpmlint are still happy and we'll get this approved.  Thanks!

Comment 27 Jonathan Underwood 2015-03-24 20:05:26 UTC
SPEC: https://jgu.fedorapeople.org/xpra.spec
SRPM: https://jgu.fedorapeople.org/xpra-0.14.21-2.fc20.src.rpm

* Mon Mar 23 2015 Jonathan G. Underwood <jonathan.underwood@gmail.com> - 0.14.21-2
- Add conditionals for building with ffmpeg and x264 support, disabled
  by default
- Remove Provides for bundled(js-web-socket-js)
- Use system js-zlib on Fedora >= 21
- On Fedora < 21 add Provides for bundled(js-zlib)



rpmlint output on the rpms:

xpra.src:66: W: unversioned-explicit-provides bundled(js-zlib)

--> I couldn't fathom what version is bundled. I think it's ok to leave this bundled(..) provides unversioned though.

xpra.x86_64: E: explicit-lib-dependency js-zlib

--> False positive.

xpra.x86_64: W: dangling-relative-symlink /usr/share/xpra/www/include/jquery.min.js ../../../javascript/jquery/2/jquery.min.js
xpra.x86_64: W: dangling-relative-symlink /usr/share/xpra/www/include/inflate.min.js ../../../javascript/zlib/inflate.min.js
xpra.x86_64: W: dangling-relative-symlink /usr/share/xpra/www/include/deflate.min.js ../../../javascript/zlib/deflate.min.js
xpra.x86_64: W: dangling-relative-symlink /usr/share/xpra/www/include/web-socket-js ../../../javascript/web-socket-js

--> Expected

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

--> True, but that binary isn't user-callable, so doesn't need a man page. One might argue it should be in libexec I suppose. I'll query that upstream.

Comment 28 Jonathan Underwood 2015-03-24 20:14:39 UTC
Will run fedora-review against these packages and report back.

Comment 29 Jonathan Underwood 2015-03-24 20:28:05 UTC
SPEC: https://jgu.fedorapeople.org/xpra.spec
SRPM: https://jgu.fedorapeople.org/xpra-0.14.21-3.fc20.src.rpm

* Tue Mar 24 2015 Jonathan G. Underwood <jonathan.underwood@gmail.com> - 0.14.21-3
- Update license tag from GPLv2+ to GPLv2+ and BSD and LGPLv3+ and MIT


Updated license tag following running fedora-review. All else seems OK as far as I can see. Hopefully we're about good to go.

Comment 30 T.C. Hollingsworth 2015-03-24 21:54:14 UTC
Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated


Issues:
=======

All blocking issues have been addressed!


TODO Post Review:
=================

- Work with upstream to resolve the issue with resizing in GNOME Shell.

- Work on enabling OpenCL CSC support if possible.


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

C/C++:
[X]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[-]: 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.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "GPL (v2 or later)", "Unknown or generated", "MIT/X11 (BSD like)", "LGPL
     (v3 or later)", "BSD (2 clause)", "*No copyright* MIT/X11 (BSD like)".
     418 files have unknown license. Detailed output of licensecheck in
     /var/tmp/1198312-xpra/licensecheck.txt
[x]: If the package is under multiple licenses, the licensing breakdown must
     be documented in the spec.
[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[x]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: update-desktop-database is invoked in %post and %postun if package
     contains desktop file(s) with a MimeType: entry.
     Note: desktop file(s) with MimeType entry in xpra
[x]: gtk-update-icon-cache is invoked in %postun and %posttrans if package
     contains icons.
     Note: icons in xpra
[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[-]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 112640 bytes in 3 files.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least one
     supported primary architecture.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[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 requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any that
     are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: %config files are marked noreplace or the reason is justified.
[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 or desktop-
     file-validate if there is such a file.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[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

Python:
[x]: Python eggs must not download any dependencies during the build process.
[x]: A package which is used by another package via an egg interface should
     provide egg info.
[x]: 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:
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
     Note: Found : Packager: T.C. Hollingsworth <tchollingsworth@gmail.com>
     See: http://fedoraproject.org/wiki/Packaging:Guidelines#Tags
[x]: 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]: Final provides and requires are sane (see attachments).
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: Patches link to upstream bugs/comments/lists or are otherwise justified.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[!]: %check is present and all tests pass.

--> Tests are not run but likely do not work well in koji.

[x]: Packages should try to preserve timestamps of original installed files.
[x]: Spec use %global instead of %define unless justified.
     Note: %define requiring justification: %define _with_enc_x264 --without-
     enc_x264, %define _with_dec_avcodec --without-dec_avcodec, %define
     _with_csc_swscale --without-csc_swscale

--> I guess %define is required for --with(out) build conditionals?

[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 (not strictly required in GL).
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: SourceX is a working URL.

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

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: Mock build failed
     See: http://fedoraproject.org/wiki/Packaging/Guidelines#rpmlint
[x]: Large data in /usr/share should live in a noarch subpackage if package is
     arched.
[x]: Spec file according to URL is the same as in SRPM.


Installation errors
-------------------
INFO: mock.py version 1.2.7 starting (python version = 2.7.5)...
Start: init plugins
INFO: selinux enabled
Finish: init plugins
Start: run
Start: chroot init
INFO: calling preinit hooks
INFO: enabled root cache
INFO: enabled yum cache
Start: cleaning yum metadata
Finish: cleaning yum metadata
INFO: enabled ccache
Mock Version: 1.2.7
INFO: Mock Version: 1.2.7
Finish: chroot init
INFO: installing package(s): /var/tmp/1198312-xpra/results/xpra-0.14.21-3.fc22.x86_64.rpm
ERROR: Command failed. See logs for output.
 # /usr/bin/yum --installroot /var/lib/mock/fedora-22-x86_64/root/ --releasever 22 install /var/tmp/1198312-xpra/results/xpra-0.14.21-3.fc22.x86_64.rpm --setopt=tsflags=nocontexts

--> This was due to missing js-web-socket-js.  It works fine locally.

Rpmlint
-------
Checking: xpra-0.14.21-3.fc22.x86_64.rpm
          xpra-0.14.21-3.fc22.src.rpm
xpra.x86_64: E: explicit-lib-dependency js-zlib
xpra.x86_64: W: dangling-relative-symlink /usr/share/xpra/www/include/jquery.min.js ../../../javascript/jquery/2/jquery.min.js
xpra.x86_64: W: dangling-relative-symlink /usr/share/xpra/www/include/inflate.min.js ../../../javascript/zlib/inflate.min.js
xpra.x86_64: W: dangling-relative-symlink /usr/share/xpra/www/include/deflate.min.js ../../../javascript/zlib/deflate.min.js
xpra.x86_64: W: dangling-relative-symlink /usr/share/xpra/www/include/web-socket-js ../../../javascript/web-socket-js
xpra.x86_64: W: no-manual-page-for-binary xpra_Xdummy
xpra.src:66: W: unversioned-explicit-provides bundled(js-zlib)
2 packages and 0 specfiles checked; 1 errors, 6 warnings.

--> Previously justified.


Requires
--------
xpra (rpmlib, GLIBC filtered):
    /bin/sh
    /usr/bin/python
    PyOpenGL
    config(xpra)
    dbus-python
    gstreamer
    gstreamer-plugins-base
    gstreamer-plugins-good
    gstreamer-python
    js-jquery
    js-web-socket-js
    js-zlib
    libX11.so.6()(64bit)
    libXcomposite.so.1()(64bit)
    libXdamage.so.1()(64bit)
    libXext.so.6()(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)
    libm.so.6()(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)
    libvpx.so.1()(64bit)
    libwebp.so.5()(64bit)
    libxkbfile.so.1()(64bit)
    numpy
    pulseaudio
    pulseaudio-utils
    pygtkglext
    python(abi)
    python-imaging
    python-numeric
    python-rencode
    rtld(GNU_HASH)
    xorg-x11-drv-dummy
    xorg-x11-drv-void
    xorg-x11-server-Xvfb
    xorg-x11-server-utils

--> OK

Provides
--------
xpra:
    application()
    application(xpra.desktop)
    application(xpra_launcher.desktop)
    bundled(js-jquery-ui)
    config(xpra)
    mimehandler(text/x-xpraconfig)
    mimehandler(x-scheme-handler/xpra)
    xpra
    xpra(x86-64)

--> OK

Unversioned so-files
--------------------
xpra: /usr/lib64/python2.7/site-packages/xpra/codecs/argb/argb.so
xpra: /usr/lib64/python2.7/site-packages/xpra/codecs/csc_cython/colorspace_converter.so
xpra: /usr/lib64/python2.7/site-packages/xpra/codecs/vpx/decoder.so
xpra: /usr/lib64/python2.7/site-packages/xpra/codecs/vpx/encoder.so
xpra: /usr/lib64/python2.7/site-packages/xpra/codecs/webp/decode.so
xpra: /usr/lib64/python2.7/site-packages/xpra/codecs/webp/encode.so
xpra: /usr/lib64/python2.7/site-packages/xpra/codecs/xor/cyxor.so
xpra: /usr/lib64/python2.7/site-packages/xpra/gtk_common/gdk_atoms.so
xpra: /usr/lib64/python2.7/site-packages/xpra/net/bencode/cython_bencode.so
xpra: /usr/lib64/python2.7/site-packages/xpra/server/stats/cymaths.so
xpra: /usr/lib64/python2.7/site-packages/xpra/x11/bindings/core_bindings.so
xpra: /usr/lib64/python2.7/site-packages/xpra/x11/bindings/display_source.so
xpra: /usr/lib64/python2.7/site-packages/xpra/x11/bindings/keyboard_bindings.so
xpra: /usr/lib64/python2.7/site-packages/xpra/x11/bindings/randr_bindings.so
xpra: /usr/lib64/python2.7/site-packages/xpra/x11/bindings/wait_for_x_server.so
xpra: /usr/lib64/python2.7/site-packages/xpra/x11/bindings/window_bindings.so
xpra: /usr/lib64/python2.7/site-packages/xpra/x11/bindings/ximage.so
xpra: /usr/lib64/python2.7/site-packages/xpra/x11/gtk_x11/gdk_bindings.so
xpra: /usr/lib64/python2.7/site-packages/xpra/x11/gtk_x11/gdk_display_source.so

--> All in %{python_sitearch} and excluded from autoprovides, OK.

Source checksums
----------------
https://xpra.org/src/xpra-0.14.21.tar.xz :
  CHECKSUM(SHA256) this package     : 1a802c3a6dc4b51c7a729d4cf8fb97c5a645d4f87428d7379e9009f3ff4f65f3
  CHECKSUM(SHA256) upstream package : 1a802c3a6dc4b51c7a729d4cf8fb97c5a645d4f87428d7379e9009f3ff4f65f3

--> OK

Generated by fedora-review 0.5.2 (63c24cb) last change: 2014-07-14
Command line :/usr/bin/fedora-review -mfedora-22-x86_64 -b1198312
Buildroot used: fedora-22-x86_64
Active plugins: Python, Generic, Shell-api, C/C++
Disabled plugins: Java, SugarActivity, fonts, Haskell, Ocaml, Perl, R, PHP, Ruby
Disabled flags: EXARCH, EPEL5, BATCH, DISTTAG

Comment 31 T.C. Hollingsworth 2015-03-24 21:54:47 UTC
This package is APPROVED.  Thank you for your contribution to Fedora!

Comment 32 Jonathan Underwood 2015-03-25 10:11:32 UTC
Many thanks @tchollingsworth - I think it's fair to say you did the hard work for this package!

Comment 33 Jonathan Underwood 2015-03-25 10:12:58 UTC
New Package SCM Request
=======================
Package Name: xpra
Short Description: Remote display server for applications and desktops
Upstream URL: https://www.xpra.org/
Owners: jgu
Branches: f20 f21 f22
InitialCC: jgu

Comment 34 Gwyn Ciesla 2015-03-25 11:20:24 UTC
Git done (by process-git-requests).

Comment 35 Fedora Update System 2015-03-25 13:41:21 UTC
js-web-socket-js-1.0.2-3.fc22,xpra-0.14.21-3.fc22 has been submitted as an update for Fedora 22.
https://admin.fedoraproject.org/updates/js-web-socket-js-1.0.2-3.fc22,xpra-0.14.21-3.fc22

Comment 36 Fedora Update System 2015-03-25 13:44:24 UTC
xpra-0.14.21-3.fc21,js-web-socket-js-1.0.2-3.fc21 has been submitted as an update for Fedora 21.
https://admin.fedoraproject.org/updates/xpra-0.14.21-3.fc21,js-web-socket-js-1.0.2-3.fc21

Comment 37 Fedora Update System 2015-03-25 13:45:33 UTC
xpra-0.14.21-3.fc20,js-web-socket-js-1.0.2-3.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/xpra-0.14.21-3.fc20,js-web-socket-js-1.0.2-3.fc20

Comment 38 Fedora Update System 2015-03-29 04:26:32 UTC
xpra-0.14.21-3.fc20, js-web-socket-js-1.0.2-3.fc20 has been pushed to the Fedora 20 testing repository.

Comment 39 Fedora Update System 2015-04-21 18:49:49 UTC
js-web-socket-js-1.0.2-3.fc22, xpra-0.14.21-3.fc22 has been pushed to the Fedora 22 stable repository.

Comment 40 Fedora Update System 2015-05-15 13:32:42 UTC
js-web-socket-js-1.0.2-3.fc21 has been pushed to the Fedora 21 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 41 Fedora Update System 2015-05-15 13:33:26 UTC
js-web-socket-js-1.0.2-3.fc20 has been pushed to the Fedora 20 stable repository.


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