xpra icon
Bug tracker and wiki

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#851 closed defect (invalid)

Rencode error on 0.15.0 Fedora 20 and 21 builds

Reported by: J. Max Mena Owned by: Antoine Martin
Priority: major Milestone: 0.14
Component: server Version: trunk
Keywords: Cc:

Description

Running a Fedora 20 and 21 r9137 0.15.0 Servers (from xpra.org/beta):

Running an xpra list and xpra stop :13 prints the following:

[max@ueberraschung ~]$ xpra list
2015-05-01 14:10:20,452 rencode at '/usr/lib64/python2.7/site-packages/rencode/__init__.pyc' lacks versioning information
Found the following xpra sessions:
	LIVE session at :13
[max@ueberraschung ~]$ xpra stop :13
2015-05-01 14:10:24,183 rencode at '/usr/lib64/python2.7/site-packages/rencode/__init__.pyc' lacks versioning information
server requested disconnect: server shutdown
Failed to shutdown xpra at :13

In addition, I see the rencode error on server start, and on xpra info :13

Attachments (1)

rencode.diff (18.4 KB) - added by Antoine Martin 4 years ago.
diff between upstream on github and our copy

Download all attachments as: .zip

Change History (13)

comment:1 Changed 4 years ago by Antoine Martin

Milestone: 0.14
Resolution: invalid
Status: newclosed

You have not installed the latest python-rencode package, which means that you don't have xpra.org added as a repository as per the Fedora installation instructions.

Worrying thing is that other packages may also be missing and causing or hiding problems.

comment:2 Changed 4 years ago by jonathan.underwood

Resolution: invalid
Status: closedreopened

Hi Antoine,

I am re-opening this, as we're hitting this on Fedora with 0.15.0 when not using the xpra repos, but rather installing everything from the Fedora repos. I am really keen for xpra to be usable straight from the Fedora repos without enabling the xpra repos, and want to work with you towards that goal.

It seems like the underlying problem here is that you've forked rencode. Do you plan to submit the changes upstream? I haven't looked closely, but what are the reasons for, and extent of, the changes? Thanks in advance.

comment:3 Changed 4 years ago by Antoine Martin

Resolution: invalid
Status: reopenedclosed

I am really keen for xpra to be usable straight from the Fedora repos without enabling the xpra repos


This warning should make absolutely no difference in usability.


It seems like the underlying problem here is that you've forked rencode.


