Skip to content

Conversation

@cdown
Copy link

@cdown cdown commented Jan 11, 2022

Commands which fail, even at (for example) dynamic loader stage get a
pass if there's a "-" leader to indicate to ignore errors. That means
that any user using this must accept that there's a chance that the
command will not execute whatsoever.

Since these outcomes are the same as the executable not existing at all,
allow "-" to also silence the command not existing in the first place.

As an example, with:

[testenv:bogus]
commands =
-bogus

Before:

ERROR: InvocationError for command could not find executable bogus
________________________ summary ________________________
ERROR:   bogus: commands failed

After:

WARNING: command not found but explicitly ignored
cmd: bogus
________________________ summary ________________________
bogus: commands succeeded
congratulations :)

Closes #2315.

Commands which fail, even at (for example) dynamic loader stage get a
pass if there's a "-" leader to indicate to ignore errors. That means
that any user using this must accept that there's a chance that the
command will not execute whatsoever.

Since these outcomes are the same as the executable not existing at all,
allow "-" to also silence the command not existing in the first place.

As an example, with:

    [testenv:bogus]
    commands =
	-bogus

Before:

    ERROR: InvocationError for command could not find executable bogus
    ________________________ summary ________________________
    ERROR:   bogus: commands failed

After:

    WARNING: command not found but explicitly ignored
    cmd: bogus
    ________________________ summary ________________________
    bogus: commands succeeded
    congratulations :)

Closes tox-dev#2315.
@jugmac00
Copy link
Member

jugmac00 commented Jan 11, 2022

Could you elaborate on your use case?

Usually - is used to make a tox run pass even when e.g. a linter fails.

This makes sense, as the linter output is printed to stdout so it does not go by completely unnoticed but CI still succeeds - think of e.g. some whitespace issues detected by flake8.

To me an unavailable command is something completely different. That indicates a broken setup.

Without knowing more about a good reason I am -1 on this.

@cdown
Copy link
Author

cdown commented Jan 11, 2022

@jugmac00 The use case is from #python IRC:

15:02:04 <NoImNotNi> hm. okay, so, using tox, in my commands, i have a bunch of 'optional' commands (where i don't want their failure to prevent a successful tox run), so i prefix them with a hyphen as per tox docs, and this works fine, for the most part
15:02:36 <NoImNotNi> but for some commands, maybe a dependency isn't installed (e.g. npm), and so tox gets a ERROR: InvocationError: could not find executable 'npx'
15:03:09 <NoImNotNi> and this class of error apparently doesn't get ignored by the leading hyphen? because it causes the overall tox job to fail.

While there are other ways to go about this (wrapper script, etc), using - to solely indicate "pass even if [some application] fails" is not correct, because (for example) we could even be failing to load the interpreter for a script, FFI, or dynlibs and it would still pass. Thus anyone using - must accept that it will also pass on said broken setups. This just codifies that that is the case, it doesn't change the existing paradigm, the same as how make works.

Any user using - must accept that they cannot know why the command failed -- up to and including complete environment brokenness -- even in the current case. Going half way on that and saying "it's fine if the command isn't even runnable (for example, it's not even a recognised binfmt or shebang), but not fine if the command doesn't exist" doesn't make sense to me.

Copy link
Member

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@asottile
Copy link
Contributor

there is at least precedent for this in make -- as weird as it seems:

$ cat Makefile 
all:
	-wat
$ make all
wat
make: wat: Command not found
make: [Makefile:2: all] Error 127 (ignored)

@gaborbernat gaborbernat merged commit 9cc692d into tox-dev:master Jan 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ignore missing commands when ignoring errors

4 participants