launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #22777
[Merge] ~tintou/git-build-recipe:master into git-build-recipe:master
Corentin Noël has proposed merging ~tintou/git-build-recipe:master into git-build-recipe:master.
Commit message:
Handle submodules
It is possible to fetch the submodules before building the package by specifying submodule-strategy {normal, recursive} in the header of the recipe
The default behavior is not changed
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~tintou/git-build-recipe/+git/git-build-recipe-1/+merge/351699
I had to refactor the handling of the `deb-version` optional variable as there are now two optional variables.
by default, there is no additional command run, if the submodule strategy is changed then the repository is synced and updated (the behavior is inspired from what is described in https://docs.gitlab.com/ee/ci/yaml/README.html#git-submodule-strategy )
I haven't tested it on a real repository though.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~tintou/git-build-recipe:master into git-build-recipe:master.
diff --git a/gitbuildrecipe/recipe.py b/gitbuildrecipe/recipe.py
index d1700f2..b2d9703 100644
--- a/gitbuildrecipe/recipe.py
+++ b/gitbuildrecipe/recipe.py
@@ -281,6 +281,11 @@ class CheckoutFailed(GitCommandFailed):
_cmd = "checkout"
+class SubmoduleFailed(GitCommandFailed):
+
+ _cmd = "submodule"
+
+
class MergeFailed(GitCommandFailed):
_cmd = "merge"
@@ -320,6 +325,20 @@ def pull_or_clone(base_branch, target_path):
base_branch.git_call("checkout", base_branch.commit)
except subprocess.CalledProcessError as e:
raise CheckoutFailed(e.output)
+ if base_branch.submodule_strategy is not None:
+ if base_branch.submodule_strategy == "recursive":
+ try:
+ base_branch.git_call("submodule", "sync", "--recursive")
+ base_branch.git_call("submodule", "update", "--init",
+ "--recursive")
+ except subprocess.CalledProcessError as e:
+ raise SubmoduleFailed(e.output)
+ else:
+ try:
+ base_branch.git_call("submodule", "sync")
+ base_branch.git_call("submodule", "update", "--init")
+ except subprocess.CalledProcessError as e:
+ raise SubmoduleFailed(e.output)
def fetch_branches(child_branch):
@@ -608,7 +627,7 @@ class RecipeBranch:
`self.url`.
"""
- def __init__(self, name, url, revspec=None):
+ def __init__(self, name, url, submodule_strategy=None, revspec=None):
"""Create a `RecipeBranch`.
:param name: The name for the branch, or None if it is the root.
@@ -618,6 +637,7 @@ class RecipeBranch:
"""
self.name = name
self.url = url
+ self.submodule_strategy = submodule_strategy
self.revspec = revspec
self.child_branches = []
self.commit = None
@@ -715,6 +735,7 @@ class RecipeBranch:
raise AssertionError(
"%s already has a branch nested there" % location)
self.child_branches.append(NestInstruction(branch, location))
+ self.apply_submodule_strategy(location)
def run_command(self, command):
"""Set a command to be run.
@@ -751,6 +772,9 @@ class RecipeBranch:
return True
return False
+ def apply_submodule_strategy(self, subpath=None):
+ """Initialize the submodules depending of the defined strategy"""
+
def iter_all_instructions(self):
"""Iter over all instructions under this branch."""
for instruction in self.child_branches:
@@ -796,13 +820,14 @@ class RecipeBranch:
class BaseRecipeBranch(RecipeBranch):
"""The `RecipeBranch` that is at the root of a recipe."""
- def __init__(self, url, deb_version, format_version, revspec=None):
+ def __init__(self, url, deb_version, format_version,
+ submodule_strategy=None, revspec=None):
"""Create a `BaseRecipeBranch`.
:param deb_version: The template to use for the version number.
Should be None for anything except the root branch.
"""
- super().__init__(None, url, revspec=revspec)
+ super().__init__(None, url, submodule_strategy, revspec=revspec)
self.deb_version = deb_version
self.format_version = format_version
if not urlparse(url).scheme:
@@ -910,8 +935,9 @@ class RecipeParser:
self.current_indent_level = 0
self.seen_nicks = set()
self.seen_paths = {".": 1}
- version, deb_version = self.parse_header()
+ version, deb_version, submodule_strategy = self.parse_header()
self.version = version
+ self.submodule_strategy = submodule_strategy
last_instruction = None
active_branches = []
last_branch = None
@@ -940,7 +966,8 @@ class RecipeParser:
revspec = self.parse_optional_revspec()
self.new_line()
last_branch = BaseRecipeBranch(
- url, deb_version, self.version, revspec=revspec)
+ url, deb_version, self.version, self.submodule_strategy,
+ revspec=revspec)
active_branches = [last_branch]
last_instruction = ""
else:
@@ -968,7 +995,8 @@ class RecipeParser:
else:
revspec = self.parse_optional_revspec()
self.new_line(instruction)
- last_branch = RecipeBranch(branch_id, url, revspec=revspec)
+ last_branch = RecipeBranch(branch_id, url,
+ self.submodule_strategy, revspec=revspec)
if instruction == NEST_INSTRUCTION:
active_branches[-1].nest_branch(location, last_branch)
elif instruction == MERGE_INSTRUCTION:
@@ -994,9 +1022,14 @@ class RecipeParser:
if version > self.NEWEST_VERSION:
self.throw_parse_error("Unknown format: '%s'" % str(version))
self.take_chars(len(version_str))
- deb_version = self.parse_optional_deb_version()
+ options = self.parse_optional_values()
+ deb_version = options.get('deb-version', None)
+ submodule_strategy = options.get('submodule-strategy', None)
+ if submodule_strategy not in (None, 'normal', 'recursive'):
+ self.throw_parse_error("Unknown submodule strategy: '%s'" %
+ str(submodule_strategy))
self.new_line()
- return version, deb_version
+ return version, deb_version, submodule_strategy
def parse_instruction(self, permitted_instructions=None):
if self.version < 0.2:
@@ -1079,17 +1112,24 @@ class RecipeParser:
self.parse_whitespace("the revspec")
return self.take_to_whitespace("the revspec")
- def parse_optional_deb_version(self):
- self.parse_whitespace("'deb-version'", require=False)
- actual = self.peek_to_whitespace()
- if actual is None:
- # End of line, no version
- return None
- if actual != "deb-version":
- self.throw_expecting_error("deb-version", actual)
- self.take_chars(len("deb-version"))
- self.parse_whitespace("a value for 'deb-version'")
- return self.take_to_whitespace("a value for 'deb-version'")
+ def parse_optional_values(self):
+ options = dict()
+ expected_options = ("deb-version", "submodule-strategy")
+ for i in range(len(expected_options)):
+ self.parse_whitespace("next option", require=False)
+ actual = self.peek_to_whitespace()
+ if actual is None:
+ # End of line
+ continue;
+ else:
+ if actual not in expected_options:
+ self.throw_expecting_error("' or '".join(expected_options),
+ actual)
+ self.take_chars(len(actual))
+ self.parse_whitespace("a value for '%s'" % actual)
+ options[actual] = self.take_to_whitespace(
+ "a value for '%s'" % actual)
+ return options
def parse_optional_revspec(self):
self.parse_whitespace(None, require=False)
diff --git a/gitbuildrecipe/tests/test_recipe.py b/gitbuildrecipe/tests/test_recipe.py
index ed7634d..fdee41e 100644
--- a/gitbuildrecipe/tests/test_recipe.py
+++ b/gitbuildrecipe/tests/test_recipe.py
@@ -95,6 +95,17 @@ class RecipeParserTests(GitTestCase):
branch = "http://foo.org/\n";
self.get_recipe(header + branch)
+ def test_parses_reversed_strategy_deb_version(self):
+ header = ("# git-build-recipe format 0.4 submodule-strategy normal "
+ "deb-version " + self.deb_version + "\n")
+ branch = "http://foo.org/\n";
+ self.get_recipe(header + branch)
+
+ def test_parses_missing_deb_version_with_strategy(self):
+ header = "# git-build-recipe format 0.4 submodule-strategy normal\n"
+ branch = "http://foo.org/\n";
+ self.get_recipe(header + branch)
+
def tests_rejects_non_comment_to_start(self):
self.assertParseError(1, 1, "Expecting '#', got 'g'",
self.get_recipe, "git-build-recipe")
@@ -128,9 +139,9 @@ class RecipeParserTests(GitTestCase):
self.get_recipe,
"# git-build-recipe format 10000 deb-version 1\n")
- def test_rejects_invalid_deb_version_marker(self):
- self.assertParseError(1, 31, "Expecting 'deb-version', "
- "got 'deb'", self.get_recipe,
+ def test_rejects_invalid_parameter(self):
+ self.assertParseError(1, 31, "Expecting 'deb-version' or "
+ "'submodule-strategy', got 'deb'", self.get_recipe,
"# git-build-recipe format 0.1 deb")
def tests_rejects_no_deb_version_value(self):
@@ -138,9 +149,19 @@ class RecipeParserTests(GitTestCase):
"a value for 'deb-version'", self.get_recipe,
"# git-build-recipe format 0.1 deb-version")
- def tests_rejects_extra_text_after_deb_version(self):
- self.assertParseError(1, 45, "Expecting the end of the line, "
- "got 'foo'", self.get_recipe,
+ def test_unknown_submodule_strategy(self):
+ self.assertParseError(1, 53, "Unknown submodule strategy: 'foo'",
+ self.get_recipe,
+ "# git-build-recipe format 0.4 submodule-strategy foo\n")
+
+ def tests_rejects_no_submodule_strategy_value(self):
+ self.assertParseError(1, 49, "End of line while looking for "
+ "a value for 'submodule-strategy'", self.get_recipe,
+ "# git-build-recipe format 0.4 submodule-strategy")
+
+ def tests_rejects_extra_parameter(self):
+ self.assertParseError(1, 45, "Expecting 'deb-version' or "
+ "'submodule-strategy', got 'foo'", self.get_recipe,
"# git-build-recipe format 0.1 deb-version 1 foo")
def tests_rejects_indented_base_branch(self):
@@ -539,6 +560,16 @@ class BuildTreeTests(GitTestCase):
self.assertEqual(commit, target.rev_parse("HEAD"))
self.assertEqual(commit, base_branch.commit)
+ def test_build_tree_single_branch_recursive(self):
+ source = GitRepository("source")
+ commit = source.commit("one")
+ base_branch = BaseRecipeBranch("source", "1", 0.2, "recursive")
+ build_tree(base_branch, "target")
+ self.assertThat("target", PathExists())
+ target = GitRepository("target", allow_create=False)
+ self.assertEqual(commit, target.rev_parse("HEAD"))
+ self.assertEqual(commit, base_branch.commit)
+
def test_build_tree_single_branch_dir_not_branch(self):
source = GitRepository("source")
source.commit("one")
