Mirrors/fish-shell
2
0
Fork 0
mirror of https://github.com/fish-shell/fish-shell synced 2025-01-16 23:14:04 +00:00
Code Issues Projects Releases Packages Wiki Activity

Teach env_stack_impl_t to report whether it modifies a global

Browse source 
This will help in limiting variable dispatch changes to global and
principal modifications.
This commit is contained in:
ridiculousfish 2019-11-07 22:24:13 -08:00
parent d242ff1808
commit 8f3d745e60
1 changed files with 50 additions and 36 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

86
src/env.cpp
View file

@ -373,7 +373,7 @@ void env_init(const struct config_paths_t *paths /* or NULL */) {
s_universal_variables.emplace(L"");
callback_data_list_t callbacks;
s_universal_variables->initialize(callbacks);
env_universal_callbacks(&env_stack_t::principal(), callbacks);
env_universal_callbacks(&vars, callbacks);
// Do not import variables that have the same name and value as
// an exported universal variable. See issues #5258 and #5348.
@ -806,17 +806,30 @@ std::shared_ptr<environment_t> env_scoped_impl_t::snapshot() const {
return ret;
}
// A struct that wraps up the result of setting or removing a variable.
struct mod_result_t {
// The publicly visible status of the set call.
int status{ENV_OK};
// Whether we modified the global scope.
bool global_modified{false};
// Whether we modified universal variables.
bool uvar_modified{false};
explicit mod_result_t(int status) : status(status) {}
};
/// A mutable subclass of env_scoped_impl_t.
class env_stack_impl_t final : public env_scoped_impl_t {
public:
using env_scoped_impl_t::env_scoped_impl_t;
/// Set a variable under the name \p key, using the given \p mode, setting its value to \p val.
int set(const wcstring &key, env_mode_flags_t mode, wcstring_list_t val,
bool *out_needs_uvar_sync);
mod_result_t set(const wcstring &key, env_mode_flags_t mode, wcstring_list_t val);
/// Remove a variable under the name \p key.
int remove(const wcstring &key, int var_mode, bool *out_needs_uvar_sync);
mod_result_t remove(const wcstring &key, int var_mode);
/// Push a new shadowing local scope.
void push_shadowing();
@ -1072,12 +1085,12 @@ void env_stack_impl_t::set_universal(const wcstring &key, wcstring_list_t val,
uvars()->set(key, new_var);
}
int env_stack_impl_t::set(const wcstring &key, env_mode_flags_t mode, wcstring_list_t val,
bool *out_needs_uvar_sync) {
mod_result_t env_stack_impl_t::set(const wcstring &key, env_mode_flags_t mode,
wcstring_list_t val) {
const query_t query(mode);
// Handle electric and read-only variables.
if (auto ret = try_set_electric(key, query, val)) {
return *ret;
return mod_result_t{*ret};
}
// Resolve as much of our flags as we can. Note these contain maybes, and we may defer the final
@ -1097,14 +1110,17 @@ int env_stack_impl_t::set(const wcstring &key, env_mode_flags_t mode, wcstring_l
flags.pathvar = query.pathvar;
}
mod_result_t result{ENV_OK};
if (query.has_scope) {
// The user requested a particular scope.
if (query.universal) {
set_universal(key, std::move(val), query);
*out_needs_uvar_sync = true;
result.uvar_modified = true;
} else if (query.global) {
set_in_node(globals_, key, std::move(val), flags);
result.global_modified = true;
} else if (query.local) {
assert(locals_ != globals_ && "Locals should not be globals");
set_in_node(locals_, key, std::move(val), flags);
} else {
DIE("Unknown scope");
@ -1115,56 +1131,56 @@ int env_stack_impl_t::set(const wcstring &key, env_mode_flags_t mode, wcstring_l
} else if (env_node_ref_t node = find_in_chain(globals_, key)) {
// Existing global variable.
set_in_node(node, key, std::move(val), flags);
result.global_modified = true;
} else if (uvars() && uvars()->get(key)) {
// Existing universal variable.
set_universal(key, std::move(val), query);
*out_needs_uvar_sync = true;
result.uvar_modified = true;
} else {
// Unspecified scope with no existing variables.
auto node = resolve_unspecified_scope();
assert(node && "Should always resolve some scope");
set_in_node(node, key, std::move(val), flags);
result.global_modified = (node == globals_);
}
return ENV_OK;
return result;
}
int env_stack_impl_t::remove(const wcstring &key, int mode, bool *out_needs_uvar_sync) {
mod_result_t env_stack_impl_t::remove(const wcstring &key, int mode) {
const query_t query(mode);
// Users can't remove read-only keys.
if (query.user && is_read_only(key)) {
return ENV_SCOPE;
return mod_result_t{ENV_SCOPE};
}
// Helper to remove from uvars.
auto remove_from_uvars = [&] {
if (!uvars()) return false;
auto flags = uvars()->get_flags(key);
if (!flags) return false;
*out_needs_uvar_sync = true;
return uvars()->remove(key);
};
auto remove_from_uvars = [&] { return uvars() && uvars()->remove(key); };
mod_result_t result{ENV_OK};
if (query.has_scope) {
// The user requested erasing from a particular scope.
if (query.universal) {
return remove_from_uvars() ? ENV_OK : ENV_NOT_FOUND;
result.status = remove_from_uvars() ? ENV_OK : ENV_NOT_FOUND;
result.uvar_modified = true;
} else if (query.global) {
return remove_from_chain(globals_, key) ? ENV_OK : ENV_NOT_FOUND;
result.status = remove_from_chain(globals_, key) ? ENV_OK : ENV_NOT_FOUND;
result.global_modified = true;
} else if (query.local) {
return remove_from_chain(locals_, key) ? ENV_OK : ENV_NOT_FOUND;
result.status = remove_from_chain(locals_, key) ? ENV_OK : ENV_NOT_FOUND;
} else {
DIE("Unknown scope");
}
} else if (remove_from_chain(locals_, key)) {
return ENV_OK;
// pass
} else if (remove_from_chain(globals_, key)) {
return ENV_OK;
result.global_modified = true;
} else if (remove_from_uvars()) {
return ENV_OK;
result.uvar_modified = true;
} else {
return ENV_NOT_FOUND;
result.status = ENV_NOT_FOUND;
}
return result;
}
bool env_stack_t::universal_barrier() {
@ -1246,19 +1262,18 @@ int env_stack_t::set(const wcstring &key, env_mode_flags_t mode, wcstring_list_t
vals = std::move(munged_vals);
}
bool needs_uvar_sync = false;
auto ret = acquire_impl()->set(key, mode, std::move(vals), &needs_uvar_sync);
if (ret == ENV_OK) {
mod_result_t ret = acquire_impl()->set(key, mode, std::move(vals));
if (ret.status == ENV_OK) {
// Important to not hold the lock here.
env_dispatch_var_change(key, *this);
if (out_events) {
out_events->push_back(event_t::variable(key, {L"VARIABLE", L"SET", key}));
}
}
if (needs_uvar_sync) {
if (ret.uvar_modified) {
universal_barrier();
}
return ret;
return ret.status;
}
int env_stack_t::set_one(const wcstring &key, env_mode_flags_t mode, wcstring val,
@ -1274,19 +1289,18 @@ int env_stack_t::set_empty(const wcstring &key, env_mode_flags_t mode,
}
int env_stack_t::remove(const wcstring &key, int mode, std::vector<event_t> *out_events) {
bool needs_uvar_sync = false;
int ret = acquire_impl()->remove(key, mode, &needs_uvar_sync);
if (ret == ENV_OK) {
mod_result_t ret = acquire_impl()->remove(key, mode);
if (ret.status == ENV_OK) {
// Important to not hold the lock here.
env_dispatch_var_change(key, *this);
if (out_events) {
out_events->push_back(event_t::variable(key, {L"VARIABLE", L"ERASE", key}));
}
}
if (needs_uvar_sync) {
if (ret.uvar_modified) {
universal_barrier();
}
return ret;
return ret.status;
}
std::shared_ptr<const null_terminated_array_t<char>> env_stack_t::export_arr() {