Skip to content

Commit 5dde3b9

Browse files
committed
fix(run-task)!: fetch head_rev if head_ref isn't provided
BREAKING CHANGE: omitting `head_ref` no longer fetches all heads Previously we were fetching all heads in this case so that we could then run `git checkout <head_rev>` successfully. But it's much faster to just explicitly fetch `<head_rev>` in the first place. This also refactors `git_fetch` to be able to fetch multiple targets at once.
1 parent ad39854 commit 5dde3b9

File tree

2 files changed

+67
-24
lines changed

2 files changed

+67
-24
lines changed

src/taskgraph/run-task/run-task

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -568,7 +568,7 @@ def configure_volume_posix(volume, user, group, running_as_root):
568568

569569
def git_fetch(
570570
destination_path: str,
571-
ref: str,
571+
*targets: str,
572572
remote: str = "origin",
573573
tags: bool = False,
574574
env: Optional[dict[str, str]] = None,
@@ -578,7 +578,7 @@ def git_fetch(
578578
# `--force` is needed to be able to update an existing outdated tag.
579579
args.extend(["--tags", "--force"])
580580

581-
args.extend([remote, ref])
581+
args.extend([remote, *set(targets)])
582582
retry_required_command(b"vcs", args, cwd=destination_path, extra_env=env)
583583

584584

@@ -631,6 +631,8 @@ def git_checkout(
631631
ssh_key_file: Optional[Path],
632632
ssh_known_hosts_file: Optional[Path],
633633
):
634+
assert head_ref or head_rev
635+
634636
env = {
635637
# abort if transfer speed is lower than 1kB/s for 1 minute
636638
"GIT_HTTP_LOW_SPEED_LIMIT": "1024",
@@ -696,9 +698,21 @@ def git_checkout(
696698
if head_ref and not head_ref.startswith("refs/heads/") and base_repo == head_repo:
697699
tags = True
698700

699-
# If a head_ref isn't provided, we fetch all refs from head_repo, which may be slow.
700-
target = head_ref if head_ref else "+refs/heads/*:refs/remotes/work/*"
701-
git_fetch(destination_path, target, remote=head_repo, tags=tags, env=env)
701+
# Fetch head_ref and/or head_rev
702+
targets = []
703+
if head_ref:
704+
targets.append(head_ref)
705+
if not targets:
706+
# If head_ref wasn't provided, we fallback to head_rev.
707+
targets.append(head_rev)
708+
709+
git_fetch(
710+
destination_path,
711+
*targets,
712+
remote=head_repo,
713+
tags=tags,
714+
env=env,
715+
)
702716

703717
args = [
704718
"git",

test/test_scripts_run_task.py

Lines changed: 48 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import functools
12
import io
23
import os
34
import site
@@ -332,34 +333,42 @@ def mock_git_repo():
332333
["git", "config", "user.email", "[email protected]"], cwd=repo_path
333334
)
334335

335-
def _commit_file(message, filename):
336-
with open(os.path.join(repo, filename), "w") as fout:
337-
fout.write("test file content")
336+
def _commit_file(message, filename, content):
337+
filepath = os.path.join(repo, filename)
338+
os.makedirs(os.path.dirname(filepath), exist_ok=True)
339+
with open(filepath, "w") as fout:
340+
fout.write(content)
338341
subprocess.check_call(["git", "add", filename], cwd=repo_path)
339342
subprocess.check_call(["git", "commit", "-m", message], cwd=repo_path)
340343
return git_current_rev(repo_path)
341344

342345
# Commit mainfile (to main branch)
343-
main_commit = _commit_file("Initial commit", "mainfile")
346+
main_commits = [_commit_file("Initial commit", "mainfile", "foo")]
344347

345348
# New branch mybranch
346349
subprocess.check_call(["git", "checkout", "-b", "mybranch"], cwd=repo_path)
347-
# Commit branchfile to mybranch branch
348-
branch_commit = _commit_file("File in mybranch", "branchfile")
350+
# Create two commits to mybranch
351+
branch_commits = []
352+
branch_commits.append(
353+
_commit_file("Add file in mybranch2", "branchfile", "bar")
354+
)
355+
branch_commits.append(
356+
_commit_file("Update file in mybranch", "branchfile", "baz")
357+
)
349358

350359
# Set current branch back to main
351360
subprocess.check_call(["git", "checkout", "main"], cwd=repo_path)
352-
yield {"path": repo_path, "main": main_commit, "branch": branch_commit}
361+
yield {"path": repo_path, "main": main_commits, "branch": branch_commits}
353362

354363

355364
@pytest.mark.parametrize(
356-
"base_rev,head_ref,files,hash_key",
365+
"base_rev,head_ref,files,hash_key,exc",
357366
[
358-
(None, None, ["mainfile"], "main"),
359-
(None, "main", ["mainfile"], "main"),
360-
(None, "mybranch", ["mainfile", "branchfile"], "branch"),
361-
("main", "main", ["mainfile"], "main"),
362-
("main", "mybranch", ["mainfile", "branchfile"], "branch"),
367+
(None, None, ["mainfile"], "main", AssertionError),
368+
(None, "main", ["mainfile"], "main", None),
369+
(None, "mybranch", ["mainfile", "branchfile"], "branch", None),
370+
("main", "main", ["mainfile"], "main", None),
371+
("main", "mybranch", ["mainfile", "branchfile"], "branch", None),
363372
],
364373
)
365374
def test_git_checkout(
@@ -371,9 +380,11 @@ def test_git_checkout(
371380
head_ref,
372381
files,
373382
hash_key,
383+
exc,
374384
):
375385
destination = tmp_path / "destination"
376-
run_task_mod.git_checkout(
386+
run_git_checkout = functools.partial(
387+
run_task_mod.git_checkout,
377388
destination_path=destination,
378389
head_repo=mock_git_repo["path"],
379390
base_repo=mock_git_repo["path"],
@@ -383,6 +394,12 @@ def test_git_checkout(
383394
ssh_key_file=None,
384395
ssh_known_hosts_file=None,
385396
)
397+
if exc:
398+
with pytest.raises(exc):
399+
run_git_checkout()
400+
return
401+
402+
run_git_checkout()
386403

387404
# Check desired files exist
388405
for filename in files:
@@ -398,23 +415,35 @@ def test_git_checkout(
398415
assert current_branch == head_ref
399416

400417
current_rev = git_current_rev(destination)
401-
assert current_rev == mock_git_repo[hash_key]
418+
assert current_rev == mock_git_repo[hash_key][-1]
402419

403420

421+
@pytest.mark.parametrize(
422+
"head_ref,head_rev_index",
423+
(
424+
pytest.param("mybranch", 1, id="head"),
425+
pytest.param("mybranch", 0, id="non tip"),
426+
pytest.param(None, 0, id="non tip without head_ref"),
427+
),
428+
)
404429
def test_git_checkout_with_commit(
405430
mock_stdin,
406431
run_task_mod,
407432
mock_git_repo,
408433
tmp_path,
434+
head_ref,
435+
head_rev_index,
409436
):
410437
destination = tmp_path / "destination"
438+
base_rev = mock_git_repo["main"][-1]
439+
head_rev = mock_git_repo["branch"][head_rev_index]
411440
run_task_mod.git_checkout(
412441
destination_path=str(destination),
413442
head_repo=mock_git_repo["path"],
414443
base_repo=mock_git_repo["path"],
415-
base_rev=mock_git_repo["main"],
416-
head_ref="mybranch",
417-
head_rev=mock_git_repo["branch"],
444+
base_rev=base_rev,
445+
head_ref=head_ref,
446+
head_rev=head_rev,
418447
ssh_key_file=None,
419448
ssh_known_hosts_file=None,
420449
)
@@ -424,7 +453,7 @@ def test_git_checkout_with_commit(
424453
cwd=str(destination),
425454
universal_newlines=True,
426455
).strip()
427-
assert current_rev == mock_git_repo["branch"]
456+
assert current_rev == head_rev
428457

429458

430459
def test_display_python_version_should_output_python_versions_title(

0 commit comments

Comments
 (0)