Xpra: Ticket #332: 0.9.1: no "About Xpra" dialog

0.9.1 no longer able to show "About Xpra" dialog:

Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/xpra/scripts/about.py", line 78, in about
    dialog.set_comments("\n".join(get_build_info()))
  File "/usr/lib/python2.7/dist-packages/xpra/scripts/config.py", line 190, in get_build_info
    warn("Error: could not find the build information: %s", e)
TypeError: warn() takes exactly 1 argument (2 given)

This is a regression from 0.9.0.



Sat, 11 May 2013 10:24:54 GMT - Antoine Martin: owner, component changed

That's odd, the build_info.py file is created as part of the build. Do you have any changes, maybe as part of #331, that takes it out somehow?

Are you sure that this is a regression from 0.9.0?? (nothing has changed in this area between 0.9.0 and 0.9.1). If so, please bisect it (only 5 commits - should take 3 steps at most, more likely just 2) - as I am unable to reproduce this here.

r3360 avoids the double error on the error path and should allow you to see the dialog, but it does not explain why your build is missing the build_info.py - that should get fixed too.


Sat, 11 May 2013 10:49:08 GMT - onlyjob:

Ironically my build is not missing "build_info.py": here is what's in it:

LOCAL_MODIFICATIONS='unknown'
BUILD_DATE='2013-05-11'
BUILT_BY='pbuilder'
BUILT_ON='deblab'
BUILD_BIT='64bit'
BUILD_CPU='Intel(R) Pentium(R) D CPU 3.00GHz'
RELEASE_BUILD='True'
REVISION='unknown'

I was suspecting that it could it be effect of backporting r3353 but file got correctly re-generated on build...

I'll try to bisect when I can. At least I can tell that "build_info.py" is generated and installed to package...


Sat, 11 May 2013 10:52:24 GMT - Antoine Martin:

That build_info.py must be missing from your installation, otherwise you would not be seeing this error.

BTW, I'm not convinced you should be backporting stuff like r3353 when the packages have been built like this for a very long time and the next minor update is just a few days away...


Sat, 11 May 2013 11:01:29 GMT - onlyjob:

It's not missing: it is in file system and in the package.

The reason for backporting is that I can't build the package in "pbuilder" without "cython" outside of chroot... It fails on "clean" even before attempting to initialise chroot, as I explained in #331. I'm trying to backport only what's necessary, and I don't even have spare time to backport what I don't have to, least to say that I might be too lazy to do that. :)

As for next minor release it's great but since I can't read your mind I never know how soon next release is going to be...


Sat, 11 May 2013 11:07:35 GMT - Antoine Martin:

If the file is there, this should work:

python -c "from xpra.scripts.config import get_build_info;print(get_build_info())"

But I suspect it will not. It is pretty much equivalent to this (which should also work):

python -c 'from xpra.build_info import BUILT_BY, BUILT_ON, BUILD_DATE, REVISION, LOCAL_MODIFICATIONS;
print(", ".join([BUILT_BY, BUILT_ON, BUILD_DATE, REVISION, LOCAL_MODIFICATIONS]))'

As for backports.. I did say:

I will backport to 0.9.x

And minor releases do happen fairly regularly, when needed.


Sat, 11 May 2013 11:22:42 GMT - onlyjob:

For what's it worth:

On Xpra-0.9.0 strace python2.7 -c "from xpra.scripts.config import get_build_info;print(get_build_info())" 2>&1 | grep build_

