Skip to content

Remove base_ref and base_rev post-processing #813

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 14, 2025
Merged

Remove base_ref and base_rev post-processing #813

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
faf6ba4
refactor!: rename base_rev -> base in vcs.get_changed_files()
ahal Oct 8, 2025
90a856b
fix!: remove post processing of base_ref and base_rev parameters
ahal Oct 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 2 additions & 79 deletions src/taskgraph/decision.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
from taskgraph.util import json
from taskgraph.util.python_path import find_object
from taskgraph.util.schema import Schema, validate_schema
from taskgraph.util.vcs import Repository, get_repository
from taskgraph.util.vcs import get_repository
from taskgraph.util.yaml import load_yaml

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -192,25 +192,10 @@ def get_decision_parameters(graph_config, options):
except UnicodeDecodeError:
commit_message = ""

parameters["base_ref"] = _determine_more_accurate_base_ref(
repo,
candidate_base_ref=options.get("base_ref"),
head_ref=options.get("head_ref"),
base_rev=options.get("base_rev"),
)

parameters["base_rev"] = _determine_more_accurate_base_rev(
repo,
base_ref=parameters["base_ref"],
candidate_base_rev=options.get("base_rev"),
head_rev=options.get("head_rev"),
env_prefix=_get_env_prefix(graph_config),
)

