Skip to content

chore: silence platform-conditional dead-code warnings#10425

Merged
jdx merged 1 commit into
jdx:mainfrom
JamBalaya56562:chore-platform-dead-code-warnings
Jun 14, 2026
Merged

chore: silence platform-conditional dead-code warnings#10425
jdx merged 1 commit into
jdx:mainfrom
JamBalaya56562:chore-platform-dead-code-warnings

Conversation

@JamBalaya56562

@JamBalaya56562 JamBalaya56562 commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Summary

Building the mise binary on Windows surfaced a handful of dead_code / unused_import warnings (one was also firing on macOS) coming from code that is only ever used on a subset of platforms. This PR gates each item to the platform(s) that actually use it, so the default cargo check / cargo build is warning-free on every target.

No behavior change — every item is still compiled on the platform that uses it; only the copies that are unused on a given target are removed.

Changes

File Fix Why
src/system/launchd.rs #[cfg(unix)] on current_uid_from + its test only called by the #[cfg(unix)] current_uid() (Windows uses a cfg(not(unix)) fallback that returns 0)
src/ui/progress_report.rs #[cfg_attr(not(unix), allow(dead_code))] on ProgressIcon::Error the variant is matched in Display/finish_with_icon on all platforms, so it cannot be cfg-gated away, but it is only constructed by the #[cfg(unix)] brew package managers
src/oci/layer.rs #[cfg(unix)] on assert_layer_mode / assert_layer_entry_owner test helpers used only by #[cfg(unix)] permission-bit tests
src/plugins/core/erlang.rs #[cfg(linux)] on linux_precompiled_cache_name only called from the #[cfg(linux)] install_precompiled (also clears a pre-existing macOS warning, since the caller was already linux-gated)
src/config/config_file/mise_toml.rs #[cfg(unix)] on update_bootstrap_brew_tap / remove_bootstrap_brew_tap only called from the #[cfg(unix)] mise system brew tap/untap CLI
src/backend/pkgx.rs #[cfg(any(unix, test))] on shell_quote used by the #[cfg(unix)] wrapper writer and by a non-gated unit test (so test is needed for the Windows test build)
src/cmd.rs drop the redundant use std::os::windows::process::CommandExt self.cmd is a tokio::process::Command, whose raw_arg is an inherent method, so the trait import is unnecessary (hooks.rs/path.rs keep theirs because they use std::process::Command)

Verification

  • cargo fmt --all -- --check
  • cargo check --all-targets on Windows (msvc): 0 warnings, 0 errors (previously 8 warnings) ✅
  • Each cfg gate was checked against every call site to confirm it does not exclude an item that any platform still uses.

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Improved cross-platform compatibility by restricting Unix-specific helpers and behaviors so Windows builds no longer include unnecessary Unix-only logic.
  • Chores

    • Further refined conditional compilation for platform-specific utilities and test helpers (Linux/Unix/Windows) to keep builds clean and consistent.
  • Documentation

    • Updated command documentation to reflect how raw argument handling is provided by the command interface.

@coderabbitai

coderabbitai Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c61af9e3-7ec8-419b-9e23-73d4f7f723fb

📥 Commits

Reviewing files that changed from the base of the PR and between fa3ccf1 and 42d34e6.

📒 Files selected for processing (7)
  • src/backend/pkgx.rs
  • src/cmd.rs
  • src/config/config_file/mise_toml.rs
  • src/oci/layer.rs
  • src/plugins/core/erlang.rs
  • src/system/launchd.rs
  • src/ui/progress_report.rs
🚧 Files skipped from review as they are similar to previous changes (6)
  • src/backend/pkgx.rs
  • src/oci/layer.rs
  • src/system/launchd.rs
  • src/cmd.rs
  • src/plugins/core/erlang.rs
  • src/config/config_file/mise_toml.rs

📝 Walkthrough

Walkthrough

Adds #[cfg(unix)], #[cfg(any(unix, test))], and #[cfg(linux)] compile-time gates to several runtime helpers, struct methods, a lint attribute, a trait import, and test helpers across seven files, preventing platform-specific code from being compiled on non-Unix targets such as Windows.

Changes

Platform cfg gating

Layer / File(s) Summary
Runtime and test platform gates
src/backend/pkgx.rs, src/plugins/core/erlang.rs, src/system/launchd.rs, src/config/config_file/mise_toml.rs, src/oci/layer.rs
shell_quote is gated to #[cfg(any(unix, test))]; linux_precompiled_cache_name to #[cfg(linux)]; current_uid_from to #[cfg(unix)]; both MiseToml brew-tap methods to #[cfg(unix)]. Test helpers assert_layer_mode, assert_layer_entry_owner, and test test_current_uid_prefers_sudo_uid_for_root are gated to #[cfg(unix)].
Lint attribute and import cleanup
src/ui/progress_report.rs, src/cmd.rs
ProgressIcon::Warning's #[allow(dead_code)] becomes #[cfg_attr(windows, allow(dead_code))] with explanatory comments; the CommandExt trait import in cmd.rs is removed with updated doc comment noting tokio provides raw_arg as an inherent method.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐇 A cfg here, a cfg there,
Windows won't compile what Unix should bear.
Shell quote, brew tap, and erlang's cache name —
Gated with care, no dead-code to blame.
The rabbit hops on, builds tidy and clean! 🌿

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main purpose of the changeset: silencing platform-conditional dead-code warnings across multiple source files through strategic use of configuration attributes.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps

