From b6aa691e0af14a0e783a1446f794c213e75063ad Mon Sep 17 00:00:00 2001 From: Ralph Castain Date: Thu, 16 Oct 2014 12:58:56 -0700 Subject: [PATCH] Fix incorrect implementation of new MCA param mca_base_env_list - it was not picking up envars and forwarding them, but only worked if you explicitly set a value for the envar. Ensure it works for both direct and indirect launch modes. Remove stale code as this replaced orte_forward_envars. Ensure it doesn't get passed to the ORTE daemons. --- opal/mca/base/mca_base_var.c | 29 ++++++----- opal/mca/base/mca_base_var.h | 6 +++ opal/mca/pmix/cray/pmix_cray.c | 5 ++ opal/mca/pmix/s1/pmix_s1.c | 5 ++ opal/mca/pmix/s2/pmix_s2.c | 5 ++ orte/mca/plm/rsh/plm_rsh_module.c | 7 ++- orte/orted/orted_main.c | 1 + orte/runtime/orte_globals.c | 1 - orte/runtime/orte_globals.h | 1 - orte/runtime/orte_mca_params.c | 7 --- orte/tools/orterun/orterun.c | 84 ++++++++++++++----------------- 11 files changed, 80 insertions(+), 71 deletions(-) diff --git a/opal/mca/base/mca_base_var.c b/opal/mca/base/mca_base_var.c index 61f19f0e69..d8665385b1 100644 --- a/opal/mca/base/mca_base_var.c +++ b/opal/mca/base/mca_base_var.c @@ -127,7 +127,6 @@ static int mca_base_var_cache_files (bool rel_path_search); static int var_set_initial (mca_base_var_t *var); static int var_get (int vari, mca_base_var_t **var_out, bool original); static int var_value_string (mca_base_var_t *var, char **value_string); -static int mca_base_var_process_env_list(void); /* * classes @@ -261,28 +260,28 @@ int mca_base_var_init(void) mca_base_var_cache_files(false); - /* set nesessary env variables for external usage */ - mca_base_var_process_env_list(); + /* register the envar-forwarding params */ + (void)mca_base_var_register ("opal", "mca", "base", "env_list", + "Set SHELL env variables", + MCA_BASE_VAR_TYPE_STRING, NULL, 0, 0, OPAL_INFO_LVL_3, + MCA_BASE_VAR_SCOPE_READONLY, &mca_base_env_list); + (void)mca_base_var_register ("opal", "mca", "base", "env_list_delimiter", + "Set SHELL env variables delimiter. Default: semicolon ';'", + MCA_BASE_VAR_TYPE_STRING, NULL, 0, 0, OPAL_INFO_LVL_3, + MCA_BASE_VAR_SCOPE_READONLY, &mca_base_env_list_sep); } return OPAL_SUCCESS; } -static int mca_base_var_process_env_list(void) +int mca_base_var_process_env_list(char ***argv) { int i; char** tokens; char* ptr; char* param, *value; char sep; - (void)mca_base_var_register ("opal", "mca", "base", "env_list", - "Set SHELL env variables", - MCA_BASE_VAR_TYPE_STRING, NULL, 0, 0, OPAL_INFO_LVL_3, - MCA_BASE_VAR_SCOPE_READONLY, &mca_base_env_list); - (void)mca_base_var_register ("opal", "mca", "base", "env_list_delimiter", - "Set SHELL env variables delimiter. Default: semicolon ';'", - MCA_BASE_VAR_TYPE_STRING, NULL, 0, 0, OPAL_INFO_LVL_3, - MCA_BASE_VAR_SCOPE_READONLY, &mca_base_env_list_sep); + if (NULL == mca_base_env_list) { return OPAL_SUCCESS; } @@ -307,10 +306,10 @@ static int mca_base_var_process_env_list(void) value = strchr(param, '='); *value = '\0'; value++; - opal_setenv(param, value, true, &environ); + opal_setenv(param, value, true, argv); free(param); } else { - opal_setenv(tokens[i], value, true, &environ); + opal_setenv(tokens[i], value, true, argv); } } else { opal_show_help("help-mca-var.txt", "incorrect-env-list-param", @@ -321,7 +320,7 @@ static int mca_base_var_process_env_list(void) value = strchr(param, '='); *value = '\0'; value++; - opal_setenv(param, value, true, &environ); + opal_setenv(param, value, true, argv); free(param); } } diff --git a/opal/mca/base/mca_base_var.h b/opal/mca/base/mca_base_var.h index 1138336f5d..4180bbee79 100644 --- a/opal/mca/base/mca_base_var.h +++ b/opal/mca/base/mca_base_var.h @@ -722,6 +722,12 @@ OPAL_DECLSPEC int mca_base_var_dump(int vari, char ***out, mca_base_var_dump_typ #define MCA_COMPILETIME_VER "print_compiletime_version" #define MCA_RUNTIME_VER "print_runtime_version" +/* + * Parse a provided list of envars and add their local value, or + * their assigned value, to the provided argv + */ +OPAL_DECLSPEC int mca_base_var_process_env_list(char ***argv); + END_C_DECLS #endif /* OPAL_MCA_BASE_VAR_H */ diff --git a/opal/mca/pmix/cray/pmix_cray.c b/opal/mca/pmix/cray/pmix_cray.c index eabd9c2f89..a0b944ff56 100644 --- a/opal/mca/pmix/cray/pmix_cray.c +++ b/opal/mca/pmix/cray/pmix_cray.c @@ -18,7 +18,9 @@ #include "opal/types.h" #include "opal_stdint.h" +#include "opal/mca/base/mca_base_var.h" #include "opal/mca/hwloc/base/base.h" +#include "opal/util/opal_environ.h" #include "opal/util/output.h" #include "opal/util/proc.h" #include "opal/util/output.h" @@ -235,6 +237,9 @@ static int cray_init(void) } } + /* setup any local envars we were asked to do */ + mca_base_var_process_env_list(&environ); + return OPAL_SUCCESS; err_exit: PMI2_Finalize(); diff --git a/opal/mca/pmix/s1/pmix_s1.c b/opal/mca/pmix/s1/pmix_s1.c index 54eac0a042..d3ccc400c7 100644 --- a/opal/mca/pmix/s1/pmix_s1.c +++ b/opal/mca/pmix/s1/pmix_s1.c @@ -13,7 +13,9 @@ #include "opal/types.h" #include "opal_stdint.h" +#include "opal/mca/base/mca_base_var.h" #include "opal/mca/hwloc/base/base.h" +#include "opal/util/opal_environ.h" #include "opal/util/output.h" #include "opal/util/proc.h" #include "opal/util/show_help.h" @@ -290,6 +292,9 @@ static int s1_init(void) goto err_exit; } + /* setup any local envars we were asked to do */ + mca_base_var_process_env_list(&environ); + return OPAL_SUCCESS; err_exit: diff --git a/opal/mca/pmix/s2/pmix_s2.c b/opal/mca/pmix/s2/pmix_s2.c index 818dd424e9..10d4acc288 100644 --- a/opal/mca/pmix/s2/pmix_s2.c +++ b/opal/mca/pmix/s2/pmix_s2.c @@ -18,7 +18,9 @@ #include "opal/types.h" #include "opal_stdint.h" +#include "opal/mca/base/mca_base_var.h" #include "opal/mca/hwloc/base/base.h" +#include "opal/util/opal_environ.h" #include "opal/util/output.h" #include "opal/util/proc.h" #include "opal/util/show_help.h" @@ -259,6 +261,9 @@ static int s2_init(void) } } + /* setup any local envars we were asked to do */ + mca_base_var_process_env_list(&environ); + return OPAL_SUCCESS; err_exit: PMI2_Finalize(); diff --git a/orte/mca/plm/rsh/plm_rsh_module.c b/orte/mca/plm/rsh/plm_rsh_module.c index 19a5e11519..a699fb6cd3 100644 --- a/orte/mca/plm/rsh/plm_rsh_module.c +++ b/orte/mca/plm/rsh/plm_rsh_module.c @@ -573,8 +573,13 @@ static int setup_launch(int *argcptr, char ***argvptr, * only if they aren't already present */ for (i = 0; NULL != environ[i]; ++i) { + if (0 == strncmp("OMPI_MCA_mca_base_env_list", environ[i], + strlen("OMPI_MCA_mca_base_env_list"))) { + /* ignore this one */ + continue; + } if (0 == strncmp("OMPI_MCA", environ[i], 8)) { - /* check for duplicate in app->env - this + /* check for duplicate in app->env - this * would have been placed there by the * cmd line processor. By convention, we * always let the cmd line override the diff --git a/orte/orted/orted_main.c b/orte/orted/orted_main.c index 61ef22f1da..abf81a7e28 100644 --- a/orte/orted/orted_main.c +++ b/orte/orted/orted_main.c @@ -781,6 +781,7 @@ int orte_daemon(int argc, char *argv[]) "orte_ess_vpid", "orte_ess_num_procs", "orte_parent_uri", + "mca_base_env_list", NULL }; for (i=0; i < argc; i++) { diff --git a/orte/runtime/orte_globals.c b/orte/runtime/orte_globals.c index 30790f70f8..674849083b 100644 --- a/orte/runtime/orte_globals.c +++ b/orte/runtime/orte_globals.c @@ -177,7 +177,6 @@ bool orte_abort_non_zero_exit; int orte_stat_history_size; /* envars to forward */ -char *orte_forward_envars = NULL; char **orte_forwarded_envars = NULL; /* map-reduce mode */ diff --git a/orte/runtime/orte_globals.h b/orte/runtime/orte_globals.h index d341588244..e62fee3524 100644 --- a/orte/runtime/orte_globals.h +++ b/orte/runtime/orte_globals.h @@ -556,7 +556,6 @@ ORTE_DECLSPEC extern bool orte_abort_non_zero_exit; ORTE_DECLSPEC extern int orte_stat_history_size; /* envars to forward */ -ORTE_DECLSPEC extern char *orte_forward_envars; ORTE_DECLSPEC extern char **orte_forwarded_envars; /* map-reduce mode */ diff --git a/orte/runtime/orte_mca_params.c b/orte/runtime/orte_mca_params.c index 474dc2fbf9..674d547a60 100644 --- a/orte/runtime/orte_mca_params.c +++ b/orte/runtime/orte_mca_params.c @@ -658,13 +658,6 @@ int orte_register_params(void) OPAL_INFO_LVL_9, MCA_BASE_VAR_SCOPE_READONLY, &orte_stat_history_size); - orte_forward_envars = NULL; - (void) mca_base_var_register ("orte", "orte", NULL, "forward_envars", - "Comma-delimited environmental variables to forward, can include value to set", - MCA_BASE_VAR_TYPE_STRING, NULL, 0, 0, - OPAL_INFO_LVL_9, MCA_BASE_VAR_SCOPE_READONLY, - &orte_forward_envars); - orte_max_vm_size = -1; (void) mca_base_var_register ("orte", "orte", NULL, "max_vm_size", "Maximum size of virtual machine - used to subdivide allocation", diff --git a/orte/tools/orterun/orterun.c b/orte/tools/orterun/orterun.c index 5b49c639af..28fe2df792 100644 --- a/orte/tools/orterun/orterun.c +++ b/orte/tools/orterun/orterun.c @@ -834,8 +834,10 @@ int orterun(int argc, char *argv[]) */ orte_launch_environ = opal_argv_copy(environ); - /* purge an ess flag set externally */ + /* purge any ess flag set externally */ opal_unsetenv("OMPI_MCA_ess", &orte_launch_environ); + /* purge the env list, if set */ + opal_unsetenv("OMPI_MCA_mca_base_env_list", &orte_launch_environ); #if OPAL_ENABLE_FT_CR == 1 /* Disable OPAL CR notifications for this tool */ @@ -1533,6 +1535,11 @@ static int capture_cmd_line_params(int argc, int start, char **argv) for (i = 0; i < (argc-start); ++i) { if (0 == strcmp("-mca", argv[i]) || 0 == strcmp("--mca", argv[i]) ) { + /* ignore this one */ + if (0 == strcmp(argv[i+1], "mca_base_env_list")) { + i += 2; + continue; + } /* It would be nice to avoid increasing the length * of the orted cmd line by removing any non-ORTE * params. However, this raises a problem since @@ -1624,11 +1631,13 @@ static int create_app(int argc, char* argv[], opal_cmd_line_t cmd_line; char cwd[OPAL_PATH_MAX]; int i, j, count, rc; - char *param, *value, *value2; + char *param, *value; orte_app_context_t *app = NULL; bool cmd_line_made = false; bool found = false; char *appname; + char **vars; + char *env_set_flag; *made_app = false; @@ -1718,8 +1727,8 @@ static int create_app(int argc, char* argv[], } /* Did the user request to export any environment variables on the cmd line? */ + env_set_flag = getenv("OMPI_MCA_mca_base_env_list"); if (opal_cmd_line_is_taken(&cmd_line, "x")) { - char* env_set_flag = getenv("OMPI_MCA_mca_base_env_list"); if (NULL != env_set_flag) { orte_show_help("help-orterun.txt", "orterun:conflict-env-set", false); return ORTE_ERR_FATAL; @@ -1730,59 +1739,42 @@ static int create_app(int argc, char* argv[], for (i = 0; i < j; ++i) { param = opal_cmd_line_get_param(&cmd_line, "x", i, 0); - if (NULL != strchr(param, '=')) { - opal_argv_append_nosize(&app->env, param); + if (NULL != (value = strchr(param, '='))) { + /* terminate the name of the param */ + *value = '\0'; + /* step over the equals */ + value++; + /* overwrite any prior entry */ + opal_setenv(param, value, true, &app->env); /* save it for any comm_spawn'd apps */ - opal_argv_append_nosize(&orte_forwarded_envars, param); + opal_setenv(param, value, true, &orte_forwarded_envars); } else { value = getenv(param); if (NULL != value) { - if (NULL != strchr(value, '=')) { - opal_argv_append_nosize(&app->env, value); - /* save it for any comm_spawn'd apps */ - opal_argv_append_nosize(&orte_forwarded_envars, value); - } else { - asprintf(&value2, "%s=%s", param, value); - opal_argv_append_nosize(&app->env, value2); - /* save it for any comm_spawn'd apps */ - opal_argv_append_nosize(&orte_forwarded_envars, value2); - free(value2); - } + /* overwrite any prior entry */ + opal_setenv(param, value, true, &app->env); + /* save it for any comm_spawn'd apps */ + opal_setenv(param, value, true, &orte_forwarded_envars); } else { opal_output(0, "Warning: could not find environment variable \"%s\"\n", param); } } } - } - - /* Did the user request to export any environment variables via MCA param? */ - if (NULL != orte_forward_envars) { - char **vars; - vars = opal_argv_split(orte_forward_envars, ','); - for (i=0; NULL != vars[i]; i++) { - if (NULL != strchr(vars[i], '=')) { - /* user supplied a value */ - opal_argv_append_nosize(&app->env, vars[i]); + } else if (NULL != env_set_flag) { + /* set necessary env variables for external usage */ + vars = NULL; + if (OPAL_SUCCESS == mca_base_var_process_env_list(&vars) && + NULL != vars) { + for (i=0; NULL != vars[i]; i++) { + value = strchr(vars[i], '='); + /* terminate the name of the param */ + *value = '\0'; + /* step over the equals */ + value++; + /* overwrite any prior entry */ + opal_setenv(param, value, true, &app->env); /* save it for any comm_spawn'd apps */ - opal_argv_append_nosize(&orte_forwarded_envars, vars[i]); - } else { - /* get the value from the environ */ - value = getenv(vars[i]); - if (NULL != value) { - if (NULL != strchr(value, '=')) { - opal_argv_append_nosize(&app->env, value); - /* save it for any comm_spawn'd apps */ - opal_argv_append_nosize(&orte_forwarded_envars, value); - } else { - asprintf(&value2, "%s=%s", vars[i], value); - opal_argv_append_nosize(&app->env, value2); - /* save it for any comm_spawn'd apps */ - opal_argv_append_nosize(&orte_forwarded_envars, value2); - free(value2); - } - } else { - opal_output(0, "Warning: could not find environment variable \"%s\"\n", param); - } + opal_setenv(param, value, true, &orte_forwarded_envars); } } opal_argv_free(vars);