ci: test-checks.sh all sbf code & use nightly only #30602

ryoqun · 2023-03-06T01:04:58Z

Problem

~~When overloaded with rather hard rustc bump (#10585), I've sneaked some piggybacked changes to ci. specifically at 78dc87d~~

~~EDIT: scope changed to: make some ci jobs faster~~

EDIT2: sbf code isn't cargo {check/clippy}-ed.

Summary of Changes

~~Document the intention properly.~~

EDIT2: use host toolchain for cargo {check/clippy}-ing sbf code.

ryoqun · 2023-03-06T01:24:22Z

ci/test-checks.sh

@@ -64,7 +69,8 @@ if [[ $CI_BASE_BRANCH = "$EDGE_CHANNEL" ]]; then
    exit "$check_status"
  fi

-   # Ensure nightly and --benches
+   # Ensure nightly; there's some test code, which is enabled only under


ryoqun · 2023-03-06T01:26:01Z

ci/test-checks.sh

@@ -56,7 +56,10 @@ export RUSTFLAGS="-D warnings -A incomplete_features"

 # Only force up-to-date lock files on edge
 if [[ $CI_BASE_BRANCH = "$EDGE_CHANNEL" ]]; then
-  if _ scripts/cargo-for-all-lock-files.sh check --locked --all-targets; then


failed like below:

https://buildkite.com/solana-labs/solana/builds/91305#0186b473-a055-49f4-b758-3495864a9f1b/124-714

Checking solana-cargo-build-bpf v1.16.0 (/solana/sdk/cargo-build-bpf) error[E0554]: `#![feature]` may not be used on the stable release channel --> sdk/benches/slot_hashes.rs:1:12 | 1 | #![feature(test)] | ^^^^ error[E0554]: `#![feature]` may not be used on the stable release channel --> sdk/benches/slot_history.rs:1:12 | 1 | #![feature(test)] | ^^^^ error[E0554]: `#![feature]` may not be used on the stable release channel --> sdk/benches/accounts.rs:1:12 | 1 | #![feature(test)] | ^^^^

ryoqun · 2023-03-06T06:57:30Z

hmm, I've hit by this #15080

https://buildkite.com/solana-labs/solana/builds/91313#0186b5a6-896f-41a9-89bf-2a905aff28ac/1247-1274:

  --- stderr
  + ./sbf-tools/rust/bin/rustc --version
  + ./sbf-tools/rust/bin/rustc --print sysroot
  + set +e
  + rustup toolchain uninstall sbf
  info: no toolchain installed for 'sbf'
  + set -e
  + rustup toolchain link sbf sbf-tools/rust
  + exit 0
  make: solana-keygen: No such file or directory
  make: *** [../../../sdk/sbf/c/sbf.mk:264: ../target/debug/sbf/alloc.so] Error 127
  thread 'main' panicked at 'assertion failed: Command::new(\"make\").current_dir(\"c\").arg(\"programs\").arg(&install_dir).status().expect(\"Failed to build C-based SBF programs\").success()', build.rs:45:9
  stack backtrace:
     0: rust_begin_unwind
               at /rustc/52372f9c71d8ade4cb815524f179119656f0aa2e/library/std/src/panicking.rs:575:5
     1: core::panicking::panic_fmt
               at /rustc/52372f9c71d8ade4cb815524f179119656f0aa2e/library/core/src/panicking.rs:64:14
     2: core::panicking::panic
               at /rustc/52372f9c71d8ade4cb815524f179119656f0aa2e/library/core/src/panicking.rs:114:5
     3: build_script_build::main
               at ./build.rs:45:9
     4: core::ops::function::FnOnce::call_once
               at /rustc/52372f9c71d8ade4cb815524f179119656f0aa2e/library/core/src/ops/function.rs:250:5
  note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

yihau · 2023-03-06T07:10:49Z

sounds like it needs solana-keygen 🤔
maybe we should make it to use a determinism one instead of a random one.

ryoqun · 2023-03-07T14:47:59Z

programs/sbf/Cargo.toml

+default = [
+    "sbf_c",
+    "sbf_rust",
+]


why not? :) Otherwise, we have to leak these impl-detail features to ./ci/test-checks.sh by starting manually unrolling cargo-for-all-lock-files.sh.

ryoqun · 2023-03-07T14:50:12Z

sdk/sbf/c/sbf.mk

-ifeq (,$(wildcard $(subst .so,-keypair.json,$1)))
-	$(_@)solana-keygen new --no-passphrase --silent -o $(subst .so,-keypair.json,$1)
-endif


seems we've reached to a consensus to remove this entirely: https://discord.com/channels/428295358100013066/560503042458517505/1082437459423543437

ryoqun · 2023-03-07T14:50:46Z

programs/sbf/tests/programs.rs

+#![allow(clippy::clone_on_copy)]
+#![allow(clippy::needless_range_loop)]
+#![allow(clippy::redundant_clone)]
+#![allow(clippy::needless_borrow)]
+#![allow(clippy::cmp_owned)]
+#![allow(clippy::needless_collect)]
+#![allow(clippy::match_like_matches_macro)]
+#![allow(clippy::unnecessary_cast)]
+#![allow(clippy::uninlined_format_args)]


all the fun excises are left to vm guys..... xD

ryoqun · 2023-03-07T15:23:29Z

programs/sbf/build.rs

+    if env::var("CI_TEST_CHECKS_FORCE_HOST_COMPILE").is_ok() {
+        println!("cargo:warning=(not a warning) Compiling with host toolchain for CI...");
+        return;
+    }
+


