From d87b6621192840a42cc4da2464cac97a8225ef3c Mon Sep 17 00:00:00 2001 From: isanae <14251494+isanae@users.noreply.github.com> Date: Thu, 19 Nov 2020 07:57:05 -0500 Subject: [PATCH] fixed process, the pipes can be empty now if they're not required comments --- src/core/env.cpp | 96 +++++++++++++++++++++++++++++++------------- src/core/env.h | 77 +++++++++++++++++++++++++++++++++-- src/core/process.cpp | 16 ++++++-- 3 files changed, 155 insertions(+), 34 deletions(-) diff --git a/src/core/env.cpp b/src/core/env.cpp index 141a597..53c5963 100644 --- a/src/core/env.cpp +++ b/src/core/env.cpp @@ -10,8 +10,13 @@ namespace mob { +// retrieves the Visual Studio environment variables for the given architecture; +// this is pretty expensive, so it's called on demand and only once, and is +// stored as a static variable in vs_x86() and vs_x64() below +// env get_vcvars_env(arch a) { + // translate arch to the string needed by vcvars std::string arch_s; switch (a) @@ -31,22 +36,37 @@ env get_vcvars_env(arch a) gcx().trace(context::generic, "looking for vcvars for {}", arch_s); + + // the only way to get these variables is to + // 1) run vcvars in a cmd instance, + // 2) call `set`, which outputs all the variables to stdout, and + // 3) parse it + // + // the process class doesn't really have a good way of dealing with this + // and it's not worth adding all this crap to it just for vcvars, so most + // of this is done manually + + // stdout will be redirected to this const fs::path tmp = make_temp_file(); - // "vcvarsall.bat" amd64 && set > temp_file + // runs `"vcvarsall.bat" amd64 && set > temp_file` const std::string cmd = "\"" + path_to_utf8(vs::vcvars()) + "\" " + arch_s + " && set > \"" + path_to_utf8(tmp) + "\""; + // cmd_unicode() is necessary so `set` outputs in utf16 instead of codepage process::raw(gcx(), cmd) .cmd_unicode(true) .run(); gcx().trace(context::generic, "reading from {}", tmp); + // reads the file, converting utf16 to utf8 std::stringstream ss(op::read_text_file(gcx(), encodings::utf16, tmp)); op::delete_file(gcx(), tmp); + // `ss` contains all the variables in utf8 + env e; gcx().trace(context::generic, "parsing variables"); @@ -108,20 +128,24 @@ env env::vs(arch a) env::env() : own_(false) { + // empty env, does not own } env::env(const env& e) : data_(e.data_), own_(false) { + // copy data, does not own } env::env(env&& e) : data_(std::move(e.data_)), own_(e.own_) { + // move data, owns if `e` did } env& env::operator=(const env& e) { + // copy data, does not own data_ = e.data_; own_ = false; return *this; @@ -129,6 +153,7 @@ env& env::operator=(const env& e) env& env::operator=(env&& e) { + // copy data, owns if `e` did data_ = std::move(e.data_); own_ = e.own_; return *this; @@ -168,6 +193,7 @@ env& env::change_path(const std::vector& v, flags f) { case replace: { + // convert to utf16 strings, join with ; const auto strings = mob::map(v, [&](auto&& p){ return p.native(); }); @@ -182,6 +208,7 @@ env& env::change_path(const std::vector& v, flags f) if (current) path = *current; + // append all paths as utf16 strings to the current value, if any for (auto&& p : v) { if (!path.empty()) @@ -199,6 +226,7 @@ env& env::change_path(const std::vector& v, flags f) if (current) path = *current; + // prepend all paths as utf16 strings to the current value, if any for (auto&& p : v) { if (!path.empty()) @@ -277,19 +305,12 @@ env::map env::get_map() const return data_->vars; } -void env::set_from(const env& e) +void env::create_sys() const { - copy_for_write(); + // CreateProcess() wants a string where every key=value is separated by a + // null and also terminated by a null, so there are two null characters at + // the end - if (e.data_) - { - for (auto&& v : e.data_->vars) - set_impl(v.first, v.second, replace); - } -} - -void env::create() const -{ data_->sys.clear(); for (auto&& v : data_->vars) @@ -303,16 +324,7 @@ void env::create() const std::wstring* env::find(std::wstring_view name) { - if (!data_) - return {}; - - for (auto itor=data_->vars.begin(); itor!=data_->vars.end(); ++itor) - { - if (_wcsicmp(itor->first.c_str(), name.data()) == 0) - return &itor->second; - } - - return {}; + return const_cast(std::as_const(*this).find(name)); } const std::wstring* env::find(std::wstring_view name) const @@ -334,10 +346,11 @@ void* env::get_unicode_pointers() const if (!data_ || data_->vars.empty()) return nullptr; + // create string if it doesn't exist { std::scoped_lock lock(data_->m); if (data_->sys.empty()) - create(); + create_sys(); } return (void*)data_->sys.c_str(); @@ -347,6 +360,9 @@ void env::copy_for_write() { if (own_) { + // this is called every time something is about to change; if this + // instance already owns the data, the sys strings must still be cleared + // out so they're recreated if get_unicode_pointers() is every called if (data_) data_->sys.clear(); @@ -355,50 +371,75 @@ void env::copy_for_write() if (data_) { + // remember the shared data auto shared = data_; + + // create a new owned instance data_.reset(new data); + // copying std::scoped_lock lock(shared->m); data_->vars = shared->vars; } else { + // creating own, empty data data_.reset(new data); } + // this instance owns the data own_ = true; } - +// mob's environment variables are only retrieved once and are kept in sync +// after that; this must also be thread-safe static std::mutex g_sys_env_mutex; static env g_sys_env; static bool g_sys_env_inited; + env this_env::get() { std::scoped_lock lock(g_sys_env_mutex); if (g_sys_env_inited) + { + // already done return g_sys_env; + } + + + // first time, get the variables from the system auto free = [](wchar_t* p) { FreeEnvironmentStringsW(p); }; auto env_block = std::unique_ptr{ GetEnvironmentStringsW(), free}; + // GetEnvironmentStringsW() returns a string where each variable=value + // is separated by a null character + for (const wchar_t* name = env_block.get(); *name != L'\0'; ) { + // equal sign const wchar_t* equal = std::wcschr(name, '='); + + // key std::wstring key(name, static_cast(equal - name)); - const wchar_t* pValue = equal + 1; - std::wstring value(pValue); + // value + const wchar_t* value_start = equal + 1; + std::wstring value(value_start); + // the strings contain all sorts of weird stuff, like variables to + // keep track of the current directory, those start with an equal sign, + // so just ignore them if (!key.empty()) g_sys_env.set(utf16_to_utf8(key), utf16_to_utf8(value)); - name = pValue + value.length() + 1; + // next string is one past end of value to account for null byte + name = value_start + value.length() + 1; } g_sys_env_inited = true; @@ -436,6 +477,7 @@ void this_env::set(const std::string& k, const std::string& v, env::flags f) } } + // keep in sync { std::scoped_lock lock(g_sys_env_mutex); if (g_sys_env_inited) diff --git a/src/core/env.h b/src/core/env.h index 0db542c..a832216 100644 --- a/src/core/env.h +++ b/src/core/env.h @@ -5,11 +5,16 @@ namespace mob { +// a set of environment variables; copy-on-write because this gets copied a lot +// class env { public: using map = std::map; + // used in set(); replaces, appends or prepends to a variable if it already + // exists + // enum flags { replace = 1, @@ -17,67 +22,133 @@ public: prepend }; + // Visual Studio environment variables for 32-bit + // static env vs_x86(); + + // Visual Studio environment variables for 64-bit + // static env vs_x64(); + + // Visual Studio environment variables for the given architecture + // static env vs(arch a); + + // empty set + // env(); + + // handle ref count + // env(const env& e); env(env&& e); env& operator=(const env& e); env& operator=(env&& e); + // prepends to PATH + // env& prepend_path(const fs::path& p); env& prepend_path(const std::vector& v); + + // appends to PATH + // env& append_path(const fs::path& p); env& append_path(const std::vector& v); + // sets k=v + // env& set(std::string_view k, std::string_view v, flags f=replace); env& set(std::wstring k, std::wstring v, flags f=replace); - void set_from(const env& e); - + // returns the variable' value, empty if not found + // std::string get(std::string_view k) const; + + // map of variables + // map get_map() const; + // passed to CreateProcess() in the process class; returns a pointer to a + // block of utf16 strings, owned by this, created on demand + // void* get_unicode_pointers() const; private: + // shared between copies + // struct data { std::mutex m; map vars; + + // unicode strings, see get_unicode_pointers() mutable std::wstring sys; }; + // shared data std::shared_ptr data_; + + // whether this instance owns the data, set to true in copy_for_write() + // when the data must be modified bool own_; - void create() const; + + // creates the unicode strings + // + void create_sys() const; + + // returns a pointer inside the map, null if not found + // std::wstring* find(std::wstring_view name); const std::wstring* find(std::wstring_view name) const; + + // called by set(), sets the value in the map + // void set_impl(std::wstring k, std::wstring v, flags f); + + // duplicates the data, sets own_=true + // void copy_for_write(); + + // called by the various *_path() functions, actually changes the PATH + // value + // env& change_path(const std::vector& v, flags f); }; +// represents mob's environment variables +// struct this_env { + // sets a variable + // static void set( const std::string& k, const std::string& v, env::flags f=env::replace); + // changes PATH + // static void prepend_to_path(const fs::path& p); static void append_to_path(const fs::path& p); + // returns mob's environment variables + // static env get(); + // returns a specific variable; bails out if it doesn't exist + // static std::string get(const std::string& k); + + // returns a specific variable, or empty if it doesn't exist + // static std::optional get_opt(const std::string& k); private: + // used by get() and get_opt(), does the actual work + // static std::optional get_impl(const std::string& k); }; diff --git a/src/core/process.cpp b/src/core/process.cpp index 2747e06..2cf6df9 100644 --- a/src/core/process.cpp +++ b/src/core/process.cpp @@ -542,8 +542,11 @@ void process::on_timeout(bool& already_interrupted) void process::read_pipes(bool finish) { - read_pipe(finish, io_.out, *impl_.stdout_pipe, context::std_out); - read_pipe(finish, io_.err, *impl_.stderr_pipe, context::std_err); + if (impl_.stdout_pipe) + read_pipe(finish, io_.out, *impl_.stdout_pipe, context::std_out); + + if (impl_.stderr_pipe) + read_pipe(finish, io_.err, *impl_.stderr_pipe, context::std_err); } void process::read_pipe( @@ -651,8 +654,13 @@ void process::on_completed() read_pipes(true); // loop until both pipes are closed - if (impl_.stdout_pipe->closed() && impl_.stderr_pipe->closed()) - break; + if (impl_.stdout_pipe && !impl_.stdout_pipe->closed()) + continue; + + if (impl_.stderr_pipe && !impl_.stderr_pipe->closed()) + continue; + + break; }