Skip to content

Conversation

@simonbyrne
Copy link
Contributor

@simonbyrne simonbyrne commented Aug 12, 2019

I found that I needed to apply the same change as #5597/#5598 to orterun.c for the environment variables to work correctly.

@ompiteam-bot
Copy link

Can one of the admins verify this patch?

@jsquyres
Copy link
Member

ok to test

@jsquyres
Copy link
Member

@simonbyrne Per our contribution guidelines, can you sign-off your commit? Thanks.

I found that I needed to apply the same change as open-mpi#5597 to orterun.c for the environment variables to work correctly. Signed-off-by: Simon Byrne <simonbyrne@gmail.com>
@simonbyrne
Copy link
Contributor Author

@jsquyres done.

@jsquyres
Copy link
Member

@simonbyrne Thanks for updating.

Gotta say -- this is a little surprising that for the amount of noise that was made over this feature, no one noticed this issue for a year. ☹️

@rhc54 rhc54 merged commit 4e06be1 into open-mpi:master Aug 19, 2019
@rhc54
Copy link
Contributor

rhc54 commented Aug 19, 2019

@simonbyrne Might want to cherry-pick this to the v4.0.x branch

jsquyres added a commit to jsquyres/ompi that referenced this pull request Aug 19, 2019
open-mpi#6895 fixed the code in orterun.c to allow running as root if both OMPI_ALLOW_RUN_AS_ROOT and OMPI_ALLOW_RUN_AS_ROOT_CONFIRM env vars are set. However, this env-var-checking code already exists in orte_submit.c:orte_submit_init() -- it looks like the geteuid()/getenv()-checking code here in orterun is now duplicate code. So let's just get rid of the duplicate code. Signed-off-by: Jeff Squyres <jsquyres@cisco.com>
@jsquyres
Copy link
Member

After some internal discussion, I just filed #6910 as an addendum to this PR. When we merge #6910, for simplicity, we can cherry-pick the commit from this PR and the commit from #6910 over to v4.0.x.

jsquyres added a commit to jsquyres/ompi that referenced this pull request Aug 19, 2019
open-mpi#6895 fixed the code in orterun.c to allow running as root if both OMPI_ALLOW_RUN_AS_ROOT and OMPI_ALLOW_RUN_AS_ROOT_CONFIRM env vars are set. However, this env-var-checking code already exists in orte_submit.c:orte_submit_init() -- it looks like the geteuid()/getenv()-checking code here in orterun is now duplicate code. So let's just get rid of the duplicate code. Signed-off-by: Jeff Squyres <jsquyres@cisco.com> (cherry picked from commit 197beb3)
@simonbyrne simonbyrne deleted the sb/root branch August 19, 2019 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

5 participants