`svnversion -n ..` may lead setup.py to loop forever

[goekce]

add_build_info.py uses svnversion -n .., which gets the revision of the upper level directory. If a packager directly checks the src folder out, then svnversion returns Unversioned directory. This leads to an error in setup_html5.py on lines 122 and 125:

data = data.replace('REVISION : "0",',
                    'REVISION : "%i",' % REVISION)
...
data = data.replace('LOCAL_MODIFICATIONS : "0",',
                    'LOCAL_MODIFICATIONS : "%i",' % LOCAL_MODIFICATIONS)

Someone who just wants to get this thing compiled may repair this problem by replacing %i% with %s which leads to an even worse error. Then setup.py` hangs forever at

copying css/20_progress_bar.css -> pkg/usr/share/xpra/css

Catching this error in get_svn_props() or using svnversion instead of svnversion .. could be very helpful for packagers.

PS: I recently discovered this project. I find Xpra great for remote desktop in Covid times. I am very grateful for the authors.

[goekce]

Probably I am wrong. svnversion .. does not lead to the "loop forever" behavior. I could invoke python install successfully without the packaging tool of my distro (makepkg in Archlinux).

Maybe this is related to fakeroot. I will look into that.

[Antoine Martin]

(fixing link)

[goekce]

I found the actual problem. The problem is in src/setup.py. In AUR xpra package is called xpra-svn and the pkg directory can look like /home/user/xpra-svn/pkg/xpra-svn. In this case len(dirs)=6 and i=4 this leads to the forever loop.

...
elif "pkg" in dirs:
       #archlinux
       #ie: "/build/xpra/pkg/xpra/etc" -> "etc"
       i = dirs.index("pkg")
       while i>=0 and len(dirs)>i+1:
           if dirs[i+1] == "xpra":
               dirs = dirs[i+2:]
           try:
               i = dirs.index("pkg")
           except ValueError:
               break
...

A workaround is appending or dirs[i+1] == "xpra-svn".

The description of get_base_conf_dir() suggests that this function should strip buildroot, e.g., "/build/xpra/pkg/xpra/etc" -> "etc". But in get_conf_dir() I see:

def get_conf_dir(install_dir, stripbuildroot=True):
     dirs = get_base_conf_dir(install_dir, stripbuildroot)
     dirs.append("etc")
     dirs.append("xpra")
     return os.path.join(*dirs)

This could lead to a path like etc/etc.

I also noticed that during install phase, this function was called two times with /home/user/xpra-svn/pkg/xpra-svn as the parameter. (There was no extra suffix)

Further discovery: The following elif branch checks for pkgdir which is a variable used in PKGBUILD (used by makepkg). But pkgdir is not exported AFAIK. If this elif branch is for Archlinux only, I may never work in case of makepkg.:

...
elif pkgdir and install_dir.startswith(pkgdir):
    #arch build dir:
    dirs = install_dir.lstrip(pkgdir).split(os.path.sep)
...

[Antoine Martin]

Does r27684 fix things?

[goekce]

Yeah, it does fix Antoine, thanks!

But there are still two potential problems

• the loop will run forever if 'xpra' or 'xpra-svn' is not in path

• I suggest that you add an additional check like if i changed after the last iteration
• or if 'xpra' or 'xpra-svn' is not in path, just error out

• some packagers may directly pull the "src" instead of "trunk", so svnversion .. does not always output an integer. Do you think this is packager's problem?

[Antoine Martin]

the loop will run forever if 'xpra' or 'xpra-svn' is not in path

How about r27686? (untested)

some packagers may directly pull the "src" instead of "trunk"

There used to be a reason, which I cannot remember, so r27687 switches to 'src'.
(we'll see if something breaks)

Does that work for you?

[goekce]

Yeah it works!

Grateful for your quick replies Antoine!

[migration script]

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