`espup install` detects and re-uses existing gcc, but always re-installs rust toolchain when it fails to execute it

codyps commented 1 year ago

Bug description

When running espup install after previously having installed the same esp toolchain, espup install reuses (and does not re-download) GCC or LLVM (and prints warnings that it is doing this re-use), but always re-downloads the rust toolchain.

Log of a typical run of espup install when the esp-rs toolchain is already installed.

[nix-shell:~]$ espup install
[2023-09-08T15:56:59Z INFO ] 💽  Installing the Espressif Rust ecosystem
[2023-09-08T15:56:59Z INFO ] 💡  Querying GitHub API: 'https://api.github.com/repos/esp-rs/rust-build/releases/latest'
[2023-09-08T15:57:00Z INFO ] 💡  Querying GitHub API: 'https://api.github.com/repos/esp-rs/rust-build/releases'
[2023-09-08T15:57:01Z INFO ] 🔧  Checking Rust installation
[2023-09-08T15:57:01Z INFO ] 🔧  Installing RISC-V targets ('riscv32imc-unknown-none-elf' and 'riscv32imac-unknown-none-elf') for 'nightly' toolchain
[2023-09-08T15:57:01Z WARN ] ⚠️   Previous installation of GCC exists in: '/home/cody/.rustup/toolchains/esp/xtensa-esp32s2-elf/esp-12.2.0_20230208'. Reusing this installation.
[2023-09-08T15:57:01Z WARN ] ⚠️   Previous installation of GCC exists in: '/home/cody/.rustup/toolchains/esp/xtensa-esp32s3-elf/esp-12.2.0_20230208'. Reusing this installation.
[2023-09-08T15:57:01Z WARN ] ⚠️   Previous installation of GCC exists in: '/home/cody/.rustup/toolchains/esp/riscv32-esp-elf/esp-12.2.0_20230208'. Reusing this installation.
[2023-09-08T15:57:01Z WARN ] ⚠️   Previous installation of LLVM exists in: '/home/cody/.rustup/toolchains/esp/xtensa-esp32-elf-clang/esp-16.0.0-20230516'. Reusing this installation.
[2023-09-08T15:57:01Z WARN ] ⚠️   Previous installation of GCC exists in: '/home/cody/.rustup/toolchains/esp/xtensa-esp32-elf/esp-12.2.0_20230208'. Reusing this installation.
[2023-09-08T15:57:01Z INFO ] 🔧  Uninstalling Xtensa Rust toolchain
[2023-09-08T15:57:01Z INFO ] 🔧  Installing Xtensa Rust 1.72.0.0 toolchain
[2023-09-08T15:57:01Z INFO ] 📥  Downloading file '/home/cody/.rustup/toolchains/esp/.tmpQJCDaO/rust.tar.xz' from 'https://github.com/esp-rs/rust-build/releases/download/v1.72.0.0/rust-1.72.0.0-x86_64-unknown-linux-gnu.tar.xz'
[2023-09-08T15:57:05Z INFO ] 🔧  Uncompressing tar.xz file to '/home/cody/.rustup/toolchains/esp/.tmpQJCDaO'
[2023-09-08T15:57:16Z INFO ] 🔧  Installing 'rust' component for Xtensa Rust toolchain
[2023-09-08T15:57:22Z INFO ] 📥  Downloading file '/home/cody/.rustup/toolchains/esp/.tmpRqAMrB/rust-src.tar.xz' from 'https://github.com/esp-rs/rust-build/releases/download/v1.72.0.0/rust-src-1.72.0.0.tar.xz'
[2023-09-08T15:57:22Z INFO ] 🔧  Uncompressing tar.xz file to '/home/cody/.rustup/toolchains/esp/.tmpRqAMrB'
[2023-09-08T15:57:23Z INFO ] 🔧  Installing 'rust-src' component for Xtensa Rust toolchain
[2023-09-08T15:58:23Z INFO ] 🔧  Creating export file
[2023-09-08T15:58:23Z WARN ] 💡  Please, set up the environment variables by running: '. /home/cody/export-esp.sh'
[2023-09-08T15:58:23Z WARN ] ⚠️   This step must be done every time you open a new terminal.
[2023-09-08T15:58:23Z INFO ] ✅  Installation successfully completed!

To Reproduce

Steps to reproduce the behavior:

espup install at least once
espup install a second time
Note that GCC & llvm aren't re-downloaded and re-installed, but the rust toolchain is

Expected behavior

Re-use the existing rust toolchain if it's already the right version, making re-running espup install a quick operation.

Environment

OS: nixos
espup version: 0.5.1-dev 2ac988bf

SergioGasquez commented 1 year ago

Hi! It should reuse the installation, just tried, and I can't reproduce your issue:

