Skip to content

Commit

Permalink
refactor!: revert {task_workdir} interpolation in run-task environment
Browse files Browse the repository at this point in the history
In hindsight, this ended up being a bad idea and caused lots of subtle
issues in Gecko. As an alternative, I landed a change to get
generic-worker itself to set $TASK_WORKDIR.

This will take awhile to propagate to all pools, but in the meantime
it's cleanest to revert back to the abspath method.
  • Loading branch information
ahal committed Feb 18, 2025
1 parent 6f642b5 commit bee1612
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 38 deletions.
8 changes: 8 additions & 0 deletions docs/reference/migrations.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,14 @@ Migration Guide

This page can help when migrating Taskgraph across major versions.

13.x -> 14.x
------------

* The `{task_workdir}` string in environment variables no longer gets
interpolated by `run-task`. This is backing out a feature introduced in
version 13.x, so this release introduces no new backwards incompatibilities
with version 12.x or earlier.

12.x -> 13.x
------------

Expand Down
32 changes: 14 additions & 18 deletions src/taskgraph/run-task/run-task
Original file line number Diff line number Diff line change
Expand Up @@ -1257,24 +1257,20 @@ def main(args):
for repo in repositories:
vcs_checkout_from_args(repo)

# Interpolate environment variables with defined substitution patterns. For
# example, the variable `CACHE_DIR={task_workdir}/.cache` will be
# interpolated with the task's working directory.
env_subs = {
"task_workdir": task_workdir,
}
for k, v in os.environ.items():
for subname, subval in env_subs.items():
# We check for an exact match rather than using a format string to
# avoid accidentally trying to interpolate environment variables
# that happen to contain brackets for some other reason.
pattern = f"{{{subname}}}"
if pattern in v:
os.environ[k] = v.replace(pattern, subval)
print_line(
b"setup",
b"%s is %s\n" % (k.encode("utf-8"), os.environ[k].encode("utf-8")),
)
# Convert certain well known environment variables to absolute paths.
for k in [
"CARGO_HOME",
"MOZ_FETCHES_DIR",
"PIP_CACHE_DIR",
"UPLOAD_DIR",
"UV_CACHE_DIR",
"npm_config_cache",
] + [
"{}_PATH".format(repository["project"].upper())
for repository in repositories
]:
if k in os.environ:
os.environ[k] = os.path.abspath(os.environ[k])

try:
if "MOZ_FETCHES" in os.environ:
Expand Down
2 changes: 1 addition & 1 deletion src/taskgraph/transforms/run/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ def cmp_artifacts(a):
"task-reference": json.dumps(task_fetches, sort_keys=True)
}

env.setdefault("MOZ_FETCHES_DIR", "{task_workdir}/fetches")
env.setdefault("MOZ_FETCHES_DIR", "fetches")

yield task

Expand Down
3 changes: 1 addition & 2 deletions src/taskgraph/transforms/run/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,7 @@ def support_vcs_checkout(config, task, taskdesc, repo_configs, sparse=False):
"REPOSITORIES": json.dumps(
{repo.prefix: repo.name for repo in repo_configs.values()}
),
# If vcsdir is already absolute this will return it unmodified.
"VCS_PATH": path.join("{task_workdir}", vcsdir),
"VCS_PATH": vcsdir,
}
)
for repo_config in repo_configs.values():
Expand Down
27 changes: 11 additions & 16 deletions test/test_scripts_run_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import pytest

import taskgraph
from taskgraph.util.caches import CACHES


@pytest.fixture(scope="module")
Expand Down Expand Up @@ -468,21 +469,15 @@ def inner(extra_args=None, env=None):
return inner


def test_main_interpolate_environment(run_main):
result, env = run_main(
env={
"MOZ_FETCHES_DIR": "{task_workdir}/file",
"UPLOAD_DIR": "$TASK_WORKDIR/file",
"FOO": "{foo}/file",
"BAR": "file",
}
)
def test_main_abspath_environment(run_main):
envvars = ["MOZ_FETCHES_DIR", "UPLOAD_DIR"]
envvars += [cache["env"] for cache in CACHES.values() if "env" in cache]
env = {key: "file" for key in envvars}
env["FOO"] = "file"

result, env = run_main(env=env)
assert result == 0

assert env == {
"MOZ_FETCHES_DIR": "/builds/worker/file",
"UPLOAD_DIR": "$TASK_WORKDIR/file",
"FOO": "{foo}/file",
"BAR": "file",
"TASK_WORKDIR": "/builds/worker",
}
assert env.get("FOO") == "file"
for key in envvars:
assert env[key] == "/builds/worker/file"
2 changes: 1 addition & 1 deletion test/test_transforms_run_run_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ def assert_generic_worker(task):
"HG_STORE_PATH": "y:/hg-shared",
"MOZ_SCM_LEVEL": "1",
"REPOSITORIES": '{"ci": "Taskgraph"}',
"VCS_PATH": "{task_workdir}/build/src",
"VCS_PATH": "build/src",
},
"implementation": "generic-worker",
"mounts": [
Expand Down

0 comments on commit bee1612

Please sign in to comment.