stat("/usr/lib/python2.7/dist-packages/xpra/build_info", 0x7fffa9e81820) = -1 ENOENT (No such file or directory)
open("/usr/lib/python2.7/dist-packages/xpra/build_info.x86_64-linux-gnu.so", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/lib/python2.7/dist-packages/xpra/build_info.so", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/lib/python2.7/dist-packages/xpra/build_infomodule.so", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/lib/python2.7/dist-packages/xpra/build_info.py", O_RDONLY) = 3
open("/usr/lib/python2.7/dist-packages/xpra/build_info.pyc", O_RDONLY) = 4

On Xpra-0.9.1 strace python2.7 -c "from xpra.scripts.config import get_build_info;print(get_build_info())" 2>&1 | grep build_:

stat("xpra/build_info", 0x7fff155f6840) = -1 ENOENT (No such file or directory)
open("xpra/build_info.x86_64-linux-gnu.so", O_RDONLY) = -1 ENOENT (No such file or directory)
open("xpra/build_info.so", O_RDONLY)    = -1 ENOENT (No such file or directory)
open("xpra/build_infomodule.so", O_RDONLY) = -1 ENOENT (No such file or directory)
open("xpra/build_info.py", O_RDONLY)    = -1 ENOENT (No such file or directory)
open("xpra/build_info.pyc", O_RDONLY)   = -1 ENOENT (No such file or directory)
write(2, "  File \"xpra/scripts/config.py\","..., 61  File "xpra/scripts/config.py", line 190, in get_build_info

Sat, 11 May 2013 11:27:40 GMT - Antoine Martin:

You're running it from the source tree I think ("."), which doesn't have it since it's built into somewhere else via your build scripts.

It should be fine once deployed though, unless your current working directory also has an "xpra/init.py" (then it's down to the python interpreter where it loads things from - not us)

And btw, I've reviewed every single changeset for 0.9.1 and none of them have anything to do with paths or anything like that, so I am 99% confident the problem is at your end with your (new?) build scripts.


Sat, 11 May 2013 11:35:37 GMT - onlyjob:

I _never_ run Xpra from source tree. I don't even know how. :)

I always build packages so I can see the difference between them. CWD was "/" for running those commands....

I also suspect that it could be build-related but I have only 3 commits between 0.9.0 and 0.9.1 and I can't quite see where it got broken... I'll try to rebuild 0.9.0 again to see if it still works in case something has changed in toolchain...


Sat, 11 May 2013 16:33:01 GMT - onlyjob:

Xpra-0.9.0:

python2.7 -c "from xpra.scripts.config import get_build_info;print(get_build_info())"

('Error: could not find the build information: %s', ValueError("invalid literal for int() with base 10: 'unknown'",))
['Built on deblab by debdev', '2013-05-12']

But due to "warn" modification in xpra/scripts/config.py the above warning stops: TypeError: warn() takes exactly 1 argument (2 given).

The fix for Xpra-0.9.1 is trivial:

--- a/xpra/scripts/config.py
+++ b/xpra/scripts/config.py
@@ -186,9 +186,9 @@
             info.append("revision %s" % REVISION)
         else:
             info.append("revision %s with %s local changes" % (REVISION, LOCAL_MODIFICATIONS))
     except Exception, e:
-        warn("Error: could not find the build information: %s", e)
+        warn("Warning: could not find the build information: %s" % e)
     return info
 def read_config(conf_file):

I also changed "Error" --> "Warning" as the latter IMHO is more adequate because it should be OK to continue on such minor problem.

This patch is tested.


Sun, 12 May 2013 09:39:35 GMT - Antoine Martin:

This is a workaround, not the actual fix: identical to r3360 - see comment:1. Except that missing components are real errors, not warnings: the problem is with packaging and changing the wording to try to make it sound like something isn't broken is not going to get merged.


The real problem is that the build process loses parts of the build info file. In particular, LOCAL_MODIFICATIONS which ends up being set to "unknown" and then we fail trying to parse it. Why, I'm not sure yet.


Sun, 12 May 2013 10:25:44 GMT - onlyjob:

r3360 is a fix. Just applying r3360 is enough to restore "About Xpra" dialog for 0.9.1 just like it was working in 0.9.0.

Please forgive me for misleading information but "build_info.py" was never lost. I think you were right when you suggested that I was invoking command from Xpra source tree. Nevertheless the problem is quite clear from report and first two comments.

The underlying problem is in "add_build_info.py" which tries to create "build_info.py" by extracting metadata from SVN tree. However this is meaningless for tar releases and that's how "build_info.py" from comment2 was created. That's why r3360 fixes the problem with dialog but of course it still complains about "build information".

With next official Debian package (that is based on your tar releases) build information will be populated using the following patch to get rid of warning and to provide correct revision:

--- a/add_build_info.py
+++ b/add_build_info.py
@@ -36,9 +36,9 @@
                 "REVISION" : "unknown",
                 "LOCAL_MODIFICATIONS" : "unknown"
             }
     #find revision:
-    proc = subprocess.Popen("svnversion -n", stdin=None, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True)
+    proc = subprocess.Popen("cat svn-version", stdin=None, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True)
     (out, _) = proc.communicate()
     if proc.returncode!=0:
         print("'svnversion -n' failed with return code %s" % proc.returncode)
         return  props
@@ -63,9 +63,9 @@
     rev = int(rev_str)
     props["REVISION"] = rev
     #find number of local files modified:
     changes = 0
-    proc = subprocess.Popen("svn status", stdin=None, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True)
+    proc = subprocess.Popen("echo 0", stdin=None, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True)
     (out, _) = proc.communicate()
     if proc.poll()!=0:
         print("could not get status of local files")
         return  props

Sun, 12 May 2013 10:36:05 GMT - Antoine Martin:

It is not a fix, it is a kludge. I know it restores "about xpra" but it certainly does not fix the underlying broken packaging.

The change in comment:11 is just plain wrong: only hacking away at a problem rather than figuring out where the file does get lost. (it does get deleted/overwritten/lost somewhere - and this is a debian problem only, rpms are ok)

add_build_info is not meant to override the contents of xpra/build_info with bad values - ever. It only replaces the existing values when it does manage to invoke svnversion / svn status.

And when building packages from a tree without svn, it will fail - which is absolutely fine: in that case it should leave the existing values which come in the source archive and only append what it has access to (build host, etc):

$ cat xpra-0.9.1/xpra/build_info.py
LOCAL_MODIFICATIONS='0'
REVISION='3332'
RELEASE_BUILD='True'

The problem here is that the file is missing, so the bad values get saved as defaults.


Sun, 12 May 2013 11:14:19 GMT - onlyjob:

Replying to totaam:

The problem here is that the file is missing, so the bad values get saved as defaults.

We can define problem like this but to me main and the most user-visible problem was with incorrect handling of warning for "bad values".

From packaging prospective build_info.py either should be re-generated from scratch or do not change. It is not particularly important whenever this file is not getting deleted during "clean" or if it is correctly generated outside of SVN tree on build-time... But packaging will be unnecessary complicated if build_info.py or other files are modified in place during build so please let's not do that...


Sun, 12 May 2013 11:20:37 GMT - Antoine Martin:

We can define problem like this..

I am not defining anything, this is what happens and is the real source of the problems. That's a fact.

But packaging will be unnecessary complicated if build_info.py or other files are modified in place during build so please let's not do that...

This is how it is, and has been for a while. Packaging is a curse and I wished it did not exist. But it does. Now, until someone re-writes the code and makes it work on all the platforms with all the different packaging solutions, this is how it will remain.


Sun, 12 May 2013 11:51:06 GMT - onlyjob:

Replying to totaam:

I am not defining anything, this is what happens and is the real source of the problems. That's a fact.

Whatever. :) At least we've identified yet another problem and nailed it. :)

From my prospective we can close this bug if you can accommodate r3360 to next 0.9.2 release...

I think we've learned the lesson to be careful with build_info.py as well and to make sure that it contains valid data. Now it's all good, besides what you already did no other changes are needed here.

Thanks.

Packaging is a curse and I wished it did not exist. But it does. Now, until someone re-writes the code and makes it work on all the platforms with all the different packaging solutions, this is how it will remain.

That sound a bit annoyed (or stressed?)... I hope that's not because of me...


Sun, 12 May 2013 12:37:31 GMT - Antoine Martin:

Whatever. :) At least we've identified yet another problem and nailed it. :)

