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

Build contracts and dylint driver with stable #698

Merged
merged 11 commits into from
Aug 23, 2022
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions .github/workflows/macos.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
platform:
- macos-latest
toolchain:
- nightly
- stable
runs-on: ${{ matrix.platform }}
env:
RUST_BACKTRACE: full
Expand All @@ -40,8 +40,7 @@ jobs:
with:
profile: minimal
toolchain: ${{ matrix.toolchain }}
components: rust-src
override: true
components: rust-src, rustc-dev, llvm-tools-preview

- name: Install cargo-dylint
uses: baptiste0928/cargo-install@v1
Expand All @@ -68,4 +67,4 @@ jobs:
echo "[workspace]" >> foobar/Cargo.toml
cargo run -- contract build --manifest-path=foobar/Cargo.toml
cargo run -- contract check --manifest-path=foobar/Cargo.toml
cargo run -- contract test --manifest-path=foobar/Cargo.toml
cargo run -- contract test --manifest-path=foobar/Cargo.toml
4 changes: 2 additions & 2 deletions .github/workflows/windows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
platform:
- windows-latest
toolchain:
- nightly
- stable
runs-on: ${{ matrix.platform }}
env:
RUST_BACKTRACE: full
Expand All @@ -42,7 +42,7 @@ jobs:
uses: dtolnay/rust-toolchain@stable
with:
toolchain: ${{ matrix.toolchain }}
components: rust-src
components: rust-src, rustc-dev, llvm-tools-preview

- name: Install cargo-dylint
uses: baptiste0928/cargo-install@v1
Expand Down
29 changes: 13 additions & 16 deletions .gitlab-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ variables:
RUSTY_CACHIER_SINGLE_BRANCH: master
RUSTY_CACHIER_DONT_OPERATE_ON_MAIN_BRANCH: "true"
# paritytech/contracts-ci-linux:production defaults to nightly toolchain, default rusty-cachier to it too
RUSTY_CACHIER_TOOLCHAIN: nightly
RUSTY_CACHIER_TOOLCHAIN: stable
Copy link
Contributor

Choose a reason for hiding this comment

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

We'll need to update the Docker image in paritytech/scripts after this, eh

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have no idea what this env variable actually does. It didn't change the toolchain being used to compile because that needed to be changed at other places.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This variable is used by CI tool rusty-cachier which provides reusable Rust cache for Cargo.

If should be RUSTY_CACHIER_TOOLCHAIN: nightly for all jobs which use nightly toolchain.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This variable is used by CI tool rusty-cachier which provides reusable Rust cache for Cargo.

I figured that much but what does this actually mean?

Copy link
Contributor

Choose a reason for hiding this comment

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

We'll need to update the Docker image in paritytech/scripts after this, eh

Yes, the default toolchain for the CI image is nightly. But it could be overridden in the before_script: section as it was done below.

I figured that much but what does this actually mean?

rusty-cachier related environment variables are described here. RUSTY_CACHIER_TOOLCHAIN is used to select the toolchain to construct a cache key and defaults to stable. Toolchain version is used as a part of the cache key because different Rust versions have incompatible cargo_target_dir caches.

Copy link
Contributor

Choose a reason for hiding this comment

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

