Bring ALPS ODLS up to par regarding wdir. #3310
Conversation
Signed-off-by: Mark Santcroos <mark.santcroos@rutgers.edu>
|
|
||
| /* take us to the correct wdir */ | ||
| if (NULL != cd->wdir) { | ||
| chdir(cd->wdir); |
There was a problem hiding this comment.
I'd like to see this handle the case that chdir returns with error.
There was a problem hiding this comment.
I don't disagree strongly, although I think your request is already covered by the call to orte_util_check_context_cwd() (https://github.com/open-mpi/ompi/blob/master/orte/util/context_fns.c#L56).
Note that my addition is mirroring the existing code in the default ODLS module (https://github.com/open-mpi/ompi/blob/master/orte/mca/odls/default/odls_default_module.c#L417).
There was a problem hiding this comment.
I kinda agree with @marksantcroos on this one. We cannot get to this code path unless the chdir already worked. I admit there is a race condition - could be that the directory got removed since we tested it. Since we do have a way of returning the error, it wouldn't be hard to do so if it truly is a concern.
FWIW: I'll probably go back and add it to the default one, just for aesthetics.
|
@rhc54 Would you like me to add the check in both places? Happy to do so. |
|
Oh - yes please! Much appreciated 😄 |
|
I'm okay with improving possible error return from |
Signed-off-by: Mark Santcroos <mark.santcroos@rutgers.edu>
|
Very nice - thanks!!! |
Hold your guns! ;) It turned out to be less straight forward than I anticipated ... Just noticed that basename is not set in the Cray case. |
|
I think that should be fine |
Signed-off-by: Mark Santcroos <mark.santcroos@rutgers.edu>
|
Ok, thanks for the feedback. Tested on Titan and MacOS, its good to go for me. |
3 participants
This fixes #3296 which was caused by #3217.
Signed-off-by: Mark Santcroos mark.santcroos@rutgers.edu