Skip to content

Commit 62e27c4

Browse files
committed
fix: parse CUMULUSCI_EXTRA_YAML as comma-separated, use pathlib
Splitting the env var on ":" broke on Windows where drive letters contain colons (e.g. C:\tmp\a.yml). Switch to comma, the convention used everywhere else in CCI for list-valued strings, via process_list_arg. Replace os.path.isfile/open with pathlib.Path. Update all CLI help strings, docstring, and docs.
1 parent 6f892cc commit 62e27c4

6 files changed

Lines changed: 25 additions & 20 deletions

File tree

cumulusci/cli/extra_yaml.py

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,16 @@
44
kwarg, which already merges into the project config via the existing YAML
55
merge stack.
66
"""
7+
78
import os
9+
from pathlib import Path
810
from typing import Optional, Tuple
911

1012
import click
1113
import yaml
1214

1315
from cumulusci.core.exceptions import CumulusCIUsageError
14-
from cumulusci.core.utils import dictmerge
16+
from cumulusci.core.utils import dictmerge, process_list_arg
1517

1618
ENV_VAR = "CUMULUSCI_EXTRA_YAML"
1719

@@ -22,7 +24,7 @@ def resolve_extra_yaml(paths: Tuple[str, ...]) -> Optional[str]:
2224
Args:
2325
paths: Tuple of paths from Click's ``multiple=True`` option. Empty
2426
means the flag was not supplied; fall back to
25-
``CUMULUSCI_EXTRA_YAML`` (colon-separated paths).
27+
``CUMULUSCI_EXTRA_YAML`` (comma-separated paths).
2628
2729
Returns:
2830
A single YAML document representing the deep-merge of all input files
@@ -37,7 +39,7 @@ def resolve_extra_yaml(paths: Tuple[str, ...]) -> Optional[str]:
3739
if not effective_paths:
3840
env_value = os.environ.get(ENV_VAR)
3941
if env_value:
40-
effective_paths = tuple(p for p in env_value.split(":") if p)
42+
effective_paths = tuple(p for p in (process_list_arg(env_value) or []) if p)
4143

4244
if not effective_paths:
4345
return None
@@ -51,11 +53,11 @@ def resolve_extra_yaml(paths: Tuple[str, ...]) -> Optional[str]:
5153

5254
merged: dict = {}
5355
for path in effective_paths:
54-
if not os.path.isfile(path):
56+
file_path = Path(path)
57+
if not file_path.is_file():
5558
raise CumulusCIUsageError(f"--extra-yaml file not found: {path}")
5659
try:
57-
with open(path, "r", encoding="utf-8") as f:
58-
raw = f.read()
60+
raw = file_path.read_text(encoding="utf-8")
5961
except OSError as e:
6062
raise CumulusCIUsageError(f"--extra-yaml could not read {path}: {e}")
6163
try:

cumulusci/cli/flow.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,9 @@ def flow_doc(runtime, project=False):
4545
flows_by_group = group_items(flows)
4646
flow_groups = sorted(
4747
flows_by_group.keys(),
48-
key=lambda group: flow_info_groups.index(group)
49-
if group in flow_info_groups
50-
else 100,
48+
key=lambda group: (
49+
flow_info_groups.index(group) if group in flow_info_groups else 100
50+
),
5151
)
5252

5353
for group in flow_groups:
@@ -116,7 +116,7 @@ def flow_list(runtime, plain, print_json):
116116
"Path to an additional YAML file to merge into the project config "
117117
"for this command only. Can be specified multiple times; later files "
118118
"override earlier ones. Also honors CUMULUSCI_EXTRA_YAML env var "
119-
"(colon-separated paths) as a fallback."
119+
"(comma-separated paths) as a fallback."
120120
),
121121
)
122122
@pass_runtime(require_keychain=True)
@@ -164,7 +164,7 @@ def flow_info(runtime, flow_name, extra_yaml):
164164
"Path to an additional YAML file to merge into the project config "
165165
"for this command only. Can be specified multiple times; later files "
166166
"override earlier ones. Also honors CUMULUSCI_EXTRA_YAML env var "
167-
"(colon-separated paths) as a fallback."
167+
"(comma-separated paths) as a fallback."
168168
),
169169
)
170170
@pass_runtime(require_keychain=True)

cumulusci/cli/task.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ def task_doc(runtime, project=False, write=False):
105105
"Path to an additional YAML file to merge into the project config "
106106
"for this command only. Can be specified multiple times; later files "
107107
"override earlier ones. Also honors CUMULUSCI_EXTRA_YAML env var "
108-
"(colon-separated paths) as a fallback."
108+
"(comma-separated paths) as a fallback."
109109
),
110110
)
111111
@pass_runtime(require_project=False, require_keychain=True)
@@ -168,7 +168,7 @@ class RunTaskCommand(click.MultiCommand):
168168
"Path to an additional YAML file to merge into the project "
169169
"config for this command only. Can be specified multiple times; "
170170
"later files override earlier ones. Also honors "
171-
"CUMULUSCI_EXTRA_YAML env var (colon-separated paths) as a "
171+
"CUMULUSCI_EXTRA_YAML env var (comma-separated paths) as a "
172172
"fallback."
173173
),
174174
"is_flag": False,

cumulusci/cli/tests/test_extra_yaml.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,15 +56,15 @@ def test_resolve_extra_yaml__env_var_fallback(tmp_path, monkeypatch):
5656
assert "env-loaded" in result
5757

5858

59-
def test_resolve_extra_yaml__env_var_multiple_colon_separated(tmp_path, monkeypatch):
59+
def test_resolve_extra_yaml__env_var_multiple_comma_separated(tmp_path, monkeypatch):
6060
"""Env var with multiple paths produces a deep-merged document."""
6161
import yaml
6262

6363
a = tmp_path / "a.yml"
6464
a.write_text("tasks:\n a:\n description: from A\n")
6565
b = tmp_path / "b.yml"
6666
b.write_text("tasks:\n b:\n description: from B\n")
67-
monkeypatch.setenv("CUMULUSCI_EXTRA_YAML", f"{a}:{b}")
67+
monkeypatch.setenv("CUMULUSCI_EXTRA_YAML", f"{a},{b}")
6868
result = resolve_extra_yaml(())
6969
assert result is not None
7070
parsed = yaml.safe_load(result)
@@ -87,7 +87,7 @@ def test_resolve_extra_yaml__flag_overrides_env_var(tmp_path, monkeypatch):
8787
def test_resolve_extra_yaml__empty_env_var_segments_ignored(tmp_path, monkeypatch):
8888
p = tmp_path / "x.yml"
8989
p.write_text("project: {}\n")
90-
monkeypatch.setenv("CUMULUSCI_EXTRA_YAML", f"::{p}::")
90+
monkeypatch.setenv("CUMULUSCI_EXTRA_YAML", f",,{p},,")
9191
result = resolve_extra_yaml(())
9292
assert result is not None
9393
assert "project: {}" in result

docs/cli.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -492,9 +492,10 @@ $ cci task run my_task --extra-yaml migrations/v2.yml
492492
$ cci flow run dev_org --extra-yaml base.yml --extra-yaml override.yml
493493
```
494494

495-
The `CUMULUSCI_EXTRA_YAML` environment variable (colon-separated paths)
495+
The `CUMULUSCI_EXTRA_YAML` environment variable (comma-separated paths)
496496
supplies a default when the flag is absent. When both are set, the flag
497-
wins; they are not merged.
497+
wins; they are not merged. Paths containing commas cannot be expressed
498+
in the env var; pass them via the repeatable `--extra-yaml` flag instead.
498499

499500
Extra YAML is merged using the same deep-merge semantics as the project
500501
`cumulusci.yml`. Mappings and scalars are overridden; lists are

docs/env-var-reference.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,12 @@ orgs.
1818

1919
## `CUMULUSCI_EXTRA_YAML`
2020

21-
Colon-separated list of paths to additional YAML files to merge into
21+
Comma-separated list of paths to additional YAML files to merge into
2222
the project config for each `cci task run`, `cci task info`,
2323
`cci flow run`, and `cci flow info` invocation. The `--extra-yaml`
24-
flag on those commands takes precedence when both are set. See
24+
flag on those commands takes precedence when both are set. Paths
25+
containing commas cannot be expressed here; use the repeatable
26+
`--extra-yaml` flag instead. See
2527
[the CLI reference](cli.md#the-extra-yaml-flag) for merge semantics.
2628

2729
## `CUMULUSCI_KEY`

0 commit comments

Comments
 (0)