Option file (#1728)

* Add support for stage option files.

* Remove test code.

* Add JSON option file.

* Add tests form option_file.
Add grid_option_file option.

* Make sure there's no "foo.las" sitting around.

pdal/Kernel.hpp

@@ -52,12 +52,6 @@ class PointView;
 
 typedef std::shared_ptr<PointView> PointViewPtr;
 
-//
-// The application base class gives us these common options:
-//    --help / -h
-//    --verbose / -v
-//    --version
-//
 class PDAL_DLL Kernel
 {
     FRIEND_TEST(KernelTest, parseOption);

pdal/Options.cpp

@@ -34,6 +34,8 @@
 
 #include <pdal/Options.hpp>
 #include <pdal/PDALUtils.hpp>
+#include <pdal/util/FileUtils.hpp>
+#include <json/json.h>
 
 #include <iostream>
 #include <sstream>
@@ -151,4 +153,103 @@ StringList Options::toCommandLine() const
     return s;
 }
 
+Options Options::fromFile(const std::string& filename, bool throwOnOpenError)
+{
+    if (!FileUtils::fileExists(filename))
+    {
+        if (throwOnOpenError)
+            throw pdal_error("Can't read options file '" + filename + "'.");
+        else
+            return Options();
+    }
+
+    std::string s = FileUtils::readFileIntoString(filename);
+
+    size_t cnt = Utils::extractSpaces(s, 0);
+    if (s[cnt] == '{')
+        return fromJsonFile(filename, s);
+    else if (s[cnt] == '-')
+        return fromCmdlineFile(filename, s);
+    else
+        throw pdal_error("Option file '" + filename + "' not valid JSON or "
+            "command-line format.");
+}
+
+
+Options Options::fromJsonFile(const std::string& filename, const std::string& s)
+{
+    Options options;
+
+    Json::Reader reader;
+    Json::Value node;
+
+    if (!reader.parse(s, node))
+        throw pdal_error("Unable to parse options file '" + filename +
+            "' as JSON: \n" + reader.getFormattedErrorMessages());
+
+    for (const std::string& name : node.getMemberNames())
+    {
+        if (node[name].isString())
+            options.add(name, node[name].asString());
+        else if (node[name].isInt())
+            options.add(name, node[name].asInt64());
+        else if (node[name].isUInt())
+            options.add(name, node[name].asUInt64());
+        else if (node[name].isDouble())
+            options.add(name, node[name].asDouble());
+        else if (node[name].isBool())
+            options.add(name, node[name].asBool());
+        else if (node[name].isNull())
+            options.add(name, "");
+        else if (node[name].isArray() || node[name].isObject())
+        {
+            Json::FastWriter w;
+            options.add(name, w.write(node[name]));
+        }
+        else
+            throw pdal_error("Value of stage option '" +
+                name + "' in options file '" + filename +
+                "' cannot be converted.");
+    }
+    return options;
+}
+
+
+Options Options::fromCmdlineFile(const std::string& filename,
+    const std::string& s)
+{
+    Options options;
+    StringList args = Utils::simpleWordexp(s);
+
+    for (size_t i = 0; i < args.size(); ++i)
+    {
+        std::string option = args[i];
+        std::string value;
+        if (i + 1 < args.size())
+            value = args[i + 1];
+
+        if (option.size() < 3)
+            throw pdal_error("Invalid option '" + option + "' in option "
+                "file '" + filename + "'.");
+        if (option[0] != '-' || option[1] != '-')
+            throw pdal_error("Option '" + option + "' missing leading \"--\" "
+                "in option file '" + filename + "'.");
+
+        std::string::size_type pos = 2;
+        std::string::size_type count = Option::parse(option, pos);
+        std::string optionName = option.substr(2, count);
+        pos += count;
+        if (option[pos++] == '=')
+            value = option.substr(pos);
+        else
+            i++;
+        if (value.empty())
+            throw pdal_error("No value found for option '" + option + "' in "
+                "option file '" + filename + "'.");
+        Option o(optionName, value);
+        options.add(o);
+    }
+    return options;
+}
+
 } // namespace pdal

pdal/Options.hpp

@@ -224,9 +224,16 @@ class PDAL_DLL Options
 
     std::vector<Option> getOptions(std::string const& name="") const;
     StringList toCommandLine() const;
+    static Options fromFile(const std::string& filename,
+        bool throwOnOpenError = true);
 
 private:
     std::multimap<std::string, Option> m_options;
+
+    static Options fromJsonFile(const std::string& filename,
+        const std::string& s);
+    static Options fromCmdlineFile(const std::string& filename,
+        const std::string& s);
 };
 typedef std::map<std::string, Options> OptionsMap;
 