@alessandrod this is a bit aggressive, but i think it's better not to use cargo-build-sbf altogether here.

Faster build (no need to wget/tar, etc; also build cache reuse..)

Avoid broken clippy with programs/sbf/build.rs

nicer output in general.

don't pull in externally-controlled dep (i think latest solana-sbf-tools is implicitly used?)

potential downsides:

version mismatch between monorepo rust vs solana-sbf-tools => shouldn't cause too much trouble because casually monorepo is newer

potential risk of super hard-to-debug bugs due to this special cased build.rs behavior => possible, so I'm adding cargo:warning=... here for this bailout codepath.

if this makes sense, I'll finish up this pr in this direction (squash commits, toss review to @yihau, etc).

hope this is what you wanted (faster ./test-checksh.sh): a car, not faster horse. lol

haha this looks good to me, although not sure if it impacts how @dmakarov tests his changes

I think we might want to compile these programs with platform-tools (previously known as sbf-tools and bpf-tools) to prevent accidental updating to a broken toolchain.

as discussed at discord, we've reached conclution this is ok given that this is new additional fail-first test step: https://discord.com/channels/428295358100013066/560503042458517505/1082870294504538244

ryoqun · 2023-03-08T03:38:36Z

yihau

🪖

codecov · 2023-03-10T05:47:25Z

Codecov Report

Merging #30602 (ac7b881) into master (3a97986) will decrease coverage by 0.1%.
The diff coverage is n/a.

❗ Current head ac7b881 differs from pull request most recent head 0683101. Consider uploading reports for the commit 0683101 to get more accurate results

@@            Coverage Diff            @@
##           master   #30602     +/-   ##
=========================================
- Coverage    81.7%    81.6%   -0.1%     
=========================================
  Files         723      723             
  Lines      201620   201620             
=========================================
- Hits       164728   164710     -18     
- Misses      36892    36910     +18

yihau

🪖

ryoqun requested a review from alessandrod March 6, 2023 01:23

ryoqun commented Mar 6, 2023

View reviewed changes

ryoqun marked this pull request as draft March 6, 2023 04:30

ryoqun requested a review from yihau March 6, 2023 04:30

ryoqun changed the title ~~Update and comment test-checks.sh a bit~~ Arrange ci cargo invocations to be faster & non-duplicated Mar 6, 2023

ryoqun commented Mar 7, 2023

View reviewed changes

yihau previously approved these changes Mar 8, 2023

View reviewed changes

ryoqun dismissed yihau’s stale review via 3a208ed March 9, 2023 12:02

ryoqun force-pushed the ci-test-checks-comments branch from 422f0b9 to 3a208ed Compare March 9, 2023 12:02

ryoqun force-pushed the ci-test-checks-comments branch 2 times, most recently from ac7b881 to 733a63c Compare March 13, 2023 00:06

ryoqun changed the title ~~Arrange ci cargo invocations to be faster & non-duplicated~~ ci: test-checks.sh all sbf code & use nightly only Mar 13, 2023

ci: test-checks.sh all sbf code & use nightly only

0683101

ryoqun force-pushed the ci-test-checks-comments branch from 733a63c to 0683101 Compare March 13, 2023 00:14

ryoqun marked this pull request as ready for review March 13, 2023 00:19

ryoqun requested a review from yihau March 13, 2023 00:19

yihau approved these changes Mar 13, 2023

View reviewed changes

ryoqun merged commit 7f58345 into solana-labs:master Mar 13, 2023

This was referenced Mar 13, 2023

programs/sbf: bench running programs with input accounts #30577

Closed

Upgrades Rust to 1.68.0 #30664

Merged

ryoqun mentioned this pull request Jun 30, 2023

Add special feature for inter-crate safe dev-related code reuse #32169

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci: test-checks.sh all sbf code & use nightly only #30602

ci: test-checks.sh all sbf code & use nightly only #30602

ryoqun commented Mar 6, 2023 •

edited

Loading

ryoqun Mar 6, 2023

ryoqun Mar 6, 2023

ryoqun commented Mar 6, 2023

yihau commented Mar 6, 2023 •

edited

Loading

ryoqun Mar 7, 2023

ryoqun Mar 7, 2023

ryoqun Mar 7, 2023

ryoqun Mar 7, 2023

ryoqun Mar 7, 2023 •

edited

Loading

alessandrod Mar 8, 2023

dmakarov Mar 8, 2023

ryoqun Mar 8, 2023

ryoqun commented Mar 8, 2023

yihau left a comment

codecov bot commented Mar 10, 2023 •

edited

Loading

yihau left a comment

ci: test-checks.sh all sbf code & use nightly only #30602

ci: test-checks.sh all sbf code & use nightly only #30602

Conversation

ryoqun commented Mar 6, 2023 • edited Loading

Problem

Summary of Changes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryoqun commented Mar 6, 2023

yihau commented Mar 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryoqun Mar 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryoqun commented Mar 8, 2023

yihau left a comment

Choose a reason for hiding this comment

codecov bot commented Mar 10, 2023 • edited Loading

Codecov Report

yihau left a comment

Choose a reason for hiding this comment

ryoqun commented Mar 6, 2023 •

edited

Loading

yihau commented Mar 6, 2023 •

edited

Loading

ryoqun Mar 7, 2023 •

edited

Loading

codecov bot commented Mar 10, 2023 •

edited

Loading