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

Generated rustfmt/cargo-fmt shims may not work? #1305

Closed
alexcrichton opened this issue Dec 8, 2017 · 11 comments
Closed

Generated rustfmt/cargo-fmt shims may not work? #1305

alexcrichton opened this issue Dec 8, 2017 · 11 comments

Comments

@alexcrichton
Copy link
Member

@alexcrichton alexcrichton commented Dec 8, 2017

I got rustup to self-update to 1.8.0 (from the dev archives) and correctly got:

warning: tool `rustfmt` is already installed, remove it from `/home/alex/.cargo/bin`, then run `rustup update` to have rustup manage this tool.
warning: tool `cargo-fmt` is already installed, remove it from `/home/alex/.cargo/bin`, then run `rustup update` to have rustup manage this tool.

I then ran cargo uninstall rustfmt, which removed the bins, but no amount of rustup update seems to be causing the files to come into existence.

I think that these tools only come into existence during a self update?

cc @nrc

@nrc
Copy link
Member

@nrc nrc commented Dec 11, 2017

Yeah, dammit, we require a self update, but running rustup update won't do that. I wonder if it is enough to change the warning message to recommend rustup self update rather than rustup update or whether since there is nothing to update, it will skip the self update any way?

@alexcrichton
Copy link
Member Author

@alexcrichton alexcrichton commented Dec 11, 2017

Yeah I think I tried the self update and that didn't work as well (as it didn't do anything :( )

@SimonSapin
Copy link
Contributor

@SimonSapin SimonSapin commented Dec 14, 2017

I have a patch that I think fixes this, but I didn’t manage to write a test that fails on master. This passes:

diff --git a/tests/cli-self-upd.rs b/tests/cli-self-upd.rs
index c8bc9c4b..3a24f93a 100644
--- a/tests/cli-self-upd.rs
+++ b/tests/cli-self-upd.rs
@@ -1273,5 +1273,12 @@ fn update_does_not_overwrite_rustfmt() {
                          "`rustfmt` is already installed");
         assert!(rustfmt_path.exists());
         assert_eq!(utils::file_size(rustfmt_path).unwrap(), 0);
+
+        // Remove the file as suggested by the error "already installed" error message.
+        utils::remove_file("rustfmt", rustfmt_path).unwrap();
+        assert!(!rustfmt_path.exists());
+
+        expect_ok(config, &["rustup", "self", "update"]);
+        assert!(rustfmt_path.exists());
     });
 }
@nrc
Copy link
Member

@nrc nrc commented Dec 14, 2017

@SimonSapin did you confirm that doing rustup self update fails in real life? I would expect that test to fail

@SimonSapin
Copy link
Contributor

@SimonSapin SimonSapin commented Dec 14, 2017

“Real” rustup self update does nothing when the current version is the latest. Are the tests not running the same binary?

@SimonSapin
Copy link
Contributor

@SimonSapin SimonSapin commented Dec 14, 2017

#1310 hopefully fixes this.

SimonSapin added a commit to SimonSapin/rustup.rs that referenced this issue Dec 16, 2017
SimonSapin added a commit to SimonSapin/rustup.rs that referenced this issue Dec 16, 2017
@djc
Copy link

@djc djc commented Jan 9, 2018

This bug still seems to be there (I ran into this twice over the past few weeks, on different machines) with rustup-1.9.0, is that expected?

@boxofrox
Copy link

@boxofrox boxofrox commented Feb 16, 2018

I can confirm rustup self update fixed the missing rustfmt shims in my situation.

Terminal commands I ran to update rustc stable and fix broken rustfmt shims (click to expand)
$ rustup self update
info: checking for self-updates
info: downloading self-update
info: rustup updated successfully to 1.11.0
warning: tool `rustfmt` is already installed, remove it from `/home/boxofrox/.cargo/bin`, then run `rustup update` to have rustup manage this tool.
warning: tool `cargo-fmt` is already installed, remove it from `/home/boxofrox/.cargo/bin`, then run `rustup update` to have rustup manage this tool.

