Skip to content

refactor(backend): parse tool options per backend#9838

Merged
jdx merged 5 commits into
jdx:mainfrom
risu729:refactor/backend-typed-options
May 15, 2026
Merged

refactor(backend): parse tool options per backend#9838
jdx merged 5 commits into
jdx:mainfrom
risu729:refactor/backend-typed-options

Conversation

@risu729

@risu729 risu729 commented May 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add a shared BackendOptions reader for raw and platform-scoped backend tool options.
  • Add a private typed option wrapper for the unified GitHub/GitLab/Forgejo backend.
  • Move GitHub/GitLab/Forgejo option reads and lockfile option collection behind that wrapper while keeping common install/verify helpers on raw ToolVersionOptions.
  • Keep HTTP, S3, Aqua, and UBI migrations out of this PR; those are split into separate draft follow-ups.
  • Defer Backend::option_specs() metadata changes to a later PR.

Project item

Behavior notes

  • This PR is intended to be a refactor and does not change lockfile/manifest persistence classifications.
  • The refactor includes two small behavior fixes found while centralizing Git backend option reads:
    • Forgejo now shares the same git install-time option filtering as GitHub/GitLab.
    • no_app is now read through target-aware platform option lookup, so platforms.<target>.no_app works for cross-platform lock resolution.

Review

  • Gemini and Greptile AI review suggestions were addressed and their review threads are resolved.
  • Manual review focused on preserving existing string/scalar coercion and platform fallback behavior in the Git backend wrapper.

Verification

  • cargo fmt --all -- --check
  • git diff --check upstream/main...HEAD
  • GitHub CI is pending for fda4e0227 after narrowing the PR scope.

This PR was updated by an AI coding assistant.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request standardizes backend option handling by introducing a BackendOptions wrapper and refactoring the Aqua, GitHub, HTTP, S3, and Ubi backends to use specialized option structs. This change improves code consistency and simplifies option retrieval across different backend implementations. Feedback suggests further improving the BackendOptions utility by adding a platform_bool helper, which would allow the no_app option to be configured on a per-platform basis.

Comment thread src/backend/github.rs Outdated
@greptile-apps

greptile-apps Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR centralizes backend tool-option reads behind a new BackendOptions struct (and a GitBackendOptions adapter for GitHub/GitLab/Forgejo), replacing scattered inline opts.get() / lookup_platform_key* calls across github.rs with typed, named methods. Two small behavior fixes are bundled: Forgejo is now included in install_time_option_keys_for_type, and no_app is now resolved through the target-aware platform lookup so platforms.<target>.no_app is honoured during cross-platform lock resolution.

  • src/backend/options.rs (new): defines BackendOptions<'a> with str, platform_string, platform_string_for_target, platform_string_for_target_without_base, and platform_bool_for_target; the is_truthy helper accepts \"true\" and \"1\", which is slightly more permissive than the old parse::<bool>() but consistent with how other scalar options are handled.
  • src/backend/github.rs: replaces get_api_url, get_filter_bins, and inline option reads with GitBackendOptions methods; unit tests updated to match the new typed signatures.
  • src/backend/mod.rs: adds pub(crate) mod options and extends the Forgejo arm in install_time_option_keys_for_type.

Confidence Score: 5/5

Safe to merge — the change is a mechanical refactor that preserves existing option-reading semantics, with two intentional and clearly scoped behavior fixes.

All changed code paths were traced through the underlying ToolVersionOptions accessors. The typed wrappers faithfully delegate to the same lookup functions as before, and the two documented behavior changes are correct and well-tested. No regressions were found.

No files require special attention.

Important Files Changed

Filename Overview
src/backend/options.rs New shared BackendOptions wrapper; introduces platform_bool_for_target, is_truthy, and target-aware platform lookup helpers — all correctly implemented
src/backend/github.rs Migrates option reads to GitBackendOptions wrapper; removes get_api_url/get_filter_bins helpers; fixes Forgejo missing from install-time key filter and enables cross-platform no_app lookup
src/backend/mod.rs Adds options module and includes Forgejo in install_time_option_keys_for_type alongside GitHub/GitLab

Reviews (5): Last reviewed commit: "refactor(backend): split non-git option ..." | Re-trigger Greptile

Comment thread src/backend/options.rs Outdated
@risu729 risu729 marked this pull request as ready for review May 14, 2026 16:28
@jdx jdx merged commit a814b71 into jdx:main May 15, 2026
32 checks passed
@risu729 risu729 deleted the refactor/backend-typed-options branch May 15, 2026 14:17
mise-en-dev added a commit that referenced this pull request May 16, 2026
### 🐛 Bug Fixes

- **(s3)** support SSO-based AWS profiles by enabling aws-config sso
feature by @Amir-Ahmad in [#9875](#9875)

### 🚜 Refactor

- **(backend)** parse tool options per backend by @risu729 in
[#9838](#9838)

### 📦️ Dependency Updates

- update rust crate sha2 to 0.11 by @renovate[bot] in
[#9885](#9885)
- update ghcr.io/jdx/mise:alpine docker digest to 1a653b5 by
@renovate[bot] in [#9890](#9890)
- update ghcr.io/jdx/mise:rpm docker digest to 7880c74 by @renovate[bot]
in [#9892](#9892)
- update ghcr.io/jdx/mise:deb docker digest to a6fe62d by @renovate[bot]
in [#9891](#9891)
- update rust docker digest to 39d8cb3 by @renovate[bot] in
[#9899](#9899)

### New Contributors

- @Amir-Ahmad made their first contribution in
[#9875](#9875)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants