Hi Dan
This looks broadly fine but there are a couple of issues.
This file mixes spaces with tabs, which is a really bad idea when
writing Python, please make sure indent is all done with spaces!
I ran migrations/run-all, and hit CTRL+C, but it kept going anyway, and
dumped loads of text over my shell which was annoying. That's probably a
bug in the migrations/run-all script.
When I did manage to kill this migration script, it left a
'git___git_baserock_org_delta_gnome_gnome_desktop' directory hanging
around in my current working directory.
It would be cool if there was an option to reuse an existing git cache
instead of downloading everything again. It's usable as it is now though.
There seems to be a bit of copy+paste in here from
migrations/006-specify-build-system.py, but I think it's fine in this case.
On 18/02/16 15:46, Daniel Firth wrote:
From: Dan Firth <dan.firth(a)codethink.co.uk>
---
migrations/008-submodules-in-strata.py | 219 +++++++++++++++++++++++++++++++++
1 file changed, 219 insertions(+)
create mode 100755 migrations/008-submodules-in-strata.py
diff --git a/migrations/008-submodules-in-strata.py
b/migrations/008-submodules-in-strata.py
new file mode 100755
index 0000000..21b3b71
--- /dev/null
+++ b/migrations/008-submodules-in-strata.py
@@ -0,0 +1,219 @@
+#!/usr/bin/env python
+# Copyright (C) 2015 Codethink Limited
It's 2016
...
+'''Migration to Baserock Definitions format version 8.
+
+In definitions version 8, submodules must be declared explicitly for all chunks.
+
+'''
Is this description right? Surely it's still possible to use submodules
without declaring them in the stratum, it's just more difficult that way?
+import requests
+import yaml
This 'yaml' import isn't used anywhere
+import string
...
+# From ybd.git file repos.py at commit
eb3bf397ba729387f0d4145a8df8d3c1f9eb707f
+
+def get_repo_url(repo):
+ for alias, url in REPO_ALIASES.items():
+ repo = repo.replace(alias, url)
+ if repo.endswith('.git'):
+ repo = repo[:-4]
+ return repo
+
+def get_repo_name(repo):
+ ''' Convert URIs to strings that only contain digits, letters, _ and %.
+ NOTE: this naming scheme is based on what lorry uses
+ '''
+ valid_chars = string.digits + string.ascii_letters + '%_'
+ transl = lambda x: x if x in valid_chars else '_'
+ return ''.join([transl(x) for x in get_repo_url(repo)])
+
+
+## End of code based on morph.git file buildsystem.py.
This doesn't match up
with the comment at the start of this block, which
is correct?
+def add_submodules_to_strata(contents, filename):
+ assert contents['kind'] == 'stratum'
+
+ changed = False
+ for chunk_ref in contents.get('chunks', []):
+ chunk_git_url = get_repo_url(chunk_ref['repo'])
+ chunk_git_ref = chunk_ref['ref']
+
+ if 'submodules' in chunk_ref:
+ continue
+ try:
+ toplevel_file_list = get_toplevel_file_list_from_repo(
+ chunk_git_url, chunk_git_ref)
+ except Exception as e:
+ message = (
+ "Unable to look up one or more repos on remote Git "
+ "server %s. If you are using a Trove that is not %s, "
+ "please edit the TROVE_HOST constant in this script "
+ "and run it again" % (TROVE_HOST, TROVE_HOST))
...
I got this error, but it was pretty useless because it didn't tell me
what repo it had a problem with, and the TROVE_HOST constant is correct.
I suggest changing it to this:
+ except Exception as e:
+ message = (
+ "Unable to look up repo %s on remote Git server %s.
Check that "
+ "the repo URL is correct." % (chunk_git_url, TROVE_HOST))
+ warning = (
+ "If you are using a Trove that is not %s, please edit the "
+ "TROVE_HOST constant in this script and run it again." %
+ TROVE_HOST)
+ if FAIL_ON_REMOTE_CACHE_ERRORS:
+ raise RuntimeError(message + " " + warning)
+ else:
+ warnings.warn(message)
+ warnings.warn(warning)
+ continue
The reason for separating the message into 2 is that warnings.warn()
hides duplicate messages. So with this I see useful warnings, but not noise:
WARNING: Unable to look up repo
http://gitorious.org/supybot/supybot on remote Git server
git.baserock.org. Check that the repo URL is correct.
WARNING: If you are using a Trove that is not
git.baserock.org,
please edit the TROVE_HOST constant in this script and run it again.
WARNING: Unable to look up repo
git://github.com/mgedmin/irclog2html on remote Git server
git.baserock.org. Check that the repo URL is correct.
WARNING: Unable to look up repo
git://git.baserock.org/delta/diff-lcs on remote Git server
git.baserock.org. Check that the repo URL is correct.
WARNING: Unable to look up repo
git://github.com/sonyxperiadev/pygerrit on remote Git server
git.baserock.org. Check that the repo URL is correct.
Most of these errors are because I'm running it in infrastruture.git,
which points to some repos that are not repos are mirrored on the Trove
(oops!).
Other than that, this is really cool, thanks!
--
Sam Thursfield, Codethink Ltd.
Office telephone: +44 161 236 5575