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

🚩 add flag to only install riscv if user wants #391

Merged
merged 9 commits into from
Nov 8, 2023

Conversation

Vollbrecht
Copy link
Contributor

No description provided.

@Vollbrecht
Copy link
Contributor Author

i tried to test the bin directly in a container, though i got some strange runtime panics i dont fully understand why they happening,,,

Cannot drop a runtime in a context where blocking is not allowed. This happens when a runtime is dropped from within an asynchronous context

stack backtrace:
   0: rust_begin_unwind
             at /rustc/189d6c71f3bb6c52113b5639a80839791974fd22/library/std/src/panicking.rs:645:5
   1: core::panicking::panic_fmt
             at /rustc/189d6c71f3bb6c52113b5639a80839791974fd22/library/core/src/panicking.rs:72:14
   2: tokio::runtime::blocking::shutdown::Receiver::wait
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.33.0/src/runtime/blocking/shutdown.rs:51:21
   3: tokio::runtime::blocking::pool::BlockingPool::shutdown
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.33.0/src/runtime/blocking/pool.rs:261:12
   4: <tokio::runtime::blocking::pool::BlockingPool as core::ops::drop::Drop>::drop
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.33.0/src/runtime/blocking/pool.rs:278:9
   5: core::ptr::drop_in_place<tokio::runtime::blocking::pool::BlockingPool>
             at /rustc/189d6c71f3bb6c52113b5639a80839791974fd22/library/core/src/ptr/mod.rs:498:1
   6: core::ptr::drop_in_place<tokio::runtime::runtime::Runtime>
             at /rustc/189d6c71f3bb6c52113b5639a80839791974fd22/library/core/src/ptr/mod.rs:498:1
   7: reqwest::blocking::wait::enter
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/reqwest-0.11.22/src/blocking/wait.rs:76:21
   8: reqwest::blocking::wait::timeout
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/reqwest-0.11.22/src/blocking/wait.rs:13:5
   9: reqwest::blocking::client::ClientHandle::new
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/reqwest-0.11.22/src/blocking/client.rs:1075:15
  10: reqwest::blocking::client::ClientBuilder::build
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/reqwest-0.11.22/src/blocking/client.rs:103:9
  11: reqwest::blocking::client::Client::new
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/reqwest-0.11.22/src/blocking/client.rs:877:9
  12: espup::toolchain::github_query
             at ./espup/src/toolchain/mod.rs:287:18

Also note to self need to update readme and changelog file

Copy link
Member

@SergioGasquez SergioGasquez left a comment

Choose a reason for hiding this comment

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

Using the suggested changes:

esp@2293cd89985c:~/espup$ cargo r -r -- install
    Finished release [optimized] target(s) in 0.09s
     Running `target/release/espup install`
[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]: Downloading 'rust.tar.xz'
[info]: Downloading 'idf_tool_xtensa_elf_clang.tar.xz'
[info]: Downloading 'xtensa-esp-elf.tar.xz'
[info]: Creating symlink between '/home/esp/.rustup/toolchains/esp/xtensa-esp32-elf-clang/esp-16.0.0-20230516/esp-clang/lib' and '/home/esp/.espup/esp-clang'
[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 need to set up some environment variables by running: '. /home/esp/export-esp.sh'
        This step must be done every time you open a new terminal.
            See other methods for setting the environment in https://esp-rs.github.io/book/installation/riscv-and-xtensa.html#3-set-up-the-environment-variables

Note that no GCC RISC-V is installed, and using the --ulp flag produces:

esp@2293cd89985c:~/espup$ cargo r -r -- install --ulp
    Finished release [optimized] target(s) in 0.09s
     Running `target/release/espup install --ulp`
[info]: Installing the Espressif Rust ecosystem
[info]: Checking Rust installation
LLVM Path: /home/esp/.rustup/toolchains/esp/xtensa-esp32-elf-clang/esp-16.0.0-20230516
[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 (riscv32-esp-elf)
[info]: Installing GCC (xtensa-esp-elf)
[info]: Downloading 'idf_tool_xtensa_elf_clang.tar.xz'
[info]: Downloading 'rust.tar.xz'
[info]: Downloading 'riscv32-esp-elf.tar.xz'
[info]: Downloading 'xtensa-esp-elf.tar.xz'
[info]: Creating symlink between '/home/esp/.rustup/toolchains/esp/xtensa-esp32-elf-clang/esp-16.0.0-20230516/esp-clang/lib' and '/home/esp/.espup/esp-clang'
[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 need to set up some environment variables by running: '. /home/esp/export-esp.sh'
        This step must be done every time you open a new terminal.
            See other methods for setting the environment in https://esp-rs.github.io/book/installation/riscv-and-xtensa.html#3-set-up-the-environment-variables

src/toolchain/mod.rs Outdated Show resolved Hide resolved
src/toolchain/mod.rs Outdated Show resolved Hide resolved
@SergioGasquez
Copy link
Member

SergioGasquez commented Nov 8, 2023

I've been thinking about the --ulp flag, I think it might be misleading. Users might want to install the Espressif RISC-V GCC but not use the ULP coprocessor, maybe they want to use it for an ESP32-C3. Perhaps the one that you proposed is better: --esp-riscv-gcc. At the end, its about installing Espressif RISC-V GCC toolchain or not, not much to do with ULP cores, right?

Sorry for the confusion, during yesterdays chat I was on my phone and not thinking too much about it.

@Vollbrecht
Copy link
Contributor Author

i updated the flag name and run clippy. Still puzzled why it did not work in my container, maybe because i build with native openssl or something?

@SergioGasquez
Copy link
Member

i updated the flag name and run clippy. Still puzzled why it did not work in my container, maybe because i build with native openssl or something?

Are you using the Dockerfile under .devcontainer folder? The tests that I did, and the logs that I've posted were using that devcontainer.

@Vollbrecht
Copy link
Contributor Author

Vollbrecht commented Nov 8, 2023

hmm it seams my problem was that i just moved the compiled binary manual with cargo build && cp target/debug/espup ${HOME}/.cargo/bin/espup && ${HOME}/.cargo/bin/espup --version and after using it in next step it was just not working correclty, running the build command like you did with cargo run was no problem ... hmm.

EDIT: i am a dummy espup is not a standalone bin right? it needs its lib dependency

Copy link
Member

@SergioGasquez SergioGasquez left a comment

Choose a reason for hiding this comment

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

Left some formatting comments.

src/cli.rs Outdated Show resolved Hide resolved
src/toolchain/mod.rs Outdated Show resolved Hide resolved
Vollbrecht and others added 3 commits November 8, 2023 16:13
Co-authored-by: Sergio Gasquez Arcos <sergio.gasquez@gmail.com>
Co-authored-by: Sergio Gasquez Arcos <sergio.gasquez@gmail.com>
Copy link
Member

@SergioGasquez SergioGasquez left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks

@Vollbrecht
Copy link
Contributor Author

Vollbrecht commented Nov 8, 2023

Ty for the review, with this change alone we probably fasten the xtensa-action as its now dll/install less then before

@SergioGasquez SergioGasquez merged commit 7dc66b0 into esp-rs:main Nov 8, 2023
16 checks passed
@SergioGasquez
Copy link
Member

Ty for the review, with this change alone we probably fasten the xtensa-action as its now dll/install less then before

Yes! I will do some further testing tomorrow and cut a release! Kind of unlucky timing since yesterday morning I released 0.8.0

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