-
Notifications
You must be signed in to change notification settings - Fork 53
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
VS Code Dev Container Updates #169
VS Code Dev Container Updates #169
Conversation
cargo/.devcontainer/Dockerfile
Outdated
# Run installation to fix docker "cargo-espflash" | ||
# /lib/x86_64-linux-gnu/libc.so.6: version | ||
# `GLIBC_2.32' not found error | ||
RUN cargo install espflash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
technically the container itself does not need espflash, as the tool is either on the host machine that does the flashing, or the user uses the web-flash technology to flash from the container - through the browser - onto the device
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you describe your workflow why you think this should be included? Are you trying to execute espflash with something like docker exec from the host?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like the benefit of using a dev container is all the tooling build dependencies are embedded in the container to compile the app and not on the host machine, but you bring up a good point about the host machine and needing to flash on the host machine, which is what I ended up doing.
In my workflow, I did the cargo clean
and cargo build
in the container and did the flashing on the host. I have not tried web-flash yet. I can try that out.
If you want to approve the other devcontainer.json updates, I can update my PR and remove this update. Let me know @Vollbrecht Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like the benefit of using a dev container is all the tooling build dependencies are embedded in the container to compile the app and not on the host machine, but you bring up a good point about the host machine and needing to flash on the host machine, which is what I ended up doing.
Yeah this is a complicated story, on some platforms its possible to forward the usb connection from the host into the container itself. But this is limited, it doesn't work on windows host and i think even on macos its not without problems. So such a solution would only run on linux. This contradicts the goal of the fully isolated build env, thats why we also provide the option with web-flash.
But its a compromise, espflash as a tool is more capable, and also if you want for example to debug your esp with jtag, you also would need to run openocd on your host machine and then forward the ports from openocd into the container.
If you want to approve the other devcontainer.json updates, I can update my PR and remove this update. Let me know @Vollbrecht Thank you!
Yeah it seems the other part of your change seems good.
I see that the Dockerfile dll's currently cargo-espflash we probably should remove that part. If you change your PR we can merge it i think no problem, I also see that the Dockerfile could see some love regarding updating the base-image to bookworm. I think i will make a followup PR changing the base image to an official rust nightly bookworm one, that way it should be a bit faster to setup, but it need some tweaks as the rust image install cargo into the global /usr/local/cargo instead of the home folder
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated!!
Ty for your PR. I created #170 and added your changes into it and Co-Authored you there in the commit. That makes this obsolete, i hope that is OK with you. |
No description provided.