JeffFlinn / boost-process

proposed portable process library for boost
27 stars 4 forks source link

How to set an environment variable, especially on the POSIX side? #3

Closed leutloff closed 6 years ago

leutloff commented 12 years ago

Hi,

thank you for providing the process library. It works really good, even on an ancient Debian Lenny system.

Now I must add an environment variable when running an executable. But the POSIX side seams not to be implemented. I am willing to provide a patch to get it working, but I am not sure what the desired way is.

Here is the existing part of the code that should be enhanced with a TEXINPUTS variable, e.g. in shell syntax: TEXINPUTS=../newpath:$TEXINPUTS.

    const fs::path exePdfLatex = fs::path("/usr/bin/pdflatex");
    const fs::path BERGOUTDIR = fs::path("out");
    bio::file_descriptor_sink tex_log(texLogfile.c_str());
    bp::monitor c12 = bp::make_child(
                    bp::paths(exePdfLatex.c_str(), BERGOUTDIR.c_str())
                    , bp::arg("-interaction=nonstopmode")
                    , bp::arg("-file-line-error")
                    , bp::arg("-output-directory=out")
                    , bp::arg("filename.tex")
                    , bp::std_out_to(tex_log)
                    , bp::std_err_to(tex_log)
                    );

Complete code is available from https://github.com/leutloff/berg/blob/master/src/srv/maker/maker.cpp

The example code referenced int issue #1, is from another process implementation - right?

  bp::context ctx;
  ctx.environment = bp::self::get_environment();
  bp::child c = bp::launch((*i).process.ProcessName(), (*i).process.Args(), ctx);

So I would aim for a solution like

    const fs::path exePdfLatex = fs::path("/usr/bin/pdflatex");
    const fs::path BERGOUTDIR = fs::path("out");
    bio::file_descriptor_sink tex_log(texLogfile.c_str());
    bp::monitor c12 = bp::make_child(
                    bp::paths(exePdfLatex.c_str(), BERGOUTDIR.c_str())
                    , bp::arg("-interaction=nonstopmode")
                    , bp::arg("-file-line-error")
                    , bp::arg("-output-directory=out")
                    , bp::arg("filename.tex")
                    , bp::env("TEXINPUTS", bp::paths("new value", "existing values"))
                    , bp::std_out_to(tex_log)
                    , bp::std_err_to(tex_log)
                    );

This will be implemented similar to bp::arg. The in make_child mentioned environment variables are added to the existing environment variables of the main process and overwriting existing ones.

Would you accept a patch that is heading in the outlined direction?

Are there already any test cases available that I could use as an example?

JeffFlinn commented 12 years ago

Hi,

On Tue, Sep 18, 2012 at 4:27 AM, Christian Leutloff < notifications@github.com> wrote:

Hi,

thank you for providing the process library. It works really good, even on an ancient Debian Lenny system.

That's great that you are finding the lib useful!

Now I must add an environment variable when running an executable. But the POSIX side seams not to be implemented. I am willing to provide a patch to get it working, but I am not sure what the desired way is.

Here is the existing part of the code that should be enhanced with a TEXINPUTS variable, e.g. in shell syntax: TEXINPUTS=../newpath:$TEXINPUTS.

const fs::path exePdfLatex = fs::path("/usr/bin/pdflatex");
const fs::path BERGOUTDIR = fs::path("out");
bio::file_descriptor_sink tex_log(texLogfile.c_str());
bp::monitor c12 = bp::make_child(
                bp::paths(exePdfLatex.c_str(), BERGOUTDIR.c_str())
                , bp::arg("-interaction=nonstopmode")
                , bp::arg("-file-line-error")
                , bp::arg("-output-directory=out")
                , bp::arg("filename.tex")
                , bp::std_out_to(tex_log)
                , bp::std_err_to(tex_log)
                );

Complete code is available from https://github.com/leutloff/berg/blob/master/src/srv/maker/maker.cpp

The example code referenced int issue #1https://github.com/JeffFlinn/boost-process/issues/1, is from another process implementation - right?

bp::context ctx; ctx.environment = bp::self::get_environment(); bp::child c = bp::launch((_i).process.ProcessName(), (_i).process.Args(), ctx);

You are correct that the boost::process::context related code is not from the lib here on github, it's from one of the earlier attempts at a process lib for boost.

So I would aim for a solution like

const fs::path exePdfLatex = fs::path("/usr/bin/pdflatex");
const fs::path BERGOUTDIR = fs::path("out");
bio::file_descriptor_sink tex_log(texLogfile.c_str());
bp::monitor c12 = bp::make_child(
                bp::paths(exePdfLatex.c_str(), BERGOUTDIR.c_str())
                , bp::arg("-interaction=nonstopmode")
                , bp::arg("-file-line-error")
                , bp::arg("-output-directory=out")
                , bp::arg("filename.tex")
                , bp::env("TEXINPUTS", bp::paths("new value", "existing values"))
                , bp::std_out_to(tex_log)
                , bp::std_err_to(tex_log)
                );

This will be implemented similar to bp::arg. The in make_child mentioned environment variables are added to the existing environment variables of the main process and overwriting existing ones.

Would you accept a patch that is heading in the outlined direction?

I'll definitely accept patches! Both windows and posix inherit the parent's environment by default without any environment initializer. Doh, the existing posix environment initializer is just wrong. It actually looks like a mix of posix/windows code, sorry. It should implement the posix pre_fork_parent method rather than the windows pre_create method. I'll get that fixed tonight.

With the above fix the posix/windows environment initializer as coded will really just make explicit the current implicit behavior. That is the child process will have the same environment as the parent.

I'm thinking the args/arg initializers can provide some inspiration. Your arg related code above can also be written as:

bp::args("-interaction=nonstopmode")("-file-line-error")("-output-directory=out")("filename.tex")

This approach follows that of boost::spirit::symbols, allowing composition of arg instances into an args container. I've got notes related to ideas on specifying additions/subtractions of specific env variables given a snapshot of the parents environment. I'll get those to you this evening.

Are there already any test cases available that I could use as an example?

Test cases and docs are currently sorely lacking. I haven't had much need for environment variables different from the parent's, which is why environment initializers are lacking.

— Reply to this email directly or view it on GitHubhttps://github.com/JeffFlinn/boost-process/issues/3.

I will provide any help needed.

Thanks, Jeff

leutloff commented 12 years ago

Good news. Thanks.

While you are on it, you may want to take a look at the fork of git://github.com/ShuminHuang/boost-process.git (https://github.com/ShuminHuang/boost-process). That fork collects all the existing patches to boost-process and provides a first test case and the old attempts to boost::process as tar archives. I have scanned the patches, commented sometimes on them, and got issues corrected.

leutloff commented 6 years ago

Superceded by official integration into boost.