❯ espup install
[2023-09-12T09:15:43Z INFO ] 💽  Installing the Espressif Rust ecosystem
[2023-09-12T09:15:43Z INFO ] 💡  Querying GitHub API: 'https://api.github.com/repos/esp-rs/rust-build/releases/latest'
[2023-09-12T09:15:43Z INFO ] 💡  Querying GitHub API: 'https://api.github.com/repos/esp-rs/rust-build/releases'
[2023-09-12T09:15:44Z INFO ] 🔧  Checking Rust installation
[2023-09-12T09:15:44Z INFO ] 🔧  Installing RISC-V targets ('riscv32imc-unknown-none-elf' and 'riscv32imac-unknown-none-elf') for 'nightly' toolchain
[2023-09-12T09:15:44Z WARN ] ⚠️   Previous installation of GCC exists in: '/home/sergio/.rustup/toolchains/esp/xtensa-esp32s3-elf/esp-12.2.0_20230208'. Reusing this installation.
[2023-09-12T09:15:44Z WARN ] ⚠️   Previous installation of GCC exists in: '/home/sergio/.rustup/toolchains/esp/xtensa-esp32s2-elf/esp-12.2.0_20230208'. Reusing this installation.
[2023-09-12T09:15:44Z WARN ] ⚠️   Previous installation of GCC exists in: '/home/sergio/.rustup/toolchains/esp/xtensa-esp32-elf/esp-12.2.0_20230208'. Reusing this installation.
[2023-09-12T09:15:44Z WARN ] ⚠️   Previous installation of GCC exists in: '/home/sergio/.rustup/toolchains/esp/riscv32-esp-elf/esp-12.2.0_20230208'. Reusing this installation.
[2023-09-12T09:15:44Z WARN ] ⚠️   Previous installation of LLVM exists in: '/home/sergio/.rustup/toolchains/esp/xtensa-esp32-elf-clang/esp-16.0.0-20230516'. Reusing this installation.
[2023-09-12T09:15:44Z WARN ] ⚠️   Previous installation of Xtensa Rust 1.72.0.0 exists in: '/home/sergio/.rustup/toolchains/esp'. Reusing this installation.
[2023-09-12T09:15:45Z INFO ] 🔧  Creating export file
[2023-09-12T09:15:45Z WARN ] 💡  Please, set up the environment variables by running: '. /home/sergio/export-esp.sh'
[2023-09-12T09:15:45Z WARN ] ⚠️   This step must be done every time you open a new terminal.
[2023-09-12T09:15:45Z INFO ] ✅  Installation successfully completed!

I think what happened to you is that you had an older Xtensa Rust version, and when you ran the espup install it updated your Xtensa Rust to the latest version.

SergioGasquez commented 1 year ago

Hello @jmesmon, did you have a chance to verify the behavior?¿

codyps commented 1 year ago

Ya, I took another look, and here's what is happening:

When espup goes to check the current toolchain, it tries to execute the binaries.
In my test system, the binaries failed to execute because I don't have the dynamic linker they're expecting
espup sees that execution failure but doesn't print any error for the user and reinstalls the rust toolchain
repeat.

(the "doesn't have the expected dynamic linker" bit is because this is nixos, which doesn't use the same filesystem layout. They carry patches to fixup binaries rustup downloads. I suspect I'll need to do the same thing for espup as long as it ships executables with a dependency on a dynamic linker in a particular location)

SergioGasquez commented 1 year ago

Do you have any suggestion on how to improve this on espup side? I don't Use Nix/NixOs

codyps commented 1 year ago

Things that could help here:

print a warning/error message when execution of the compiler fails when trying to detect the version
provide a hook in espup to run something (script/executable/etc) to post-process a toolchain after it installs. Should be part of some config/env that can be set for all espup invocations. This would (potentially) let me have something hooked into espup to fixup the dynamic linker path without patching espup source.
ship static executables for the rustc/cargo/other toolchain binaries so that no post-processing of the toolchain is needed.

SergioGasquez commented 1 year ago

Thanks for the suggestions, just created #357 which adds a warn message when failing to detect the version.

ship static executables for the rustc/cargo/other toolchain binaries so that no post-processing of the toolchain is needed.

This should be done in https://github.com/esp-rs/rust-build maybe we can open an issue for this.

LucaFulchir commented 1 year ago

I'm trying to make it work on NixOS, too.

after running espup install I ran a custom script to fix the path of the interpreters and system libraries of all executables under ~/.rustup

espup install will not reinstall things twice.

$ rustup show     
Default host: x86_64-unknown-linux-gnu
rustup home:  /home/luker/.rustup

installed toolchains
--------------------

nightly-x86_64-unknown-linux-gnu (default)
esp

active toolchain
----------------

esp (overridden by '/home/luker/Work/personal/EMM/emm/rust-toolchain.toml')
rustc 1.72.1-nightly (edc37d22d 2023-09-15) (1.72.1.0)