If all pipeline jobs are going to use the stable toolchain, the variable could be removed.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was more referring to the line with paritytech/contracts-ci-linux:production (which wasn't touched in this PR).

@rcny not all jobs are going to use the same toolchain, some are still gonna use nightly. What should we do in this case?

Copy link
Contributor

Choose a reason for hiding this comment

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

Add RUSTY_CACHIER_TOOLCHAIN: nightly under the variables: keyword at the specific jobs' level.


workflow:
rules:
Expand Down Expand Up @@ -49,6 +49,8 @@ workflow:
image:
name: "${CI_IMAGE}"
before_script:
- rustup default stable
- rustup component add rustfmt clippy rust-src rustc-dev llvm-tools-preview
athei marked this conversation as resolved.
Show resolved Hide resolved
- cargo -vV
- rustc -vV
- rustup show
Expand Down Expand Up @@ -91,7 +93,7 @@ fmt:
stage: check
<<: *docker-env
script:
- cargo fmt --verbose --all -- --check
- cargo +nightly fmt --verbose --all -- --check

clippy:
stage: check
Expand All @@ -107,20 +109,17 @@ test-dylint:
stage: test
<<: *docker-env
script:
# We need to fix this test to a toolchain because of the UI tests
# We can't use stable here because the UI tests freak out if there are dots in the drivers file name
athei marked this conversation as resolved.
Show resolved Hide resolved
- rustup default nightly-2022-06-30
- rustup component add rustfmt clippy rust-src rustc-dev llvm-tools-preview

- rusty-cachier snapshot create
- cd ink_linting/
- mv _Cargo.toml Cargo.toml

# Installing these components here is necessary because
# `ink_linting/` has a fixed `rust-toolchain` file.
# We can't move this line to the Docker container, since
# that would then make it impossible to upgrade the
# `ink_linting/rust-toolchain` file while still having
# this CI job succeed.
- rustup component add rustfmt clippy rust-src

- cargo check --verbose
- cargo fmt --verbose --all -- --check
- cargo +nightly fmt --verbose --all -- --check
- cargo clippy --verbose -- -D warnings;

# Needed until https://github.com/mozilla/sccache/issues/1000 is fixed.
Expand Down Expand Up @@ -155,8 +154,8 @@ test-new-project-template:

- cargo check --verbose
- cargo test --verbose --all
- cargo fmt --verbose --all -- --check
- cargo clippy --verbose --manifest-path Cargo.toml -- -D warnings -A clippy::let-unit-value;
- cargo +nightly fmt --verbose --all -- --check
- cargo clippy --verbose --manifest-path Cargo.toml -- -D warnings -A clippy::let-unit-value -A clippy::extra-unused-lifetimes
- rusty-cachier cache upload

# With the introduction of `ink_linting` in `build.rs` the installation process
Expand All @@ -166,15 +165,13 @@ test-new-project-template:
test-registry-publish-install:
stage: test
<<: *docker-env
before_script:
- !reference [.rusty-cachier, before_script]
script:
# Set up a local registry.
- mkdir -p ./estuary/crates/ ./estuary/indices/
- estuary --base-url=http://0.0.0.0:7878 --crate-dir ./estuary/crates/ --index-dir ./estuary/indices &
- mkdir .cargo
- echo -e '[registries]\nestuary = { index = "http://0.0.0.0:7878/git/index" }' > .cargo/config.toml
- echo 0000 | cargo login --registry estuary
script:
- rusty-cachier snapshot create
- cargo publish --registry estuary
- cargo install cargo-contract --index http://0.0.0.0:7878/git/index
Expand Down
3 changes: 3 additions & 0 deletions build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,9 @@ fn build_and_zip_dylint_driver(
cmd.env_remove("RUSTUP_TOOLCHAIN");
cmd.env_remove("CARGO_TARGET_DIR");

// Dylint drivers need unstable features. Allow them on a stable toolchain.
cmd.env("RUSTC_BOOTSTRAP", "1");

println!(
"Setting cargo working dir to '{}'",
ink_dylint_driver_dir.display()
Expand Down
7 changes: 5 additions & 2 deletions ink_linting/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,8 @@ cargo build --release

# Run the linting on a contract.
DYLINT_LIBRARY_PATH=$PWD/target/release cargo dylint contract_instantiated
--manifest-path ../ink/examples/erc20/Cargo.toml
```
--manifest-path ../ink/examples/erc20/Cargo.toml

# The UI tests are written against a specific toolchain version. If you wish to run them locally:
cargo +nightly-2022-06-30 test
athei marked this conversation as resolved.
Show resolved Hide resolved
```
6 changes: 0 additions & 6 deletions ink_linting/rust-toolchain.toml

This file was deleted.

21 changes: 16 additions & 5 deletions src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use anyhow::{
};
use heck::ToUpperCamelCase as _;
use rustc_version::Channel;
use semver::Version;
use std::{
ffi::OsStr,
fs,
Expand All @@ -41,15 +42,22 @@ use std::{
/// Check whether the current rust channel is valid: `nightly` is recommended.
pub fn assert_channel() -> Result<()> {
let meta = rustc_version::version_meta()?;
let min_version = Version::new(1, 63, 0);
athei marked this conversation as resolved.
Show resolved Hide resolved
match meta.channel {
Channel::Dev | Channel::Nightly => Ok(()),
Channel::Stable | Channel::Beta => {
Channel::Stable if meta.semver >= min_version => Ok(()),
athei marked this conversation as resolved.
Show resolved Hide resolved
Channel::Stable => {
anyhow::bail!(
"Minimum Rust version is {}. You are using {}.",
min_version,
meta.semver
)
}
_ => {
anyhow::bail!(
"cargo-contract cannot build using the {:?} channel. \
Switch to nightly. \
See https://github.com/paritytech/cargo-contract#build-requires-the-nightly-toolchain",
Contracts should be build using a stable toolchain.",
athei marked this conversation as resolved.
Show resolved Hide resolved
athei marked this conversation as resolved.
Show resolved Hide resolved
format!("{:?}", meta.channel).to_lowercase(),
);
)
}
}
}
Expand Down Expand Up @@ -93,6 +101,9 @@ where
cmd.current_dir(path);
}

// Allow nightly features on a stable toolchain
cmd.env("RUSTC_BOOTSTRAP", "1");

cmd.arg(command);
cmd.args(args);
match verbosity {
Expand Down