No, we haven't. Now I have to figure out how to prevent debuild from squashing/deleting that file.

That sound a bit annoyed (or stressed?)...

Yes, I would rather be doing other things than packaging.


Sun, 12 May 2013 23:04:51 GMT - onlyjob:

Replying to totaam:

Whatever. :) At least we've identified yet another problem and nailed it. :)

No, we haven't.

I meant the problem with broken warning. It was fixed, wasn't it? (certainly it was a minor problem)

Now I have to figure out how to prevent debuild from squashing/deleting that file.

Or can you make sure that it re-created properly with correct data? Also it would be OK to leave the file as is and do not touch it on build. You can avoid deletion by removing file from debian/clean.

Yes, I would rather be doing other things than packaging.

Debian packages will be closely following your releases so you don't have to worry about this unless you want to provide your own packages.


Mon, 13 May 2013 09:18:19 GMT - Antoine Martin:

You can avoid deletion by removing file from debian/clean.

AFAICT this file is not in debian/clean

Debian packages will be closely following your releases so you don't have to worry about this unless you want to provide your own packages.

Well, Debian Wheezy ships a buggy version, and let's not even talk about Squeeze, or all the Ubuntus.

So yes, I will continue to provide packages so people can get a decent version of xpra without too much effort. As I have done for 4 years now.


