Repository navigation
Per-host dependency derivation for launch-manager install sets - #28
Merged
Merged
Conversation
…against it A workspace whose machines are not all big enough to install everything ends up with the launch files in one package and the dependencies in sibling install sets, one per host. Neither check could make sense of that: the package naming a dependency is not the package declaring it, so athena_driver_launch reported 12 missing exec_depends to the manifest validator and 27 undeclared references to the launch-tree check, none of them real. The workaround was --exclude-package, which switched the checks off and took the real findings with them. The mapping is already in the repository twice. launch_manager_configs assigns a component to a host; the component definition names the launch file it starts. Following that launch file derives what a host needs, so the install sets become checked rather than hand-maintained. A package claims a host in its own export section: <export><launch_manager_host>athena-gripper</launch_manager_host></export> Custom export children are safe: the schema check only inspects top-level children, BuildTypeExportStep leaves the rest alone, and a formatter round-trip leaves the tag byte-identical. With no such export anywhere the run behaves exactly as before - verified against athena_driver_launch, which reports the same 12 until the tags are added. The derivation is a lower bound and the fill is additive only. 15 of athena_driver_launch_main's 32 dependencies are named nowhere a launch-file scan reaches: controller plugins, pluginlib libraries, and packages handed to a generic component as an argument via a config file. A declared dependency that was not derived is left alone and not reported; removing one would uninstall something the robot needs. Nothing is filled from an incomplete derivation. A component whose definition cannot be found - usually because launch_manager_common_components is not installed - means the set is short by an unknown amount, so the run says so and writes nothing. A host the configuration launches on that no package claims is reported from the run rather than from a file: the gap is the absence of a package, so no per-package check could notice it. athena-jetson runs three components and has no install set today. On athena_driver_launch this turns 12 findings into 1 for the manifest validator and 27 into 5 for the launch-tree check, and the ones that remain are real - including a component naming a launch file the package does not ship. Auto-fill adds exactly the seven, two and two entries the derivation names, and removes nothing.
check-launch-tree knew that an install set may declare what the configuration
package's launch files name, but when nothing declared it the finding still
read "not declared by athena_driver_launch" - naming the package the reference
was written in rather than the one that should have declared it. A CI run of
this hook alone gave a true failure pointing at the wrong manifest.
It now derives what each host launches, the same way the manifest validator
does, and names the install sets that want the package:
error ros2_vitals is not declared by any install set that launches it
wanted by athena-arm-base - athena_driver_launch_arm_base/package.xml
wanted by athena-gripper - athena_driver_launch_gripper/package.xml
wanted by athena-jetson - no package claims this host, so nothing
declares what it runs
named in launch_manager_components/ros2_vitals.yaml:3
Still one finding per package rather than one per host, so the count does not
change - only the attribution. Deriving is only done once some package claims a
host, since walking every component's launch tree is the expensive part and
there is nothing to say without one.
Redirection applies to the configuration package alone. Everywhere else the
manifest a reference was written in is the manifest that should declare it, and
there is nothing to redirect.
The hook still does not check install sets or fill anything - that stays with
the manifest validator, so the same defect is not reported twice.
Scope's host_packages and install_sets defaulted to {}. A NamedTuple default is
one object shared by every instance that omits the field, so anything written
to one scope's map would have been visible from the next and from every scope
built afterwards. Nothing mutates them today, which is the only reason it was
not already a bug. They are MappingProxyType now, so a future write raises
instead of leaking.
An install set claiming a host reported "claims host 'gripper', which no
launch-manager configuration mentions" whenever the derivation had failed - two
configuration packages, say - because a failed derivation yields no hosts at
all. That sends the reader to fix a manifest that is already correct. When the
derivation is incomplete the step now says it cannot tell, and says why.
A run whose only finding was a host with no install set printed its error and
then "Corrected `package.xml` files successfully". The exit code was right; the
summary was not. Creating the missing package is not something the tool can do,
so it now counts among the unresolvable errors and the summary says so.
The test for the third one passed with the fix reverted, twice. First the
fixture left a second error that reached the same summary by another route;
then the minimal manifests it wrote were missing <version>, <description>,
<maintainer> and <license>, and those raise a critical error of their own. The
fixture now writes complete manifests, and all three fixes fail their test when
reverted.
|
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Some workspaces keep every launch file in one package and declare the dependencies in sibling install sets, one per machine, the shape you get when a robot's computers aren't all big enough to install everything. Neither check could make sense of that: the package naming a dependency isn't the package declaring it. On
athena_driver_launchthat meant 12 false findings fromformat-package-xmland 27 fromcheck-launch-tree, worked around with--exclude-package, which switched the checks off entirely.The mapping already exists in the repo:
launch_manager_configsassigns a component to a host, and the component definition names the launch file it starts. Following that launch file derives what each host needs.Usage
A package says which machine it is the install set for:
The validator then checks that install set against everything its host launches, and --auto-fill-missing-deps adds what's missing. check-launch-tree stops blaming the shared launch package and names the install set instead.