$ rustup update stable
info: syncing channel updates for 'stable-x86_64-unknown-linux-gnu'
info: latest update on 2018-02-15, rust version 1.24.0 (4d90ac38c 2018-02-12)
info: downloading component 'rustc'
 47.8 MiB /  47.8 MiB (100 %)  16.6 MiB/s ETA:   0 s
info: downloading component 'rust-std'
 67.2 MiB /  67.2 MiB (100 %)  18.8 MiB/s ETA:   0 s
info: downloading component 'cargo'
info: downloading component 'rust-docs'
  4.6 MiB /   4.6 MiB (100 %)   3.5 MiB/s ETA:   0 s
info: downloading component 'rust-src'
info: removing component 'rustc'
info: removing component 'rust-std'
info: removing component 'cargo'
info: removing component 'rust-docs'
info: removing component 'rust-src'
info: installing component 'rustc'
info: installing component 'rust-std'
info: installing component 'cargo'
info: installing component 'rust-docs'
info: installing component 'rust-src'

  stable-x86_64-unknown-linux-gnu updated - rustc 1.24.0 (4d90ac38c 2018-02-12)

$ cargo uninstall rustfmt-nightly
    Removing /home/boxofrox/.cargo/bin/cargo-fmt
    Removing /home/boxofrox/.cargo/bin/git-rustfmt
    Removing /home/boxofrox/.cargo/bin/rustfmt
    Removing /home/boxofrox/.cargo/bin/rustfmt-format-diff

$ rustup component add rustfmt-preview
info: downloading component 'rustfmt-preview'
info: installing component 'rustfmt-preview'

$ which rustfmt
rustfmt not found

$ rustup self update
info: checking for self-updates

$ which rustfmt
/home/boxofrox/.cargo/bin/rustfmt

@nrc
Copy link
Member

@nrc nrc commented Feb 16, 2018

I assume that if you have a really old version of rustup and don't rustup self update you won't get the fix and thus will still have problems, but that should be fixed by self-updating.

mattico added a commit to mattico/rustup.rs that referenced this issue Apr 5, 2018
@Diggsey Diggsey added this to Proxying & telemetry in Issue Categorisation May 30, 2018
@a2bb892
Copy link

@a2bb892 a2bb892 commented Oct 1, 2019

I also ran into this same issue using rustup 1.19.0 (2af131cf9 2019-09-08). After doing rustup update:

warning: tool `rustfmt` is already installed, remove it from `/home/shaun/.cargo/bin`, then run `rustup update` to have rustup manage this tool.
warning: tool `cargo-fmt` is already installed, remove it from `/home/shaun/.cargo/bin`, then run `rustup update` to have rustup manage this tool.
$ rm /home/shaun/.cargo/bin/rustfmt
$ rm /home/shaun/.cargo/bin/cargo-fmt

$ rustup update
info: syncing channel updates for 'stable-x86_64-unknown-linux-gnu'
info: checking for self-updates

  stable-x86_64-unknown-linux-gnu unchanged - rustc 1.38.0 (625451e37 2019-09-23)

$ rustfmt
error: 'rustfmt' is not installed for the toolchain 'stable-x86_64-unknown-linux-gnu'
To install, run `rustup component add rustfmt --toolchain stable-x86_64-unknown-linux-gnu`

$ rustup component add rustfmt --toolchain stable-x86_64-unknown-linux-gnu
info: downloading component 'rustfmt'
info: installing component 'rustfmt'

I also tried removing rustfmt again, and did rustup self update and that also fixed it, although the only output that command gave was:

info: checking for self-updates 
@kinnison
Copy link
Collaborator

@kinnison kinnison commented Oct 29, 2019

I think this is resolved by the self-update behaviour described above. As such, I'm closing this issue. If problems remain then please file new issues describing them.

@kinnison kinnison closed this Oct 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Issue Categorisation
Proxying & telemetry
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants