From 6781900f98da233cd51ffbae7d93ee1728ed833e Mon Sep 17 00:00:00 2001 From: Jeff Squyres Date: Thu, 26 May 2005 15:57:48 +0000 Subject: [PATCH] (copied from an e-mail, just so that I don't have to re-type the entire explanation ;-) ) Our Abaqus friends just pointed out another bug to me. We have the "-x" option to orterun to export environment variables to newly-started processes. However, it doesn't work if the environment variable is already set in the target environment. For example: mpirun -x LD_LIBRARY_PATH -np 2 a.out The app context correctly contains LD_LIBRARY_PATH and its value, and that app context correctly propagates out to the orted and is present when we fork/exec a.out. However, if LD_LIBRARY_PATH is already set in the newly-started process' environment, the fork pls won't override it with the value from the app context. It really only has to do with the ordering of arguments in ompi_environ_merge() -- when merging to env arrays together, we "prefer" one set to the other if there are duplicate names. I think that if the user wants to override variables (even variables like LD_LIBRARY_PATH), we should let them -- it's necessary for some applications (like in Abaqus' case). If they screw it up, it's their fault (e.g., setting some LD_LIBRARY_PATH that won't work). That being said, we should *not* allow them to override specific MCA parameters that are necessary for startup -- that's easy to accomplish by setting up that stuff *after* we merge in the context app environment. Also note that I am *only* speaking about the fork pls here -- so this only applies to started ORTE job processes, not the orted. So an easy re-order to do the following: env_copy = merge(environ and context->app) ompi_setenv(...MCA params necessary for startup..., env_copy) execve(..., env_copy) does what we want. This commit was SVN r5878. --- src/mca/pls/fork/pls_fork_module.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/mca/pls/fork/pls_fork_module.c b/src/mca/pls/fork/pls_fork_module.c index c15c252ed9..2b2d6d30e7 100644 --- a/src/mca/pls/fork/pls_fork_module.c +++ b/src/mca/pls/fork/pls_fork_module.c @@ -144,7 +144,7 @@ static int orte_pls_fork_proc( if(pid == 0) { char* param; char* uri; - char **new_env, **environ_copy; + char **environ_copy; #if 0 /* for gperf - setup a new directory for each executable */ @@ -164,8 +164,13 @@ static int orte_pls_fork_proc( } #endif - /* setup base environment */ - environ_copy = ompi_argv_copy(environ); + /* setup base environment: copy the current environ and merge + in the app context environ */ + if (NULL != context->env) { + environ_copy = ompi_environ_merge(environ, context->env); + } else { + environ_copy = ompi_argv_copy(environ); + } param = mca_base_param_environ_variable("rmgr","bootproxy","jobid"); ompi_unsetenv(param, &environ_copy); @@ -190,7 +195,7 @@ static int orte_pls_fork_proc( ompi_setenv(param, uri, true, &environ_copy); free(param); free(uri); - + /* setup gpr contact info */ if(NULL != orte_process_info.gpr_replica_uri) { uri = strdup(orte_process_info.gpr_replica_uri); @@ -209,10 +214,6 @@ static int orte_pls_fork_proc( /* setup stdout/stderr */ orte_iof_base_setup_child(&opts); - /* execute application */ - new_env = ompi_environ_merge(context->env, environ_copy); - ompi_argv_free(environ_copy); - if (context->argv == NULL) { context->argv = malloc(sizeof(char*)*2); context->argv[0] = strdup(context->app); @@ -242,7 +243,7 @@ static int orte_pls_fork_proc( /* Exec the new executable */ - execve(context->app, context->argv, new_env); + execve(context->app, context->argv, environ_copy); ompi_output(0, "orte_pls_fork: %s - %s\n", context->app, ompi_argv_join(context->argv, ' ')); ompi_output(0, "orte_pls_fork: execv failed with errno=%d\n", errno);