Skip to content

Commit

Permalink
Make the single argument constructor inherit the env
Browse files Browse the repository at this point in the history 
Signed-off-by: Michael Carroll <[email protected]>
  • Loading branch information
@mjcarroll
mjcarroll committed Nov 1, 2023
1 parent 55451cc commit d64ff9b
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 42 deletions.
84 changes: 62 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,80 @@ 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();
}

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

this->process = new subprocess_s;

std::vector<std::string> environmentStr;
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 &[key, val] : this->environment)
{
environmentStr.push_back(key + "=" + val);
environmentCstr.push_back(environmentStr.back().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 +122,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 +145,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 +162,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 +179,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 +193,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
11 changes: 3 additions & 8 deletions test/integration/subprocess/subprocess_main.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@

class OutputSink
{
public: OutputSink(const std::string &_dest):
dest(_dest)
public: explicit OutputSink(std::string _dest):
dest(std::move(_dest))
{
}

Expand All @@ -40,7 +40,6 @@ class OutputSink
{
std::cerr << val << std::endl;
}
return;
}

private: std::string dest;
Expand Down Expand Up @@ -69,10 +68,6 @@ int main(int argc, char **argv)
std::this_thread::sleep_for(std::chrono::milliseconds(iter_ms));
}

std::string env_var;
if(gz::utils::env("ENV_VAR", env_var))
{
sink.Write("ENV_VAR=" + env_var);
}
sink.Write(gz::utils::printenv());
return 0;
}
59 changes: 47 additions & 12 deletions test/integration/subprocess_TEST.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

#include <chrono>
#include <thread>
#include "gz/utils/Environment.hh"

using Subprocess = gz::utils::Subprocess;

Expand All @@ -33,6 +34,28 @@ TEST(Subprocess, CreateInvalid)
EXPECT_FALSE(proc.Alive());
}

/////////////////////////////////////////////////
TEST(Subprocess, CreateInvalidSpaces)
{
// Test if a user passes a string with spaces, rather than
// a vector of strings
std::string command(kExecutablePath);
command.append(" --help");
auto proc = Subprocess({command});

// Block until the executable is done
auto ret = proc.Join();
EXPECT_EQ(-1, ret);

EXPECT_FALSE(proc.Alive());

auto cout = proc.Stdout();
auto cerr = proc.Stdout();

EXPECT_TRUE(cout.empty());
EXPECT_TRUE(cerr.empty());
}

/////////////////////////////////////////////////
TEST(Subprocess, CreateValid)
{
Expand Down Expand Up @@ -94,38 +117,50 @@ TEST(Subprocess, Cerr)
/////////////////////////////////////////////////
TEST(Subprocess, Environment)
{
ASSERT_TRUE(gz::utils::clearenv());
ASSERT_TRUE(gz::utils::setenv("GZ_FOO_KEY", "GZ_FOO_VAL"));
ASSERT_TRUE(gz::utils::setenv("GZ_BAR_KEY", "GZ_BAR_VAL"));
ASSERT_TRUE(gz::utils::setenv("GZ_BAZ_KEY", "GZ_BAZ_VAL"));

{
auto proc = Subprocess({kExecutablePath, "--output=cout"},
{"ENV_VAR=foobar"});
// Default behavior is to inherit the environment
auto proc = Subprocess({kExecutablePath, "--output=cout"});
// Block until the executable is done
auto ret = proc.Join();
EXPECT_EQ(0u, ret);

auto cout = proc.Stdout();
EXPECT_NE(std::string::npos, cout.find("ENV_VAR=foobar"));
EXPECT_NE(std::string::npos, cout.find("GZ_FOO_KEY=GZ_FOO_VAL"));
EXPECT_NE(std::string::npos, cout.find("GZ_BAR_KEY=GZ_BAR_VAL"));
EXPECT_NE(std::string::npos, cout.find("GZ_BAZ_KEY=GZ_BAZ_VAL"));
}

{
auto proc = Subprocess({kExecutablePath, "--output=cerr"},
{"ENV_VAR=foobar2"});
// Passing an empty map as the second arg clears the environment
auto proc = Subprocess({kExecutablePath, "--output=cout"}, {});
// Block until the executable is done
auto ret = proc.Join();
EXPECT_EQ(0u, ret);

auto cerr = proc.Stderr();
EXPECT_NE(std::string::npos, cerr.find("ENV_VAR=foobar2"));
auto cout = proc.Stdout();
EXPECT_EQ(std::string::npos, cout.find("GZ_FOO_KEY=GZ_FOO_VAL"));
EXPECT_EQ(std::string::npos, cout.find("GZ_BAR_KEY=GZ_BAR_VAL"));
EXPECT_EQ(std::string::npos, cout.find("GZ_BAZ_KEY=GZ_BAZ_VAL"));
}

{
auto proc = Subprocess({kExecutablePath},
{"ENV_VAR=foobar3"});
// Passing a map sets those variables, clearing the rest
auto proc = Subprocess({kExecutablePath, "--output=cout"}, {
{"QUX_KEY", "QUX_VAL"}
});
// Block until the executable is done
auto ret = proc.Join();
EXPECT_EQ(0u, ret);

auto cout = proc.Stdout();
auto cerr = proc.Stderr();
EXPECT_EQ(std::string::npos, cerr.find("ENV_VAR=foobar3"));
EXPECT_EQ(std::string::npos, cout.find("ENV_VAR=foobar3"));
EXPECT_EQ(std::string::npos, cout.find("GZ_FOO_KEY=GZ_FOO_VAL"));
EXPECT_EQ(std::string::npos, cout.find("GZ_BAR_KEY=GZ_BAR_VAL"));
EXPECT_EQ(std::string::npos, cout.find("GZ_BAZ_KEY=GZ_BAZ_VAL"));
EXPECT_NE(std::string::npos, cout.find("QUX_KEY=QUX_VAL"));
}
}

0 comments on commit d64ff9b

Please sign in to comment.