$ rustc -vV  
rustc 1.72.1-nightly (edc37d22d 2023-09-15) (1.72.1.0)
binary: rustc
commit-hash: edc37d22db1cc1b112b91addeb1f79951c58e661
commit-date: 2023-09-15
host: x86_64-unknown-linux-gnu
release: 1.72.1-nightly
LLVM version: 16.0.4

However, running cargo build on the generic template (no-std or std, esp32-s3 or esp32-c3) always terminates with:

error: Package `test v0.0.0 (/home/luker/.rustup/toolchains/esp/lib/rustlib/src/rust/library/test)` does not have the feature `backtrace`

...which does not make much sense to me. But apparently it means I was using cargo from nixos instead of the one installed by espup, because then I tried:

$ export PATH="/home/luker/.rustup/toolchains/esp/bin:$PATH"

And it starts compiling. After a bit I get

error: linker `xtensa-esp32s3-elf-gcc` not found

..which actually already is in my path:

$ which xtensa-esp32s3-elf-gcc     
/home/luker/.rustup/toolchains/esp/xtensa-esp32s3-elf/esp-12.2.0_20230208/xtensa-esp32s3-elf/bin/xtensa-esp32s3-elf-gcc

...and now I'm stuck

LucaFulchir commented 1 year ago

ok, turns out I didn't get all the executables. Once I did, cargo build works. finally.

I can share my script if anyone is interested, but it's a bit of an hack

SergioGasquez commented 1 year ago

ok, turns out I didn't get all the executables. Once I did, cargo build works. finally.

I can share my script if anyone is interested, but it's a bit of an hack

Yes, if you don't mind sharing it I think it might be useful for other users!

SergioGasquez commented 1 year ago

Merged #357 but still might be worth to keep the issue open, if you have any suggestion that makes Nix users the life easier without introducing too much complexity to espup, I'm all ears!

LucaFulchir commented 1 year ago

I copied the script here: https://gist.github.com/LucaFulchir/a4ea1d76045868437e61180f6dcf5c41

it's not the proper "Nix way", but it Works For Me, so good enough for me.

As it stands I guess the proper nix way of fixing this would be to write a wrapper for espup, and if the script is called with install or update commands, find and run the proper patchelf on the binaries. ...But I'm not a package maintainer so I might be wrong.

the wrapper would have to do something like what I do in the script, but should run only in the proper directories to be faster, I didn't bother making it faster.

I might open a bug report on nipkgpks if there isn't one already, this should probably not be handled by espup, unless you want to distribute purely static binaries or bring nix in your workflow

--Edit: Actually it seems that nix patches rustup to run patchelf by itself, and the patch is really small, too: https://github.com/nagy/nixpkgs/blob/nixos-unstable/pkgs/development/tools/rust/rustup/0001-dynamically-patchelf-binaries.patch

I guess espup could integrate something similar, but it would need an extra check to only run patchelf if the system is NixOS

I'm unsure if we should propose to esupup to maintain it, or to the nix package managers

SergioGasquez commented 1 year ago

I copied the script here: https://gist.github.com/LucaFulchir/a4ea1d76045868437e61180f6dcf5c41

it's not the proper "Nix way", but it Works For Me, so good enough for me.

As it stands I guess the proper nix way of fixing this would be to write a wrapper for espup, and if the script is called with install or update commands, find and run the proper patchelf on the binaries. ...But I'm not a package maintainer so I might be wrong.

the wrapper would have to do something like what I do in the script, but should run only in the proper directories to be faster, I didn't bother making it faster.

I might open a bug report on nipkgpks if there isn't one already, this should probably not be handled by espup, unless you want to distribute purely static binaries or bring nix in your workflow

--Edit: Actually it seems that nix patches rustup to run patchelf by itself, and the patch is really small, too: https://github.com/nagy/nixpkgs/blob/nixos-unstable/pkgs/development/tools/rust/rustup/0001-dynamically-patchelf-binaries.patch

I guess espup could integrate something similar, but it would need an extra check to only run patchelf if the system is NixOS

I'm unsure if we should propose to esupup to maintain it, or to the nix package managers

Thanks for sharing your script! To be honest, I've never used Nix or NixOS, so I don't feel comfortable enough to maintain such feature.

i-am-logger commented 7 months ago

note: this is on nixOS

@SergioGasquez do you have any example on how to nix build for Xtensa i'm struggling with it but i think if there is something working i can try push the fix to rustup nix package

this is the error i'm getting:

SergioGasquez commented 7 months ago

note: this is on nixOS

@SergioGasquez do you have any example on how to nix build for Xtensa i'm struggling with it but i think if there is something working i can try push the fix to rustup nix package

this is the error i'm getting:

https://github.com/esp-rs/esp-idf-template/issues/64#issuecomment-1308878695 and https://github.com/esp-rs/esp-idf-sys/issues/184 could help you. I have no experience with NixOS, but I think there are several users on the Matrix channel that have used it, I'll try to gather some more information.

esp-rs / espup