Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

strict_errexit message missing code location #2118

Open
ale5000-git opened this issue Nov 2, 2024 · 5 comments
Open

strict_errexit message missing code location #2118

ale5000-git opened this issue Nov 2, 2024 · 5 comments

Comments

@ale5000-git
Copy link

ale5000-git commented Nov 2, 2024

I'm using ysh from here: https://op.oilshell.org/uuu/github-jobs/8206/cpp-tarball.wwz/_release/oils-for-unix.tar

I was trying to run this script with ysh: https://github.com/micro5k/microg-unofficial-installer/blob/main/tools/bits-info.sh
and the only thing it say is this:
[??? no location ???] fatal: strict_errexit only allows simple commands in conditionals (got command.Redirect).

I think it should at least say the line number.

andychu pushed a commit that referenced this issue Nov 3, 2024
This gives a location for issue #2118.

But it would better to attribute it to a child node of the redirect, not
the redirect itself.
andychu pushed a commit that referenced this issue Nov 3, 2024
And point to a more precise location.

This affects the error in issue #2118.

[doc] Fix for CI
@andychu
Copy link
Contributor

andychu commented Nov 3, 2024

Thanks for the report - I improved the message to look like this

$ bin/ysh bits-info.sh 
  {
  ^
bits-info.sh:19: fatal: Command conditionals should only have one status, not BraceGroup (strict_errexit, OILS-ERR-300)

OILS-ERR-300 refers to a new message in the error catalog, which will be rendered shortly

http://op.oilshell.org/uuu/github-jobs/8214/

@andychu
Copy link
Contributor

andychu commented Nov 3, 2024

There is a long doc about out error handling philosophy, which is explicit and is designed not to lose any exit codes

http://www.oilshell.org/release/0.23.0/doc/error-handling.html

{
  command 1> /dev/null -v ':'
} 2> /dev/null || command()

Hm arguably it could be relaxed for this case, because there is only one command.

The problem is when the {} contains two commands - it is ambiguous. Not sure if that is possibel though


I think that statement can be written

command -v ':' 1>/dev/null 2>/dev/null 

that is probably the same thing

$ command zz
-bash: zz: command not found

$ command 2>/dev/null zz

@ale5000-git
Copy link
Author

ale5000-git commented Nov 3, 2024

The problem is that the shell that don't support command actually print the error outside the command so command -v ':' 1>/dev/null 2>/dev/null don't hide the error.

Initially the check was command -v command but then another shell (hush) fail because command can't check itself there (most likely a bug) so I have changed it to check : that always exist (so at the end it actualy check command).

@andychu andychu changed the title ysh cannot find error location strict_errexit message missing code location Nov 3, 2024
@andychu
Copy link
Contributor

andychu commented Nov 3, 2024

OK the docs rendered now:

https://op.oilshell.org/uuu/github-jobs/8214/ovm-tarball.wwz/_release/VERSION/doc/error-catalog.html


I think the idioms for OSH and YSH will be very different here

obviously if you switch to YSH, then the script won't work under any other shell

But for OSH, there is an extremely large common subset with bash and POSIX shell


thanks for reporting the missing location!

@ale5000-git
Copy link
Author

ale5000-git commented Nov 3, 2024

obviously if you switch to YSH, then the script won't work under any other shell

I will try make at least a part of the script work on both; the script is already tested on more than 10 shells so it doesn't use much advanced features.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants