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:

Reply via email to