AP_HAL_SITL: work around bug in dash #27742
Conversation
libraries/AP_HAL_SITL/system.cpp
Outdated
| // work around a bug in dash; its manpage says it searches $PATH | ||
| // but it does't. https://github.com/ArduPilot/ardupilot/issues/24245 | ||
| const char *cmd_fmt = "sh %s %d >%s 2>&1"; | ||
| if (getenv("AP_DASH_HACKERY")) { |
There was a problem hiding this comment.
how does this env variable get set?
There was a problem hiding this comment.
By the user.
They already need to be running SITL (or autotest) with their cwd set to a place we're not expecting, and to have modified their $PATH to include scripts, so I figured an additional environment variable would be OK.
This patch was designed as least-risk-to-existing-users.
There was a problem hiding this comment.
I'm thinking it might be easier to understand (for the users like me w/ weird setups) if the env var was something more like AP_OVERRIDE_PATH_TO_SCRIPTS=/path/to/scripts, to just specify the full path to where dumpstack/dumpcore are to be found, rather than the two-step process of adding that dir to PATH and also setting this other var.
There was a problem hiding this comment.
I'm thinking it might be easier to understand (for the users like me w/ weird setups) if the env var was something more like AP_OVERRIDE_PATH_TO_SCRIPTS=/path/to/scripts, to just specify the full path to where dumpstack/dumpcore are to be found, rather than the two-step process of adding that dir to PATH and also setting this other var.
Yeah, I considered doing this approach but went with your suggestion instead as a tested solution :-)
Decided at DevCall to go with this, looking for an env variable speciying the path. The reason I prefer this one is that you don't need to modify your path to find the tool....
dash doesn't search the path for scripts, so allow user to specify path to ArduPilot scripts
peterbarker
force-pushed
the
pr/dash-hack
branch
from
53d5c0d
to
c99c982
Compare
|
Fixed to use a path from the environment to the ArduPilot scripts. |
Closes #24245