Skip to content

Conversation

@mcoil1
Copy link
Contributor

@mcoil1 mcoil1 commented Nov 10, 2019

This change fixes the misleading error message from mpiexec with missing #! interpreter (#4528). I added a conditional to check whether the file exists, which determines whether to output the default error string (missing file) or a new string (bad interpreter).

Fixes #4528

Signed-off-by: Maxwell Coil [email protected]

@ompiteam-bot
Copy link

Can one of the admins verify this patch?

@jsquyres
Copy link
Member

ok to test

@mcoil1 mcoil1 force-pushed the pr/fix-misleading-error-message branch from 86940f4 to 5ac46cc Compare November 11, 2019 02:49
getcwd(dir, sizeof(dir));
struct stat stats;
char* msg;
if(0 == stat(cd->app->app, &stats)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What if the app is a binary with unresolved dependencies?
I am afraid the error message will be "Bad interpreter" in that case.

Copy link
Member

Choose a reason for hiding this comment

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

I agree; this would batch too many failure modes (wrong architecture, missing libraries, wrong os, etc.) into one error code, and worse, it would be coming from us, not the OS>

Copy link
Contributor

Choose a reason for hiding this comment

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

one option could be to stat() the file, and if successful and it starts with #!, then add a hint into the error message that the MPI app is indeed a script and the error could come from the interpreter.

Copy link
Member

@jsquyres jsquyres Nov 14, 2019

Choose a reason for hiding this comment

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

On Linux, it looks like errno == ENOENT means either the file was missing or it's a bad interpreter. Looks like it's the same behavior on MacOS.

So adding a check for errno == ENOENT would likely be sufficient for this small fix.

This change fixes the misleading error message. I added a conditional to
determine whether the error is due to a missing file or a bad interpreter.
If it is the latter, a new, more precise error message will be displayed.

Fixes open-mpi#4528

Signed-off-by: Maxwell Coil <[email protected]>
@mcoil1 mcoil1 force-pushed the pr/fix-misleading-error-message branch from 5ac46cc to 9b73f6a Compare November 14, 2019 23:07
@mcoil1
Copy link
Contributor Author

mcoil1 commented Nov 14, 2019

I added the ENOENT check.

Copy link
Member

@jsquyres jsquyres left a comment

Choose a reason for hiding this comment

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

@ggouaillardet @bwbarrett Any other comments on this PR?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Misleading error message from mpiexec with missing #! interpreter

5 participants