[fetch] Actually fetch from the provided ref.
Previously there was confusion around ref vs revision; The checkout
function signature expected a ref, but was always being passed a revision.
Now we pass both the ref and the revision when we have them; The fetch
operation will now provide the ref during the fetch which should allow
non-master refs.
R=gbeaty@chromium.org, tandrii@chromium.org
Change-Id: Ie14f7c3f388f90bfeb49ad8a4145c04305dee1b4
Reviewed-on: https://chromium-review.googlesource.com/c/infra/luci/recipes-py/+/1693768
Reviewed-by: Garrett Beaty <gbeaty@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Robbie Iannucci <iannucci@chromium.org>
(cherry picked from commit b077e6d7b5e202070b28b0e7e1be95396c423eec)
Reviewed-on: https://chromium-review.googlesource.com/c/infra/luci/recipes-py/+/1698495
diff --git a/recipe_engine/internal/autoroll_impl/candidate_algorithm.py b/recipe_engine/internal/autoroll_impl/candidate_algorithm.py
index b68d56c..015965d 100644
--- a/recipe_engine/internal/autoroll_impl/candidate_algorithm.py
+++ b/recipe_engine/internal/autoroll_impl/candidate_algorithm.py
@@ -184,7 +184,7 @@
# seemed like a worse alternative.
dep_path = os.path.join(recipe_deps.recipe_deps_path, repo_name)
backend = GitBackend(dep_path, dep.url)
- backend.checkout(dep.revision)
+ backend.checkout(dep.branch, dep.revision)
# Add any newly discovered repos to our repos set. We don't replace
# repos because we want to keep the metadata for all already-rolled
diff --git a/recipe_engine/internal/fetch.py b/recipe_engine/internal/fetch.py
index 577dce3..306df1c 100644
--- a/recipe_engine/internal/fetch.py
+++ b/recipe_engine/internal/fetch.py
@@ -120,12 +120,13 @@
"""
raise NotImplementedError()
- def checkout(self, refspec):
+ def checkout(self, refspec, revision=None):
"""Checks out given |repo| at |refspec| to |checkout_dir|.
Args:
refspec (str) - a git refspec which is resolvable on the
remote git repo, e.g. 'refs/heads/master', 'deadbeef...face', etc.
+ revesion (str|None) - A pre-resolved revision to checkout.
"""
# TODO(iannucci): Alter the contract for this method so that it only checks
# out the files referred to according to the rules that the bundle
@@ -295,8 +296,9 @@
LOGGER.info('fetching %s', self.repo_url)
self._git(*args)
- def checkout(self, refspec):
- revision = self.resolve_refspec(refspec)
+ def checkout(self, refspec, revision=None):
+ if not revision:
+ revision = self.resolve_refspec(refspec)
LOGGER.info('Checking out %r in %s (%s)',
revision, self.checkout_dir, self.repo_url)
@@ -333,7 +335,7 @@
if bool(rev)
]
- def _resolve_refspec_impl(self, revision):
+ def _resolve_refspec_impl(self, refspec):
self._ensure_local_repo_exists()
# Can return e.g.
#
@@ -346,10 +348,10 @@
ref: csum
for csum, ref in (
l.split()
- for l in self._git('ls-remote', source, revision).splitlines()
+ for l in self._git('ls-remote', source, refspec).splitlines()
)
}
- rslt = mapping[revision]
+ rslt = mapping[refspec]
assert self.is_resolved_revision(rslt), repr(rslt)
return rslt
diff --git a/recipe_engine/internal/recipe_deps.py b/recipe_engine/internal/recipe_deps.py
index 54ae911..e365499 100644
--- a/recipe_engine/internal/recipe_deps.py
+++ b/recipe_engine/internal/recipe_deps.py
@@ -164,7 +164,7 @@
dep_path = os.path.join(recipe_deps_path, repo_name)
backend = fetch.GitBackend(dep_path, dep.url)
- backend.checkout(dep.revision)
+ backend.checkout(dep.branch, dep.revision)
repos[repo_name] = RecipeRepo.create(ret, dep_path, backend=backend)
# This makes `repos` unmodifiable. object.__setattr__ is needed to get
diff --git a/unittests/fetch_test.py b/unittests/fetch_test.py
index 2cd37aa..065c6f4 100755
--- a/unittests/fetch_test.py
+++ b/unittests/fetch_test.py
@@ -99,8 +99,8 @@
]
def g_ls_remote(self):
- return self.g(['-C', 'dir', 'ls-remote', 'repo', 'revision'],
- 'a'*40 + '\trevision')
+ return self.g(['-C', 'dir', 'ls-remote', 'repo', 'ref'],
+ 'a'*40 + '\tref')
@mock.patch('os.path.isdir')
@mock.patch(fetch.__name__+'.GitBackend._execute')
@@ -108,13 +108,12 @@
isdir.return_value = False
git.side_effect = multi(*([
self.g(['init', 'dir']),
- self.g_ls_remote(),
] + self.g_metadata_calls() + [
self.g(['-C', 'dir', 'diff', '--quiet', 'a'*40], CPE('', 1)),
self.g(['-C', 'dir', 'reset', '-q', '--hard', 'a'*40])
]))
- fetch.GitBackend('dir', 'repo').checkout('revision')
+ fetch.GitBackend('dir', 'repo').checkout('ref', 'a'*40)
self.assertMultiDone(git)
@@ -129,7 +128,7 @@
self.g(['-C', 'dir', 'reset', '-q', '--hard', 'a'*40])
]))
- fetch.GitBackend('dir', 'repo').checkout('revision')
+ fetch.GitBackend('dir', 'repo').checkout('ref')
self.assertMultiDone(git)
isdir.assert_has_calls([
@@ -140,13 +139,12 @@
@mock.patch(fetch.__name__+'.GitBackend._execute')
def test_existing_checkout_same_revision(self, git, isdir):
isdir.return_value = True
- git.side_effect = multi(*([
- self.g_ls_remote(),
- ] + self.g_metadata_calls() + [
+ git.side_effect = multi(*(
+ self.g_metadata_calls() + [
self.g(['-C', 'dir', 'diff', '--quiet', 'a'*40]),
]))
- fetch.GitBackend('dir', 'repo').checkout('revision')
+ fetch.GitBackend('dir', 'repo').checkout('ref', 'a'*40)
self.assertMultiDone(git)
isdir.assert_has_calls([
@@ -162,7 +160,7 @@
git.side_effect = _mock_execute
with self.assertRaises(exceptions.GitFetchError):
- fetch.GitBackend('dir', 'repo').checkout('revision')
+ fetch.GitBackend('dir', 'repo').checkout('ref', 'a'*40)
git.assert_called_once_with('GIT', 'init', 'dir')
@@ -171,18 +169,16 @@
def test_rev_parse_fail(self, git, isdir):
isdir.return_value = True
git.side_effect = multi(*(
- self.g_ls_remote(),
-
self.g(
['-C', 'dir', 'show', '-s', '--format=%aE%n%ct%n%B', 'a'*40],
CPE(1, 'nope')),
- self.g(['-C', 'dir', 'fetch', 'repo', 'revision']),
+ self.g(['-C', 'dir', 'fetch', 'repo', 'ref']),
self.g(['-C', 'dir', 'diff', '--quiet', 'a'*40], CPE('', 1)),
self.g(['-C', 'dir', 'reset', '-q', '--hard', 'a'*40]),
))
- fetch.GitBackend('dir', 'repo').checkout('revision')
+ fetch.GitBackend('dir', 'repo').checkout('ref', 'a'*40)
self.assertMultiDone(git)
@@ -195,7 +191,7 @@
self.g_ls_remote(),
] + self.g_metadata_calls()))
- result = fetch.GitBackend('dir', 'repo').commit_metadata('revision')
+ result = fetch.GitBackend('dir', 'repo').commit_metadata('ref')
self.assertEqual(result, fetch.CommitMetadata(
revision = 'a'*40,
author_email = 'foo@example.com',
@@ -219,7 +215,7 @@
self.g_ls_remote(),
] + self.g_metadata_calls(config=spec)))
- result = fetch.GitBackend('dir', 'repo').commit_metadata('revision')
+ result = fetch.GitBackend('dir', 'repo').commit_metadata('ref')
self.assertEqual(result, fetch.CommitMetadata(
revision = 'a'*40,
author_email = 'foo@example.com',
@@ -242,7 +238,7 @@
self.g_ls_remote(),
] + self.g_metadata_calls(config=spec, diff=tuple([IRC]))))
- result = fetch.GitBackend('dir', 'repo').commit_metadata('revision')
+ result = fetch.GitBackend('dir', 'repo').commit_metadata('ref')
self.assertEqual(result, fetch.CommitMetadata(
revision = 'a'*40,
author_email = 'foo@example.com',
@@ -264,7 +260,7 @@
self.g_ls_remote()
] + self.g_metadata_calls(config=spec, diff=tuple(['recipes/foo']))))
- result = fetch.GitBackend('dir', 'repo').commit_metadata('revision')
+ result = fetch.GitBackend('dir', 'repo').commit_metadata('ref')
self.assertEqual(result, fetch.CommitMetadata(
revision = 'a'*40,
author_email = 'foo@example.com',
@@ -288,7 +284,7 @@
self.g_ls_remote()
] + self.g_metadata_calls(config=spec)))
- result = fetch.GitBackend('dir', 'repo').commit_metadata('revision')
+ result = fetch.GitBackend('dir', 'repo').commit_metadata('ref')
self.assertEqual(result, fetch.CommitMetadata(
revision = 'a'*40,
author_email = 'foo@example.com',
@@ -311,7 +307,7 @@
self.g_ls_remote()
] + self.g_metadata_calls(config=spec, diff=['.gitattributes'])))
- result = fetch.GitBackend('dir', 'repo').commit_metadata('revision')
+ result = fetch.GitBackend('dir', 'repo').commit_metadata('ref')
self.assertEqual(result, fetch.CommitMetadata(
revision = 'a'*40,
author_email = 'foo@example.com',
@@ -333,7 +329,7 @@
self.g_ls_remote()
] + self.g_metadata_calls(config=spec, diff=['subdir/.gitattributes'])))
- result = fetch.GitBackend('dir', 'repo').commit_metadata('revision')
+ result = fetch.GitBackend('dir', 'repo').commit_metadata('ref')
self.assertEqual(result, fetch.CommitMetadata(
revision = 'a'*40,
author_email = 'foo@example.com',
