CI: Re-write `run_task.sh` #2633

tcharding · 2024-03-26T02:30:24Z

Recently we re-wrote CI to increase VM level parallelism, in hindsite this has proved to be not that great because:

It resulted in approx 180 jobs
We are on free tier so only get 20 jobs (VMs) at a time so its slow to run
The UI is annoying to dig through the long job list to find failures

Have another go at organising the jobs with the main aim of shortening total run time and making it easier to quickly see fails.

Re-write the run_task.sh script, notable moving manifest handling to the workflow. Also don't bother testing with beta toolchain.

Note on review

The diff is hard to read for rust.yml, I tried splitting out a bunch of separate patches but it resulted in the same thing (because there are so many identical lines in the yaml file). I suggest just looking at the yaml file and not the diff.

coveralls · 2024-03-26T02:35:12Z

Pull Request Test Coverage Report for Build 8840820999

Details

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 83.131%

Totals
Change from base Build 8832141948:	0.0%
Covered Lines:	19195
Relevant Lines:	23090

💛 - Coveralls

tcharding · 2024-03-26T03:40:02Z

WASM fail needs further investigation with fresh eyes.

tcharding · 2024-03-26T22:46:21Z

Bizzare, I can't work out why the wasm test has stopped working, I also can't get it running right now on this branch or master. I have been away since 3am though.

apoelstra · 2024-03-27T18:15:10Z

Needs rebase after #2635.

tcharding · 2024-03-27T22:12:55Z

Needs #2637

tcharding · 2024-03-27T23:41:24Z

Since we have to mereg #2637 with red CI and this PR does not conflict and is only red because of the same reason as 2637 we could consider this for one-ack merge immediately. The only one this is likely of interest to is @Kixunil and he is not about at this minute to see it.

tcharding · 2024-03-28T02:46:03Z

