Fix env parsing by placing it before executable parsing (#81)

* Fix env parsing by placing it before executable parsing Signed-off-by: Nate Koenig <[email protected]> * Add tests to #81 (#82) This adds a simple check to verify that the env directives apply to everything in the ignition namespace, as fixed in #81. The test itself is a bit of a workaround. We don't have a mechanism for checking the manager output, and executable command directives cannot expand environment variables. This writes a script that is executed by the test to verify that a file specified by an environment variable is touch-ed Signed-off-by: Michael Carroll <[email protected]> Co-authored-by: Nate Koenig <[email protected]> Co-authored-by: Michael Carroll <[email protected]>
gazebosim · Jan 8, 2021 · 1870459 · 1870459
1 parent 0986169
commit 1870459
Show file tree

Hide file tree

Showing 2 changed files with 108 additions and 4 deletions.
diff --git a/src/Manager.cc b/src/Manager.cc
@@ -521,6 +521,9 @@ bool ManagerPrivate::ParseConfig(const std::string &_config)
     ignerr << "Invalid config file,m issing <ignition> element\n";
     return false;
   }
+  // Keep the environment variables in memory. See manpage for putenv.
+  this->envs = this->ParseEnvs(root);
+  this->SetEnvs(this->envs);
 
   // Parse and create all the <executable> elements.
   this->ParseExecutables(root);
@@ -529,10 +532,6 @@ bool ManagerPrivate::ParseConfig(const std::string &_config)
   if (this->master)
     this->ParseExecutableWrappers(root);
 
-  // Keep the environment variables in memory. See manpage for putenv.
-  this->envs = this->ParseEnvs(root);
-  this->SetEnvs(this->envs);
-
   // Parse and create all the <plugin> elements.
   if (this->master)
   {

diff --git a/src/Manager_TEST.cc b/src/Manager_TEST.cc
@@ -17,8 +17,46 @@
 
 #include <gtest/gtest.h>
 #include <ignition/common/Console.hh>
+#include <ignition/common/Filesystem.hh>
+
+#include <ignition/utilities/ExtraTestMacros.hh>
+
+#include <sys/stat.h>
+
 #include "Manager.hh"
 
+static constexpr char kTestScriptPath[] = "/tmp/ign-launch.sh";
+
+/////////////////////////////////////////////////
+bool RemoveTestScript()
+{
+  // Remove the file if it already exists
+  if (ignition::common::isFile(kTestScriptPath))
+  {
+    if (!ignition::common::removeFile(kTestScriptPath))
+    {
+      return false;
+    }
+  }
+  return true;
+}
+
+/////////////////////////////////////////////////
+bool WriteTestScript()
+{
+  if (!RemoveTestScript())
+    return false;
+
+  // Write a simple script and mark it executable
+  std::ofstream ofs(kTestScriptPath);
+  ofs << R"(#!/usr/bin/env bash
+echo $TEST_VAR
+touch $TEST_VAR
+)";
+  chmod(kTestScriptPath, S_IRWXU);
+  return true;
+}
+
 /////////////////////////////////////////////////
 TEST(Ignition_TEST, RunEmptyConfig)
 {
@@ -81,6 +119,73 @@ TEST(Ignition_TEST, RunLs)
   EXPECT_TRUE(mgr.RunConfig(config));
 }
 
+
+/////////////////////////////////////////////////
+TEST(Ignition_TEST, IGN_UTILS_TEST_DISABLED_ON_WIN32(RunEnvPre))
+{
+  // Test that environment is applied regardless of order
+  std::string testPath = "/tmp/ign-launch-env-test-pre";
+
+  if (ignition::common::isFile(testPath))
+  {
+    ASSERT_TRUE(ignition::common::removeFile(testPath));
+  }
+
+  ASSERT_TRUE(WriteTestScript());
+
+  std::string config = R"(
+<ignition version='1.0'>
+  <env>
+    <name>TEST_VAR</name>
+    <value>)" + testPath + R"(</value>
+  </env>
+  <executable name='touch'>
+    <command>/tmp/ign-launch.sh</command>
+  </executable>
+</ignition>
+)";
+
+  ignition::launch::Manager mgr;
+
+  EXPECT_TRUE(mgr.RunConfig(config));
+  EXPECT_TRUE(ignition::common::isFile(testPath));
+  EXPECT_TRUE(ignition::common::removeFile(testPath));
+  EXPECT_TRUE(RemoveTestScript());
+}
+
+/////////////////////////////////////////////////
+TEST(Ignition_TEST, IGN_UTILS_TEST_DISABLED_ON_WIN32(RunEnvPost))
+{
+  // Test that environment is applied regardless of order
+  std::string testPath = "/tmp/ign-launch-env-test-post";
+
+  if (ignition::common::isFile(testPath))
+  {
+    ASSERT_TRUE(ignition::common::removeFile(testPath));
+  }
+
+  ASSERT_TRUE(WriteTestScript());
+
+  std::string config = R"(
+<ignition version='1.0'>
+  <executable name='touch'>
+    <command>/tmp/ign-launch.sh</command>
+  </executable>
+  <env>
+    <name>TEST_VAR</name>
+    <value>)" + testPath + R"(</value>
+  </env>
+</ignition>
+)";
+
+  ignition::launch::Manager mgr;
+
+  EXPECT_TRUE(mgr.RunConfig(config));
+  EXPECT_TRUE(ignition::common::isFile(testPath));
+  EXPECT_TRUE(ignition::common::removeFile(testPath));
+  EXPECT_TRUE(RemoveTestScript());
+}
+
 /////////////////////////////////////////////////
 int main(int argc, char **argv)
 {