# Define default filter list, as most configurations shouldn't need
# custom filters.
parameters["files_changed"] = repo.get_changed_files(
rev=parameters["head_rev"], base_rev=parameters["base_rev"]
rev=parameters["head_rev"], base=parameters["base_rev"]
)
parameters["filters"] = [
"target_tasks_method",
Expand Down Expand Up @@ -284,68 +269,6 @@ def get_decision_parameters(graph_config, options):
return result


def _determine_more_accurate_base_ref(repo, candidate_base_ref, head_ref, base_rev):
base_ref = candidate_base_ref

if not candidate_base_ref:
base_ref = repo.default_branch
elif candidate_base_ref == head_ref and base_rev == Repository.NULL_REVISION:
logger.info(
"base_ref and head_ref are identical but base_rev equals the null revision. "
"This is a new branch but Github didn't identify its actual base."
)
base_ref = repo.default_branch

if base_ref != candidate_base_ref:
logger.info(
f'base_ref has been reset from "{candidate_base_ref}" to "{base_ref}".'
)

return base_ref


def _determine_more_accurate_base_rev(
repo, base_ref, candidate_base_rev, head_rev, env_prefix
):
if not candidate_base_rev:
logger.info("base_rev is not set.")
base_ref_or_rev = base_ref
elif candidate_base_rev == Repository.NULL_REVISION:
logger.info("base_rev equals the null revision. This branch is a new one.")
base_ref_or_rev = base_ref
elif not repo.does_revision_exist_locally(candidate_base_rev):
logger.warning(
"base_rev does not exist locally. It is likely because the branch was force-pushed. "
"taskgraph is not able to assess how many commits were changed and assumes it is only "
f"the last one. Please set the {env_prefix.upper()}_BASE_REV environment variable "
"in the decision task and provide `--base-rev` to taskgraph."
)
base_ref_or_rev = base_ref
else:
base_ref_or_rev = candidate_base_rev

if base_ref_or_rev == base_ref:
logger.info(
f'Using base_ref "{base_ref}" to determine latest common revision...'
)

base_rev = repo.find_latest_common_revision(base_ref_or_rev, head_rev)
if base_rev != candidate_base_rev:
if base_ref_or_rev == candidate_base_rev:
logger.info("base_rev is not an ancestor of head_rev.")

logger.info(
f'base_rev has been reset from "{candidate_base_rev}" to "{base_rev}".'
)

return base_rev


def _get_env_prefix(graph_config):
repo_keys = list(graph_config["taskgraph"].get("repositories", {}).keys())
return repo_keys[0] if repo_keys else ""


def set_try_config(parameters, task_config_file):
if os.path.isfile(task_config_file):
logger.info(f"using try tasks from {task_config_file}")
Expand Down
2 changes: 1 addition & 1 deletion src/taskgraph/parameters.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class ParameterMismatch(Exception):
base_schema = Schema(
{
Required("base_repository"): str,
Required("base_ref"): str,
Optional("base_ref"): str,
Required("base_rev"): str,
Required("build_date"): int,
Required("build_number"): int,
Expand Down
26 changes: 12 additions & 14 deletions src/taskgraph/util/vcs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,12 +140,12 @@ def get_changed_files(
diff_filter: Optional[str],
mode: Optional[str],
rev: Optional[str],
base_rev: Optional[str],
base: Optional[str],
) -> List[str]:
"""Return a list of files that are changed in:
* either this repository's working copy,
* or at a given revision (``rev``)
* or between 2 revisions (``base_rev`` and ``rev``)
* or between 2 revisions (``base`` and ``rev``)

``diff_filter`` controls which kinds of modifications are returned.
It is a string which may only contain the following characters:
Expand All @@ -162,9 +162,9 @@ def get_changed_files(
``rev`` is a specifier for which changesets to consider for
changes. The exact meaning depends on the vcs system being used.

``base_rev`` specifies the range of changesets. This parameter cannot
``base`` specifies the range of changesets. This parameter cannot
be used without ``rev``. The range includes ``rev`` but excludes
``base_rev``.
``base``.
"""

@abstractmethod
Expand Down Expand Up @@ -293,17 +293,17 @@ def get_tracked_files(self, *paths, rev=None):
rev = rev or "."
return self.run("files", "-r", rev, *paths).splitlines()

def get_changed_files(self, diff_filter=None, mode=None, rev=None, base_rev=None):
def get_changed_files(self, diff_filter=None, mode=None, rev=None, base=None):
diff_filter = diff_filter or "ADM"
if rev is None:
if base_rev is not None:
raise ValueError("Cannot specify `base_rev` without `rev`")
if base is not None:
raise ValueError("Cannot specify `base` without `rev`")
# Use --no-status to print just the filename.
df = self._format_diff_filter(diff_filter, for_status=True)
return self.run("status", "--no-status", f"-{df}").splitlines()
else:
template = self._files_template(diff_filter)
revision_argument = rev if base_rev is None else f"{rev} % {base_rev}"
revision_argument = rev if base is None else f"{rev} % {base}"
return self.run("log", "-r", revision_argument, "-T", template).splitlines()

def get_outgoing_files(self, diff_filter="ADM", upstream=None):
Expand Down Expand Up @@ -479,23 +479,21 @@ def get_tracked_files(self, *paths, rev=None):
rev = rev or "HEAD"
return self.run("ls-tree", "-r", "--name-only", rev, *paths).splitlines()

def get_changed_files(self, diff_filter=None, mode=None, rev=None, base_rev=None):
def get_changed_files(self, diff_filter=None, mode=None, rev=None, base=None):
diff_filter = diff_filter or "ADM"
mode = mode or "unstaged"
assert all(f.lower() in self._valid_diff_filter for f in diff_filter)

if rev is None:
if base_rev is not None:
raise ValueError("Cannot specify `base_rev` without `rev`")
if base is not None:
raise ValueError("Cannot specify `base` without `rev`")
cmd = ["diff"]
if mode == "staged":
cmd.append("--cached")
elif mode == "all":
cmd.append("HEAD")
else:
revision_argument = (
f"{rev}~1..{rev}" if base_rev is None else f"{base_rev}..{rev}"
)
revision_argument = f"{rev}~1..{rev}" if base is None else f"{base}..{rev}"
cmd = ["log", "--format=format:", revision_argument]

cmd.append("--name-only")
Expand Down
91 changes: 2 additions & 89 deletions test/test_decision.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@
import unittest
from pathlib import Path

import pytest

from taskgraph import decision
from taskgraph.util.vcs import GitRepository, HgRepository
from taskgraph.util.yaml import load_yaml
Expand Down Expand Up @@ -54,6 +52,7 @@ class TestGetDecisionParameters(unittest.TestCase):
def setUp(self):
self.options = {
"base_repository": "https://hg.mozilla.org/mozilla-unified",
"base_rev": "aaaa",
"head_repository": "https://hg.mozilla.org/mozilla-central",
"head_rev": "bbbb",
"head_ref": "default",
Expand All @@ -67,27 +66,11 @@ def setUp(self):
"tasks_for": "hg-push",
"level": "3",
}
self.old_determine_more_accurate_base_rev = (
decision._determine_more_accurate_base_rev
)
decision._determine_more_accurate_base_rev = lambda *_, **__: "aaaa"
self.old_determine_more_accurate_base_ref = (
decision._determine_more_accurate_base_ref
)
decision._determine_more_accurate_base_ref = lambda *_, **__: "aaaa"

def tearDown(self):
decision._determine_more_accurate_base_rev = (
self.old_determine_more_accurate_base_rev
)
decision._determine_more_accurate_base_ref = (
self.old_determine_more_accurate_base_ref
)

def test_simple_options(self, mock_files_changed):
mock_files_changed.return_value = ["foo.txt"]
params = decision.get_decision_parameters(FAKE_GRAPH_CONFIG, self.options)
mock_files_changed.assert_called_once_with(rev="bbbb", base_rev="aaaa")
mock_files_changed.assert_called_once_with(rev="bbbb", base="aaaa")
self.assertEqual(params["build_date"], 1503691511)
self.assertEqual(params["head_tag"], "v0.0.1")
self.assertEqual(params["pushlog_id"], "143")
Expand Down Expand Up @@ -154,73 +137,3 @@ def test_dontbuild_commit_message_yields_default_target_tasks_method(
self.options["tasks_for"] = "hg-push"
params = decision.get_decision_parameters(FAKE_GRAPH_CONFIG, self.options)
self.assertEqual(params["target_tasks_method"], "nothing")


@pytest.mark.parametrize(
"candidate_base_ref, base_rev, expected_base_ref",
(
("", "base-rev", "default-branch"),
("head-ref", "base-rev", "head-ref"),
("head-ref", "0000000000000000000000000000000000000000", "default-branch"),
),
)
def test_determine_more_accurate_base_ref(
candidate_base_ref, base_rev, expected_base_ref
):
repo_mock = unittest.mock.MagicMock()
repo_mock.default_branch = "default-branch"

assert (
decision._determine_more_accurate_base_ref(
repo_mock, candidate_base_ref, "head-ref", base_rev
)
== expected_base_ref
)


@pytest.mark.parametrize(
"common_rev, candidate_base_rev, expected_base_ref_or_rev, expected_base_rev",
(
("found-rev", "", "base-ref", "found-rev"),
(
"found-rev",
"0000000000000000000000000000000000000000",
"base-ref",
"found-rev",
),
("found-rev", "non-existing-rev", "base-ref", "found-rev"),
("existing-rev", "existing-rev", "existing-rev", "existing-rev"),
),
)
def test_determine_more_accurate_base_rev(
common_rev, candidate_base_rev, expected_base_ref_or_rev, expected_base_rev
):
repo_mock = unittest.mock.MagicMock()
repo_mock.find_latest_common_revision.return_value = common_rev
repo_mock.does_revision_exist_locally = lambda rev: rev == "existing-rev"

assert (
decision._determine_more_accurate_base_rev(
repo_mock, "base-ref", candidate_base_rev, "head-rev", env_prefix="PREFIX"
)
== expected_base_rev
)
repo_mock.find_latest_common_revision.assert_called_once_with(
expected_base_ref_or_rev, "head-rev"
)


@pytest.mark.parametrize(
"graph_config, expected_value",
(
({"taskgraph": {}}, ""),
({"taskgraph": {"repositories": {}}}, ""),
({"taskgraph": {"repositories": {"mobile": {}}}}, "mobile"),
(
{"taskgraph": {"repositories": {"mobile": {}, "some-other-repo": {}}}},
"mobile",
),
),
)
def test_get_env_prefix(graph_config, expected_value):
assert decision._get_env_prefix(graph_config) == expected_value
Loading