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

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

Open
codyps opened this issue Sep 8, 2023 · 14 comments · Fixed by #357
Assignees
Labels
bug Something isn't working

Comments

@codyps
Copy link

codyps commented Sep 8, 2023

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:

  1. espup install at least once
  2. espup install a second time
  3. 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 2ac988b
@codyps codyps added the bug Something isn't working label Sep 8, 2023
@github-project-automation github-project-automation bot moved this to Todo in esp-rs Sep 8, 2023
@SergioGasquez SergioGasquez self-assigned this Sep 12, 2023
@SergioGasquez
Copy link
Member

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 SergioGasquez moved this from Todo to In Progress in esp-rs Sep 13, 2023
@SergioGasquez SergioGasquez moved this from In Progress to Todo in esp-rs Sep 13, 2023
@SergioGasquez
Copy link
Member

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

@codyps
Copy link
Author

codyps commented Sep 26, 2023

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
Copy link
Member

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

@codyps
Copy link
Author

codyps commented Sep 26, 2023

Things that could help here:

  1. print a warning/error message when execution of the compiler fails when trying to detect the version
  2. 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.
  3. ship static executables for the rustc/cargo/other toolchain binaries so that no post-processing of the toolchain is needed.

@SergioGasquez SergioGasquez linked a pull request Sep 27, 2023 that will close this issue
@SergioGasquez
Copy link
Member

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

  1. 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.

@SergioGasquez SergioGasquez moved this from Todo to In Progress in esp-rs Sep 27, 2023
@LucaFulchir
Copy link

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
Copy link

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

@codyps codyps changed the title espup install detects and re-uses existing gcc, but always re-installs rust toolchain espup install detects and re-uses existing gcc, but always re-installs rust toolchain when it fails to execute it Sep 30, 2023
@SergioGasquez
Copy link
Member

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!

@github-project-automation github-project-automation bot moved this from In Progress to Done in esp-rs Oct 2, 2023
@SergioGasquez SergioGasquez reopened this Oct 2, 2023
@github-project-automation github-project-automation bot moved this from Done to In Progress in esp-rs Oct 2, 2023
@SergioGasquez
Copy link
Member

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
Copy link

LucaFulchir commented Oct 2, 2023

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
Copy link
Member

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
Copy link

i-am-logger commented Mar 7, 2024

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:

image

@SergioGasquez
Copy link
Member

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:

image

esp-rs/esp-idf-template#64 (comment) and esp-rs/esp-idf-sys#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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Todo
Development

Successfully merging a pull request may close this issue.

4 participants