pdal/Stage.cpp

@@ -98,9 +98,22 @@ void Stage::addAllArgs(ProgramArgs& args)
 void Stage::handleOptions()
 {
     addAllArgs(*m_args);
+
+    StringList files = m_options.getValues("option_file");
+    for (std::string& file : files)
+        m_options.addConditional(Options::fromFile(file));
+    m_options.remove(Option("option_file", 0));
+
+    // Special stuff for GRiD so that no error is thrown when a file
+    // isn't found.
+    files = m_options.getValues("grid_option_file");
+    for (std::string& file : files)
+        m_options.addConditional(Options::fromFile(file, false));
+    m_options.remove(Option("grid_option_file", 0));
+
+    StringList cmdline = m_options.toCommandLine();
     try
     {
-        StringList cmdline = m_options.toCommandLine();
         m_args->parse(cmdline);
     }
     catch (arg_error error)
@@ -410,6 +423,11 @@ void Stage::l_addArgs(ProgramArgs& args)
 {
     args.add("user_data", "User JSON", m_userDataJSON);
     args.add("log", "Debug output filename", m_logname);
+    // We never really bind anything to this variable.  We extract the option
+    // before parsing the command line.  This entry allows a line in the
+    // help and options list.
+    args.add("option_file", "File from which to read additional options",
+        m_optionFile);
     readerAddArgs(args);
 }
 

pdal/Stage.hpp

@@ -358,6 +358,9 @@ class PDAL_DLL Stage
     std::string m_userDataJSON;
     point_count_t m_pointCount;
     point_count_t m_faceCount;
+    // This is never used, but we want something to bind to the argument
+    // we stick in ProgramArgs so that it shows up in help and an options list.
+    std::string m_optionFile;
 
     Stage& operator=(const Stage&); // not implemented
     Stage(const Stage&); // not implemented

test/data/apps/bad_cmd_opt

@@ -0,0 +1 @@
+--foobar=Classification[0:3]

test/data/apps/bad_json_opt

@@ -0,0 +1,3 @@
+{
+    "somejunk":"Classification[0:3]"
+}

test/data/apps/good_cmd_opt

@@ -0,0 +1 @@
+--limits=Classification[0:3]

test/data/apps/good_json_opt

@@ -0,0 +1,3 @@
+{
+    "limits":"Classification[0:3]"
+}

test/unit/apps/AppTest.cpp

@@ -64,4 +64,51 @@ TEST(PdalApp, log)
     EXPECT_TRUE(output.find("PDAL Debug") == std::string::npos);
 }
 
+TEST(PdalApp, option_file)
+{
+    std::string output;
+
+    std::string baseCommand = appName() + " translate " +
+        Support::datapath("las/simple.las") + " " +
+        Support::temppath("out.las") + " -f filters.range ";
+    std::string command;
+
+    Utils::run_shell_command(baseCommand + " 2>&1", output);
+    EXPECT_TRUE(output.find("Missing value") != std::string::npos);
+
+    command = baseCommand +
+        "--filters.range.option_file=" + Support::datapath("apps/nofile") +
+        " 2>&1";
+    Utils::run_shell_command(command, output);
+    EXPECT_TRUE(output.find("Can't read") != std::string::npos);
+
+    command = baseCommand +
+        "--filters.range.option_file=" +
+        Support::datapath("apps/good_cmd_opt") +
+        " 2>&1";
+    Utils::run_shell_command(command, output);
+    EXPECT_TRUE(output.empty());
+
+    command = baseCommand +
+        "--filters.range.option_file=" +
+        Support::datapath("apps/good_json_opt") +
+        " 2>&1";
+    Utils::run_shell_command(command, output);
+    EXPECT_TRUE(output.empty());
+
+    command = baseCommand +
+        "--filters.range.option_file=" +
+        Support::datapath("apps/bad_cmd_opt") +
+        " 2>&1";
+    Utils::run_shell_command(command, output);
+    EXPECT_TRUE(output.find("Unexpected argument") != std::string::npos);
+
+    command = baseCommand +
+        "--filters.range.option_file=" +
+        Support::datapath("apps/bad_json_opt") +
+        " 2>&1";
+    Utils::run_shell_command(command, output);
+    EXPECT_TRUE(output.find("Unexpected argument") != std::string::npos);
+}
+
 } // unnamed namespace

test/unit/apps/TranslateTest.cpp

@@ -93,6 +93,7 @@ TEST(translateTest, t2)
         output), 0);
 
     // Check that we fail with no bad input file.
+    FileUtils::deleteFile("foo.las");
     EXPECT_NE(runTranslate("foo.las " + out + " --json=\"" + json + "\"",
         output), 0);