Bug 1198312

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> - 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

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.

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

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.

> 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?  :-)

(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.

(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?

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> - 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

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> - 0.14.19-3
- Use js-jquery package only on F22 or later - not available on
  earlier distros

For anyone wanting to test packages, I've created a COPR repository here:

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

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> - 0.14.19-4
- Add --with-Xdummy and --with-Xdummy_wrapper build options since Xorg
  not installed at build time so autodetection fails

(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.

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

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.

(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.

(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.

(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?

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>
     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

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)

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.

BZ #1199189 is a review request for the unbundled js-web-socket-js package.

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> - 0.14.19-6
- Unbundle js-query even on Fedora 20

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

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> - 0.14.21-1
- Update to 0.14.21

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> - 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 :).

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

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!

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> - 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.

Will run fedora-review against these packages and report back.

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> - 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.

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>
     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

This package is APPROVED.  Thank you for your contribution to Fedora!

Many thanks @tchollingsworth - I think it's fair to say you did the hard work for this package!

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

Git done (by process-git-requests).

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

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

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

xpra-0.14.21-3.fc20, js-web-socket-js-1.0.2-3.fc20 has been pushed to the Fedora 20 testing repository.

js-web-socket-js-1.0.2-3.fc22, xpra-0.14.21-3.fc22 has been pushed to the Fedora 22 stable repository.

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.

js-web-socket-js-1.0.2-3.fc20 has been pushed to the Fedora 20 stable repository.