Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(doctor): identify missing/extra shims #1524

Merged
merged 1 commit into from
Jan 25, 2024

Conversation

Ajpantuso
Copy link
Contributor

Summary

Closes #1511

These changes enable mise doctor to identify missing or extra shims and will request users to run mise reshim if needed.

Additional Notes

  • Quite a bit of refactoring here really just to make the changes more obvious.
  • Uses checks rather than a warning, but can be trivially updated to warnings if desired

src/cli/doctor.rs Show resolved Hide resolved
@Ajpantuso Ajpantuso force-pushed the apantuso/doctor_prescribe_reshim branch from 1fb05b0 to 2fc8e38 Compare January 25, 2024 16:12
src/cli/doctor.rs Show resolved Hide resolved
src/cli/doctor.rs Outdated Show resolved Hide resolved
src/cli/doctor.rs Outdated Show resolved Hide resolved
src/cli/doctor.rs Outdated Show resolved Hide resolved
@Ajpantuso Ajpantuso force-pushed the apantuso/doctor_prescribe_reshim branch from 2fc8e38 to 8d98224 Compare January 25, 2024 16:39
@Ajpantuso Ajpantuso force-pushed the apantuso/doctor_prescribe_reshim branch from 8d98224 to 6db79cb Compare January 25, 2024 16:40

let shims_to_add = shims.difference(&existing_shims);
let shims_to_remove = existing_shims.difference(&shims);
let (shims_to_add, shims_to_remove) = get_shim_diffs(&mise_bin, ts)?;
Copy link
Owner

Choose a reason for hiding this comment

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

I was sure this would need to be refactored, but it looks like it wasn't too bad. I actually refactored this at one point to use the set difference and I think that paid off. Before it just did the calculation and symlink edits at the same time.

fn list_tool_bins(t: Arc<dyn Forge>, tv: &ToolVersion) -> Result<Vec<String>> {
Ok(t.list_bin_paths(tv)?
.into_iter()
.par_bridge()
Copy link
Owner

@jdx jdx Jan 25, 2024

Choose a reason for hiding this comment

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

I do wonder if using parallelization is actually worth it for stuff like this. I think maybe if someone had like 100 tools (and I know some people do), it might help.

That said, when I have tried to benchmark stuff like this in the past with rust it's basically impossible to tell the difference at least with hyperfine. I think to really find out we'd have to have isolated benchmarks.

@jdx jdx merged commit 0737239 into jdx:main Jan 25, 2024
7 checks passed
@jdx
Copy link
Owner

jdx commented Jan 25, 2024

also, keep in the back of your mind other potential checks we might be able to add here

@Ajpantuso Ajpantuso deleted the apantuso/doctor_prescribe_reshim branch January 25, 2024 16:54
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.

doctor: warn if shims are missing
2 participants