Mon, 13 May 2013 09:59:14 GMT - onlyjob:

Replying to totaam:

You can avoid deletion by removing file from debian/clean.

AFAICT this file is not in debian/clean

Then it is not a packaging problem but perhaps a python build system conf. :)

Well, Debian Wheezy ships a buggy version

Please speak to us about such issues. Until it was released you never mentioned that you don't want it to be in Wheezy. (I vaguely remember that you've said something that 0.3.* is made for distributions). We could exclude Xpra from stable Debian release and provide an up-to-date version through backports. By the way I'm going to keep fresh Xpra version in backports anyway...

, and let's not even talk about Squeeze, or all the Ubuntus.

I'm with you, but once again please let us know if you want something excluded and we'll see what can be done.

So yes, I will continue to provide packages so people can get a decent version of xpra without too much effort. As I have done for 4 years now.

:)


Mon, 13 May 2013 10:05:37 GMT - Antoine Martin:

I vaguely remember that you've said something that 0.3.* is made for distributions

Yes, it definitely was, and then things moved on to the newer stable branches and this one was left to rot. I was spending a lot of effort maintaining multiple stable branches so that distributions could update at their own pace and avoid the problems with bleeding edge versions, but when I realized that they were just sticking with the old buggy one I gave up on them.
No-one should be running 0.3.x right now, the fact that debian "stable" ships this very buggy version speaks for itself..


Mon, 13 May 2013 11:16:13 GMT - onlyjob:

Replying to totaam:

No-one should be running 0.3.x right now, the fact that debian "stable" ships this very buggy version speaks for itself..

Well that warning came too late... Why you didn't tell us not to ship it? We could remove it but you never warned us...

Maybe even a note or warning in README could do... Nevertheless I'm sure we'll communicate better in the future. :)


Mon, 13 May 2013 11:17:21 GMT - Antoine Martin:

Why you didn't tell us...

Oh, I did.


Sat, 06 Jul 2013 07:57:29 GMT - Antoine Martin: status changed; resolution set

r3781 solves this properly for 0.10 by splitting the build info (generated at build time) from the source info


Fri, 02 Aug 2013 05:44:19 GMT - Antoine Martin: milestone set

added milestone


Sat, 23 Jan 2021 04:51:56 GMT - migration script:

this ticket has been moved to: https://github.com/Xpra-org/xpra/issues/332