reactive plugin charm tools version validation code is britle
Metadata
Current evaluation
Fixed by merged PR #866: commit 35ec089e updated reactive-plugin charm-tools validation to use JSON output; merge c40c30c2 closed #864.
Resolution: closed resolved
Reason: PR #866 was merged into main as c40c30c2, with resolving commit 35ec089e updating the reactive plugin to consume charm-tools JSON version output instead of relying on newline parsing. This directly addressed the brittleness reported in #864.
Evaluated by: mr-cal-bot
No scores available.
Issue body
The charm tools may soon remove the now redundant legacy charm store code, when that happens the output from the version command may no longer use the format expected here:
https://github.com/canonical/charmcraft/blob/dbf82a0fbaa6633248bb213d3943603c1e7c50a6/charmcraft/reactive_plugin.py#L64-L65
I would recommend either using the `--format json` argument or looking for the `charm-tools` string in the output rather than relying on there being a newline character there.
Ref: https://github.com/juju/charm-tools/blob/17812f0e00ad634f8440053d43d1a5370a71fa50/charmtools/version.py#L15-L25
Evaluation history
| Date | Model | Scores | Action | Summary |
|---|---|---|---|---|
| stealth/space-bunny-alpha | — | closed resolved | Fixed by merged PR #866: commit 35ec089e updated reactive-plugin charm-tools validation to use JSON output; merge c40c30c2 closed #864. | |
| stealth/space-bunny-alpha | — | closed resolved | Resolved by merged PR #866 (35ec089e), which updated the reactive plugin to use charm-tools' JSON version output instead of brittle newline parsing; merge c40c30c2 closed #864. | |
| stealth/space-bunny-alpha | — | closed resolved | Merged PR #866 in commit 35ec089e, replacing newline-dependent charm-tools version validation with robust checks. | |
| qwen3.6-35b-a3b-mtp-q6 | — | — | Closed without resolution. The reactive plugin's brittle charm-tools version validation was flagged for potential breakage. Suggestions to use JSON format or string matching were not implemented. No comments or fixes were applied, indicating the issue was abandoned. | |
| qwen/qwen3.6-35b-a3b | — | — | Reactive plugin version validation relied on a fragile charm-tools output format. Recommended using JSON format or string matching. Closed without comments or resolution, indicating the report was abandoned. | |
| qwen3.6-35b-a3b-mtp-q6 | — | — | Closed without action. The report flagged brittle version validation in the reactive plugin relying on a specific charm tools output format. Recommended using JSON formatting or string matching instead. No resolution was applied. |
Update history
No update history recorded yet.
Related work
-
Likely Fixed By:
canonical/charmcraft#866
(confidence 100%)
Merged PR #866 contains the fix in commit 35ec089e and was merged as c40c30c2.
Related issues
| Issue | Project | State | Summary | Similarity |
|---|---|---|---|---|
| #866 reactive_plugin: Fix charm tools version validation | charmcraft | merged | Merged PR #866 as commit 35ec089e (merge c40c30c2), replacing brittle charm-tools version parsing with JSON output and fixing #864. | |
| #613 plugins: reactive using charm tool (CRAFT-699) | charmcraft | merged | Merged PR #613 (merge commit 32c8650a, branch charm-tools-plugin) added the reactive charm plugin built on the charm tool, implementing CRAFT-699; 11 files, +499/-8. | |
| #869 reactive_plugin: Properly check returncode from charm tool | charmcraft | merged | Merged in commit 7046f61d (merge ecce30c6), fixing reactive_plugin to validate the charm tool return code and always log it; resolves #804. | |
| #610 many: add reactive charm plugin | charmcraft | closed | PR #610 (charm-tools-based reactive plugin) was closed unmerged on 2021-12-14 by sergiusens in favor of #613, which reimplemented the plugin on the charm tool (CRAFT-699) and merged as 32c8650a. | |
| #804 The reactive plugin is not correctly checking if `charm` failed | charmcraft | closed | Fixed in merged PR #869 (commit 7046f61d), adding negative-return-code handling and return-code logging for both charm invocations. | |
| #1326 fix(test): fix reactive plugin test if 'charm' tool is installed | charmcraft | merged | Merged test-only fix making reactive plugin tests deterministic when the charm executable is installed, via PR #1326 and commit 4a573702. |