smarr/ReBench

ReBench environment variable inconsistency

o- opened this issue · 6 comments

o- commented

We just had a long and very confused session of tracking down diverging results compared across machines...

Given we call ReBench with some environment variables set, for instance ENABLE_TEST=1 rebench someExperiment.

When rebench-denoise fails, then ENABLE_TEST is passed to the experiment process, when rebench-denoise succeeds, it is not passed to the process.

What we expected is consistent behavior, i.e., either pass env variables or not.

The reason is this line -- the environment is not carried over as a side-effect of calling sudo.

smarr commented

Ugh, sorry about that.

Do you have a fork with support for defining environment variables in the config? I think @charig may have added that a long while back. (mostly just curios, because there's a PR from @OctaveLarose I still haven't gotten to)

A quick google search suggests that I should add the -E/--preserve-env flag to the sudo call.
https://www.petefreitag.com/item/877.cfm

o- commented

Do you have a fork with support for defining environment variables in the config? I think @charig may have added that a long while back.

yes, but we stopped using it, since I was reluctant to maintain it...

(mostly just curios, because there's a PR from @OctaveLarose I still haven't gotten to)

that's the one I am trying out right now. I guess learning from this experience it seems extra important to know that the variables in the config have precedence.

A quick google search suggests that I should add the -E/--preserve-env flag to the sudo call.

can confirm. though I am actually unsure if maybe always clearing all env variables is the better idea for repeatability. especially if there is a proper way of setting them.

FYI in the meantime, you may want to specify your env variables in the sudoers file to always pass them to sudo (https://wiki.archlinux.org/title/Sudo#Environment_variables), if they're provided with the same keys/values pair every time.

Or I suppose you could just edit the executor.py file and manually add them there before the sudo call that you've pointed out / add the -E flag. Not the best solutions, obviously.

smarr commented

Ok, my current plan is to put together a small PR just setting -E as an interim solution and adding a test to check that env vars are properly passed on.
Hopefully in the next couple of days.

Then, later, we can see how to handle @OctaveLarose's PR.

There's also still PR #159 to avoid issues in docker, though, I think I'll get only to one small change at a time.

smarr commented

With #174 configuration support for environment variables landed.

Though, this introduces major, breaking changes around them.

Environment variables are now not propagated, at all, in the standard case.
And, when one configures environment variables, only those are passed on through sudo, using --preserve-env=list-of-var-names.

In some way, this is more consistent, and hopefully reduces one more way how a system can unexpectedly change the configuration of benchmarks in an "undocumented" way.

In practice, this may turn out to be too restrictive. So, we'll need to see how well it works.

smarr commented

I'll close this issue for now, but feel free to reopen.