diff options
author | Todd Gamblin <tgamblin@llnl.gov> | 2023-12-21 09:06:06 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-12-21 09:06:06 -0800 |
commit | 0a5f2fc94d7093d57a5f1926f1ebbe035066ad9c (patch) | |
tree | 39ef098f7dcf5513795bdf29f085d4fd7284bbb2 | |
parent | 45b2c207dbf6f9ed1b77de01ee15972d5d7014ec (diff) | |
download | spack-0a5f2fc94d7093d57a5f1926f1ebbe035066ad9c.tar.gz spack-0a5f2fc94d7093d57a5f1926f1ebbe035066ad9c.tar.bz2 spack-0a5f2fc94d7093d57a5f1926f1ebbe035066ad9c.tar.xz spack-0a5f2fc94d7093d57a5f1926f1ebbe035066ad9c.zip |
specs: Better error messages for badly quoted specs (#41805)
If you are calling Spack from the python API, you might have written something like this
before #41529:
```
find = SpackCommand("find")
find('--format={name}', 'saxpy@1.0.0', '+rocm', 'amdgpu_target="gfx90a"')
```
But with the breaking change in #41529, you should write:
```
find = SpackCommand("find")
find('--format={name}', 'gromacs', '+rocm', 'amdgpu_target=gfx90a')
```
Note that we don't need quotes in Python strings, and that this is what would come in
via argv if you typed a quoted variant on the CLI.
The error messages for strings like this are not great -- you get something like this:
```
==> No package matches the query: gromacs+rocm amdgpu_target="gfx90a"
```
Which doesn't indicate that the issue might be your quoting. This is because we were
simply outputting the argv we got, instead of using spec.format() to output the error
message. This PR fixes such errors to use `spec.format()` and to look like this:
```
==> No package matches the query: gromacs+rocm amdgpu_target='"gfx90a"'
```
So users should have an easier time understanding that Spack considers the variant value
to contain quotes here.
- [x] update ConstraintAction to store parsed Specs
- [x] refactor commands to display formatted parsed Specs instead of raw input
-rw-r--r-- | lib/spack/spack/cmd/common/arguments.py | 11 | ||||
-rw-r--r-- | lib/spack/spack/cmd/find.py | 6 | ||||
-rw-r--r-- | lib/spack/spack/cmd/load.py | 8 | ||||
-rw-r--r-- | lib/spack/spack/cmd/modules/__init__.py | 18 |
4 files changed, 18 insertions, 25 deletions
diff --git a/lib/spack/spack/cmd/common/arguments.py b/lib/spack/spack/cmd/common/arguments.py index c30b06ce74..c72833e9de 100644 --- a/lib/spack/spack/cmd/common/arguments.py +++ b/lib/spack/spack/cmd/common/arguments.py @@ -67,12 +67,13 @@ class ConstraintAction(argparse.Action): def __call__(self, parser, namespace, values, option_string=None): # Query specs from command line - self.values = values - namespace.constraint = values + self.constraint = namespace.constraint = values + self.constraint_specs = namespace.constraint_specs = [] namespace.specs = self._specs def _specs(self, **kwargs): - qspecs = spack.cmd.parse_specs(self.values) + # store parsed specs in spec.constraint after a call to specs() + self.constraint_specs[:] = spack.cmd.parse_specs(self.constraint) # If an environment is provided, we'll restrict the search to # only its installed packages. @@ -81,12 +82,12 @@ class ConstraintAction(argparse.Action): kwargs["hashes"] = set(env.all_hashes()) # return everything for an empty query. - if not qspecs: + if not self.constraint_specs: return spack.store.STORE.db.query(**kwargs) # Return only matching stuff otherwise. specs = {} - for spec in qspecs: + for spec in self.constraint_specs: for s in spack.store.STORE.db.query(spec, **kwargs): # This is fast for already-concrete specs specs[s.dag_hash()] = s diff --git a/lib/spack/spack/cmd/find.py b/lib/spack/spack/cmd/find.py index 0dbd751c41..3092ec73b7 100644 --- a/lib/spack/spack/cmd/find.py +++ b/lib/spack/spack/cmd/find.py @@ -261,10 +261,8 @@ def find(parser, args): # Exit early with an error code if no package matches the constraint if not results and args.constraint: - msg = "No package matches the query: {0}" - msg = msg.format(" ".join(args.constraint)) - tty.msg(msg) - raise SystemExit(1) + constraint_str = " ".join(str(s) for s in args.constraint_specs) + tty.die(f"No package matches the query: {constraint_str}") # If tags have been specified on the command line, filter by tags if args.tags: diff --git a/lib/spack/spack/cmd/load.py b/lib/spack/spack/cmd/load.py index 3945651341..50f04f6a4c 100644 --- a/lib/spack/spack/cmd/load.py +++ b/lib/spack/spack/cmd/load.py @@ -98,15 +98,15 @@ def load(parser, args): spack.cmd.display_specs(results) return + constraint_specs = spack.cmd.parse_specs(args.constraint) specs = [ - spack.cmd.disambiguate_spec(spec, env, first=args.load_first) - for spec in spack.cmd.parse_specs(args.constraint) + spack.cmd.disambiguate_spec(spec, env, first=args.load_first) for spec in constraint_specs ] if not args.shell: - specs_str = " ".join(args.constraint) or "SPECS" + specs_str = " ".join(str(s) for s in constraint_specs) or "SPECS" spack.cmd.common.shell_init_instructions( - "spack load", " eval `spack load {sh_arg} %s`" % specs_str + "spack load", f" eval `spack load {{sh_arg}} {specs_str}`" ) return 1 diff --git a/lib/spack/spack/cmd/modules/__init__.py b/lib/spack/spack/cmd/modules/__init__.py index c63e14aba5..0e8147cc45 100644 --- a/lib/spack/spack/cmd/modules/__init__.py +++ b/lib/spack/spack/cmd/modules/__init__.py @@ -388,21 +388,15 @@ def modules_cmd(parser, args, module_type, callbacks=callbacks): callbacks[args.subparser_name](module_type, specs, args) except MultipleSpecsMatch: - msg = "the constraint '{query}' matches multiple packages:\n" + query = " ".join(str(s) for s in args.constraint_specs) + msg = f"the constraint '{query}' matches multiple packages:\n" for s in specs: spec_fmt = "{hash:7} {name}{@version}{%compiler}" spec_fmt += "{compiler_flags}{variants}{arch=architecture}" msg += "\t" + s.cformat(spec_fmt) + "\n" - tty.error(msg.format(query=args.constraint)) - tty.die( - "In this context exactly **one** match is needed: " - "please specify your constraints better." - ) + tty.die(msg, "In this context exactly *one* match is needed.") except NoSpecMatches: - msg = "the constraint '{query}' matches no package." - tty.error(msg.format(query=args.constraint)) - tty.die( - "In this context exactly **one** match is needed: " - "please specify your constraints better." - ) + query = " ".join(str(s) for s in args.constraint_specs) + msg = f"the constraint '{query}' matches no package." + tty.die(msg, "In this context exactly *one* match is needed.") |