On Thu, Jun 23, 2011 at 02:35:51PM +0200, Guido Günther wrote: > On Sun, Jun 19, 2011 at 03:55:02PM -0500, Courtney Bane wrote: > > Sure. I've attached an updated patch that handles both forms of homedir > > URLs. This one is a little bit more involved, since I had to modify the > > remote script, in addition to the URL parsing. (Note that, according to > > the bash manpage, the "~username/" part can't be quoted, and from > > testing, dash has the same behavior.) > > Great. Thanks! We only need to... > > [snip patch] > > ...make sure we move the %(base)s into the quotes like "%(base)s%(dir)s" > to be safe when the path contains spaces.
The problem with doing that, is that if anything in the "~username/" part is quoted, the tilde expansion doesn't work anymore. From the bash(1) manpage: > If a word begins with an unquoted tilde character (`~'), all of the > characters preceding the first unquoted slash (or all characters, if > there is no unquoted slash) are considered a tilde-prefix. If none of > the characters in the tilde-prefix are quoted, the characters in the > tilde-prefix following the tilde are treated as a possible login name. > If this login name is the null string, the tilde is replaced with the > value of the shell parameter HOME. If HOME is unset, the home > directory of the user executing the shell is substituted instead. > Otherwise, the tilde-prefix is replaced with the home directory > associated with the specified login name. The equivalent section in the dash(1) manpage isn't as detailed, but from my testing, it works the same way (at least with regards to quoting). (This is why I added the "base" key to the dict returned by parse_remote(); if the tilde expansion would have worked with the path quoted, I would have just modified that.) What I can do instead is to tighten up the regex that extracts the ~username part of the URL such that there can't be any shell special characters or whitespace in it; that will eliminate the need to quote it. The regex I'm currently looking at[1] will accept alphanumerics, underscores, and hyphens; can you think of anything else that should be included? > It would be great if you could also add some doctests (there are > examples in gbp/deb.py) to parse_remote but that's not a must. I've added a number of doctests in my latest patch. I also noticed that the invocation of ssh to run the remote script didn't work properly if there was an explicit port in the URL. I've gone ahead and fixed that as well. [1] r"/(~[a-zA-Z0-9_-]*/)(.*)"
diff --git a/debian/rules b/debian/rules index 86334e9..c76d008 100755 --- a/debian/rules +++ b/debian/rules @@ -59,7 +59,7 @@ checks: links_stamp export GIT_AUTHOR_EMAIL=te...@example.com; \ export GIT_COMMITTER_NAME=$$GIT_AUTHOR_NAME; \ export GIT_COMMITTER_EMAIL=$$GIT_AUTHOR_EMAIL; \ - nosetests --with-doctest + nosetests --exe --with-doctest endif %.py: % diff --git a/gbp-create-remote-repo b/gbp-create-remote-repo index e7cce9b..4d48752 100755 --- a/gbp-create-remote-repo +++ b/gbp-create-remote-repo @@ -26,6 +26,7 @@ import os, os.path import urlparse import subprocess import tty, termios +import re import gbp.deb as du from gbp.command_wrappers import (CommandExecFailed, PristineTar, GitCommand, GitFetch) @@ -49,16 +50,64 @@ def print_config(remote, branches): def parse_remote(remote_url, name, pkg): - """Sanity check our remote URL""" + """ + Sanity check our remote URL + + >>> parse_remote("ssh://host/path/%(pkg)s", "origin", "package") == {'pkg': 'package', 'url': 'ssh://host/path/package', 'dir': '/path/package', 'base': '', 'host': 'host', 'port': None, 'name': 'origin'} + True + >>> parse_remote("ssh://host:22/path/repo.git", "origin", "package") == {'pkg': 'package', 'url': 'ssh://host:22/path/repo.git', 'dir': '/path/repo.git', 'base': '', 'host': 'host', 'port': '22', 'name': 'origin'} + True + >>> parse_remote("ssh://host:22/~/path/%(pkg)s.git", "origin", "package") == {'pkg': 'package', 'url': 'ssh://host:22/~/path/package.git', 'dir': 'path/package.git', 'base': '~/', 'host': 'host', 'port': '22', 'name': 'origin'} + True + >>> parse_remote("ssh://host:22/~user/path/%(pkg)s.git", "origin", "package") == {'pkg': 'package', 'url': 'ssh://host:22/~user/path/package.git', 'dir': 'path/package.git', 'base': '~user/', 'host': 'host', 'port': '22', 'name': 'origin'} + True + >>> parse_remote("git://host/repo.git", "origin", "package") + Traceback (most recent call last): + ... + GbpError: Remote URL must use ssh protocol. + >>> parse_remote("ssh://host/path/repo", "origin", "package") + Traceback (most recent call last): + ... + GbpError: Remote URL needs to contain either a repository name or '%(pkg)s' + >>> parse_remote("ssh://host:asdf/path/%(pkg)s.git", "origin", "package") + Traceback (most recent call last): + ... + GbpError: Remote URL contains invalid port. + >>> parse_remote("ssh://host/~us er/path/%(pkg)s.git", "origin", "package") + Traceback (most recent call last): + ... + GbpError: Remote URL contains invalid ~username expansion. + """ frags = urlparse.urlparse(remote_url) if frags.scheme not in ['ssh', 'git+ssh']: raise GbpError, "Remote URL must use ssh protocol." if not '%(pkg)s' in remote_url and not remote_url.endswith(".git"): raise GbpError, "Remote URL needs to contain either a repository name or '%(pkg)s'" + + if ":" in frags.netloc: + (host, port) = frags.netloc.split(":", 1) + if not re.match(r"^[0-9]+$", port): + raise GbpError, "Remote URL contains invalid port." + else: + host = frags.netloc + port = None + + if frags.path.startswith("/~"): + m = re.match(r"/(~[a-zA-Z0-9_-]*/)(.*)", frags.path) + if not m: + raise GbpError, "Remote URL contains invalid ~username expansion." + base = m.group(1) + path = m.group(2) + else: + base = "" + path = frags.path + remote = { 'pkg' : pkg, 'url' : remote_url % { 'pkg': pkg }, - 'dir' : frags.path % { 'pkg': pkg }, - 'host': frags.netloc, + 'dir' : path % { 'pkg': pkg }, + 'base': base, + 'host': host, + 'port': port, 'name': name} return remote @@ -150,29 +199,31 @@ def main(argv): raise GbpError, "Aborted." # Create and run the remote script - ssh = 'ssh %(host)s sh' % remote remote_script = """ -cat <<EOF set -e umask 002 -if [ -d "%(dir)s" ]; then - echo "Repository at \"%(dir)s\" already exists - giving up." +if [ -d %(base)s"%(dir)s" ]; then + echo "Repository at \"%(base)s%(dir)s\" already exists - giving up." exit 1 fi -mkdir -p "%(dir)s" -cd "%(dir)s" +mkdir -p %(base)s"%(dir)s" +cd %(base)s"%(dir)s" git init --bare --shared echo "%(pkg)s packaging" > description -EOF""" % remote +""" % remote if options.verbose: print remote_script - p1 = subprocess.Popen([remote_script], stdout=subprocess.PIPE, shell=True) - p2 = subprocess.Popen([ssh], stdin=p1.stdout, shell=True) - p2.communicate() - if p2.returncode: - raise GbpError, "Error creating remote repository" + ssh = ["ssh"] + if remote["port"]: + ssh.extend(["-p", remote["port"]]) + ssh.extend([remote["host"], "sh"]) + + proc = subprocess.Popen(ssh, stdin=subprocess.PIPE) + proc.communicate(remote_script) + if proc.returncode: + raise GbpError, "Error creating remote repository" push_branches(remote, branches) if options.track: