Add filtering mechanism for executable prefix application #522
Conversation
…utables Signed-off-by: Cameron Miller <cammlle@amazon.com>
Signed-off-by: Cameron Miller <cammlle@amazon.com>
Signed-off-by: Cameron Miller <cammlle@amazon.com>
camm73
force-pushed
the
cameron/support-launch-prefix-filter
branch
from
ad4dc2c
to
1be7845
Compare
Signed-off-by: Cameron Miller <cammlle@amazon.com>
camm73
force-pushed
the
cameron/support-launch-prefix-filter
branch
from
1be7845
to
7177e69
Compare
| filter_str = perform_substitutions(context, self.__prefix_filter) | ||
| regex_filter = None | ||
| if len(filter_str): | ||
| regex_filter = re.compile(filter_str) | ||
| if regex_filter is not None and regex_filter.match(os.path.basename(cmd[0])): | ||
| cmd = shlex.split(perform_substitutions(context, self.__prefix)) + cmd | ||
|
|
||
| # Apply to all if no filter is provided or filter is overridden | ||
| if regex_filter is None or self.__override_filter: | ||
| cmd = shlex.split(perform_substitutions(context, self.__prefix)) + cmd |
There was a problem hiding this comment.
@camm73 meta: hmm, if I read this correctly the prefix filter is irrelevant when a prefix is explicitly given. Nevermind the potential code simplification, that's actually a bit counter-intuitive when attempting to target an specific executable.
There was a problem hiding this comment.
My thinking behind this was based on the fact that whenever a user sets a specific prefix in their launch file it currently overrides any launch-prefix provided in the launch configuration and applies the specific prefix:
launch/launch/launch/actions/execute_process.py
Lines 230 to 232 in f891aed
Since the specific prefix already overrides a prefix passed via --launch-prefix, I figured it should also do the same when --launch-prefix and --launch-prefix-filter are used in conjunction, for the sake of consistency.
If you think it's more intuitive to remove the override_filter feature though, I can definitely do that.
There was a problem hiding this comment.
I figured it should also do the same when --launch-prefix and --launch-prefix-filter are used in conjunction, for the sake of consistency.
That's fair. Should be fine so long as we document it.
If you think it's more intuitive to remove the override_filter feature though, I can definitely do that.
Not so much removing the feature. My naive (and wrong) expectation coming into launch could be that ros2 launch --launch-prefix-filter some_executable --launch-prefix gdb --args takes precedence for some_executable over whatever was specified as prefix. So long as we document this is NOT the case, it'll be fine.
There was a problem hiding this comment.
Specifically, let's document this behavior in ExecuteProcess.__init__() docstring.
| filter_str = perform_substitutions(context, self.__prefix_filter) | ||
| regex_filter = None | ||
| if len(filter_str): | ||
| regex_filter = re.compile(filter_str) | ||
| if regex_filter is not None and regex_filter.match(os.path.basename(cmd[0])): | ||
| cmd = shlex.split(perform_substitutions(context, self.__prefix)) + cmd | ||
|
|
||
| # Apply to all if no filter is provided or filter is overridden | ||
| if regex_filter is None or self.__override_filter: | ||
| cmd = shlex.split(perform_substitutions(context, self.__prefix)) + cmd |
There was a problem hiding this comment.
Specifically, let's document this behavior in ExecuteProcess.__init__() docstring.
Co-authored-by: Michel Hidalgo <michel@ekumenlabs.com> Signed-off-by: Cameron Miller <cammlle@amazon.com>
Co-authored-by: Michel Hidalgo <michel@ekumenlabs.com> Signed-off-by: Cameron Miller <cammlle@amazon.com>
Signed-off-by: Cameron Miller <cammlle@amazon.com>
Signed-off-by: Cameron Miller <cammlle@amazon.com>
camm73
force-pushed
the
cameron/support-launch-prefix-filter
branch
from
a23f33c
to
9db2e8d
Compare
Signed-off-by: Cameron Miller <cammlle@amazon.com>
|
Alright, going in! |
In order to support the
--launch-prefix-filterargument proposed in ros2/launch_ros#257, the regex filter supplied via thelaunch-prefix-filterLaunchConfiguration must be matched against the executable name. In the case of a match, the prefix passed via thelaunch-prefixLaunchConfiguration is prepended to the executable command. If a match is not found, the prefix is not applied.Additionally, if the user provides a specific prefix for an executable in the launch file, the filter will be overridden and the prefix will be applied as expected.
Corresponding PR: ros2/launch_ros#261 will implement the
--launch-prefix-filterargument intolaunch_ros.