Skip to content
This repository was archived by the owner on Sep 30, 2022. It is now read-only.

Conversation

@rhc54
Copy link

@rhc54 rhc54 commented Aug 12, 2016

(cherry picked from commit open-mpi/ompi@1c44543)

@rhc54 rhc54 added the bug label Aug 12, 2016
@rhc54 rhc54 added this to the v2.0.1 milestone Aug 12, 2016
@lanl-ompi
Copy link
Contributor

Test FAILed.

@mellanox-github
Copy link

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ompi-release-pr/2085/ for details.

@mellanox-github
Copy link

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ompi-release-pr/2087/ for details.

@jsquyres
Copy link
Member

@rhc54 I note that this is quite a bit more than the cherry-picked open-mpi/ompi@1c44543. Was that intentional?

@jsquyres
Copy link
Member

Doing a little log spelunking, it looks like this also includes open-mpi/ompi@5717b75.

* environment variables. If so, setup the path and argv[0].
* Note that we allow the user to specify the launch agent
* even if they are in a Grid Engine environment */
if (0 == strcmp(mca_plm_rsh_component.agent, "ssh : rsh")) {
Copy link
Member

Choose a reason for hiding this comment

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

Minor nit: open-mpi/ompi#1972 might be a more robust way to check to see if we're not using the default value...?

@jsquyres
Copy link
Member

Other than the two minor nits (which can be fixed after this PR), as long as open-mpi/ompi@5717b75 was intended to be in this PR, 👍

(cherry picked from commit open-mpi/ompi@1c44543)

Restore the rsh template creation code

(cherry picked from commit open-mpi/ompi@5717b75)

Further cleanup getpwuid usage - try it first (unless completely disabled), and then silently failover to try other methods.

(cherry picked from commit open-mpi/ompi@9f43db7)
@rhc54
Copy link
Author

rhc54 commented Aug 16, 2016

I didn't worry about the history of the changes - I just fixed the problem 😄

This should now be complete. We can add the better way of detecting default later.

@jsquyres
Copy link
Member

You really like squashing your commits, don't you? 😄 It took a minute to figure out what was new in your new commit (i.e., the getpwent stuff). Looks good.

I agree that the (potentially) more robust check can be added later.

@jsquyres
Copy link
Member

@hppritcha Once CI finishes, this is good to go.

@mellanox-github
Copy link

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ompi-release-pr/2091/ for details.

@jsquyres
Copy link
Member

False Travis failure (we killed the build because it was taking hours and it hadn't even start yet).

@jsquyres jsquyres merged commit a85789d into open-mpi:v2.x Aug 16, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants