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

update devcontainer to bookworm container using rust-nightly #170

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Vollbrecht
Copy link
Collaborator

supersedes #169

@SergioGasquez
Copy link
Member

What about using idf-rust tags, which are generated in this workflow from this container file. The container file could probably be improved, and IIRC, you did some work on that a while ago? We could pick that work, finish it and leverage it here too

@Vollbrecht
Copy link
Collaborator Author

Vollbrecht commented Nov 4, 2023

What about using idf-rust tags, which are generated in this workflow from this container file.

If i understand you correctly you want to (depending on the tag) just link to an existing build image from the repo? yeah we may should do this.

As you see i already only include the xtensa toolchain if riscv is not set in this. Also i removed espflash from the container and some libusb dep's that really only needed if one would attempt to forward the usb-connection.

I think its good to switch from base bookworm to the rust-lang nightly image. Your opinion here?

I also thinking about adding an unrelated thing, but useful for guys using the container to put the compiled binary into a different outdir with this option inside the config.toml

[build]
.....
out-dir = "bin" # copy build artifacts to bin/

[unstable] 
unstable-options = true # for out-dir usage

So if one wants to flash from hosts its simpler to locate the elf file

@Vollbrecht
Copy link
Collaborator Author

Vollbrecht commented Nov 5, 2023

the container don't build on ci because the action itself sources the source /home/esp/export-esp.sh. I could adapt the action with a simple if / else, but i think i will look into the published idf-rust containers first

@Vollbrecht
Copy link
Collaborator Author

for now i created esp-rs/rust-build#247 where we can coordinate potential improvements for all projects

rm "${HOME}/.cargo/bin/web-flash.zip" && \
chmod u+x "${HOME}/.cargo/bin/web-flash"
RUN ARCH=$(${RUST_INSTALL_DIR}/bin/rustup show | grep "Default host" | sed -e 's/.* //') && \
{% if arch != "riscv" -%}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{% if arch != "riscv" -%}
{% if arch == "xtensa" -%}

Comment on lines -35 to -41
curl -L "https://github.com/esp-rs/espflash/releases/latest/download/cargo-espflash-${ARCH}.zip" -o "${HOME}/.cargo/bin/cargo-espflash.zip" && \
unzip "${HOME}/.cargo/bin/cargo-espflash.zip" -d "${HOME}/.cargo/bin/" && \
rm "${HOME}/.cargo/bin/cargo-espflash.zip" && \
chmod u+x "${HOME}/.cargo/bin/cargo-espflash" && \
curl -L "https://github.com/esp-rs/embuild/releases/latest/download/ldproxy-${ARCH}.zip" -o "${HOME}/.cargo/bin/ldproxy.zip" && \
unzip "${HOME}/.cargo/bin/ldproxy.zip" -d "${HOME}/.cargo/bin/" && \
rm "${HOME}/.cargo/bin/ldproxy.zip" && \
Copy link
Member

Choose a reason for hiding this comment

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

I see some value on having espflash or cargo-espflash installed, they offer subcomands like save-image that can be really helpful even in a container, and also the user would like to forward the usb-connection and flash from the container as you mentioned.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah i am on board with it, i just stripped them in here with the mind of a potential container that is often build. If we follow that what i wrote on esp-rs/rust-lang issue i think it should be all included in both the all and the target specific images

@SergioGasquez
Copy link
Member

I think its good to switch from base bookworm to the rust-lang nightly image. Your opinion here?

There is even a nightly-bookworm-slim which is 240 MB smaller, we could also evaluate it.

Regarding setting the output to bin I dont have an strong opinion, to be honest. Im used to locate the elf files under the target folder, but I might be a bit biased. Also, espflash/espfhash can be very handy here.

@Vollbrecht
Copy link
Collaborator Author

I think its good to switch from base bookworm to the rust-lang nightly image. Your opinion here?

There is even a nightly-bookworm-slim which is 240 MB smaller, we could also evaluate it.

this should be the same image just from docker-hub and not from ghcr ?

Regarding setting the output to bin I dont have an strong opinion, to be honest. Im used to locate the elf files under the target folder, but I might be a bit biased. Also, espflash/espfhash can be very handy here.

Does espflash correctly autofinds debug/release elf on no_std projects? for my projects i always need to give it the path to the elf

@SergioGasquez
Copy link
Member

this should be the same image just from docker-hub and not from ghcr ?

Just checked and its also available in GitHub registry: https://github.com/rust-lang/docker-rust-nightly/pkgs/container/rust/144487152?tag=nightly-bookworm-slim

Does espflash correctly autofinds debug/release elf on no_std projects? for my projects i always need to give it the path to the elf

No, I meant it for the save-image subcomand

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