Skip to content

Commit

Permalink
Make the single argument constructor inherit the env (#113)
Browse files Browse the repository at this point in the history 
Signed-off-by: Michael Carroll <[email protected]>
  • Loading branch information
@mjcarroll
mjcarroll authored Nov 8, 2023
1 parent 8529918 commit db61818
Show file tree
Hide file tree
Showing 6 changed files with 259 additions and 62 deletions.
19 changes: 19 additions & 0 deletions include/gz/utils/Environment.hh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

#include <string>
#include <unordered_map>
#include <vector>

namespace gz
{
Expand Down Expand Up @@ -78,6 +79,24 @@ bool GZ_UTILS_VISIBLE clearenv();
/// \brief Type alias for a collection of environment variables
using EnvironmentMap = std::unordered_map<std::string, std::string>;

/// \brief Type alias for a collection of environment variables
/// Each entry is of the form KEY=VAL
using EnvironmentStrings = std::vector<std::string>;

/// \brief Convert a vector of environment variables to a map
///
/// \param[in] _envStrings Vector collection of environment variables
/// \return Mapped collection of environment variables.
EnvironmentMap GZ_UTILS_VISIBLE envStringsToMap(
const EnvironmentStrings &_envStrings);

/// \brief Convert a map of environment variables to a vector
///
/// \param[in] _envMap Collection of mapped environment variables.
/// \return Vector collection of environment variables.
EnvironmentStrings GZ_UTILS_VISIBLE envMapToStrings(
const EnvironmentMap &_envMap);

/// \brief Retrieve all current environment variables
///
/// Note: This function is not thread-safe and should not be called
Expand Down
99 changes: 77 additions & 22 deletions include/gz/utils/Subprocess.hh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,12 @@
#define GZ_UTILS__SUBPROCESS_HH_

#include "detail/subprocess.h"
#include "gz/utils/Environment.hh"

#include <iostream>
#include <string>
#include <vector>
#include <utility>

#include <gz/utils/detail/subprocess.h>

Expand All @@ -32,46 +36,95 @@ namespace gz
namespace utils
{

/// \brief Create a RAII-type object that encapsulates a subprocess.
class Subprocess
{
/// \brief Constructor
///
/// This variant will spawn a subprocess that inherits the environment
/// from the calling process.
///
/// \param[in] _commandLine set of arguments starting with an executable
/// used to spawn the subprocess
public: explicit Subprocess(const std::vector<std::string> &_commandLine):
commandLine(_commandLine),
environment({}),
inheritEnvironment(true)
{
this->Create();
}

/// \brief Constructor
///
/// This variant will spawn a subprocess that uses the user-specified
/// environment
///
/// \param[in] _commandLine set of arguments starting with an executable
/// used to spawn the subprocess
/// \param[in] _environment environment variables to set in the spawned
/// subprocess
public: Subprocess(const std::vector<std::string> &_commandLine,
const std::vector<std::string> &_environment = {}):
gz::utils::EnvironmentMap _environment):
commandLine(_commandLine),
environment(_environment)
environment(std::move(_environment)),
inheritEnvironment(false)
{
this->Create();
}

/// \brief Constructor
///
/// This variant will spawn a subprocess that uses the user-specified
/// environment
///
/// \param[in] _commandLine set of arguments starting with an executable
/// used to spawn the subprocess
/// \param[in] _environment environment variables to set in the spawned
/// subprocess
public: Subprocess(const std::vector<std::string> &_commandLine,
const gz::utils::EnvironmentStrings &_environment):
Subprocess(_commandLine, gz::utils::envStringsToMap(_environment))
{
}


private: void Create()
{
if (this->process)
if (this->process != nullptr)
return;

this->process = new subprocess_s;

auto environmentStr = gz::utils::envMapToStrings(this->environment);
std::vector<const char*> environmentCstr;
std::vector<const char*> commandLineCstr;

for (const auto &val : this->commandLine)
{
commandLineCstr.push_back(val.c_str());
}
commandLineCstr.push_back(nullptr);

std::vector<const char*> environmentCstr;
for (const auto &val : this->environment)
if (!this->inheritEnvironment)
{
environmentCstr.push_back(val.c_str());
for (const auto &val : environmentStr)
{
environmentCstr.push_back(val.c_str());
}
environmentCstr.push_back(nullptr);
}
environmentCstr.push_back(nullptr);

int ret = -1;
if (this->environment.size())
if (!this->inheritEnvironment)
{
ret = subprocess_create_ex(commandLineCstr.data(),
0, environmentCstr.data(), this->process);
}
else
{
ret = subprocess_create(commandLineCstr.data(), 0, this->process);
ret = subprocess_create(commandLineCstr.data(),
subprocess_option_inherit_environment,
this->process);
}

if (0 != ret)
Expand All @@ -84,21 +137,21 @@ class Subprocess

public: ~Subprocess()
{
if (this->process)
if (this->process != nullptr)
subprocess_destroy(this->process);
delete this->process;
}

public: std::string Stdout()
{
std::string result{""};
if (this->process)
std::string result;
if (this->process != nullptr)
{
auto p_stdout = subprocess_stdout(this->process);
auto *p_stdout = subprocess_stdout(this->process);
char buffer[128];
while (!feof(p_stdout))
{
if (fgets(buffer, 128, p_stdout) != NULL)
if (fgets(buffer, 128, p_stdout) != nullptr)
result += buffer;
}
}
Expand All @@ -107,14 +160,14 @@ class Subprocess

public: std::string Stderr()
{
std::string result{""};
if (this->process)
std::string result;
if (this->process != nullptr)
{
auto p_stdout = subprocess_stderr(this->process);
auto *p_stdout = subprocess_stderr(this->process);
char buffer[128];
while (!feof(p_stdout))
{
if (fgets(buffer, 128, p_stdout) != NULL)
if (fgets(buffer, 128, p_stdout) != nullptr)
result += buffer;
}
}
Expand All @@ -124,15 +177,15 @@ class Subprocess

public: bool Alive()
{
if (this->process)
if (this->process != nullptr)
return subprocess_alive(this->process);
else
return false;
}

public: bool Terminate()
{
if (this->process)
if (this->process != nullptr)
return subprocess_terminate(this->process) != 0;
else
return false;
Expand All @@ -141,7 +194,7 @@ class Subprocess
public: int Join()
{
int return_code = -1;
if (this->process)
if (this->process != nullptr)
{
auto ret = subprocess_join(this->process, &return_code);
if (ret != 0)
Expand All @@ -155,7 +208,9 @@ class Subprocess

protected: std::vector<std::string> commandLine;

protected: std::vector<std::string> environment;
protected: EnvironmentMap environment;

protected: bool inheritEnvironment;

protected: subprocess_s * process {nullptr};
};
Expand Down
55 changes: 34 additions & 21 deletions src/Environment.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,22 +128,13 @@ bool clearenv()
}
#endif
return success;

}

/////////////////////////////////////////////////
EnvironmentMap env()
{
EnvironmentMap ret;

// Helper function to split KEY=VAL
auto split = [](const std::string &_inp)
{
return std::make_pair(
_inp.substr(0, _inp.find('=')),
_inp.substr(_inp.find('=') + 1));
};

char **currentEnv = nullptr;
#ifdef _WIN32
currentEnv = *__p__environ();
Expand All @@ -155,11 +146,12 @@ EnvironmentMap env()
if (currentEnv == nullptr)
return {};

std::vector<std::string> envStrings;
for (; *currentEnv; ++currentEnv)
{
ret.emplace(split(*currentEnv));
envStrings.emplace_back(*currentEnv);
}
return ret;
return envStringsToMap(envStrings);
}

/////////////////////////////////////////////////
Expand All @@ -174,20 +166,41 @@ bool setenv(const EnvironmentMap &_vars)
}

/////////////////////////////////////////////////
std::string printenv()
EnvironmentMap envStringsToMap(const EnvironmentStrings &_envStrings)
{
std::string ret;
// Variables are in an unordered_map as we generally don't
// care, but for printing sort for consistent display
auto currentEnv = env();
EnvironmentMap ret;
for (const auto &pair : _envStrings)
{
auto eqPos = pair.find('=');
if (eqPos != std::string::npos)
{
ret.emplace(pair.substr(0, eqPos), pair.substr(eqPos + 1));
}
}
return ret;
}

/////////////////////////////////////////////////
EnvironmentStrings envMapToStrings(const EnvironmentMap &_envMap)
{
EnvironmentStrings ret;
auto sorted = std::vector<std::pair<std::string, std::string>>(
currentEnv.begin(), currentEnv.end());
_envMap.begin(), _envMap.end());
std::sort(sorted.begin(), sorted.end());
for (const auto &[key, value] : sorted)
for (auto [key, value] : sorted)
{
ret.push_back(key + "=" + value);
}
return ret;
}

/////////////////////////////////////////////////
std::string printenv()
{
std::string ret;
for (const auto &entry : envMapToStrings(env()))
{
ret.append(key);
ret.append("=");
ret.append(value);
ret.append(entry);
ret.append("\n");
}
return ret;
Expand Down
36 changes: 36 additions & 0 deletions src/Environment_TEST.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,3 +175,39 @@ TEST(Environment, printenv)
EXPECT_EQ(gz::utils::printenv(),
"GZ_BAR_KEY=GZ_BAR_VAL\nGZ_BAZ_KEY=GZ_BAZ_VAL\nGZ_FOO_KEY=GZ_FOO_VAL\n");
}

/////////////////////////////////////////////////
TEST(Environment, envStringsToMap)
{
gz::utils::EnvironmentStrings strings;
strings.emplace_back("GZ_FOO_KEY=GZ_FOO_VAL");
strings.emplace_back("GZ_BAR_KEY=GZ_BAR_VAL");
strings.emplace_back("GZ_BAZ_KEY=GZ_BAZ_VAL");
strings.emplace_back("BAD_KEY");

{
auto envMap = gz::utils::envStringsToMap(strings);
EXPECT_EQ(3u, envMap.size());
EXPECT_EQ("GZ_FOO_VAL", envMap["GZ_FOO_KEY"]);
EXPECT_EQ("GZ_BAR_VAL", envMap["GZ_BAR_KEY"]);
EXPECT_EQ("GZ_BAZ_VAL", envMap["GZ_BAZ_KEY"]);
}
}

/////////////////////////////////////////////////
TEST(Environment, envMapToStrings)
{
gz::utils::EnvironmentMap envMap;
envMap.insert({{"GZ_FOO_KEY"}, {"GZ_FOO_VAL"}});
envMap.insert({{"GZ_BAR_KEY"}, {"GZ_BAR_VAL"}});
envMap.insert({{"GZ_BAZ_KEY"}, {"GZ_BAZ_VAL"}});

{
auto envStrings = gz::utils::envMapToStrings(envMap);

EXPECT_EQ(3u, envStrings.size());
EXPECT_EQ("GZ_BAR_KEY=GZ_BAR_VAL", envStrings[0]);
EXPECT_EQ("GZ_BAZ_KEY=GZ_BAZ_VAL", envStrings[1]);
EXPECT_EQ("GZ_FOO_KEY=GZ_FOO_VAL", envStrings[2]);
}
}
Loading

0 comments on commit db61818

Please sign in to comment.