Fork might be too strong a word, just patched and produced a snapshot to make it easier to package for us. (since we've been packaging it for a very long time now, not just for Fedora and centos, but also win32, osx, and Debian / Ubuntu..)

Do you plan to submit the changes upstream?


Done already, a long time ago. (though it looks like googlecode ate it)
IIRC, there is only one change: we've added versioning information.


what are the reasons for, and extent of, the changes


As per above, just adding version information.


AFAICT, your options are (both are trivial):

  • apply the same patch to rencode
  • patch xpra to ignore the lack of version information

FYI: the vast majority of bugs that get reported are due to funky installations (missing dependencies, unusual file locations, etc) and sometimes because of unusual client-server combinations. Having complete version information about all the components is extremely useful to us. (something we've expanded further in the upcoming 0.16 release, to record many of the build-time library version details)

I am closing as invalid again, because we will not be changing the xpra code to remove this warning anytime soon, sorry!

comment:4 Changed 4 years ago by Antoine Martin

Just one more thing: although this sort of duplicating may be frowned upon, I claim that our decision to avoid relying on unreliable external code hosting systems has been vindicated once again. (often a problem with sourceforge, or in this case with googlecode)
These services come and go, they change their download links for no good reason and without warning (how many times has webp moved its download folder for example?) - and we need to have a reliable build infrastructure. Not spending hours updating all the branches to new the new shiny URL every time we need to make new builds. (or miss new releases because they're not where we expect them to be)

comment:5 Changed 4 years ago by jonathan.underwood

Hi Antoine,

I wasn't looking to pass any kind of judgement on your development practices, so please don't take my comment about patching/forking/bundling rencode as any kind of criticism, personal or professional. In fact, you've been very helpful in unbundling rencode from the xpra tarball - that has saved a lot of packaging work for me - prior to that we had to patch xpra to use the system installed rencode, and that patch needed to be rebased with each release.

I understand your reasons for for shipping your own rencode packages. My main concern is ensuring that patches go upstream so we can incorporate them in distros, and to ease shipping and integration of xpra with distros as much as possible. It seems that it is easily solved in this case.

For the record, rencode has moved to github:

https://github.com/aresch/rencode

I'm not an svn guru, so haven't worked out how to find all the commits which touch your rencode - any chance you could give me the magic voodoo?

Changed 4 years ago by Antoine Martin

Attachment: rencode.diff added

diff between upstream on github and our copy

comment:6 Changed 4 years ago by jonathan.underwood

Thanks - I've already done the diff, but I was hoping to pull out the individual revisions so I could look at the log messages for the changes and perhaps generate a patchset for upstream submission rather than a big diff. Anyhow, I'll continue to read the svn manual, or perhaps create a git repo from the svn repo.

comment:7 Changed 4 years ago by Antoine Martin

Sorry, there is no history, and that's kind of why I placed it under our svn: so we could track changes when they are made in the future - which did not happen, no bugs or fixes since it got checked in...
svn log rencode shows:

------------------------------------------------------------------------
r7859 | antoine | 2014-10-02 07:57:55 +0200 (Thu, 02 Oct 2014) | 1 line

forgot to bump version here..
------------------------------------------------------------------------
r7672 | antoine | 2014-09-17 16:38:24 +0200 (Wed, 17 Sep 2014) | 1 line

ensure build files don't get added to svn
------------------------------------------------------------------------
r7671 | antoine | 2014-09-17 16:17:13 +0200 (Wed, 17 Sep 2014) | 9 lines

#683: unbundle python-rencode
* move it to its own directory
* remove build option in our setup.py
* re-instate utf8 option in rencode.loads (even though we don't use it)
* fix some tests (make it python2.6 compatible)
* add spec file
* depend on new package from xpra specfile
* load from global location in packet_encoding.py
* bump the version no
------------------------------------------------------------------------

From the diff attached, I can see the following changes by scrolling through it:

  • indentation tweaks (could easily merge back - whichever one is right)
  • README changes - not sure, not looked at those in detail
  • use stricter imports (avoid 'import *' which can cause lint warnings)
  • faster int2byte (remove runtime branching)
  • more indentation fixes: use 4 spaces consistently
  • remove unused variables i and j
  • use context manager rather than try + finally
  • constified some values to avoid cython warnings (that was the case then, maybe newer Cython versions don't require those workarounds?)
  • don't silently ignore types we don't handle: raise an exception
  • some #@DuplicatedSignature to silence pydev (my IDE) - this can be ignored
  • old references to "Deluge" instead of "rencode" - I should merge this
  • fixes for python2.6: don't use assertIsInstance
  • remove unused import

comment:8 Changed 4 years ago by jonathan.underwood

Thanks Antoine, that explains why I wasn't getting anywhere with svn... I thought it was just my Sunday brain.

I'll open an issue on the upstream github repo pointing to this page and asking to merge the changes.

comment:9 Changed 4 years ago by Antoine Martin

Just to be clear: not all changes should be merged, for example the #@DuplicatedSignature is only useful for pydev users, and I should probably merge a few things myself (Deluge vs rencode, documentation?)

And it would probably be worth bumping the revision to 1.1 or something, to get back in sync.

comment:10 Changed 4 years ago by jonathan.underwood

OK, I have broken the diff up into a series of patches and issued a github pull request. See:

https://github.com/aresch/rencode/pull/3

comment:11 Changed 4 years ago by Antoine Martin

Awesome! thanks.

comment:12 Changed 4 years ago by jonathan.underwood

OK, my pull requst has been merged upstream. While reviewing, the strategy for detecting python 3 was changed from what you have in your tree...

You were doing:

py3 = sys.version >= '3'

whereas what we've merged does:

py3 = sys.version[0] >= 3

Andrew has also made a release:

https://github.com/aresch/rencode/releases/tag/v1.0.3

So, hopefully this brings everything back together.

Note: See TracTickets for help on using tickets.