From 5c7dad67d1125a5e5a30037466e82c862302fce2 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Fri, 23 Sep 2022 19:00:53 +0200 Subject: [PATCH 01/21] CI check for rust feature bleed Signed-off-by: Oliver Tale-Yazdi --- scripts/ci/gitlab/pipeline/check.yml | 11 ++++ scripts/ci/gitlab/rust-features.sh | 96 ++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+) create mode 100755 scripts/ci/gitlab/rust-features.sh diff --git a/scripts/ci/gitlab/pipeline/check.yml b/scripts/ci/gitlab/pipeline/check.yml index 3166e13313f2a..05041393d1f47 100644 --- a/scripts/ci/gitlab/pipeline/check.yml +++ b/scripts/ci/gitlab/pipeline/check.yml @@ -33,8 +33,19 @@ test-dependency-rules: variables: CI_IMAGE: "paritytech/tools:latest" script: + - cargo install cargo-workspaces - ./scripts/ci/gitlab/ensure-deps.sh +test-rust-features: + stage: check + extends: + - .kubernetes-env + - .test-refs-no-trigger-prs-only + variables: + CI_IMAGE: "paritytech/tools:latest" + script: + - ./scripts/ci/gitlab/rust-features.sh + test-prometheus-alerting-rules: stage: check extends: .kubernetes-env diff --git a/scripts/ci/gitlab/rust-features.sh b/scripts/ci/gitlab/rust-features.sh new file mode 100755 index 0000000000000..7d743d1b277b0 --- /dev/null +++ b/scripts/ci/gitlab/rust-features.sh @@ -0,0 +1,96 @@ +#!/bin/bash + +# This file is part of Substrate. +# Copyright (C) 2022 Parity Technologies (UK) Ltd. +# SPDX-License-Identifier: Apache-2.0 +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +####################################################################### +# +# This script checks that no crate enables a specific feature in its +# `default` feature set. It's important to check that since features +# are used to gate specific functionality which should only be enabled +# if the feature is present. +# +# Has `cargo-workspaces` as optional dependency: +# https://github.com/pksunkara/cargo-workspaces +# +# Invocation scheme: +# ./rust-features.sh +# +####################################################################### + +SUBSTRATE_ROOT=$1 +declare -a RULES=( + "default,std never implies runtime-benchmarks" + "default,std never implies try-runtime" +) + +function check_does_not_imply() { + ENABLED=$1 + STAYS_DISABLED=$2 + echo "📏 Checking that '$ENABLED' does not imply '$STAYS_DISABLED'" + + RET=0 + # Check if the forbidden feature is enabled anywhere in the workspace. + cargo tree --no-default-features --locked --offline --workspace -e features --features "$ENABLED" | grep -q "feature \"$STAYS_DISABLED\"" || RET=$? + if [ $RET -ne 0 ]; then + echo "✅ Feature '$ENABLED' does not imply '$STAYS_DISABLED' in the workspace" + return + else + echo "❌ Feature '$ENABLED' implies '$STAYS_DISABLED' in the workspace" + fi + + # Check if `cargo-workspaces` is installed. + # If not, the user only gets a generic error instead of a nicer one. + if ! cargo workspaces --version > /dev/null 2>&1; then + echo "❌ Install 'cargo-workspaces' for a more detailed error message." + exit 1 + fi + + CRATES=`cargo workspaces list --all | egrep -o '^(\w|-)+'` + FAILED=0 + PASSED=0 + + for CRATE in $CRATES; do + RET=0 + OUTPUT=$(cargo tree --no-default-features --locked --offline -e features --features $ENABLED -p $CRATE 2>&1) + IS_NOT_SUPPORTED=$(echo $OUTPUT | grep -q "not supported for packages in this workspace" || echo $?) + + if [ $IS_NOT_SUPPORTED -eq 0 ]; then + # This case just means that the pallet does not support the + # requested feature which is fine. + PASSED=$((PASSED+1)) + elif echo "$OUTPUT" | grep -q "feature \"$STAYS_DISABLED\""; then + echo "❌ Feature '$ENABLED' implies '$STAYS_DISABLED' in $CRATE; enabled by" + echo "$OUTPUT" | grep -w "feature \"$STAYS_DISABLED\"" | head -n 1 + FAILED=$((FAILED+1)) + else + PASSED=$((PASSED+1)) + fi + done + + TOTAL=$((PASSED + FAILED)) + echo "Checked $TOTAL crates in total of which $FAILED failed and $PASSED passed." + + if [ $FAILED -ne 0 ]; then + exit 1 + fi +} + +cd "$SUBSTRATE_ROOT" + +for RULE in "${RULES[@]}"; do + read -a strarr <<< "$RULE" # uses default whitespace IFS + check_does_not_imply "${strarr[0]}" "${strarr[3]}" +done From 9cb864250194dda271506ffa4239dfe8374d9da7 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Fri, 23 Sep 2022 19:11:25 +0200 Subject: [PATCH 02/21] Cargo not available Signed-off-by: Oliver Tale-Yazdi --- scripts/ci/gitlab/pipeline/check.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/ci/gitlab/pipeline/check.yml b/scripts/ci/gitlab/pipeline/check.yml index 05041393d1f47..b1c8659ff6a6e 100644 --- a/scripts/ci/gitlab/pipeline/check.yml +++ b/scripts/ci/gitlab/pipeline/check.yml @@ -33,7 +33,6 @@ test-dependency-rules: variables: CI_IMAGE: "paritytech/tools:latest" script: - - cargo install cargo-workspaces - ./scripts/ci/gitlab/ensure-deps.sh test-rust-features: From 893e44837dd4e313cc6cd129d06838929c27de4e Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Fri, 23 Sep 2022 19:19:02 +0200 Subject: [PATCH 03/21] Handle missing programs Signed-off-by: Oliver Tale-Yazdi --- scripts/ci/gitlab/pipeline/check.yml | 2 +- scripts/ci/gitlab/rust-features.sh | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/scripts/ci/gitlab/pipeline/check.yml b/scripts/ci/gitlab/pipeline/check.yml index b1c8659ff6a6e..1a067ca6fe373 100644 --- a/scripts/ci/gitlab/pipeline/check.yml +++ b/scripts/ci/gitlab/pipeline/check.yml @@ -43,7 +43,7 @@ test-rust-features: variables: CI_IMAGE: "paritytech/tools:latest" script: - - ./scripts/ci/gitlab/rust-features.sh + - bash ./scripts/ci/gitlab/rust-features.sh . test-prometheus-alerting-rules: stage: check diff --git a/scripts/ci/gitlab/rust-features.sh b/scripts/ci/gitlab/rust-features.sh index 7d743d1b277b0..d91b253e4c3de 100755 --- a/scripts/ci/gitlab/rust-features.sh +++ b/scripts/ci/gitlab/rust-features.sh @@ -30,6 +30,8 @@ # ####################################################################### +set -eu + SUBSTRATE_ROOT=$1 declare -a RULES=( "default,std never implies runtime-benchmarks" @@ -64,7 +66,7 @@ function check_does_not_imply() { for CRATE in $CRATES; do RET=0 - OUTPUT=$(cargo tree --no-default-features --locked --offline -e features --features $ENABLED -p $CRATE 2>&1) + OUTPUT=$(cargo tree --no-default-features --locked --offline -e features --features $ENABLED -p $CRATE 2>&1 || true) IS_NOT_SUPPORTED=$(echo $OUTPUT | grep -q "not supported for packages in this workspace" || echo $?) if [ $IS_NOT_SUPPORTED -eq 0 ]; then From 14976ccc621988c4ba90814ff6d700927664245d Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Fri, 23 Sep 2022 19:35:03 +0200 Subject: [PATCH 04/21] Check for deps Signed-off-by: Oliver Tale-Yazdi --- scripts/ci/gitlab/rust-features.sh | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/scripts/ci/gitlab/rust-features.sh b/scripts/ci/gitlab/rust-features.sh index d91b253e4c3de..d8a827407b129 100755 --- a/scripts/ci/gitlab/rust-features.sh +++ b/scripts/ci/gitlab/rust-features.sh @@ -32,6 +32,11 @@ set -eu +# Check that cargo, grep and egrep are installed. +command -v cargo >/dev/null 2>&1 || { echo >&2 "cargo is required but not installed. Aborting."; exit 1; } +command -v grep >/dev/null 2>&1 || { echo >&2 "grep is required but not installed. Aborting."; exit 1; } +command -v egrep >/dev/null 2>&1 || { echo >&2 "egrep is required but not installed. Aborting."; exit 1; } + SUBSTRATE_ROOT=$1 declare -a RULES=( "default,std never implies runtime-benchmarks" @@ -63,6 +68,7 @@ function check_does_not_imply() { CRATES=`cargo workspaces list --all | egrep -o '^(\w|-)+'` FAILED=0 PASSED=0 + echo "🔍 Checking individual crates" for CRATE in $CRATES; do RET=0 @@ -93,6 +99,6 @@ function check_does_not_imply() { cd "$SUBSTRATE_ROOT" for RULE in "${RULES[@]}"; do - read -a strarr <<< "$RULE" # uses default whitespace IFS - check_does_not_imply "${strarr[0]}" "${strarr[3]}" + read -a splits <<< "$RULE" + check_does_not_imply "${splits[0]}" "${splits[3]}" done From 7a886bc56fac8f5051ceef12cf6b75d2bf5d05b8 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 26 Sep 2022 12:31:59 +0200 Subject: [PATCH 05/21] Add doc Signed-off-by: Oliver Tale-Yazdi --- scripts/ci/gitlab/rust-features.sh | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/scripts/ci/gitlab/rust-features.sh b/scripts/ci/gitlab/rust-features.sh index d8a827407b129..e23c99a9df486 100755 --- a/scripts/ci/gitlab/rust-features.sh +++ b/scripts/ci/gitlab/rust-features.sh @@ -28,6 +28,13 @@ # Invocation scheme: # ./rust-features.sh # +# The steps of this script: +# 1. Check that all required dependencies are installed. +# 2. Check that all rules are fullfilled for the whole workspace. If not: +# 3. Print an error and check if `cargo-workspaces` is installed. If so: +# 4. List all crates and go through each one to find the offending crate. +# 5. Print all offending crates and exit with failure. +# ####################################################################### set -eu From 2ea9fc34911c212436945e4e4337b659a2d5be18 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 26 Sep 2022 12:32:34 +0200 Subject: [PATCH 06/21] Use correct CI image Signed-off-by: Oliver Tale-Yazdi --- scripts/ci/gitlab/pipeline/check.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/scripts/ci/gitlab/pipeline/check.yml b/scripts/ci/gitlab/pipeline/check.yml index 1a067ca6fe373..44fd6349da299 100644 --- a/scripts/ci/gitlab/pipeline/check.yml +++ b/scripts/ci/gitlab/pipeline/check.yml @@ -40,8 +40,6 @@ test-rust-features: extends: - .kubernetes-env - .test-refs-no-trigger-prs-only - variables: - CI_IMAGE: "paritytech/tools:latest" script: - bash ./scripts/ci/gitlab/rust-features.sh . From af8065fe44ceb4e21ebbc5c02b7216adfb27d62b Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 26 Sep 2022 14:10:32 +0200 Subject: [PATCH 07/21] Remove --offline Signed-off-by: Oliver Tale-Yazdi --- scripts/ci/gitlab/rust-features.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/ci/gitlab/rust-features.sh b/scripts/ci/gitlab/rust-features.sh index e23c99a9df486..c925cd8fcc24a 100755 --- a/scripts/ci/gitlab/rust-features.sh +++ b/scripts/ci/gitlab/rust-features.sh @@ -57,7 +57,7 @@ function check_does_not_imply() { RET=0 # Check if the forbidden feature is enabled anywhere in the workspace. - cargo tree --no-default-features --locked --offline --workspace -e features --features "$ENABLED" | grep -q "feature \"$STAYS_DISABLED\"" || RET=$? + cargo tree --no-default-features --locked --workspace -e features --features "$ENABLED" | grep -q "feature \"$STAYS_DISABLED\"" || RET=$? if [ $RET -ne 0 ]; then echo "✅ Feature '$ENABLED' does not imply '$STAYS_DISABLED' in the workspace" return @@ -79,7 +79,7 @@ function check_does_not_imply() { for CRATE in $CRATES; do RET=0 - OUTPUT=$(cargo tree --no-default-features --locked --offline -e features --features $ENABLED -p $CRATE 2>&1 || true) + OUTPUT=$(cargo tree --no-default-features --locked -e features --features $ENABLED -p $CRATE 2>&1 || true) IS_NOT_SUPPORTED=$(echo $OUTPUT | grep -q "not supported for packages in this workspace" || echo $?) if [ $IS_NOT_SUPPORTED -eq 0 ]; then From 3a137b56afe91295e502f984197973b359ed02d6 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 26 Sep 2022 16:03:56 +0200 Subject: [PATCH 08/21] Install cargo-workspaces Signed-off-by: Oliver Tale-Yazdi --- scripts/ci/gitlab/pipeline/check.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/ci/gitlab/pipeline/check.yml b/scripts/ci/gitlab/pipeline/check.yml index 44fd6349da299..fc50592753618 100644 --- a/scripts/ci/gitlab/pipeline/check.yml +++ b/scripts/ci/gitlab/pipeline/check.yml @@ -41,6 +41,7 @@ test-rust-features: - .kubernetes-env - .test-refs-no-trigger-prs-only script: + - cargo install --git https://github.com/pksunkara/cargo-workspaces --tag v0.2.35 --force cargo-workspaces - bash ./scripts/ci/gitlab/rust-features.sh . test-prometheus-alerting-rules: From 9639de656a1f6a9acf841fbe33ad63422abea895 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 26 Sep 2022 16:27:40 +0200 Subject: [PATCH 09/21] Remove cargo-workspaces dep Signed-off-by: Oliver Tale-Yazdi --- scripts/ci/gitlab/pipeline/check.yml | 1 - scripts/ci/gitlab/rust-features.sh | 46 +++++++++++++--------------- 2 files changed, 21 insertions(+), 26 deletions(-) diff --git a/scripts/ci/gitlab/pipeline/check.yml b/scripts/ci/gitlab/pipeline/check.yml index fc50592753618..44fd6349da299 100644 --- a/scripts/ci/gitlab/pipeline/check.yml +++ b/scripts/ci/gitlab/pipeline/check.yml @@ -41,7 +41,6 @@ test-rust-features: - .kubernetes-env - .test-refs-no-trigger-prs-only script: - - cargo install --git https://github.com/pksunkara/cargo-workspaces --tag v0.2.35 --force cargo-workspaces - bash ./scripts/ci/gitlab/rust-features.sh . test-prometheus-alerting-rules: diff --git a/scripts/ci/gitlab/rust-features.sh b/scripts/ci/gitlab/rust-features.sh index c925cd8fcc24a..136183b485c01 100755 --- a/scripts/ci/gitlab/rust-features.sh +++ b/scripts/ci/gitlab/rust-features.sh @@ -22,17 +22,13 @@ # are used to gate specific functionality which should only be enabled # if the feature is present. # -# Has `cargo-workspaces` as optional dependency: -# https://github.com/pksunkara/cargo-workspaces -# # Invocation scheme: # ./rust-features.sh # # The steps of this script: # 1. Check that all required dependencies are installed. # 2. Check that all rules are fullfilled for the whole workspace. If not: -# 3. Print an error and check if `cargo-workspaces` is installed. If so: -# 4. List all crates and go through each one to find the offending crate. +# 4. Check all crates to find the offending ones. # 5. Print all offending crates and exit with failure. # ####################################################################### @@ -53,33 +49,28 @@ declare -a RULES=( function check_does_not_imply() { ENABLED=$1 STAYS_DISABLED=$2 - echo "📏 Checking that '$ENABLED' does not imply '$STAYS_DISABLED'" + echo "📏 Checking that $ENABLED does not imply $STAYS_DISABLED ..." RET=0 # Check if the forbidden feature is enabled anywhere in the workspace. cargo tree --no-default-features --locked --workspace -e features --features "$ENABLED" | grep -q "feature \"$STAYS_DISABLED\"" || RET=$? if [ $RET -ne 0 ]; then - echo "✅ Feature '$ENABLED' does not imply '$STAYS_DISABLED' in the workspace" + echo "✅ $ENABLED does not imply $STAYS_DISABLED in the workspace" return else - echo "❌ Feature '$ENABLED' implies '$STAYS_DISABLED' in the workspace" - fi - - # Check if `cargo-workspaces` is installed. - # If not, the user only gets a generic error instead of a nicer one. - if ! cargo workspaces --version > /dev/null 2>&1; then - echo "❌ Install 'cargo-workspaces' for a more detailed error message." - exit 1 + echo "❌ $ENABLED implies $STAYS_DISABLED in the workspace" fi - CRATES=`cargo workspaces list --all | egrep -o '^(\w|-)+'` + # Find all Cargo.toml files but exclude the root one since we know that it is broken. + CARGOS=`find $SUBSTRATE_ROOT -name Cargo.toml -not -path "$SUBSTRATE_ROOT/Cargo.toml"` FAILED=0 PASSED=0 - echo "🔍 Checking individual crates" + # number of all cargos + echo "🔍 Checking individual crates - this takes some time." - for CRATE in $CRATES; do + for CARGO in $CARGOS; do RET=0 - OUTPUT=$(cargo tree --no-default-features --locked -e features --features $ENABLED -p $CRATE 2>&1 || true) + OUTPUT=$(cargo tree --no-default-features --locked -e features --features $ENABLED --manifest-path $CARGO 2>&1 || true) IS_NOT_SUPPORTED=$(echo $OUTPUT | grep -q "not supported for packages in this workspace" || echo $?) if [ $IS_NOT_SUPPORTED -eq 0 ]; then @@ -87,7 +78,8 @@ function check_does_not_imply() { # requested feature which is fine. PASSED=$((PASSED+1)) elif echo "$OUTPUT" | grep -q "feature \"$STAYS_DISABLED\""; then - echo "❌ Feature '$ENABLED' implies '$STAYS_DISABLED' in $CRATE; enabled by" + echo "❌ Violation in $CARGO enabled by dependency" + # Best effort hint for which dependency needs to be fixed. echo "$OUTPUT" | grep -w "feature \"$STAYS_DISABLED\"" | head -n 1 FAILED=$((FAILED+1)) else @@ -97,15 +89,19 @@ function check_does_not_imply() { TOTAL=$((PASSED + FAILED)) echo "Checked $TOTAL crates in total of which $FAILED failed and $PASSED passed." - - if [ $FAILED -ne 0 ]; then - exit 1 - fi + echo "Exiting with code 1" + exit 1 } cd "$SUBSTRATE_ROOT" for RULE in "${RULES[@]}"; do read -a splits <<< "$RULE" - check_does_not_imply "${splits[0]}" "${splits[3]}" + ENABLED=${splits[0]} + STAYS_DISABLED=${splits[3]} + # Split ENABLED at , and check each one. + IFS=',' read -ra ENABLED_SPLIT <<< "$ENABLED" + for ENABLED_SPLIT in "${ENABLED_SPLIT[@]}"; do + check_does_not_imply "$ENABLED_SPLIT" "$STAYS_DISABLED" + done done From 5cf9457207ebf3a55bd4d9e505d234748e2e7c89 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 26 Sep 2022 16:35:53 +0200 Subject: [PATCH 10/21] Fix try-runtime feature Signed-off-by: Oliver Tale-Yazdi --- frame/try-runtime/Cargo.toml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frame/try-runtime/Cargo.toml b/frame/try-runtime/Cargo.toml index dd77c0438d71f..51b6f91784594 100644 --- a/frame/try-runtime/Cargo.toml +++ b/frame/try-runtime/Cargo.toml @@ -14,7 +14,7 @@ targets = ["x86_64-unknown-linux-gnu"] [dependencies] codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false, features = ["derive"]} -frame-support = { version = "4.0.0-dev", default-features = false, features = [ "try-runtime" ], path = "../support" } +frame-support = { version = "4.0.0-dev", default-features = false, path = "../support" } sp-api = { version = "4.0.0-dev", default-features = false, path = "../../primitives/api" } sp-runtime = { version = "6.0.0", default-features = false, path = "../../primitives/runtime" } sp-std = { version = "4.0.0", default-features = false, path = "../../primitives/std" } @@ -28,3 +28,6 @@ std = [ "sp-runtime/std", "sp-std/std", ] +try-runtime = [ + "frame-support/try-runtime", +] From 040dc2347df4a82002fc1c3624249979e47aa35c Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 26 Sep 2022 16:47:24 +0200 Subject: [PATCH 11/21] Fix features Signed-off-by: Oliver Tale-Yazdi --- frame/try-runtime/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/frame/try-runtime/src/lib.rs b/frame/try-runtime/src/lib.rs index 7fec92712cd19..07ca4a526ac62 100644 --- a/frame/try-runtime/src/lib.rs +++ b/frame/try-runtime/src/lib.rs @@ -18,6 +18,7 @@ //! Supporting types for try-runtime, testing and dry-running commands. #![cfg_attr(not(feature = "std"), no_std)] +#![cfg(features = "try-runtime")] pub use frame_support::traits::TryStateSelect; use frame_support::weights::Weight; From 15e3ac514d74cd5353eab3b86a6625e4633d9a9f Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Tue, 27 Sep 2022 13:31:16 +0200 Subject: [PATCH 12/21] Fix features Signed-off-by: Oliver Tale-Yazdi --- bin/node-template/node/Cargo.toml | 2 +- bin/node/cli/Cargo.toml | 2 +- .../support/procedural/src/pallet/expand/genesis_build.rs | 2 +- frame/try-runtime/src/lib.rs | 2 +- utils/frame/try-runtime/cli/Cargo.toml | 7 ++++++- utils/frame/try-runtime/cli/src/lib.rs | 2 ++ 6 files changed, 12 insertions(+), 5 deletions(-) diff --git a/bin/node-template/node/Cargo.toml b/bin/node-template/node/Cargo.toml index dc1d3caa8b75d..b3915a7d39834 100644 --- a/bin/node-template/node/Cargo.toml +++ b/bin/node-template/node/Cargo.toml @@ -75,4 +75,4 @@ runtime-benchmarks = [ ] # Enable features that allow the runtime to be tried and debugged. Name might be subject to change # in the near future. -try-runtime = ["node-template-runtime/try-runtime", "try-runtime-cli"] +try-runtime = ["node-template-runtime/try-runtime", "try-runtime-cli/try-runtime"] diff --git a/bin/node/cli/Cargo.toml b/bin/node/cli/Cargo.toml index edb434e3c04e3..8d1b43f7c4548 100644 --- a/bin/node/cli/Cargo.toml +++ b/bin/node/cli/Cargo.toml @@ -165,7 +165,7 @@ runtime-benchmarks = [ ] # Enable features that allow the runtime to be tried and debugged. Name might be subject to change # in the near future. -try-runtime = ["kitchensink-runtime/try-runtime", "try-runtime-cli"] +try-runtime = ["kitchensink-runtime/try-runtime", "try-runtime-cli/try-runtime"] [[bench]] name = "transaction_pool" diff --git a/frame/support/procedural/src/pallet/expand/genesis_build.rs b/frame/support/procedural/src/pallet/expand/genesis_build.rs index 53d0695e5f971..d19476779011b 100644 --- a/frame/support/procedural/src/pallet/expand/genesis_build.rs +++ b/frame/support/procedural/src/pallet/expand/genesis_build.rs @@ -19,7 +19,7 @@ use crate::pallet::Def; /// /// * implement the trait `sp_runtime::BuildModuleGenesisStorage` -/// * add #[cfg(features = "std")] to GenesisBuild implementation. +/// * add #[cfg(feature = "std")] to GenesisBuild implementation. pub fn expand_genesis_build(def: &mut Def) -> proc_macro2::TokenStream { let genesis_config = if let Some(genesis_config) = &def.genesis_config { genesis_config diff --git a/frame/try-runtime/src/lib.rs b/frame/try-runtime/src/lib.rs index 07ca4a526ac62..ed1247bd8e6f2 100644 --- a/frame/try-runtime/src/lib.rs +++ b/frame/try-runtime/src/lib.rs @@ -18,7 +18,7 @@ //! Supporting types for try-runtime, testing and dry-running commands. #![cfg_attr(not(feature = "std"), no_std)] -#![cfg(features = "try-runtime")] +#![cfg(feature = "try-runtime")] pub use frame_support::traits::TryStateSelect; use frame_support::weights::Weight; diff --git a/utils/frame/try-runtime/cli/Cargo.toml b/utils/frame/try-runtime/cli/Cargo.toml index 56ead30644d86..46955daf0c820 100644 --- a/utils/frame/try-runtime/cli/Cargo.toml +++ b/utils/frame/try-runtime/cli/Cargo.toml @@ -31,7 +31,12 @@ sp-keystore = { version = "0.12.0", path = "../../../../primitives/keystore" } sp-runtime = { version = "6.0.0", path = "../../../../primitives/runtime" } sp-state-machine = { version = "0.12.0", path = "../../../../primitives/state-machine" } sp-version = { version = "5.0.0", path = "../../../../primitives/version" } -frame-try-runtime = { path = "../../../../frame/try-runtime" } +frame-try-runtime = { optional = true, path = "../../../../frame/try-runtime" } [dev-dependencies] tokio = "1.17.0" + +[features] +try-runtime = [ + "frame-try-runtime/try-runtime", +] diff --git a/utils/frame/try-runtime/cli/src/lib.rs b/utils/frame/try-runtime/cli/src/lib.rs index b8a2779d57e19..0215f2569ab6f 100644 --- a/utils/frame/try-runtime/cli/src/lib.rs +++ b/utils/frame/try-runtime/cli/src/lib.rs @@ -265,6 +265,8 @@ //! -s snap \ //! ``` +#![cfg(feature = "try-runtime")] + use parity_scale_codec::Decode; use remote_externalities::{ rpc_api::RpcService, Builder, Mode, OfflineConfig, OnlineConfig, SnapshotConfig, From c35256e1227915d4a50a96e0c664e75afdf831ca Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Tue, 27 Sep 2022 14:32:23 +0200 Subject: [PATCH 13/21] Fix more features... Signed-off-by: Oliver Tale-Yazdi --- bin/node-template/runtime/Cargo.toml | 2 +- bin/node/runtime/Cargo.toml | 2 +- frame/executive/Cargo.toml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bin/node-template/runtime/Cargo.toml b/bin/node-template/runtime/Cargo.toml index 45ab4939e311c..139264657f89d 100644 --- a/bin/node-template/runtime/Cargo.toml +++ b/bin/node-template/runtime/Cargo.toml @@ -99,7 +99,7 @@ runtime-benchmarks = [ "sp-runtime/runtime-benchmarks", ] try-runtime = [ - "frame-try-runtime", + "frame-try-runtime/try-runtime", "frame-executive/try-runtime", "frame-system/try-runtime", "frame-support/try-runtime", diff --git a/bin/node/runtime/Cargo.toml b/bin/node/runtime/Cargo.toml index e722024231651..5a0a90c96cd3f 100644 --- a/bin/node/runtime/Cargo.toml +++ b/bin/node/runtime/Cargo.toml @@ -256,7 +256,7 @@ runtime-benchmarks = [ "frame-system-benchmarking/runtime-benchmarks", ] try-runtime = [ - "frame-try-runtime", + "frame-try-runtime/try-runtime", "frame-executive/try-runtime", "frame-system/try-runtime", "frame-support/try-runtime", diff --git a/frame/executive/Cargo.toml b/frame/executive/Cargo.toml index cd5c793ee2ee0..f6f5175d63bb9 100644 --- a/frame/executive/Cargo.toml +++ b/frame/executive/Cargo.toml @@ -49,4 +49,4 @@ std = [ "sp-std/std", "sp-tracing/std", ] -try-runtime = ["frame-support/try-runtime", "frame-try-runtime" ] +try-runtime = ["frame-support/try-runtime", "frame-try-runtime/try-runtime" ] From 1d4f80a8f5b918561ce52d2e4dfd22da442b2206 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 6 Oct 2022 20:35:17 +0200 Subject: [PATCH 14/21] Use pipeline-script Signed-off-by: Oliver Tale-Yazdi --- scripts/ci/gitlab/pipeline/check.yml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/scripts/ci/gitlab/pipeline/check.yml b/scripts/ci/gitlab/pipeline/check.yml index 44fd6349da299..d86dad5011c62 100644 --- a/scripts/ci/gitlab/pipeline/check.yml +++ b/scripts/ci/gitlab/pipeline/check.yml @@ -41,7 +41,12 @@ test-rust-features: - .kubernetes-env - .test-refs-no-trigger-prs-only script: - - bash ./scripts/ci/gitlab/rust-features.sh . + # TODO @ggwpez change the branch to $PIPELINE_SCRIPTS_TAG + - git clone + --depth=1 + "--branch=https://github.com/paritytech/pipeline-scripts/pull/81" + https://github.com/paritytech/pipeline-scripts + - bash ./pipeline-scripts/rust-features.sh . test-prometheus-alerting-rules: stage: check @@ -59,4 +64,3 @@ test-prometheus-alerting-rules: - promtool check rules ./scripts/ci/monitoring/alerting-rules/alerting-rules.yaml - cat ./scripts/ci/monitoring/alerting-rules/alerting-rules.yaml | promtool test rules ./scripts/ci/monitoring/alerting-rules/alerting-rule-tests.yaml - From 8d597ec5afd4b9beb6dff717eabc4ba3c41df00f Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 6 Oct 2022 20:38:16 +0200 Subject: [PATCH 15/21] =?UTF-8?q?=F0=9F=A4=A1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Oliver Tale-Yazdi --- scripts/ci/gitlab/pipeline/check.yml | 2 +- scripts/ci/gitlab/rust-features.sh | 107 --------------------------- 2 files changed, 1 insertion(+), 108 deletions(-) delete mode 100755 scripts/ci/gitlab/rust-features.sh diff --git a/scripts/ci/gitlab/pipeline/check.yml b/scripts/ci/gitlab/pipeline/check.yml index d86dad5011c62..f34c9f80dab6f 100644 --- a/scripts/ci/gitlab/pipeline/check.yml +++ b/scripts/ci/gitlab/pipeline/check.yml @@ -44,7 +44,7 @@ test-rust-features: # TODO @ggwpez change the branch to $PIPELINE_SCRIPTS_TAG - git clone --depth=1 - "--branch=https://github.com/paritytech/pipeline-scripts/pull/81" + "--branch=oty-check-rust-feature" https://github.com/paritytech/pipeline-scripts - bash ./pipeline-scripts/rust-features.sh . diff --git a/scripts/ci/gitlab/rust-features.sh b/scripts/ci/gitlab/rust-features.sh deleted file mode 100755 index 136183b485c01..0000000000000 --- a/scripts/ci/gitlab/rust-features.sh +++ /dev/null @@ -1,107 +0,0 @@ -#!/bin/bash - -# This file is part of Substrate. -# Copyright (C) 2022 Parity Technologies (UK) Ltd. -# SPDX-License-Identifier: Apache-2.0 -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -####################################################################### -# -# This script checks that no crate enables a specific feature in its -# `default` feature set. It's important to check that since features -# are used to gate specific functionality which should only be enabled -# if the feature is present. -# -# Invocation scheme: -# ./rust-features.sh -# -# The steps of this script: -# 1. Check that all required dependencies are installed. -# 2. Check that all rules are fullfilled for the whole workspace. If not: -# 4. Check all crates to find the offending ones. -# 5. Print all offending crates and exit with failure. -# -####################################################################### - -set -eu - -# Check that cargo, grep and egrep are installed. -command -v cargo >/dev/null 2>&1 || { echo >&2 "cargo is required but not installed. Aborting."; exit 1; } -command -v grep >/dev/null 2>&1 || { echo >&2 "grep is required but not installed. Aborting."; exit 1; } -command -v egrep >/dev/null 2>&1 || { echo >&2 "egrep is required but not installed. Aborting."; exit 1; } - -SUBSTRATE_ROOT=$1 -declare -a RULES=( - "default,std never implies runtime-benchmarks" - "default,std never implies try-runtime" -) - -function check_does_not_imply() { - ENABLED=$1 - STAYS_DISABLED=$2 - echo "📏 Checking that $ENABLED does not imply $STAYS_DISABLED ..." - - RET=0 - # Check if the forbidden feature is enabled anywhere in the workspace. - cargo tree --no-default-features --locked --workspace -e features --features "$ENABLED" | grep -q "feature \"$STAYS_DISABLED\"" || RET=$? - if [ $RET -ne 0 ]; then - echo "✅ $ENABLED does not imply $STAYS_DISABLED in the workspace" - return - else - echo "❌ $ENABLED implies $STAYS_DISABLED in the workspace" - fi - - # Find all Cargo.toml files but exclude the root one since we know that it is broken. - CARGOS=`find $SUBSTRATE_ROOT -name Cargo.toml -not -path "$SUBSTRATE_ROOT/Cargo.toml"` - FAILED=0 - PASSED=0 - # number of all cargos - echo "🔍 Checking individual crates - this takes some time." - - for CARGO in $CARGOS; do - RET=0 - OUTPUT=$(cargo tree --no-default-features --locked -e features --features $ENABLED --manifest-path $CARGO 2>&1 || true) - IS_NOT_SUPPORTED=$(echo $OUTPUT | grep -q "not supported for packages in this workspace" || echo $?) - - if [ $IS_NOT_SUPPORTED -eq 0 ]; then - # This case just means that the pallet does not support the - # requested feature which is fine. - PASSED=$((PASSED+1)) - elif echo "$OUTPUT" | grep -q "feature \"$STAYS_DISABLED\""; then - echo "❌ Violation in $CARGO enabled by dependency" - # Best effort hint for which dependency needs to be fixed. - echo "$OUTPUT" | grep -w "feature \"$STAYS_DISABLED\"" | head -n 1 - FAILED=$((FAILED+1)) - else - PASSED=$((PASSED+1)) - fi - done - - TOTAL=$((PASSED + FAILED)) - echo "Checked $TOTAL crates in total of which $FAILED failed and $PASSED passed." - echo "Exiting with code 1" - exit 1 -} - -cd "$SUBSTRATE_ROOT" - -for RULE in "${RULES[@]}"; do - read -a splits <<< "$RULE" - ENABLED=${splits[0]} - STAYS_DISABLED=${splits[3]} - # Split ENABLED at , and check each one. - IFS=',' read -ra ENABLED_SPLIT <<< "$ENABLED" - for ENABLED_SPLIT in "${ENABLED_SPLIT[@]}"; do - check_does_not_imply "$ENABLED_SPLIT" "$STAYS_DISABLED" - done -done From ad2746aa117fa7cb473521113a9bec89aaf26484 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 6 Oct 2022 20:41:40 +0200 Subject: [PATCH 16/21] Make stupid change to test the CI Signed-off-by: Oliver Tale-Yazdi --- frame/balances/Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/frame/balances/Cargo.toml b/frame/balances/Cargo.toml index fd2312993b7e7..ad0380651dfc5 100644 --- a/frame/balances/Cargo.toml +++ b/frame/balances/Cargo.toml @@ -38,6 +38,7 @@ std = [ "scale-info/std", "sp-runtime/std", "sp-std/std", + "frame-benchmarking/runtime-benchmarks", ] runtime-benchmarks = ["frame-benchmarking/runtime-benchmarks"] try-runtime = ["frame-support/try-runtime"] From a6a8ed89083b283a3b51ce49ebeae686cd807d13 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 6 Oct 2022 20:49:39 +0200 Subject: [PATCH 17/21] This reverts commit ad2746aa117fa7cb473521113a9bec89aaf26484. --- frame/balances/Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/frame/balances/Cargo.toml b/frame/balances/Cargo.toml index ad0380651dfc5..fd2312993b7e7 100644 --- a/frame/balances/Cargo.toml +++ b/frame/balances/Cargo.toml @@ -38,7 +38,6 @@ std = [ "scale-info/std", "sp-runtime/std", "sp-std/std", - "frame-benchmarking/runtime-benchmarks", ] runtime-benchmarks = ["frame-benchmarking/runtime-benchmarks"] try-runtime = ["frame-support/try-runtime"] From c4eb7d9dec1a210ba72c2701862f950d3550870c Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 6 Oct 2022 20:50:30 +0200 Subject: [PATCH 18/21] Use correct branch Signed-off-by: Oliver Tale-Yazdi --- scripts/ci/gitlab/pipeline/check.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/scripts/ci/gitlab/pipeline/check.yml b/scripts/ci/gitlab/pipeline/check.yml index f34c9f80dab6f..55f0061501076 100644 --- a/scripts/ci/gitlab/pipeline/check.yml +++ b/scripts/ci/gitlab/pipeline/check.yml @@ -41,10 +41,9 @@ test-rust-features: - .kubernetes-env - .test-refs-no-trigger-prs-only script: - # TODO @ggwpez change the branch to $PIPELINE_SCRIPTS_TAG - git clone --depth=1 - "--branch=oty-check-rust-feature" + --branch="$PIPELINE_SCRIPTS_TAG" https://github.com/paritytech/pipeline-scripts - bash ./pipeline-scripts/rust-features.sh . From 82862092ee744a62f0192f43cfb1e47873dc67a7 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 12 Oct 2022 16:33:18 +0200 Subject: [PATCH 19/21] Allow failure Signed-off-by: Oliver Tale-Yazdi --- scripts/ci/gitlab/pipeline/check.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/ci/gitlab/pipeline/check.yml b/scripts/ci/gitlab/pipeline/check.yml index 55f0061501076..878c46f32e850 100644 --- a/scripts/ci/gitlab/pipeline/check.yml +++ b/scripts/ci/gitlab/pipeline/check.yml @@ -40,6 +40,7 @@ test-rust-features: extends: - .kubernetes-env - .test-refs-no-trigger-prs-only + allow_failure: true script: - git clone --depth=1 From 16ec00e1675c7ec57c988315549ff71e832a3093 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 6 Oct 2022 20:41:40 +0200 Subject: [PATCH 20/21] Make stupid change to test the CI Signed-off-by: Oliver Tale-Yazdi --- frame/balances/Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/frame/balances/Cargo.toml b/frame/balances/Cargo.toml index fd2312993b7e7..ad0380651dfc5 100644 --- a/frame/balances/Cargo.toml +++ b/frame/balances/Cargo.toml @@ -38,6 +38,7 @@ std = [ "scale-info/std", "sp-runtime/std", "sp-std/std", + "frame-benchmarking/runtime-benchmarks", ] runtime-benchmarks = ["frame-benchmarking/runtime-benchmarks"] try-runtime = ["frame-support/try-runtime"] From d9b5428b2ca49824c6fa2e6364da3a3ba2b1267a Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 12 Oct 2022 16:42:28 +0200 Subject: [PATCH 21/21] Revert "Make stupid change to test the CI" This reverts commit 16ec00e1675c7ec57c988315549ff71e832a3093. --- frame/balances/Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/frame/balances/Cargo.toml b/frame/balances/Cargo.toml index ad0380651dfc5..fd2312993b7e7 100644 --- a/frame/balances/Cargo.toml +++ b/frame/balances/Cargo.toml @@ -38,7 +38,6 @@ std = [ "scale-info/std", "sp-runtime/std", "sp-std/std", - "frame-benchmarking/runtime-benchmarks", ] runtime-benchmarks = ["frame-benchmarking/runtime-benchmarks"] try-runtime = ["frame-support/try-runtime"]