Skip to content

Commit c49b05d

Browse files
committed
refactor!: revert {task_workdir} interpolation in run-task environment
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.
1 parent 6f642b5 commit c49b05d

File tree

7 files changed

+39
-41
lines changed

7 files changed

+39
-41
lines changed

docs/reference/migrations.rst

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,14 @@ Migration Guide
33

44
This page can help when migrating Taskgraph across major versions.
55

6+
13.x -> 14.x
7+
------------
8+
9+
* The `{task_workdir}` string in environment variables no longer gets
10+
interpolated by `run-task`. This is backing out a feature introduced in
11+
version 13.x, so this release introduces no new backwards incompatibilities
12+
with version 12.x or earlier.
13+
614
12.x -> 13.x
715
------------
816

src/taskgraph/run-task/run-task

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1257,24 +1257,20 @@ def main(args):
12571257
for repo in repositories:
12581258
vcs_checkout_from_args(repo)
12591259

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

12791275
try:
12801276
if "MOZ_FETCHES" in os.environ:

src/taskgraph/transforms/run/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ def cmp_artifacts(a):
330330
"task-reference": json.dumps(task_fetches, sort_keys=True)
331331
}
332332

333-
env.setdefault("MOZ_FETCHES_DIR", "{task_workdir}/fetches")
333+
env.setdefault("MOZ_FETCHES_DIR", "fetches")
334334

335335
yield task
336336

src/taskgraph/transforms/run/common.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,7 @@ def support_vcs_checkout(config, task, taskdesc, repo_configs, sparse=False):
110110
"REPOSITORIES": json.dumps(
111111
{repo.prefix: repo.name for repo in repo_configs.values()}
112112
),
113-
# If vcsdir is already absolute this will return it unmodified.
114-
"VCS_PATH": path.join("{task_workdir}", vcsdir),
113+
"VCS_PATH": vcsdir,
115114
}
116115
)
117116
for repo_config in repo_configs.values():
@@ -199,5 +198,5 @@ def support_caches(
199198
# If cache_dir is already absolute, the `.join` call returns it as
200199
# is. In that case, {task_workdir} will get interpolated by
201200
# run-task.
202-
env[cache_cfg["env"]] = path.join("{task_workdir}", cache_dir)
201+
env[cache_cfg["env"]] = cache_dir
203202
add_cache(task, taskdesc, cache_name, cache_dir)

test/test_scripts_run_task.py

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import pytest
1414

1515
import taskgraph
16+
from taskgraph.util.caches import CACHES
1617

1718

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

470471

471-
def test_main_interpolate_environment(run_main):
472-
result, env = run_main(
473-
env={
474-
"MOZ_FETCHES_DIR": "{task_workdir}/file",
475-
"UPLOAD_DIR": "$TASK_WORKDIR/file",
476-
"FOO": "{foo}/file",
477-
"BAR": "file",
478-
}
479-
)
472+
def test_main_abspath_environment(run_main):
473+
envvars = ["MOZ_FETCHES_DIR", "UPLOAD_DIR"]
474+
envvars += [cache["env"] for cache in CACHES.values() if "env" in cache]
475+
env = {key: "file" for key in envvars}
476+
env["FOO"] = "file"
477+
478+
result, env = run_main(env=env)
480479
assert result == 0
481480

482-
assert env == {
483-
"MOZ_FETCHES_DIR": "/builds/worker/file",
484-
"UPLOAD_DIR": "$TASK_WORKDIR/file",
485-
"FOO": "{foo}/file",
486-
"BAR": "file",
487-
"TASK_WORKDIR": "/builds/worker",
488-
}
481+
assert env.get("FOO") == "file"
482+
for key in envvars:
483+
assert env[key] == "/builds/worker/file"

test/test_transforms_run_run_task.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ def assert_generic_worker(task):
114114
"HG_STORE_PATH": "y:/hg-shared",
115115
"MOZ_SCM_LEVEL": "1",
116116
"REPOSITORIES": '{"ci": "Taskgraph"}',
117-
"VCS_PATH": "{task_workdir}/build/src",
117+
"VCS_PATH": "build/src",
118118
},
119119
"implementation": "generic-worker",
120120
"mounts": [

uv.lock

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)