On 10/12/26 18:19, Joseph Myers wrote:
On Sun, 11 Oct 2026, Jacob Bachmeyer wrote:
On 10/9/26 11:42, Joseph Myers wrote:
On Thu, 8 Oct 2026, Jacob Bachmeyer wrote:
Testsuites are free to define their own Tcl variables and to use
namespaces
outside of ::dejagnu:: and ::DejaGnu:: (which are reserved for the
framework). What prevents the GCC testsuite from tracking its own
environment
overrides and building the final list at each remote_exec call?
There are about 70 remote_exec calls in the GCC testsuites (mostly for
host, some for build or target), and one remote_load, and about 20
${tool}_load. And the main way tests are run is via dg-test which is
provided by DejaGnu (and which uses ${tool}_load, which uses remote_load,
which uses call_remote, which uses standard_load if the board doesn't
provide its own *_load, which uses remote_exec).
No problem there; the environment override list parameter need only also be
added to remote_load, which needs to be documented first. ("So *that* is what
it does...") I will be writing documentation for it soon.
A design requiring most of 70 remote_exec calls to be adjusted to use an
extra argument, and additional calls made sure to use that argument
whenever they are added (alternatively: all replaced by calls to a local
wrapper, and then making sure no more direct remote_exec calls for host or
target are added in future) doesn't seem that robust compared to having a
standard system for accumulating environment overrides (where the extra
argument to remote_exec takes first precedence, then the accumulated
overrides).
I dispute your premise---this is the kind of adjustment that a moderate
amount of Emacs Lisp code can do quite easily. There is a sample in
DejaGnu's ChangeLog from the last time I had to update GPL notices.
Also, not all of the remote_exec calls may even need to pass environment
overrides. You previously said that only some tests rely on them.
Further, fixing PR44602 required auditing the entire DejaGnu code base
for uses of catch(n) and uplevel(n). Most uses of catch only required
changing double quotes to braces, but some required more extensive
work. This is the nature of maintaining software.
Lastly on this point, the optional arguments to remote_exec/remote_load
are an intermediate step that incurs only a minor future maintenance
burden on DejaGnu. Accepting the API you propose would make later tying
standing environment overrides to test groups at best very tedious and
at worst impossible without breaking backwards compatibility.
On the other hand, if the GCC testsuite defines its own variables to
accumulate environment overrides, in the future it will be able to
offload dynamic extent management by simply asking the framework to
save/restore those variables.
The dg-test procedure in dg.exp uses ${tool}_load (provided by the testsuite)
to actually run a compiled target executable. You therefore already have the
opportunity to pick up environment overrides from testsuite-specific locations
and pass them to remote_load.
GCC wraps the version of ${tool}_load provided in standard.exp, rather
than replacing it.
That is an example of a procedure that *should* be replaced: the
documentation says that it is supplied by the testsuite. Why DejaGnu
even *has* a default implementation is a mystery to me, from before my time.
The ${tool}_load callback invoked by dg.exp:dg-test is given only the name of
the output executable from the test GCC run. That will not change, but, since
${tool}_load is defined by the testsuite, it can obtain an environment
override list from wherever the testsuite keeps it.
Replacing ${tool}_load seems potentially less robust than wrapping it,
given that such a replacement won't then account for future changes made
to the default ${tool}_load in DejaGnu.
At this point, the only foreseeable future change to ${tool}_load in
DejaGnu is its possible deprecation and eventual removal. The manual
states that ${tool}_load is supplied by the testsuite and its arguments
are defined by the testsuite, although this is inaccurate: dg.exp
expects to pass an executable name to ${tool}_load.
On the other hand, the only place where the framework ever calls
${tool}_load is in a block annotated FIXME...
I will have to think about this.
DejaGnu will eventually have support for binding such values to groups of
tests at various levels, probably in a more generic form of "save/restore
these Tcl variables". This infrastructure is not yet implemented. In the
mean time, DejaGnu will not be adding global state that risks creating
"landmines" for testsuites to trip over.
In brief, your proposed API creates the opportunity for BAR.exp to quietly
depend on "left over" state from FOO.exp if the latter fails to clean up its
state. Such a situation is *certain* to break when native parallel testing is
implemented and FOO and BAR no longer execute in the same process.
The landmines already exist, in the form of the global ::env that's
already used - sometimes in a form that doesn't properly restore the value
at the end of FOO.exp (as well as having problems with not working
properly with remote host or target). Part of the benefit of adding a
system for environment overrides in DejaGnu is to defuse those landmines
by adding something that is easier to use correctly than ::env - when
replacing ::env uses, it's natural also to add saving / restoring in
places that didn't already do it, via use of push / pop operations,
because it's easier when the standard feature already exists than when you
need to code it yourself.
The existence of current landmines is not an excuse to create more of
them. Modifying ::env without restoring it is the same problem that
your proposed API introduces.
Replacing one "landmine" hazard with another does not fix anything.
My GCC patches don't replace all environment manipulation, just many of
the places including all those that are most important for remote target
testing, but they certainly make an incremental improvement over direct
::env use and facilitate further such improvements. That's the expected
use of such a feature - replacing ::env, not introducing global state to
testsuites that lack it.
If you are using ::env to manipulate the environment of remote
processes, you already have a problem. Further, as you have said
before, using ::env to manage "target" (or even "host") variables is
wrong because those environments are distinct from "build" and should
not be conflated.
Lastly on this point, ::env is defined by Tcl; it is not a DejaGnu feature.
Also, a standard DejaGnu environment scheme could be documented as one
where you cannot rely on whether state persists beyond the end of a .exp
file and should always clean up before the end of a .exp file, and then in
future DejaGnu could be made to do such cleanup - whereas that would be
more problematic if relying on GCC-specific variables.
It will be trivial in the future: run each test script in its own Tcl
context (see interp(n) for the mechanism). Delete the interpreter and
all of its state, whether in framework-defined or testsuite-defined
variables, is cleaned up. DejaGnu's own testsuite already does
something similar for internal unit tests.
Documenting that testsuites cannot rely on cleanup would be like posting
a "landmine here" sign instead of defusing the mine.
This last detail tells me that putting this in board_info is actually
wrong,
because the same board may need different environment overrides for
different
roles.
Tracking things in a separate variable (say env_info instead of
board_info) and then making remote_exec / remote_load extract the
overrides (based on whether "build", "host" or "target" is passed to
remote_*) and pass them down (together with any overrides that were
directly given to remote_load / remote_exec) doesn't particularly seem
more complicated than putting things in board_info, just different. It
could even use procedure names such as remote_pushenv as in my patch, or a
common remote_env, or something else.
The "build", "host", and "target" names are aliases, and remote_* also accepts
the name of any defined board, including (via support in call_remote) any
board for which load_board_description can locate a description file, although
overrides could be associated with those aliases instead of specific boards.
The suggestion of associating just with the aliases "build", "host",
"target" (and thus checking in remote_load and remote_exec before the
translation from alias to board name takes place) was to support the case
where host and target use the same board but need different overrides.
Suppose some overrides are registered under the build/host/target alias
and some are registered under the actual board name. How should those
interact and is there a single answer to that question that is
appropriate for all testsuites?
This could also be complicated, as DejaGnu supports testing on multiple
targets in a single run. How to distinguish between environment overrides
meant for all targets and those meant only for the current target? Further,
some targets in a multi-target run may support environment overrides while
others do not.
Presumably the target overrides should be cleared for each target. At
present, various things are cleared in reset_vars; either reset_vars or
setup_target_hook or push_target or some other such place could clear all
the environment override state for the target. But it's easier for them
to do so if there's a standard way in DejaGnu to store environment
overrides that one of those functions can know about, rather than each
testsuite doing its own thing.
If the environment overrides have dynamic extent, then there is no
problem because the framework will have popped the relevant stack. This
is a good argument for tying standard environment overrides to the
upcoming "testcase group" system.
It *does* need a way for a board to declare whether it has environment
variable support. There are tests currently skipped in the remote case
that should be skipped for a board lacking that support. But there are
also tests that just continue to run with best efforts - we don't want
executing everything on the host to fail because it doesn't know how to
set LC_ALL=C on that board, just because LC_ALL=C is used globally for
everything executed on the host, we want the tests to continue to run and
hope that in fact the board defaults to the C locale.
Are you suggesting that being unable to set LC_ALL=C on the *target* would
cause a failure because LC_ALL=C was set on a native *host*? I thought we had
already dropped the idea of using ::env as a template precisely *because*
remote nodes may need different environments.
No, I'm suggesting that remote_exec host should not fail just because
there's a request to pass through LC_ALL=C that it can't handle. The
testsuite should decide if certain environment variable settings are ones
that should result in skipping a test (ideally a result of UNRESOLVED)
versus "best efforts" ones that may not be relevant because the particular
test setup has another way of handling those (many path-related variables
are irrelevant for installed testing, for example; LC_ALL=C is irrelevant
on a host where that's the default).
No, remote_exec will not fail if the target does not support environment
overrides. Instead, a WARNING (or perhaps ERROR?) will be reported when
the environment override list is discarded. Remember that older versions
of the framework will silently discard the extra argument, so passing
environment overrides to remote_exec or remote_load is always "best
effort" anyway.
So having
${board}_exec reject calls with environment variables when unsupported is
not a good solution - and remote_exec needs to take care to work with
existing board files that define ${board}_exec to take exactly five
arguments.
Tcl's info(n) provides for that, and a WARNING (or maybe an ERROR?) will be
emitted when an environment override list is known to be dropped. The fly in
That would be incredibly noisy and undesirable (even if it could be
overridden by making the board file take "args" to ignore the request). A
single GCC testsuite run produces over 700000 PASSes; adding a similar
number of WARNING or ERROR is hardly a good idea (even a single ERROR is
something to avoid, given that they normally mean "a typo in a test
stopped a large part of the testsuite from being run at all", so adding
"expected" ERRORs would drown out the serious ones).
How is "the testsuite attempted to do something that the target is
*known* to not support" not worthy of at least a WARNING to the user?
Think about the developer who is trying to figure out why a test
FAILed---and the cause is that an environment setting needed for that
test to pass did not propagate to the target.
There needs to be *something* in the log to point at the cause of such a
failure.
You can test if environment overrides are supported on a target by using
env(1) or, if the target lacks env(1), building a trivial program that prints
the value of an environment variable and running it.
Although you can do that in a testsuite (the GCC testsuite has very many
such tests for target properties), since it's a property of the board file
as much as one that needs determining based on executing test code I think
it makes sense to have a way for the board file to communicate it - which
also avoids every testsuite needing to implement its own support for such
a check (including caching and invalidating the cache on target change to
avoid repeating the test compilation thousands of times).
If you are talking about a board description file, then we can add a key
to board_info that declares support for environment overrides on a
specific board. However, your initial patch appears to put this on the
*protocol* rather than the board, which I anticipate will have obnoxious
edge cases, especially with near-bare-metal targets: Murphy's Law says
that there is a board out there with an SSH-capable debug monitor that
has no environment at all.
-- Jacob
