ValueError when target name contains a `.`
Closed this issue · 1 comments
reiyw commented
The BazelContainer documentation uses image.tar as an example target name, which actually returns a ValueError.
Traceback
Traceback (most recent call last):
File "/Users/ryo.takahashi/ghq/github.com/deepmind/xmanager/.venv/bin/xmanager", line 33, in <module>
sys.exit(load_entry_point('xmanager', 'console_scripts', 'xmanager')())
File "/Users/ryo.takahashi/ghq/github.com/deepmind/xmanager/xmanager/cli/cli.py", line 65, in entrypoint
app.run(main)
File "/Users/ryo.takahashi/ghq/github.com/deepmind/xmanager/.venv/lib/python3.9/site-packages/absl/app.py", line 312, in run
_run_main(main, args)
File "/Users/ryo.takahashi/ghq/github.com/deepmind/xmanager/.venv/lib/python3.9/site-packages/absl/app.py", line 258, in _run_main
sys.exit(main(argv))
File "/Users/ryo.takahashi/ghq/github.com/deepmind/xmanager/xmanager/cli/cli.py", line 41, in main
app.run(m.main, argv=argv)
File "/Users/ryo.takahashi/ghq/github.com/deepmind/xmanager/.venv/lib/python3.9/site-packages/absl/app.py", line 312, in run
_run_main(main, args)
File "/Users/ryo.takahashi/ghq/github.com/deepmind/xmanager/.venv/lib/python3.9/site-packages/absl/app.py", line 258, in _run_main
sys.exit(main(argv))
File "/Users/ryo.takahashi/ghq/github.com/deepmind/xmanager/launcher.py", line 7, in main
[executable] = experiment.package(
File "/Users/ryo.takahashi/ghq/github.com/deepmind/xmanager/xmanager/xm/core.py", line 636, in package
return cls._async_packager.package(packageables)
File "/Users/ryo.takahashi/ghq/github.com/deepmind/xmanager/xmanager/xm/async_packager.py", line 114, in package
executables = self._package_batch(packageables)
File "/Users/ryo.takahashi/ghq/github.com/deepmind/xmanager/xmanager/xm_local/packaging/router.py", line 112, in package
bazel_kinds = bazel_service.fetch_kinds(bazel_labels)
File "/Users/ryo.takahashi/ghq/github.com/deepmind/xmanager/xmanager/xm_local/packaging/bazel_tools.py", line 186, in fetch_kinds
labels = [_assemble_label(_lex_label(label)) for label in labels]
File "/Users/ryo.takahashi/ghq/github.com/deepmind/xmanager/xmanager/xm_local/packaging/bazel_tools.py", line 186, in <listcomp>
labels = [_assemble_label(_lex_label(label)) for label in labels]
File "/Users/ryo.takahashi/ghq/github.com/deepmind/xmanager/xmanager/xm_local/packaging/bazel_tools.py", line 160, in _lex_label
raise ValueError(f'{label} is not an absolute Bazel label')
ValueError: //path/to/target:label.tar is not an absolute Bazel label
This error is caused by _LABEL_LEXER. This regex does not allow the inclusion of a single . which is not an expansion, so the label name in the example will not match.
https://github.com/deepmind/xmanager/blob/18652570332e284a6b2c184e6ab943ca56f6a11a/xmanager/xm_local/packaging/bazel_tools.py#L149-L152
The immediate solution that comes to mind is to allow containing . in the regex, then look for a consecutive . in post-processing like the following:
diff --git a/xmanager/xm_local/packaging/bazel_tools.py b/xmanager/xm_local/packaging/bazel_tools.py
index 694f001..4dc52b0 100644
--- a/xmanager/xm_local/packaging/bazel_tools.py
+++ b/xmanager/xm_local/packaging/bazel_tools.py
@@ -147,7 +147,7 @@ def _build_multiple_targets(
# Expansions (`...`, `*`) are not allowed.
-_NAME_RE = '[^:/.*]+'
+_NAME_RE = '[^:/*]+'
_LABEL_LEXER = re.compile(
f'^//(?P<packages>{_NAME_RE}(/{_NAME_RE})*)?(?P<target>:{_NAME_RE})?$')
_LexedLabel = Tuple[List[str], str]
@@ -156,8 +156,10 @@ _LexedLabel = Tuple[List[str], str]
def _lex_label(label: str) -> _LexedLabel:
"""Splits the label into packages and target."""
match = _LABEL_LEXER.match(label)
- if match is None:
- raise ValueError(f'{label} is not an absolute Bazel label')
+ for g in match.groups('packages'):
+ print('group:', g)
+ if '..' in g:
+ raise ValueError(f'{label} is not an absolute Bazel label')
groups = match.groupdict()
packages: Optional[str] = groups['packages']
target: Optional[str] = groups['target']