xpra icon
Bug tracker and wiki

Opened 6 weeks ago

Closed 5 weeks ago

#2901 closed enhancement (fixed)

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

Reported by: goekce Owned by: goekce
Priority: minor Milestone:
Component: packaging Version: trunk
Keywords: Cc:

Description (last modified by Antoine Martin)

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.

Change History (8)

comment:1 Changed 6 weeks ago by goekce

Priority: majorminor

comment:2 Changed 6 weeks ago by 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.

comment:3 Changed 6 weeks ago by Antoine Martin

Description: modified (diff)
Owner: changed from Antoine Martin to goekce

(fixing link)

comment:4 Changed 6 weeks ago by 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)

In case when the package is built by makepkg (AUR), which sets pkgdir env. variable, then actually the next branch should be taken, which does not require dangerous hacks:

...
elif pkgdir and install_dir.startswith(pkgdir):
    #arch build dir:
    dirs = install_dir.lstrip(pkgdir).split(os.path.sep)
...
Version 2, edited 6 weeks ago by goekce (previous) (next) (diff)

comment:5 Changed 6 weeks ago by Antoine Martin

Does r27684 fix things?

comment:6 Changed 5 weeks ago by 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?

comment:7 Changed 5 weeks ago by 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?

comment:8 Changed 5 weeks ago by goekce

Resolution: fixed
Status: newclosed

Yeah it works!

Grateful for your quick replies Antoine!

Note: See TracTickets for help on using tickets.