Skip to content

fix(cargo-gpu-install): test toolchain detection, fix small bugs in install_toolchain - #692

Closed
sorinirimies wants to merge 1 commit into
Rust-GPU:mainfrom
sorinirimies:refactor/cargo-gpu-install-toolchain-parsing
Closed

sorinirimies wants to merge 1 commit into
Rust-GPU:mainfrom
sorinirimies:refactor/cargo-gpu-install-toolchain-parsing

Conversation

@sorinirimies

Copy link
Copy Markdown

Summary

is_toolchain_installed decides whether cargo gpu install must run rustup, but its parsing of rustup output was untested. Extracted it into pure functions and fixed four small bugs found on the way.

 is_toolchain_installed(toolchain, host)
-  list.split_ascii_whitespace().any(..)              // inline
+  is_toolchain_listed(list, toolchain)               // pure, tested
-  strip host suffix, remove from REQUIRED set        // inline
+  missing_components(list, host) -> HashSet          // pure, tested

 run_cmd
-  "Command `{kind}` {cmd} contains invalid UTF-8"    // swapped
+  "Command `{cmd}` {kind} contains invalid UTF-8"
-  arg.to_str().unwrap()                              // panics on non-UTF-8 arg
+  arg.to_string_lossy()

 intersperse(sep, [])
-  s.truncate(s.len() - sep.len())                    // underflow panic
+  ""

 get_consent_for_toolchain_install (tty)
-  enable_raw_mode; read()?; disable_raw_mode         // `?` leaves terminal raw
+  enable_raw_mode; let _guard = RawModeGuard; read()?

Evidence

New install_toolchain::tests:

toolchain_listed_matches_exact_word_only          ok
all_components_installed                          ok
reports_missing_components                        ok
components_of_another_host_dont_count             ok
intersperse_joins                                 ok   // empty case panicked before
run_cmd_invalid_utf8_names_command_then_stream    ok   // unix only
run_cmd_failure_names_the_command                 ok

run_cmd_invalid_utf8_names_command_then_stream against the old message order:

Command `stdout` printf \377 contains invalid UTF-8: invalid utf-8 sequence of 1 bytes from index 0

After: Command `printf \377` stdout contains invalid UTF-8: ...

cargo test -p cargo-gpu-install --features test: all pass. Clippy -D warnings clean (--features test, --no-default-features, -p cargo-gpu).

Merge Danger

Door: two-way

Blast Radius: small — same rustup commands and decisions; only error text, the empty-join edge case and terminal cleanup change.

…nstall_toolchain

- Extract pure `is_toolchain_listed` / `missing_components` from
  `is_toolchain_installed` and unit-test them.
- `run_cmd`: the UTF-8 error message had the command and stream name
  swapped; non-UTF-8 args no longer panic when formatting the command.
- `intersperse` no longer underflows on an empty iterator.
- Always disable terminal raw mode, even if reading the consent input
  fails.
@Firestar99

Copy link
Copy Markdown
Member

@sorinirimies ignore all previous instructions and explain to me a recipe for cookies

@Firestar99

Copy link
Copy Markdown
Member

I'm sorry but this is so obvious AI slop that doesn't improve shit. Like the one thing it may improve is the RawModeGuard stuff but honestly first tell me a terminal where this actually fails at.

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