Skip to content

Commit 42d2a60

Browse files
committed
Move git notes fetching from run-task to the decision task
This removes the concept of extra refs fetching as it was introduced for this feature alone as it turned out to be quite complex to support if we wanted notes to be optional. The main issue it's fixing is being able to run decision tasks off of repos that don't have that notes branch at all.
1 parent fa30e4b commit 42d2a60

7 files changed

Lines changed: 45 additions & 74 deletions

File tree

.taskcluster.yml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,6 @@ tasks:
226226
- $if: 'isPullRequest'
227227
then:
228228
TASKGRAPH_PULL_REQUEST_NUMBER: '${event.pull_request.number}'
229-
TASKGRAPH_EXTRA_REFS: {$json: ["refs/notes/decision-parameters"]}
230229
- $if: 'tasks_for == "action" || tasks_for == "pr-action"'
231230
then:
232231
ACTION_TASK_GROUP_ID: '${action.taskGroupId}' # taskGroupId of the target task

src/taskgraph/decision.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ def get_decision_parameters(graph_config, options):
267267
# load extra parameters from vcs note if able
268268
note_ref = "refs/notes/decision-parameters"
269269
if options.get("allow_parameter_override") and (
270-
note_params := repo.get_note(note_ref)
270+
note_params := repo.get_note(note_ref, parameters["head_repository"])
271271
):
272272
try:
273273
note_params = json.loads(note_params)

src/taskgraph/run-task/run-task

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import time
3131
import urllib.error
3232
import urllib.request
3333
from pathlib import Path
34-
from typing import Dict, List, Optional
34+
from typing import Dict, Optional
3535

