Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

feat: Update primary Substrate Docker instructions. Remove and redirect legacy erroneous substrate-node-template Docker instructions #13437

Merged
merged 11 commits into from
Mar 8, 2023

Conversation

ltfschoen
Copy link
Contributor

@ltfschoen
Copy link
Contributor Author

ltfschoen commented Feb 22, 2023

I'm on a slow machine so it'll take a while to test before I create the PR.

Additional changes that could be made but are excluded from this PR are:

  • Use a environment variable like DEBUG=true build.sh that gets passed as an argument into the Dockerfile to conditionally use cargo build --locked --release and target/release if DEBUG=false is provided, and otherwise use cargo build --locked and target/debug if its DEBUG=true is provided.

  • Does not allow them to run the following too:

./run.sh cargo --version
./run.sh rustc --version
./run.sh rustup --version

I made an attempt at trying to get this to work in this commit 69de77d#diff-5ec7001467db95278ca884bf1446c5c3138d23ffd1a2938ac92adfe6c57cef51R39, but even though i build the image with changes in the 2nd build like updating the ssl ca certificates, installing relevant linker programs for rustc and substrate, installing rustup and only giving the substrate user permissions, and updating rustup default stable in the container and updating the $PATH with CARGO_HOME=$HOME/.cargo and RUSTUP_HOME=$CARGO_HOME/bin, when i tried to run those commands it'd give errors like info: no updatable toolchains installed error: rustup is not installed at '/root/.cargo' even though it had been added there.
an example was the following, even though i'd already installed rustup in the default directory in the 2nd build stage.

./run.sh rustup default nightly && rustc --version
*** Running Substrate Docker container with provided arguments: rustup default nightly

info: syncing channel updates for 'nightly-x86_64-unknown-linux-gnu'
warning: Signature verification failed for 'https://static.rust-lang.org/dist/channel-rust-nightly.toml'
info: latest update on 2023-02-25, rust version 1.69.0-nightly (c5c7d2b37 2023-02-24)
info: downloading component 'cargo'

Command 'rustc' not found, but can be installed with:
apt install rustc

without updating the ssl ca certificates in the container i'd get error like:

./run.sh rustup default stable
*** Running Substrate Docker container with provided arguments: rustup default stable

info: syncing channel updates for 'stable-x86_64-unknown-linux-gnu'
error: could not download file from 'https://static.rust-lang.org/dist/channel-rust-stable.toml.sha256' to '/substrate/.rustup/tmp/d5qo0tzv22ukcd44_file': failed to make network request: error sending request for url (https://static.rust-lang.org/dist/channel-rust-stable.toml.sha256): error trying to connect: error:1416F086:SSL routines:tls_process_server_certificate:certificate verify failed:ssl/statem/statem_clnt.c:1921: (unable to get local issuer certificate): error trying to connect: error:1416F086:SSL routines:tls_process_server_certificate:certificate verify failed:ssl/statem/statem_clnt.c:1921: (unable to get local issuer certificate): error:1416F086:SSL routines:tls_process_server_certificate:certificate verify failed:ssl/statem/statem_clnt.c:1921: (unable to get local issuer certificate): error:1416F086:SSL routines:tls_process_server_certificate:certificate verify failed:ssl/statem/statem_clnt.c:1921:

and sometimes when i'd run ./run.sh rustup update even if rustup had been installed in 2nd stage of dockerfile, it'd give error:

info: no updatable toolchains installed
error: rustup is not installed at '/root/.cargo'

@ltfschoen ltfschoen marked this pull request as ready for review February 25, 2023 07:56
@@ -0,0 +1,8 @@
[workspace]
Copy link
Member

Choose a reason for hiding this comment

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

Please remove

Copy link
Contributor Author

@ltfschoen ltfschoen Mar 4, 2023

Choose a reason for hiding this comment

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

i've removed that file now.
i checked it still builds successfully when i ran the following on ubuntu 22.04 LTS after removing that file:

cd bin/node-template && \
curl --proto '=https' --tlsv1.3 https://sh.rustup.rs -sSf | sh -s -- -y && \
. $HOME/.cargo/env && \
sudo apt install -y build-essential libclang-dev protobuf-compiler && \
rustup update nightly && \
rustup target add wasm32-unknown-unknown --toolchain nightly && \
cargo build --release

bin/node-template/README.md Outdated Show resolved Hide resolved
bin/node-template/README.md Outdated Show resolved Hide resolved
bin/node-template/README.md Outdated Show resolved Hide resolved
.gitignore Show resolved Hide resolved
@dmitry-markin dmitry-markin added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Mar 7, 2023
@bkchr
Copy link
Member

bkchr commented Mar 7, 2023

@ltfschoen please merge master

@ltfschoen
Copy link
Contributor Author

@ltfschoen please merge master

i've merged latest master and fixed conflicts

@dmitry-markin dmitry-markin merged commit 77c675b into paritytech:master Mar 8, 2023
@dmitry-markin
Copy link
Contributor

@ltfschoen thanks for contributing!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants