commands: Show less generic error when no manifest is loaded

[57300]

Minor quirk that stuck out to me while testing the recent changes. Before 23db6e1, if I were to try west manifest --resolve in Zephyr without cloning bsim, I would see this:

FATAL ERROR: project bsim (tools/bsim): cannot import contents of west.yml; do you need to run "west update"?

which is helpful enough. Now, this generic message gets displayed:

FATAL ERROR: can't run west manifest; it requires the manifest, which was not available. Try "west manifest --validate" to debug.

Ironically, west manifest --validate produces the same error, which is not very helpful. So, I tried to fix that.

[57300]

@mbolivar-nordic Hi, I can't add you as reviewer, but do you want to look over this?

[mbolivar-nordic]

I think this should probably be handled in the relevant built-in commands themselves by checking WestCommand.has_manifest and erroring out as necessary. Did you consider this? If so, why did you reject it?

[57300]

I think this should probably be handled in the relevant built-in commands themselves by checking WestCommand.has_manifest and erroring out as necessary. Did you consider this? If so, why did you reject it?

Yes, I had briefly considered it, but I figured it would probably involve passing WestApp.mle to each command, so that it could still report why not self.has_manifest. I decided against it, thinking it might be better to have common handling in one place. There's already some common handling in handle_builtin_manifest_load_err(), which seems to cover quite a few cases.

I'm fine with per-command handling, if you prefer, since it looks like only west manifest needs it right now anyway.

In fact, on second thought, the easiest fix for west manifest specifically might be to just remove it from this list:

west/src/west/app/main.py

Lines 277 to 283 in 9e8f500

	# A few commands are always safe to run without a manifest.
	# The update command is sometimes safe and sometimes not, but
	# we need to include it in this list because it's the only way
	# to fix a manifest-rev revision in a project which is being
	# imported to point from a bogus manifest to a non-bogus one.
	no_manifest_ok = ['help', 'config', 'topdir', 'init', 'manifest',
	'update']

since it will always bail without a manifest:

west/src/west/app/project.py

Lines 547 to 548 in 9e8f500

	def do_run(self, args, user_args):
	manifest = self.manifest

[marc-hb]

bsim seems to have been made optional now by Zephyr commit f30f08a3ce75 and enabling babblesim in the project-filter now fails with FATAL ERROR: "west manifest --resolve" is not (yet) supported when the manifest.project-filter option is set. So I'm no sure it's possible to reproduce the original issue upstream.

However I tried with a totally different project and as of the current west version 0612a6d the issue does not seem to have changed.

Considering Marti left the project, I have successfully tested a much simpler and zero-risk "fix" similar to 45762a0

diff --git a/src/west/app/main.py b/src/west/app/main.py
index 2b4addb03e2d..b678893911f1 100755
--- a/src/west/app/main.py
+++ b/src/west/app/main.py
@@ -917,7 +917,7 @@ class WestArgumentParser(argparse.ArgumentParser):
                     # warn them again.
                     append('Cannot load extension commands; '
                            'help for them is not available.')
-                    append('(To debug, try: "west manifest --validate".)')
+                    append('(To debug, try: "west -vv manifest --validate".)')
                     append('')
             else:
                 # TODO we may want to be more aggressive about loading
diff --git a/src/west/commands.py b/src/west/commands.py
index 2e5a4b89b555..1abb0fcd17d2 100644
--- a/src/west/commands.py
+++ b/src/west/commands.py
@@ -260,7 +260,7 @@ class WestCommand(ABC):
         if self._manifest is None:
             self.die(f"can't run west {self.name};",
                      "it requires the manifest, which was not available.",
-                     'Try "west manifest --validate" to debug.')
+                     'Try "west -vv manifest --validate" to debug.')
         return self._manifest

     def _set_manifest(self, manifest: Optional[Manifest]):

[marc-hb]

Minor -vv mitigation patch submitted in #734

EDIT: merged.

[marc-hb]

This change is located in a fairly complex area little or no test coverage: error handling.

I don't have the time to learn about it but I don't think the value is worth the risk. My 2 cents, sorry.

[pdgendt]

I'll move it out of the 1.3 list for now.