Fix run_cmd error handling

[Flamefire]

Don't raise an error when log_ok=False is passed, i.e. the caller wants to handle the error.

The naming is confusing and the dependency on the strictness level is likely wrong. E.g. the caller may not expect a failing command to return from this function but to raise an error instead. However the user might have set the strictness to IGNORE which defeats the purpose of log_ok.

This change now allows to use log_all=True, log_ok=False to log the command and output and handle potential errors at the call site.

This is very likely the intended behavior, see e.g.

easybuild-framework/easybuild/framework/extension.py

Lines 266 to 267 in a8c0cad

	# set log_ok to False so we can catch the error instead of run_cmd
	(output, ec) = run_cmd(cmd, log_ok=False, simple=False, regexp=False, inp=stdin)

[boegel]

I understand the current behavior of run_cmd is bloody confusing, and the meaning of the different parameters is unclear at best, but I'm not sure changes like this are a good idea. It's very difficult to assess the impact of this, I think.

I think we need a new run* function that properly supports the different use cases, like:

• just run a shell command, raise an error if it fails (based on exit code) -- default
• run a shell command, return output (stderr+stdout combined, or optionally split) and/or exit code

Logging behavior should probably always be the same (always log the output + exit code, no matter the use case?).

We can start slow, by implementing a new run* function that only supports the most common use case (or whichever one you have a need for), and gradually extend it's capabilities. Once we have a fully fleshed out new shiny run* implementation, we can deprecate run_cmd (and run_cmd_qa command) and make sure we use the new run* everywhere, both in framework itself and in easyblocks (that's perhaps a change to only make in EasyBuild v5.0, to avoid surprises).

[Flamefire]

I agree that a rewrite is overdue, but this is IMO a bugfix. See the description: In some places we expect that with log_ok set to False this will NOT raise an Exception.

The lengthy comment serves as a guidance for the future run-function