greptile-apps Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds platform-conditional #[cfg(...)] and #[cfg_attr(...)] gates to silence dead_code/unused_import warnings that appeared when building on Windows (and in one case macOS), with no behavior change on any platform.

  • Seven targeted fixes across six files: unix-only helpers in launchd.rs, mise_toml.rs, and oci/layer.rs get #[cfg(unix)]; the linux-only linux_precompiled_cache_name in erlang.rs gets #[cfg(linux)] (a build.rs alias for target_os = "linux"); ProgressIcon::Error gets #[cfg_attr(windows, allow(dead_code))] since it must stay in the enum for exhaustive matching; and shell_quote in pkgx.rs gets #[cfg(any(unix, test))] so Windows test builds can still compile it.
  • The redundant use std::os::windows::process::CommandExt import in cmd.rs is dropped because tokio::process::Command::raw_arg is already an inherent method.

Confidence Score: 5/5

Safe to merge — purely additive cfg gates with no logic changes and verified call-site coverage on every target.

Every change is a mechanical cfg annotation that exactly mirrors the platform scope of the item's only call sites. The build.rs confirms cfg(linux) is a valid alias. No runtime paths, data structures, or public APIs are altered.

No files require special attention.

Important Files Changed

Filename Overview
src/backend/pkgx.rs Added #[cfg(any(unix, test))] to shell_quote so it compiles on unix (where it's called) and in tests (Windows test builds), eliminating the dead_code warning on non-unix targets.
src/cmd.rs Removed redundant use std::os::windows::process::CommandExt import from raw_arg; tokio::process::Command exposes raw_arg as an inherent method so the trait is unnecessary.
src/config/config_file/mise_toml.rs Added #[cfg(unix)] to update_bootstrap_brew_tap and remove_bootstrap_brew_tap; both methods are only called from unix-gated brew CLI commands.
src/oci/layer.rs Added #[cfg(unix)] to test-helper functions assert_layer_mode and assert_layer_entry_owner, matching the unix-gated tests that consume them.
src/plugins/core/erlang.rs Added #[cfg(linux)] (a valid build.rs alias for target_os = "linux") to linux_precompiled_cache_name, matching its sole caller install_precompiled which was already linux-gated.
src/system/launchd.rs Added #[cfg(unix)] to current_uid_from and its test, matching the unix branch of current_uid() that calls it.
src/ui/progress_report.rs Added #[cfg_attr(windows, allow(dead_code))] to ProgressIcon::Error; the variant must remain in the enum for exhaustive match arms on all platforms, but is only constructed by unix-only brew package managers.

Reviews (2): Last reviewed commit: "chore: gate platform-conditional code to..." | Re-trigger Greptile

Comment thread src/ui/progress_report.rs Outdated
Warning,
// Constructed only by the brew package managers (`#[cfg(unix)]`), so it reads
// as never-constructed on non-unix builds.
#[cfg_attr(not(unix), allow(dead_code))]

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

i think it probably should be windows instead of not(unix)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good call — switched to #[cfg_attr(windows, allow(dead_code))]. mise's only non-unix target is windows, so this is equivalent here but more explicit about where the variant goes unconstructed. Updated in 42d34e6.

@jdx

jdx commented Jun 14, 2026

Copy link
Copy Markdown
Owner

if you don't verify this in ci it's just going to break again

Building the `mise` binary on Windows (and one item on macOS) surfaced
several dead_code / unused-import warnings from unix/linux-only helpers,
a test-only enum construction, and a redundant trait import. Gate each
item to the platform(s) that actually use it so the default
`cargo check`/`cargo build` is warning-free on every target.

- src/system/launchd.rs: cfg(unix) on current_uid_from + its test
- src/ui/progress_report.rs: allow(dead_code) on ProgressIcon::Error for
  non-unix (matched everywhere, but constructed only by the unix brew
  package managers)
- src/oci/layer.rs: cfg(unix) on the assert_layer_mode /
  assert_layer_entry_owner test helpers
- src/plugins/core/erlang.rs: cfg(linux) on linux_precompiled_cache_name
  (also clears a pre-existing macOS dead_code warning)
- src/config/config_file/mise_toml.rs: cfg(unix) on
  update_bootstrap_brew_tap / remove_bootstrap_brew_tap
- src/backend/pkgx.rs: cfg(any(unix, test)) on shell_quote (used by the
  unix wrapper writer and by a non-gated unit test)
- src/cmd.rs: drop the redundant CommandExt import (tokio's
  Command::raw_arg is an inherent method)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@JamBalaya56562 JamBalaya56562 force-pushed the chore-platform-dead-code-warnings branch from fa3ccf1 to 42d34e6 Compare June 14, 2026 07:49
@JamBalaya56562 JamBalaya56562 requested a review from jdx June 14, 2026 07:55
@jdx jdx merged commit f39f973 into jdx:main Jun 14, 2026
33 checks passed
@JamBalaya56562 JamBalaya56562 deleted the chore-platform-dead-code-warnings branch June 14, 2026 09:11
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