Wow, this re-write uncovered a bunch of bugs in CI. Oh and the chat bot led me on a lot of wild goose chases, its good but it talks a lot of nonsense. (raised #2638)

apoelstra · 2024-04-11T12:29:21Z

contrib/run_task.sh

+build_and_test() {
+    # Building the fuzz crate is more-or-less just a sanity check.
+    # pushd "$REPO_DIR/fuzz" > /dev/null
+    # cargo --locked build


In 39f23d9:

If we are going to comment these out we might as well just remove them.

Ah that's a fail while testing the script yesterday. Big diffs are bad even for the developer :)

tcharding · 2024-04-11T19:37:05Z

Changes in force push:

Uncomment fuzz stuff
Source hahes/test_vars.rs in asan test
Add some comments on where global vars are set

tcharding · 2024-04-11T20:16:17Z

More changes, try even harder to document exactly whats going on in the script, bash hides bugs real easy.

tcharding · 2024-04-11T20:28:49Z

Trivial changes to rust.yaml in an attempt to make reading the file easier, and catching mistakes easier - cleaned up job names and comments.

tcharding · 2024-04-11T20:35:19Z

OMG, the wasm test is not actually running, the && statement is exiting with 1 but the test is still passing ...

+ wasm-pack build
Error: crate-type must be cdylib to compile to wasm32-unknown-unknown. Add the following to your Cargo.toml file:

[lib]
crate-type = ["cdylib", "rlib"]
Caused by: crate-type must be cdylib to compile to wasm32-unknown-unknown. Add the following to your Cargo.toml file:

[lib]
crate-type = ["cdylib", "rlib"]

tcharding · 2024-04-15T21:58:10Z

I am continuing to be puzzled by why the WASM test fails here and not without this PR, a random example is #2686

No difference between the Cargo-recent.toml files
No difference between the hashes/Cargo.toml files
No difference in how the hashes manifest is patched during the job
No difference in how wasm-pack is called

Yet the installed packages differ dramatically (eg wasm-pack is not installed here but `wasm-bindgen-backend)?

tcharding · 2024-04-25T23:16:37Z

BOOM! I finally found the bug! We are not using the Cargo-recent.toml in CI currently but I was using it all along in this PR!!!!! Green CI on the wasm job without Cargo-recent - ~~now I just need to update recent to something that works.~~

Since we patch the hashes manifest before running the wasm tests I believe the current lock files are correct to not include wasm stuff. This PR will maintain (and document) that behaviour.

Recently we re-wrote CI to increase VM level parallelism, in hindsite this has proved to be not that great because: - It resulted in approx 180 jobs - We are on free tier so only get 20 jobs (VMs) at a time so its slow to run - The UI is annoying to dig through the long job list to find failures Have another go at organising the jobs with the main aim of shortening total run time and making it easier to quickly see fails. Re-write the `run_task.sh` script, notable moving manifest handling to the workflow. Also don't bother testing with beta toolchain. WASM Note Removes the `cdylib` and `rlib` from the manifest patching during wasm build - I do not know the following: - Why this breaks on this PR but not on other PRs - Why I can't get wasm test to run locally on master but PRs are passing - What the `cdylib` and `rlib` were meant to be doing This is the docs from: https://doc.rust-lang.org/reference/linkage.html * --crate-type=cdylib, #![crate_type = "cdylib"] - A dynamic system library will be produced. This is used when compiling a dynamic library to be loaded from another language. This output type will create *.so files on Linux, *.dylib files on macOS, and *.dll files on Windows. * --crate-type=rlib, #![crate_type = "rlib"] - A "Rust library" file will be produced. This is used as an intermediate artifact and can be thought of as a "static Rust library". These rlib files, unlike staticlib files, are interpreted by the compiler in future linkage. This essentially means that rustc will look for metadata in rlib files like it looks for metadata in dynamic libraries. This form of output is used to produce statically linked executables as well as staticlib outputs.

apoelstra · 2024-04-27T14:04:51Z

I believe the current lock files are correct to not include wasm stuff.

Yes. We can have a separate lockfile for wasm stuff if you think it'd help but we don't want to poison (and more than double) our real lockfiles.

Awesome that you tracked this down :)

apoelstra

ACK 26b9782

apoelstra · 2024-04-27T14:27:25Z

Will be able to one-ACK merge in 12 days, assuming no comments, questions or NACKs. I am starting the 2-week counter from the most recent force-push because there were significant changes before then and because CI was failing which I suspect was discouraging reviewers. (Personally I had the tab open, intending to help investigate the WASM thing, for at least the last 2 weeks, and did nothing..)

sanket1729

ACK 26b9782.

This is a superficial review. I only checked we are not missing any checks in the new rewrite. Btw, why does the UI say 21 checks instead of 20?

tcharding · 2024-04-29T03:51:34Z

I counted the Prepare job as "job zero" so there is 21 total jobs. I figured this was ok because it runs quickly and has to run before a bunch of the others anyway.

github-actions bot added ci test labels Mar 26, 2024

tcharding force-pushed the 03-24-ci-reduce-jobs branch 9 times, most recently from 5daeab2 to c67d916 Compare March 26, 2024 03:27

tcharding force-pushed the 03-24-ci-reduce-jobs branch 2 times, most recently from da0b9a3 to 503bbb7 Compare March 26, 2024 22:12

tcharding force-pushed the 03-24-ci-reduce-jobs branch from 503bbb7 to 62fdf77 Compare March 26, 2024 22:47

tcharding mentioned this pull request Mar 26, 2024

Run fuzzer daily #2634

Merged

tcharding changed the title ~~CI: Introduce new test.sh script~~ CI: Re-write run_task.sh Mar 26, 2024

tcharding force-pushed the 03-24-ci-reduce-jobs branch from 62fdf77 to 27e2b58 Compare March 26, 2024 23:43

tcharding force-pushed the 03-24-ci-reduce-jobs branch 2 times, most recently from 36cea30 to a101daa Compare March 27, 2024 21:52

tcharding force-pushed the 03-24-ci-reduce-jobs branch 3 times, most recently from 661599d to 0c9e1dd Compare March 28, 2024 02:45

tcharding force-pushed the 03-24-ci-reduce-jobs branch from 0c9e1dd to 03276f0 Compare March 28, 2024 02:52

github-actions bot added C-internals PRs modifying the internals crate C-io PRs modifying the io crate C-base58 labels Apr 10, 2024

tcharding force-pushed the 03-24-ci-reduce-jobs branch 2 times, most recently from 68d38c0 to 39f23d9 Compare April 10, 2024 21:32

apoelstra reviewed Apr 11, 2024

View reviewed changes

tcharding force-pushed the 03-24-ci-reduce-jobs branch from 39f23d9 to 9e70d85 Compare April 11, 2024 20:14

tcharding force-pushed the 03-24-ci-reduce-jobs branch from 9e70d85 to f3e028f Compare April 11, 2024 20:28

tcharding force-pushed the 03-24-ci-reduce-jobs branch 6 times, most recently from ba57bac to c48865a Compare April 15, 2024 21:34

tcharding force-pushed the 03-24-ci-reduce-jobs branch 2 times, most recently from 1dd98b2 to eb9130b Compare April 25, 2024 23:12

tcharding force-pushed the 03-24-ci-reduce-jobs branch from eb9130b to 26b9782 Compare April 25, 2024 23:41

apoelstra approved these changes Apr 27, 2024

View reviewed changes

sanket1729 approved these changes Apr 29, 2024

View reviewed changes

apoelstra merged commit 819eaa9 into rust-bitcoin:master Apr 29, 2024
21 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CI: Re-write `run_task.sh` #2633

CI: Re-write `run_task.sh` #2633

tcharding commented Mar 26, 2024 •

edited

coveralls commented Mar 26, 2024 •

edited

tcharding commented Mar 26, 2024

tcharding commented Mar 26, 2024

apoelstra commented Mar 27, 2024

tcharding commented Mar 27, 2024

tcharding commented Mar 27, 2024 •

edited

tcharding commented Mar 28, 2024 •

edited

apoelstra Apr 11, 2024

tcharding Apr 11, 2024

tcharding commented Apr 11, 2024

tcharding commented Apr 11, 2024

tcharding commented Apr 11, 2024 •

edited

tcharding commented Apr 11, 2024

tcharding commented Apr 15, 2024

tcharding commented Apr 25, 2024 •

edited

apoelstra commented Apr 27, 2024

apoelstra left a comment

apoelstra commented Apr 27, 2024

sanket1729 left a comment

tcharding commented Apr 29, 2024 •

edited

CI: Re-write run_task.sh #2633

CI: Re-write run_task.sh #2633

Conversation

tcharding commented Mar 26, 2024 • edited

Note on review

coveralls commented Mar 26, 2024 • edited

Pull Request Test Coverage Report for Build 8840820999

Details

💛 - Coveralls

tcharding commented Mar 26, 2024

tcharding commented Mar 26, 2024

apoelstra commented Mar 27, 2024

tcharding commented Mar 27, 2024

tcharding commented Mar 27, 2024 • edited

tcharding commented Mar 28, 2024 • edited

apoelstra Apr 11, 2024

Choose a reason for hiding this comment

tcharding Apr 11, 2024

Choose a reason for hiding this comment

tcharding commented Apr 11, 2024

tcharding commented Apr 11, 2024

tcharding commented Apr 11, 2024 • edited

tcharding commented Apr 11, 2024

tcharding commented Apr 15, 2024

tcharding commented Apr 25, 2024 • edited

apoelstra commented Apr 27, 2024

apoelstra left a comment

Choose a reason for hiding this comment

apoelstra commented Apr 27, 2024

sanket1729 left a comment

Choose a reason for hiding this comment

tcharding commented Apr 29, 2024 • edited

CI: Re-write `run_task.sh` #2633

CI: Re-write `run_task.sh` #2633

tcharding commented Mar 26, 2024 •

edited

coveralls commented Mar 26, 2024 •

edited

tcharding commented Mar 27, 2024 •

edited

tcharding commented Mar 28, 2024 •

edited

tcharding commented Apr 11, 2024 •

edited

tcharding commented Apr 25, 2024 •

edited

tcharding commented Apr 29, 2024 •

edited