"auto& prm : cmd" does work. Thank you, sending v4 now. ----- Original Message ----- > From: "Nadav Har'El" <[email protected]> > To: "Justin Cinkelj" <[email protected]> > Cc: "Osv Dev" <[email protected]> > Sent: Monday, January 9, 2017 9:29:13 PM > Subject: Re: [PATCH v2] command line: allow --env in runscript > > On Mon, Jan 9, 2017 at 10:06 PM, Justin Cinkelj <[email protected]> > wrote: > > > On FC25 test fails. It is compiler and/or std:: lib dependent. > > > > Diff between v2 nad v3: > > - av.reserve(ac+1); > > - size_t jj = 0; > > - for (auto prm : cmd) { > > - av[jj++] = prm.c_str(); > > + av.reserve(ac); > > + for (auto it=cmd.begin(); it!=cmd.end(); it++) { > > + av.push_back(it->c_str()); > > > > reserve() is not resize(), so I should use push_back, not [] (but even > > with that, code works). > > I also don't need to reserve ac+1 elements. > > > > Problem was in "(auto prm : cmd)". I guess this creates temporal > > std::string, and (temporal) prm.c_str() > > is saved into av. > > > Right. Would "auto& prm : cmd" work? Note the ampersand. It takes the > strings by reference, not making a temporary copy of each. > > > > On next iteration, (new?) temporal prm.c_str contains different value, but > > might be at same address. > > So in the failed testcase on FC25, av ended containing 4 pointers, all > > with same value. > > Using slightly longer iterator version avoids temporal std::string. > > > > Justin > > > > ----- Original Message ----- > > > From: "Justin Cinkelj" <[email protected]> > > > To: "Nadav Har'El" <[email protected]> > > > Cc: "Osv Dev" <[email protected]> > > > Sent: Monday, January 9, 2017 1:14:30 PM > > > Subject: Re: [PATCH v2] command line: allow --env in runscript > > > > > > [xlab@mike-c7 osv]$ sudo ./scripts/run.py -nvd -e > > '/tests/tst-commands.so' > > > ... > > > PASS: runscript multiple lines, multiple commands per line, with args > > > and quotes > > > PASS: runscript with --env > > > SUMMARY: 14 tests, 0 failures > > > /etc/qemu-ifdown: could not launch network script > > > [xlab@mike-c7 osv]$ > > > > > > So debug build on centos7 was OK. And also release build was OK. > > > You tested release, on fedora 25? I will retry that. > > > > > > Justin > > > > > > On 01/09/2017 10:28 AM, Nadav Har'El wrote: > > > > 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] <mailto:[email protected]> > > > > > > > > On Mon, Jan 9, 2017 at 10:11 AM, Justin Cinkelj > > > > <[email protected] <mailto:[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] > > > > <mailto:[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 > > > > <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] > > > > <mailto:osv-dev%[email protected]>. > > > > For more options, visit https://groups.google.com/d/optout > > > > <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.
