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

Setup user environment #381

Closed
wants to merge 33 commits into from
Closed

Conversation

SergioGasquez
Copy link
Member

@SergioGasquez SergioGasquez commented Oct 21, 2023

This PR basically copies how rustup handles the user environment modification.

  • Added a --no-modify-env flag that avoids modifying the user environment.
  • In Unix systems, we will try to detect the available shells, and write the source command in the shell rc files.
    • Uninstall command will clear the rc files too.
    • Two exports scripts, .sh and .fish are generated $HOME/.rustup/toolchain/<esp_toolchain>
      • In case the user is using the --no-modify-env flag, he needs to source one of these files.
  • In Windows systems, we will set the proper environment variables, unless the -no-modify-env flag is used. (we already do this)
    • Uninstall command will remove the proper environment variables
    • Two export scripts, .bat and .ps1, are generated under $HOME/.rustup/toolchain/<esp_toolchain>
      • In case the user is using the --no-modify-env flag, he needs to source one of these files.
  • Adjusted the logs to be more like rustup

This would be the usual output of espup install for a Unix system:

[info]: Installing the Espressif Rust ecosystem
[info]: Checking Rust installation
[info]: Installing Xtensa Rust 1.73.0.1 toolchain
[info]: Installing Xtensa LLVM
[info]: Installing RISC-V Rust targets ('riscv32imc-unknown-none-elf' and 'riscv32imac-unknown-none-elf') for 'nightly' toolchain
[info]: Installing GCC (xtensa-esp-elf)
[info]: Installing GCC (riscv32-esp-elf)
[info]: Downloading 'rust.tar.xz'
[info]: Downloading 'xtensa-esp-elf.tar.xz'
[info]: Downloading 'idf_tool_xtensa_elf_clang.tar.xz'
[info]: Downloading 'riscv32-esp-elf.tar.xz'
[info]: Installing 'rust' component for Xtensa Rust toolchain
[info]: Downloading 'rust-src.tar.xz'
[info]: Installing 'rust-src' component for Xtensa Rust toolchain
[info]: Installation successfully completed!
        To get started you may need to restart your current shell.
        To configure your current shell, run:
        '. /home/esp/.rustup/toolchains/esp/env' or '. /home/esp/.rustup/toolchains/esp/env.fish' depending on your shell

@SergioGasquez SergioGasquez marked this pull request as ready for review October 23, 2023 14:24
}
}

set_env_variable("PATH", &path)?;
Copy link

Choose a reason for hiding this comment

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

I have significant concern about adding some specific versions of Xtensa and RISC-V GCC to the global PATH variable. Before IDF has adopted the "export.sh" approach to managing the environment, we got many bug reports which ended up being due to the compiler version mismatch, for example because the user has been using two versions of IDF (and hence to versions of the compiler) in parallel.

Copy link
Member Author

@SergioGasquez SergioGasquez Oct 25, 2023

Choose a reason for hiding this comment

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

Right, there are some scenarios where this could happen, the main scenario being, that we update GCC version.

  • One possible solution to this could be storing GCC in $HOME/.rustup/toolchains/esp/<arch>-esp-elf instead of $HOME/.rustup/toolchains/esp/<arch>-esp-elf/<gcc_version/<arch>-esp-elf
    • This would avoid having old GCC version in the user path
    • Note that since the latest GCC release, Xtensa got unified GCC under xtensa-esp-elf

Also, if we are able to use LLD as linker, which should be at some point soon, we don't need to modify user PATH, as:

  • std already downloads (and uses) GCC when building the project
  • no_std only uses GCC as linker, and it would be replaced by LLD
    In this case, we only need to set up the LIBCLANG_PATH as it's required by std projects.

Copy link
Member Author

Choose a reason for hiding this comment

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

Just a quick note, if we remove the <gcc_version> from the installation path, we should check the version when installing (eg using <target>-gcc --version)

src/toolchain/llvm.rs Outdated Show resolved Hide resolved
@SergioGasquez SergioGasquez marked this pull request as draft December 29, 2023 11:48
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