Hi Jusin, thanks. I think this version is indeed cleaner.

However, I tried:

scripts/build image=tests
scripts/run.py -e tests/tst-commands.so

and I get a failure:

...
PASS: runscript multiple lines
PASS: runscript multiple lines, multiple commands per line, with args and
quotes
FAIL: runscript with --env
SUMMARY: 14 tests, 1 failures

Could you please look into it?

Thanks,
Nadav.




--
Nadav Har'El
[email protected]

On Mon, Jan 9, 2017 at 10:11 AM, Justin Cinkelj <[email protected]>
wrote:

> If multiple scripts try to set the same environment variable, then the
> last one wins.
>
> Fixes #819
>
> Signed-off-by: Justin Cinkelj <[email protected]>
>
> # Conflicts:
> #       loader.cc
> ---
>  core/commands.cc      | 64 ++++++++++++++++++++++++++++++
> +++++++++++++++++++++
>  tests/tst-commands.cc | 52 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 116 insertions(+)
>
> diff --git a/core/commands.cc b/core/commands.cc
> index b2b3a60..1ba6bf8 100644
> --- a/core/commands.cc
> +++ b/core/commands.cc
> @@ -12,6 +12,8 @@
>
>  #include <boost/config/warning_disable.hpp>
>  #include <boost/spirit/include/qi.hpp>
> +#include <boost/program_options.hpp>
> +#include <osv/power.hh>
>  #include <osv/commands.hh>
>  #include <osv/align.hh>
>  #include <sys/types.h>
> @@ -90,6 +92,66 @@ parse_command_line_min(const std::string line, bool &ok)
>  }
>
>  /*
> +In each runscript line, first N args starting with - are options.
> +Parse options and remove them from result.
> +
> +Options are applied immediately, just as in loader.cc parse_options().
> +So if two scripts set the same environment variable, then the last one
> wins.
> +Applying all options before running any command is also safer than trying
> to
> +apply options for each script at script execution (second script would
> modify
> +environment setup by the first script, causing a race).
> +*/
> +static void runscript_process_options(std::vector<std::vector<std::string>
> >& result) {
> +    namespace bpo = boost::program_options;
> +    namespace bpos = boost::program_options::command_line_style;
> +    // don't allow --foo bar (require --foo=bar) so we can find the first
> non-option
> +    // argument
> +    int style = bpos::unix_style & ~(bpos::long_allow_next |
> bpos::short_allow_next);
> +    bpo::options_description desc("OSv runscript options");
> +    desc.add_options()
> +        ("env", bpo::value<std::vector<std::string>>(), "set Unix-like
> environment variable (putenv())");
> +
> +    for (size_t ii=0; ii<result.size(); ii++) {
> +        auto cmd = result[ii];
> +        bpo::variables_map vars;
> +
> +        std::vector<const char*> args = { "dummy-string" };
> +        // due to https://svn.boost.org/trac/boost/ticket/6991, we can't
> terminate
> +        // command line parsing on the executable name, so we need to
> look for it
> +        // ourselves
> +        auto ac = cmd.size();
> +        auto av = std::vector<const char*>();
> +        av.reserve(ac+1);
> +        size_t jj = 0;
> +        for (auto prm : cmd) {
> +            av[jj++] = prm.c_str();
> +        }
> +        auto nr_options = std::find_if(av.data(), av.data() + ac,
> +                                       [](const char* arg) { return
> arg[0] != '-'; }) - av.data();
> +        std::copy(av.data(), av.data() + nr_options,
> std::back_inserter(args));
> +
> +        try {
> +            bpo::store(bpo::parse_command_line(args.size(), args.data(),
> desc, style), vars);
> +        } catch(std::exception &e) {
> +            std::cout << e.what() << '\n';
> +            std::cout << desc << '\n';
> +            osv::poweroff();
> +        }
> +        bpo::notify(vars);
> +
> +        if (vars.count("env")) {
> +            for (auto t : vars["env"].as<std::vector<std::string>>()) {
> +                debug("Setting in environment: %s\n", t);
> +                putenv(strdup(t.c_str()));
> +            }
> +        }
> +
> +        cmd.erase(cmd.begin(), cmd.begin() + nr_options);
> +        result[ii] = cmd;
> +    }
> +}
> +
> +/*
>  If cmd starts with "runcript file", read content of file and
>  return vector of all programs to be run.
>  File can contain multiple commands per line.
> @@ -125,6 +187,8 @@ std::vector<std::vector<std::string>>
> runscript_expand(const std::vector<std::st
>                  ok = false;
>                  return result2;
>              }
> +            // process and remove options from command
> +            runscript_process_options(result3);
>              result2.insert(result2.end(), result3.begin(), result3.end());
>              line_num++;
>          }
> diff --git a/tests/tst-commands.cc b/tests/tst-commands.cc
> index de0c143..7d0a679 100644
> --- a/tests/tst-commands.cc
> +++ b/tests/tst-commands.cc
> @@ -625,6 +625,56 @@ static bool test_runscript_multiline_
> multiple_commands_per_line_with_args_quotes
>      return true;
>  }
>
> +static bool test_runscript_with_env()
> +{
> +    std::ofstream of1("/myscript", std::ios::out | std::ios::binary);
> +    of1 << "--env=ASDF=ttrt /prog1 pp1a pp1b\n";
> +    of1.close();
> +
> +    std::vector<std::vector<std::string> > result;
> +    std::vector<std::string> cmd = { "/prog1" };
> +    size_t expected_size[] = {4};
> +    bool ok;
> +
> +    if (NULL != getenv("ASDF")) {
> +        return false;
> +    }
> +
> +    result = osv::parse_command_line(
> +        std::string("runscript \"/myscript\";  "),
> +        ok);
> +
> +    if (!ok) {
> +        return false;
> +    }
> +
> +    if (result.size() != 1) {
> +        return false;
> +    }
> +
> +    for (size_t i = 0; i < result.size(); i++) {
> +        if (result[i].size() != expected_size[i]) {
> +            return false;
> +        }
> +        if (result[i][0] != cmd[i]) {
> +            return false;
> +        }
> +    }
> +
> +    if (result[0][1] != std::string("pp1a")) {
> +        return false;
> +    }
> +    if (result[0][2] != std::string("pp1b")) {
> +        return false;
> +    }
> +
> +    if (std::string("ttrt") != getenv("ASDF")) {
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
>  int main(int argc, char *argv[])
>  {
>      report(test_parse_simplest(), "simplest command line");
> @@ -651,6 +701,8 @@ int main(int argc, char *argv[])
>             "runscript multiple lines");
>      report(test_runscript_multiline_multiple_commands_
> per_line_with_args_quotes(),
>             "runscript multiple lines, multiple commands per line, with
> args and quotes");
> +    report(test_runscript_with_env(),
> +           "runscript with --env");
>      printf("SUMMARY: %d tests, %d failures\n", tests, fails);
>      return 0;
>  }
> --
> 2.9.3
>
> --
> You received this message because you are subscribed to the Google Groups
> "OSv Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to [email protected].
> For more options, visit https://groups.google.com/d/optout.
>

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to