Skip to content

Windows: "called Option::unwrap() on a None value" #3936

Description

@Baiyuetribe

Verification

Problem

windows on Arm64 still not work.

thread 'main' panicked at src\cli\self_update.rs:652:45:
called `Option::unwrap()` on a `None` value
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Steps

reproduce

# my device: windows arm64
git clone https://github.com/rust-lang/rustup.git
cargo build --release

after,i got rustup-init.exe.
image

Possible Solution(s)

No response

Notes

No response

Rustup version

latest

Installed toolchains

none

OS version

windows arm64
### Tasks
- [ ] https://github.com/rust-lang/rustup/pull/3938

Activity

  1. rami3l commented on Jul 11, 2024

    @rami3l
    Member

    @Baiyuetribe Sorry for your bad experience!

    Do you have the HOME environment variable set (https://superuser.com/a/1617017)? Rustup depends on that to work.

  2. rami3l commented on Jul 11, 2024

    @rami3l
  3. ChrisDenton commented on Jul 11, 2024

    @ChrisDenton
    Member

    Do you have the HOME environment variable set (https://superuser.com/a/1617017)? Rustup depends on that to work.

    Wait, why is rustup doing this directly? I thought it used the home crate? Did this change?

  4. ChrisDenton commented on Jul 11, 2024

    @ChrisDenton
    Member

    Needing the HOME environment variable seems like a regression.

  5. rami3l commented on Jul 11, 2024

    @rami3l
    Member

    Do you have the HOME environment variable set (https://superuser.com/a/1617017)? Rustup depends on that to work.

    Wait, why is rustup doing this directly? I thought it used the home crate? Did this change?

    @ChrisDenton Blame: Introduced in 0997247, four years ago.

  6. ChrisDenton commented on Jul 11, 2024

    @ChrisDenton
    Member

    That's super weird because I have installed it within that time without having HOME. I mean, it doesn't exist by default.

  7. ChrisDenton commented on Jul 11, 2024

    @ChrisDenton
    Member

    It seems like we have two home_dir functions that do different things:

    pub(crate) fn home_dir(&self) -> Option<PathBuf> {
    home::env::home_dir_with_env(self)
    }

    impl home::env::Env for Process {
    fn home_dir(&self) -> Option<PathBuf> {
    match self {
    Process::OSProcess(_) => self.var("HOME").ok().map(|v| v.into()),

  8. rami3l commented on Jul 11, 2024

    @rami3l
    Member

    It seems like we have two home_dir functions that do different things:

    pub(crate) fn home_dir(&self) -> Option<PathBuf> {
    home::env::home_dir_with_env(self)
    }

    impl home::env::Env for Process {
    fn home_dir(&self) -> Option<PathBuf> {
    match self {
    Process::OSProcess(_) => self.var("HOME").ok().map(|v| v.into()),

    @ChrisDenton My theory is that the former calls the latter with an indirection, in that home_dir_with_env() accepts a home::env::Env and calls its home_dir():

    /// Returns the path of the current user's home directory from [`Env::home_dir`].
    pub fn home_dir_with_env(env: &dyn Env) -> Option<PathBuf> {
        env.home_dir()
    }
  9. ChrisDenton commented on Jul 11, 2024

    @ChrisDenton
    Member

    Then I'm super confused why this isn't a problem for x86_64.

  10. rami3l commented on Jul 11, 2024

    @rami3l
    Member

    Then I'm super confused why this isn't a problem for x86_64.

    @ChrisDenton I believe it should be, but we shouldn't trust the CI runner because from its POV setting HOME doesn't break anything.

  11. ChrisDenton commented on Jul 11, 2024

    @ChrisDenton
    Member

    Ah, I see. From testing it seems the published x86_64 installer works but not if I build from master. So it seems like this did regress at some point. I'm less confused now. This is a major regression because it means rustup-init is broken by default on Windows.

    It also seems weird that we have two ways of getting rustup home that are implemented completely differently. In some places we use home::rustup_home and in other places it's manually implemented using process.home_dir().unwrap().join(".rustup")

  12. rami3l commented on Jul 11, 2024

    @rami3l
    Member

    Ah, I see. From testing it seems the published x86_64 installer works but not if I build from master. So it seems like this did regress at some point. I'm less confused now. This is a major regression because it means rustup-init is broken by default on Windows.

    @ChrisDenton You mean somewhere between master and the v1.27.1 tag? Could you do a bisect in addition since I don't have a Windows machine near me, many thanks in advance 🙇

    It also seems weird that we have two ways of getting rustup home that are implemented completely differently. In some places we use home::rustup_home and in other places it's manually implemented using process.home_dir().unwrap().join(".rustup")

    That'll be a very good place to improve indeed.

  13. changed the title [-]windows on Arm64 still not work.[/-] [+]Windows: "called `Option::unwrap()` on a `None` value called `Option::unwrap()` on a `None` value"[/+] on Jul 11, 2024
  14. ChrisDenton commented on Jul 11, 2024

    @ChrisDenton
    Member

    Could you do a bisect

    Sure. It regressed in 204c8a9.

    That'll be a very good place to improve indeed.

    I think we should calculate rustup/cargo home once then cache it somewhere. Also those sanity checks look very unix-specific (unix paths and bash scripts). Not sure if they're relevant to Windows. Maybe there should be greater separation between platform specific bits. But that's a bigger job.

  15. rami3l commented on Jul 11, 2024

    @rami3l
    Member

    @ChrisDenton Thanks a lot for helping out!

    Also those sanity checks look very unix-specific (unix paths and bash scripts). Not sure if they're relevant to Windows.

    Rustup used to have runtime detection of OS instead of conditional compilation. The idea of this check is that it should be a no-op on Windows... I've set it as a part of #2424.

    It regressed in 204c8a9.

    cc @djc (Looks like OS_ENV should be used for OSProcess. The two branches are written in the wrong order.)

  16. rami3l commented on Jul 11, 2024

    @rami3l
    Member

    @Baiyuetribe Given @ChrisDenton's analysis, https://win.rustup.rs/aarch64 should work for you without problems.

  17. rami3l commented on Jul 11, 2024

    @rami3l
    Member

    Please note that #3938 does NOT close this.

    We'll still need to:

    Looks like do_pre_install_sanity_checks() dates back from prehistoric times (v0.1.9, 0080271), to defend against the old Rust distribution tools (namely "multirust.sh, rustc tarballs, and rustup.sh").

    Now let's look at the files it verifies:

    • manifest-rustc (without any prefix) is almost certainly gone: I cannot see this on my machine.
    • uninstall.sh is... gone as well? Not quite sure, but a solution like this will definitely not work with Windows.
    • ~/.rustup/rustup-version is not for the Rust version of rustup, but rather its prototype rustup.sh (so there's no point in changing this to rustup_home()!), implemented in shell script, so is Unix only.

    ... so this function serves no purpose, at least not on Windows. My suggestion for now is to revert 0080271 completely. @djc @ChrisDenton what do you think?

  18. Baiyuetribe commented on Jul 11, 2024

    @Baiyuetribe
    Author

    @Baiyuetribe Given @ChrisDenton's analysis, https://win.rustup.rs/aarch64 should work for you without problems.

    thanks,it works 👍🏻

  19. changed the title [-]Windows: "called `Option::unwrap()` on a `None` value called `Option::unwrap()` on a `None` value"[/-] [+]Windows: "called `Option::unwrap()` on a `None` value"[/+] on Jul 11, 2024
  20. ChrisDenton commented on Jul 12, 2024

    @ChrisDenton
    Member

    My suggestion for now is to revert 0080271 completely

    Makes sense to me! I think it served it's purpose in helping migration to rustup in ancient times but I don't think we still need to handle pre-1.0 stuff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions