Catkin plugin: Refactor build.
Metadata
Current evaluation
Merged in snapcraft#175 via commit 9045874fd, refactoring Catkin environment, dependency handling, build logic, source-space support, and tests.
Resolution: closed resolved
Reason: PR #175 was merged as 59434ff3a after implementation commit 9045874fd. It simplified Catkin build and dependency handling and brought the plugin to 100% unit-test coverage.
Evaluated by: mr-cal-bot
No scores available.
Issue body
This PR contains a number of small refactors:
- Change `env()` to only contain variables for run-time, and document each of them.
- Only obtain dependencies upon `pull()`-- for `build()` they should just be ready to go.
- Extract dependency resolution into a standalone function that can be tested in isolation.
- Change `build()` to be less clever and more simple/verbose/robust.
- Add `source-space` keyword to get rid of custom `build()`.
- Get unit testing coverage of the Catkin plugin to 100%.
Evaluation history
| Date | Model | Scores | Action | Summary |
|---|---|---|---|---|
| stealth/space-bunny-alpha | — | closed resolved | Merged in snapcraft#175 via commit 9045874fd, refactoring Catkin environment, dependency handling, build logic, source-space support, and tests. | |
| stealth/space-bunny-alpha | — | closed resolved | Merged in snapcraft#175 (commit 9045874fd), refactoring the Catkin plugin’s environment, dependency handling, build logic, source-space support, and tests. | |
| qwen/qwen3.6-35b-a3b | — | — | Merged after addressing reviewer feedback on error handling and testing strategy. The update refactored the Catkin plugin build process, simplified build logic, extracted dependency resolution, added a source-space keyword, and achieved 100% unit test coverage. | |
| qwen3.6-35b-a3b-mtp-q6 | — | — | Merged following reviewer approval. Refactored the Catkin plugin build by isolating dependency resolution, simplifying the build method, and achieving 100% test coverage. Clarified environment variables and added a source-space keyword. |
Update history
No update history recorded yet.
Related issues
| Issue | Project | State | Summary | Similarity |
|---|---|---|---|---|
| #1721168 Catkin plugin: support building the entire workspace | snapcraft (launchpad) | closed | Implemented in snapcraft PR #1743 (commit ec4cac26a, "catkin plugin: support building entire workspace"), merged ~Nov 2017. catkin-packages became optional: omitting it builds the whole workspace, while an empty list still means no packages. | |
| #331 Catkin plugin: Handle rosdep empty and multiple dependencies. | snapcraft | merged | PR #331 merged into snapcraft master as commit 1c1119ce3 (merge e3a2b9d03). It refactored the Catkin plugin's rosdep dependency resolution to handle empty output and multiple packages per line, fixing LP bugs #1548406 and #1548404. | |
| #840 Catkin plugin: build with in-snap python. | snapcraft | merged | Merged #840 as 7f9fa3ac1, fixing LP #1628990 by using in-snap Python during Catkin builds and adding gcc, libc-dev, and make as build packages. | |
| #1693380 Transition catkin plugin to use catkin build | snapcraft (launchpad) | closed | Superseded by merged PR canonical/snapcraft#1593 (7e31fa0dd), which added a separate beta catkin-tools plugin using catkin build while preserving catkin_make behavior. | |
| #195 Catkin plugin: Improve cmake path rewrite. | snapcraft | merged | Merged #195 via 0bb02b679 (merge commit 42b5e382c), replacing hard-coded Catkin Config.cmake paths with generic, robust rewrites to the install directory. | |
| #1628990 Catkin plugin: example doesn't build on vanilla install | snapcraft (launchpad) | closed | Fixed by merged PR snapcore/snapcraft#840 (commit 7f9fa3ac1): Catkin builds now use in-snap Python and add gcc, libc-dev and make as build-packages, so the example builds on a vanilla system. Closed by maintainer ~kyrofa. | |
| #1743 catkin plugin: support building entire workspace | snapcraft | merged | Merged #1743 in commit ec4cac26a, making catkin-packages optional so the Catkin plugin builds the entire workspace when omitted; this also resolves snapcraft#1713. | |
| #163 Catkin plugin: use rosdep for dependency resolution | snapcraft | merged | Merged in PR #163 (commit 87b2e24f6), replacing manual Catkin package.xml dependency parsing with rosdep; PR #183 backported the change to Snapcraft 1.x. | |
| #1650628 Update catkin plugin to use get_pull_properties | snapcraft (launchpad) | closed | Merged snapcraft#1001 in commit 561a04341, updating Catkin to use get_pull_properties() instead of modifying the schema and adding a unit test. | |
| #1548406 Catkin plugin: Can't handle rosdep resolving to nothing | snapcraft (launchpad) | closed | Fixed by canonical/snapcraft#331, merged as commit 1c1119ce3; Catkin now treats empty rosdep output as no dependencies instead of an error. |