Re: [PATCH] OpenMP, libgomp: Environment variable syntax extension.

[Jakub Jelinek <jakub@redhat.com>]

On Thu, Jun 30, 2022 at 01:40:24PM +0200, Jakub Jelinek via Gcc-patches wrote:
> On Fri, Jun 10, 2022 at 03:59:37PM +0200, Marcel Vollweiler wrote:
> > > I'm not sure we can rely on execv on all targets that do support libgomp.
> > > Any reason why you actually need this, rather than using
> > > dg-set-target-env-var directive(s) and perhaps return 0; if getenv doesn't
> > > return the expected values?
> > 
> > Interesting topic. After some (internal) discussions I think the best way is to
> > set the environment variables explicitely instead using dg-set-target-env-var.
> > The reason is that dg-set-target-env-var does not work for remote testing (which
> > seems to be a common test environment). For remote testing dejagnu immediately
> > aborts the test case with UNSUPPORTED which is specified in the corresponding
> > extension and makes sence from my point of view as the test assumption cannot be
> > fulfilled (since the environment variables are not set on remote targets).
> > It also means that whenever dg-set-target-env-var is set in the test file, the
> > execution of the test case is not tested on remote targets.
> 
> The only reason why dg-set-target-env-var is supported on native only right
> now is that I'm never doing remote testing myself and so couldn't test that.
> There is no inherent reason why the env vars couldn't be propagated over to
> the remote and set in the environment there.
> So trying to work around that rather than at least trying to change
> dg-set-target-env-var so that it works with the remote testing you do looks
> wrong.
> If dg-set-target-env-var can be made to work remotely, it will magically
> improve those 130+ tests that use it already together with the newly added
> tests.
> 
> So, I'd suggest to just use dg-set-target-env-var and incrementally work on
> making it work for remote testing if that is important to whomever does
> that kind of testing.  Could be e.g. a matter of invoking remotely
> env VAR1=val1 VAR2=val2 program args
> instead of program args.  If env is missing on the remote side, it could
> be UNSUPPORTED then.

essentially where we now do:
        if { [info exists set_target_env_var] \
             && [llength $set_target_env_var] != 0 } {
            if { [is_remote target] } {
                return [list "unsupported" ""]
            }
            set-target-env-var
        }
in the is_remote case check (ideally cached) whether env program works on the
target using remote exec with the env vars temporarily disabled and trying
something like
env MYVAR=value
and if that succeeds and prints MYVAR=value in the output, don't
return [list "unsupported" ""] and instead continue without doing that
set-target-env-var/restore-target-env-var stuff
and after this big block wrap also call_remote and if it is exec and the
env vars are around (same stuff as how the intercepted ${tool}_load does
this), prepend env and the vars in VAR=value format before the rest of args.

	Jakub