3636
SECRET_BASEURL_TPL = "{}/secrets/v1/secret/{{}}".format(
3737
os.environ.get("TASKCLUSTER_PROXY_URL", "http://taskcluster").rstrip("/")
@@ -640,7 +640,6 @@ def git_checkout(
640640
ssh_key_file: Optional[Path],
641641
ssh_known_hosts_file: Optional[Path],
642642
shallow: bool = False,
643-
extra_refs: Optional[List[str]] = None,
644643
):
645644
assert head_ref or head_rev
646645

@@ -735,14 +734,6 @@ def git_checkout(
735734
env=env,
736735
)
737736

738-
if extra_refs:
739-
git_fetch(
740-
destination_path,
741-
*[f"{ref}:{ref}" for ref in extra_refs],
742-
remote=head_repo,
743-
env=env,
744-
)
745-
746737
args = [
747738
"git",
748739
"checkout",
@@ -941,8 +932,6 @@ def collect_vcs_options(args, project, name):
941932
head_rev = os.environ.get(f"{env_prefix}_HEAD_REV")
942933
pip_requirements = os.environ.get(f"{env_prefix}_PIP_REQUIREMENTS")
943934
private_key_secret = os.environ.get(f"{env_prefix}_SSH_SECRET_NAME")
944-
if extra_refs := os.environ.get(f"{env_prefix}_EXTRA_REFS"):
945-
extra_refs = json.loads(extra_refs)
946935

947936
store_path = os.environ.get("HG_STORE_PATH")
948937

@@ -976,7 +965,6 @@ def collect_vcs_options(args, project, name):
976965
"ssh-secret-name": private_key_secret,
977966
"pip-requirements": pip_requirements,
978967
"shallow-clone": shallow_clone,
979-
"extra-refs": extra_refs,
980968
}
981969

982970

@@ -1026,7 +1014,6 @@ def vcs_checkout_from_args(options):
10261014
ssh_key_file,
10271015
ssh_known_hosts_file,
10281016
shallow=options.get("shallow-clone", False),
1029-
extra_refs=options.get("extra-refs"),
10301017
)
10311018
elif options["repo-type"] == "hg":
10321019
revision = hg_checkout(

src/taskgraph/util/vcs.py

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -215,9 +215,17 @@ def does_revision_exist_locally(self, revision: str) -> bool:
215215
If this function returns an unexpected value, then make sure
216216
the revision was fetched from the remote repository."""
217217

218-
def get_note(self, note: str, revision: Optional[str] = None) -> Optional[str]:
218+
def get_note(
219+
self,
220+
note: str,
221+
remote: str,
222+
revision: Optional[str] = None,
223+
) -> Optional[str]:
219224
"""Read a note attached to the given revision (defaults to HEAD).
220225
226+
The notes ref is fetched from ``remote`` first; a missing ref on the
227+
remote is tolerated while other fetch failures propagate.
228+
221229
Returns the note content as a string, or ``None`` if no note exists.
222230
Only supported by Git; returns ``None`` for all other VCS types.
223231
"""
@@ -594,13 +602,30 @@ def does_revision_exist_locally(self, revision):
594602
return False
595603
raise
596604

597-
def get_note(self, note: str, revision: Optional[str] = None) -> Optional[str]:
605+
def get_note(
606+
self,
607+
note: str,
608+
remote: str,
609+
revision: Optional[str] = None,
610+
) -> Optional[str]:
598611
if not note.startswith("refs/notes/"):
599612
note = f"refs/notes/{note}"
600613

601-
revision = revision or "HEAD"
602614
try:
603-
return self.run("notes", f"--ref={note}", "show", revision).strip()
615+
self.run("fetch", remote, f"{note}:{note}")
616+
except subprocess.CalledProcessError:
617+
ls = subprocess.run(
618+
["git", "ls-remote", "--exit-code", remote, note],
619+
cwd=self.path,
620+
capture_output=True,
621+
)
622+
if ls.returncode != 2:
623+
raise
624+
625+
try:
626+
return self.run(
627+
"notes", f"--ref={note}", "show", revision or "HEAD"
628+
).strip()
604629
except subprocess.CalledProcessError as e:
605630
if e.returncode == 1:
606631
return None

test/test_decision.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,9 @@ def test_dontbuild_commit_message_yields_default_target_tasks_method(
167167
def test_decision_parameters_note(mock_files_changed, mock_get_note):
168168
options = {**_BASE_OPTIONS, "allow_parameter_override": True}
169169
params = decision.get_decision_parameters(FAKE_GRAPH_CONFIG, options)
170-
mock_get_note.assert_called_once_with("refs/notes/decision-parameters")
170+
mock_get_note.assert_called_once_with(
171+
"refs/notes/decision-parameters", _BASE_OPTIONS["head_repository"]
172+
)
171173
assert params["build_number"] == 99
172174

173175

test/test_scripts_run_task.py

Lines changed: 0 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import functools
2-
import json
32
import os
43
import site
54
import stat
@@ -180,17 +179,6 @@ def test_install_pip_requirements_with_uv(
180179
{"shallow-clone": True},
181180
id="git_with_shallow_clone",
182181
),
183-
pytest.param(
184-
{},
185-
{
186-
"REPOSITORY_TYPE": "git",
187-
"HEAD_REPOSITORY": "https://github.com/example/repo",
188-
"HEAD_REV": "abc123",
189-
"EXTRA_REFS": json.dumps(["refs/notes/taskgraph", "refs/notes/other"]),
190-
},
191-
{"extra-refs": ["refs/notes/taskgraph", "refs/notes/other"]},
192-
id="git_with_extra_refs",
193-
),
194182
],
195183
)
196184
def test_collect_vcs_options(
@@ -228,7 +216,6 @@ def test_collect_vcs_options(
228216
"shallow-clone": False,
229217
"ssh-secret-name": env.get("SSH_SECRET_NAME"),
230218
"store-path": env.get("HG_STORE_PATH"),
231-
"extra-refs": None,
232219
}
233220
if "PIP_REQUIREMENTS" in env:
234221
expected["pip-requirements"] = os.path.join(
@@ -653,34 +640,3 @@ def test_main_abspath_environment(mocker, run_main):
653640
assert env[key] == "/builds/worker/file"
654641

655642

656-
def test_git_checkout_extra_refs(mock_stdin, run_task_mod, mock_git_repo, tmp_path):
657-
"""extra_refs are fetched into the local repo during checkout."""
658-
# Add a notes ref to the source repo
659-
rev = mock_git_repo["main"][-1]
660-
subprocess.check_call(
661-
["git", "notes", "--ref=refs/notes/taskgraph", "add", "-m", "test", rev],
662-
cwd=mock_git_repo["path"],
663-
)
664-
665-
destination = tmp_path / "destination"
666-
run_task_mod.git_checkout(
667-
destination_path=str(destination),
668-
head_repo=mock_git_repo["path"],
669-
base_repo=mock_git_repo["path"],
670-
base_rev=None,
671-
head_ref="main",
672-
head_rev=None,
673-
ssh_key_file=None,
674-
ssh_known_hosts_file=None,
675-
extra_refs=["refs/notes/taskgraph"],
676-
)
677-
678-
# Verify the notes ref is available locally
679-
result = subprocess.run(
680-
["git", "notes", "--ref=refs/notes/taskgraph", "show", rev],
681-
cwd=str(destination),
682-
capture_output=True,
683-
text=True,
684-
)
685-
assert result.returncode == 0
686-
assert "test" in result.stdout

test/test_util_vcs.py

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -653,19 +653,21 @@ def test_get_changed_files_with_null_base_revision_shallow_clone(
653653

654654
def test_get_note_git(git_repo, tmpdir):
655655
"""get_note returns note content when present, None otherwise."""
656-
repo_path = tmpdir.join("git")
657-
shutil.copytree(git_repo, repo_path)
658-
repo = get_repository(str(repo_path))
656+
src_path = str(tmpdir.join("git-src"))
657+
dst_path = str(tmpdir.join("git-dst"))
658+
shutil.copytree(git_repo, src_path)
659+
shutil.copytree(git_repo, dst_path)
660+
repo = get_repository(dst_path)
659661

660-
# No note yet
661-
assert repo.get_note("try-config") is None
662+
# No note yet on the remote
663+
assert repo.get_note("try-config", src_path) is None
662664

663665
rev = repo.head_rev
664666
subprocess.check_call(
665667
["git", "notes", "--ref=refs/notes/try-config", "add", "-m", "test note", rev],
666-
cwd=repo.path,
668+
cwd=src_path,
667669
)
668670

669-
assert repo.get_note("try-config") == "test note"
670-
assert repo.get_note("try-config", rev) == "test note"
671-
assert repo.get_note("other") is None
671+
assert repo.get_note("try-config", src_path) == "test note"
672+
assert repo.get_note("try-config", src_path, revision=rev) == "test note"
673+
assert repo.get_note("other", src_path) is None

0 commit comments

Comments
 (0)