Merge lp:~nuclearbob/utah/bzr-symlink into lp:utah

[Max Brustkern]

This patch follows symlinks in bzr branches. I've tested it on lp:~ubuntu-server-dev/utah/server-tests-quantal/preseeds/floodlight.preseed, which is a symlink to lp:~ubuntu-server-dev/utah/server-tests-quantal/preseeds/default.preseed This should fix lp:1089584

[lp:~nuclearbob/utah/bzr-symlink]

791. By Max Brustkern on 2012-12-12

Throw an exception for absolute paths

[Max Brustkern (nuclearbob)]

Now throws an exception when used on a symlink to an absolute path (i.e. lp:~nuclearbob/+junk/test/blarg)

[Joe Talbott (joetalbott)]

Looks good to me.

review: Approve

[Javier Collado (javier.collado)]

Looks nice. I've tested it and works fine as well.

Please fix the pep8 error in the exception message:
url.py:225:21: E128 continuation line under-indented for visual indent

Aside from that, may I suggest to create a link to the file returned by the
recursive call to `url_argument`?

=== modified file 'utah/url.py'
--- utah/url.py 2012-12-12 20:21:28 +0000
+++ utah/url.py 2012-12-13 08:32:33 +0000
@@ -225,7 +225,9 @@
                     'path in a bazaar branch: {}'.format(url))
             dirname = os.path.dirname(url)
             link_url = os.path.join(dirname, link_path)
- return url_argument(link_url)
+ target_path = url_argument(link_url)
+ os.remove(full_url)
+ os.link(target_path, full_url)
         elif not os.path.isfile(full_url):
             raise ArgumentTypeError("URL doesn't point to a file "
                                     'in a bazaar branch: {}'

Isn't really important, but this will preserve the original filename, so when
we look at the logs, we don't have the feeling that the preseed doesn't look
like the good one (I've used `os.link`, but `os.symlink` should work as well).

review: Needs Fixing

[lp:~nuclearbob/utah/bzr-symlink]

792. By Max Brustkern on 2012-12-13

Addressed Javier's suggestions

[Max Brustkern (nuclearbob)]

What version of pep8 are you using? That didn't come up in mine, so I may need to update. I think I have the issue addressed now, and I do like the idea of linking to the target path.

review: Needs Resubmitting

[Javier Collado (javier.collado)]

Regarding pep8, I've got version 1.3.3 that is higher than the one currently
available in the archive.

To get the latest version use:
sudo pip install --upgrade pep8

[Max Brustkern (nuclearbob)]

Since I made the changes Javier mentioned, I'm going to go ahead and merge this so we can get it into the lab.

[Javier Collado (javier.collado)]

Thanks.

review: Approve

=== modified file 'utah/url.py'
--- utah/url.py	2012-12-03 14:02:18 +0000
+++ utah/url.py	2012-12-13 13:50:24 +0000
@@ -218,7 +218,18 @@
             raise ArgumentTypeError('Bazaar export error: {}'
                                     .format(exception))
         full_url = os.path.join(tmp_dir, os.path.basename(url))
-        if not os.path.isfile(full_url):
+        if os.path.islink(full_url):
+            link_path = os.readlink(full_url)
+            if link_path.startswith('/'):
+                raise ArgumentTypeError('URL points to a link to an absolute '
+                                        'path in a bazaar branch: {}'
+                                        .format(url))
+            dirname = os.path.dirname(url)
+            link_url = os.path.join(dirname, link_path)
+            target_path = url_argument(link_url)
+            os.remove(full_url)
+            os.link(target_path, full_url)
+        elif not os.path.isfile(full_url):
             raise ArgumentTypeError("URL doesn't point to a file "
                                     'in a bazaar branch: {}'
                                     .format(url))