diff --git a/src/env.cpp b/src/env.cpp index 19b799b3c..e502daa92 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -1014,6 +1014,38 @@ static int set_umask(const wcstring_list_t &list_val) { return ENV_OK; } +/// Set a universal variable, inheriting as applicable from the given old variable. +static void env_set_internal_universal(const wcstring &key, wcstring_list_t val, + env_mode_flags_t input_var_mode) { + ASSERT_IS_MAIN_THREAD(); + if (!uvars()) return; + env_mode_flags_t var_mode = input_var_mode; + auto oldvar = uvars()->get(key); + // Resolve whether or not to export. + if ((var_mode & (ENV_EXPORT | ENV_UNEXPORT)) == 0) { + bool doexport = oldvar ? oldvar->exports() : false; + var_mode |= (doexport ? ENV_EXPORT : ENV_UNEXPORT); + } + // Resolve whether to be a path variable. + // Here we fall back to the auto-pathvar behavior. + if ((var_mode & (ENV_PATHVAR | ENV_UNPATHVAR)) == 0) { + bool dopathvar = oldvar ? oldvar->is_pathvar() : variable_should_auto_pathvar(key); + var_mode |= (dopathvar ? ENV_PATHVAR : ENV_UNPATHVAR); + } + + // Construct and set the new variable. + env_var_t::env_var_flags_t varflags = 0; + if (var_mode & ENV_EXPORT) varflags |= env_var_t::flag_export; + if (var_mode & ENV_PATHVAR) varflags |= env_var_t::flag_pathvar; + env_var_t new_var{val, varflags}; + + uvars()->set(key, new_var); + env_universal_barrier(); + if (new_var.exports() || (oldvar && oldvar->exports())) { + vars_stack().mark_changed_exported(); + } +} + /// Set the value of the environment variable whose name matches key to val. /// /// \param key The key @@ -1062,24 +1094,8 @@ static int env_set_internal(const wcstring &key, env_mode_flags_t input_var_mode } if (var_mode & ENV_UNIVERSAL) { - const bool old_export = uvars() && uvars()->get_export(key); - bool new_export; - if (var_mode & ENV_EXPORT) { - // Export the var. - new_export = true; - } else if (var_mode & ENV_UNEXPORT) { - // Unexport the var. - new_export = false; - } else { - // Not changing the export status of the var. - new_export = old_export; - } if (uvars()) { - uvars()->set(key, val, new_export); - env_universal_barrier(); - if (old_export || new_export) { - vars_stack().mark_changed_exported(); - } + env_set_internal_universal(key, std::move(val), var_mode); } } else { // Determine the node. @@ -1113,20 +1129,10 @@ static int env_set_internal(const wcstring &key, env_mode_flags_t input_var_mode set_proc_had_barrier(true); env_universal_barrier(); } - if (uvars() && uvars()->get(key)) { - bool exportv; - if (var_mode & ENV_EXPORT) { - exportv = true; - } else if (var_mode & ENV_UNEXPORT) { - exportv = false; - } else { - exportv = uvars()->get_export(key); - } - uvars()->set(key, val, exportv); - env_universal_barrier(); - done = 1; - + // Modifying an existing universal variable. + env_set_internal_universal(key, std::move(val), var_mode); + done = true; } else { // New variable with unspecified scope node = vars_stack().resolve_unspecified_scope(); @@ -1268,8 +1274,13 @@ int env_remove(const wcstring &key, int var_mode) { } if (!erased && !(var_mode & ENV_GLOBAL) && !(var_mode & ENV_LOCAL)) { - bool is_exported = uvars()->get_export(key); - erased = uvars() && uvars()->remove(key); + bool is_exported = false; + if (uvars()) { + if (auto old_flags = uvars()->get_flags(key)) { + is_exported = *old_flags & env_var_t::flag_export; + } + erased = uvars()->remove(key); + } if (erased) { env_universal_barrier(); event_t ev = event_t::variable_event(key); @@ -1377,7 +1388,7 @@ maybe_t env_get(const wcstring &key, env_mode_flags_t mode) { // universal var return that. if (uvars()) { auto var = uvars()->get(key); - if (var && (uvars()->get_export(key) ? search_exported : search_unexported)) { + if (var && (var->exports() ? search_exported : search_unexported)) { return var; } } diff --git a/src/env_universal_common.cpp b/src/env_universal_common.cpp index 2797268f9..7388b4825 100644 --- a/src/env_universal_common.cpp +++ b/src/env_universal_common.cpp @@ -255,17 +255,15 @@ maybe_t env_universal_t::get(const wcstring &name) const { return none(); } -bool env_universal_t::get_export(const wcstring &name) const { - bool result = false; +maybe_t env_universal_t::get_flags(const wcstring &name) const { var_table_t::const_iterator where = vars.find(name); if (where != vars.end()) { - result = where->second.exports(); + return where->second.get_flags(); } - return result; + return none(); } -void env_universal_t::set_internal(const wcstring &key, wcstring_list_t vals, bool exportv, - bool overwrite) { +void env_universal_t::set_internal(const wcstring &key, env_var_t var, bool overwrite) { ASSERT_IS_LOCKED(lock); if (!overwrite && this->modified.find(key) != this->modified.end()) { // This value has been modified and we're not overwriting it. Skip it. @@ -273,9 +271,8 @@ void env_universal_t::set_internal(const wcstring &key, wcstring_list_t vals, bo } env_var_t &entry = vars[key]; - if (entry.exports() != exportv || entry.as_list() != vals) { - entry.set_vals(std::move(vals)); - entry.set_exports(exportv); + if (entry != var) { + entry = var; // If we are overwriting, then this is now modified. if (overwrite) { @@ -284,9 +281,9 @@ void env_universal_t::set_internal(const wcstring &key, wcstring_list_t vals, bo } } -void env_universal_t::set(const wcstring &key, wcstring_list_t vals, bool exportv) { +void env_universal_t::set(const wcstring &key, env_var_t var) { scoped_lock locker(lock); - this->set_internal(key, std::move(vals), exportv, true /* overwrite */); + this->set_internal(key, std::move(var), true /* overwrite */); } bool env_universal_t::remove_internal(const wcstring &key) { diff --git a/src/env_universal_common.h b/src/env_universal_common.h index 7ea32e665..d418db7cc 100644 --- a/src/env_universal_common.h +++ b/src/env_universal_common.h @@ -56,7 +56,7 @@ class env_universal_t { bool load_from_path(const wcstring &path, callback_data_list_t &callbacks); void load_from_fd(int fd, callback_data_list_t &callbacks); - void set_internal(const wcstring &key, wcstring_list_t val, bool exportv, bool overwrite); + void set_internal(const wcstring &key, env_var_t var, bool overwrite); bool remove_internal(const wcstring &name); // Functions concerned with saving. @@ -93,11 +93,11 @@ class env_universal_t { // Get the value of the variable with the specified name. maybe_t get(const wcstring &name) const; - // Returns whether the variable with the given name is exported, or false if it does not exist. - bool get_export(const wcstring &name) const; + // \return flags from the variable with the given name. + maybe_t get_flags(const wcstring &name) const; // Sets a variable. - void set(const wcstring &key, wcstring_list_t val, bool exportv); + void set(const wcstring &key, env_var_t var); // Removes a variable. Returns true if it was found, false if not. bool remove(const wcstring &name); diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index ac5f58a31..1fb3d1666 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -2834,7 +2834,7 @@ static int test_universal_helper(int x) { for (int j = 0; j < UVARS_PER_THREAD; j++) { const wcstring key = format_string(L"key_%d_%d", x, j); const wcstring val = format_string(L"val_%d_%d", x, j); - uvars.set(key, {val}, false); + uvars.set(key, env_var_t{val, 0}); bool synced = uvars.sync(callbacks); if (!synced) { err(L"Failed to sync universal variables after modification"); @@ -2966,28 +2966,30 @@ static void test_universal_callbacks() { env_universal_t uvars1(UVARS_TEST_PATH); env_universal_t uvars2(UVARS_TEST_PATH); + env_var_t::env_var_flags_t noflags = 0; + // Put some variables into both. - uvars1.set(L"alpha", {L"1"}, false); - uvars1.set(L"beta", {L"1"}, false); - uvars1.set(L"delta", {L"1"}, false); - uvars1.set(L"epsilon", {L"1"}, false); - uvars1.set(L"lambda", {L"1"}, false); - uvars1.set(L"kappa", {L"1"}, false); - uvars1.set(L"omicron", {L"1"}, false); + uvars1.set(L"alpha", env_var_t{L"1", noflags}); + uvars1.set(L"beta", env_var_t{L"1", noflags}); + uvars1.set(L"delta", env_var_t{L"1", noflags}); + uvars1.set(L"epsilon", env_var_t{L"1", noflags}); + uvars1.set(L"lambda", env_var_t{L"1", noflags}); + uvars1.set(L"kappa", env_var_t{L"1", noflags}); + uvars1.set(L"omicron", env_var_t{L"1", noflags}); uvars1.sync(callbacks); uvars2.sync(callbacks); // Change uvars1. - uvars1.set(L"alpha", {L"2"}, false); // changes value - uvars1.set(L"beta", {L"1"}, true); // changes export + uvars1.set(L"alpha", env_var_t{L"2", noflags}); // changes value + uvars1.set(L"beta", env_var_t{L"1", env_var_t::flag_export}); // changes export uvars1.remove(L"delta"); // erases value - uvars1.set(L"epsilon", {L"1"}, false); // changes nothing + uvars1.set(L"epsilon", env_var_t{L"1", noflags}); // changes nothing uvars1.sync(callbacks); // Change uvars2. It should treat its value as correct and ignore changes from uvars1. - uvars2.set(L"lambda", {L"1"}, false); // same value - uvars2.set(L"kappa", {L"2"}, false); // different value + uvars2.set(L"lambda", {L"1", noflags}); // same value + uvars2.set(L"kappa", {L"2", noflags}); // different value // Now see what uvars2 sees. callbacks.clear(); @@ -3047,7 +3049,7 @@ static void test_universal_ok_to_save() { uvars.sync(cbs); cbs.clear(); do_test(!uvars.is_ok_to_save() && "Should no longer be OK to save"); - uvars.set(L"SOMEVAR", {L"SOMEVALUE"}, false); + uvars.set(L"SOMEVAR", env_var_t{wcstring{L"SOMEVALUE"}, 0}); uvars.sync(cbs); // Ensure file is same. diff --git a/tests/test3.err b/tests/test3.err index e69de29bb..30aa0088a 100644 --- a/tests/test3.err +++ b/tests/test3.err @@ -0,0 +1,3 @@ + +#################### +# Path universal variables diff --git a/tests/test3.in b/tests/test3.in index 5cad24e86..e4d8c2b42 100644 --- a/tests/test3.in +++ b/tests/test3.in @@ -311,4 +311,18 @@ set -x DONT_ESCAPE_COLONS 1: 2: :3: ; env | grep '^DONT_ESCAPE_COLONS=' set -x DONT_ESCAPE_SPACES '1 ' '2 ' ' 3 ' 4 ; env | grep '^DONT_ESCAPE_SPACES=' set -x DONT_ESCAPE_COLONS_PATH 1: 2: :3: ; env | grep '^DONT_ESCAPE_COLONS_PATH=' +logmsg Path universal variables +set __fish_test_path_not a b c +set __fish_test_PATH 1 2 3 +echo "$__fish_test_path_not $__fish_test_PATH" +set --unpath __fish_test_PATH $__fish_test_PATH +echo "$__fish_test_path_not $__fish_test_PATH" +set --path __fish_test_path_not $__fish_test_path_not +echo "$__fish_test_path_not $__fish_test_PATH" +set --path __fish_test_PATH $__fish_test_PATH +echo "$__fish_test_path_not $__fish_test_PATH" + + + + true diff --git a/tests/test3.out b/tests/test3.out index c466c8f6b..9c8bfd2da 100644 --- a/tests/test3.out +++ b/tests/test3.out @@ -46,3 +46,10 @@ MANPATH=man1:man2:man3 DONT_ESCAPE_COLONS=1: 2: :3: DONT_ESCAPE_SPACES=1 2 3 4 DONT_ESCAPE_COLONS_PATH=1::2:::3: + +#################### +# Path universal variables +a b c 1:2:3 +a b c 1 2 3 +a:b:c 1 2 3 +a:b:c 1:2:3