Add CI shell scripts #4

tcharding · 2024-05-02T00:32:51Z

We would like to put all the CI scripts in a single place instead of copied to each repository.

Add a ci/ directory and in it a run_task.sh script as well as auxilary scripts required. Include a README to document the directory.

When the following three PRs have green CI runs then I believe we can merge this. And then I will update each of the PRs to use this repo and master (instead my fork and the PR branch 05-02-ci).

rust-bitcoin: CI: Use run_task from maintainer tools rust-bitcoin#2736
rust-miniscript: CI: Use run_task from maintainer tools rust-miniscript#682
rust-bitcoincore-rpc: Use rust-bitcoin-maintainer-tools and re-write CI rust-bitcoincore-rpc#348
rust-chf https://github.com/tcharding/rust-chf/actions/runs/8931701405

All green with one minute till my End Of Day - BOOM!

apoelstra · 2024-05-02T12:57:08Z

I guess this is a draft til we nail down why it doesn't work on rust-bitcoincore-rpc?

Otherwise ACK.

storopoli

Some comments in the README.
The script looks fine.

storopoli · 2024-05-02T17:12:55Z

ci/README.md

+
+## Lock files
+
+All repositories MUST include a minimal and recent lock file:


Suggested change

All repositories MUST include a minimal and recent lock file:

All repositories MUST include a minimal and a recent lock file:

It is two right? Not one that is minimal and recent?

Lol you got me, and English is even my first language :)

storopoli · 2024-05-02T17:19:00Z

ci/README.md

+- `Cargo-recent.lock`: A manifest with some recent versions numbers that pass CI.
+- `Cargo-minimal.lock`: A manifest with some minimal version numbers that pass CI.
+
+The `run_taks.sh` script invokes copies each to `Cargo.toml` and uses `cargo --locked`.


Suggested change

The `run_taks.sh` script invokes copies each to `Cargo.toml` and uses `cargo --locked`.

The `run_tasks.sh` script will overwrite each of these `.lock` files to `Cargo.toml` and uses `cargo --locked`.

That was ambiguous. I tried to make more direct and clear.

Nice, this made me find that this text is all wrong. lock files are expected to be handled in the workflow not in the run_task.sh script.

storopoli · 2024-05-02T17:20:26Z

ci/README.md

+If MSRV build requires pinning of any dependencies then a `contrib/pin.sh` script should
+be present in the repository, for example:
+
+```yaml


Suggested change

```yaml

```bash

this is not yaml

I've removed pinning all together now.

storopoli · 2024-05-02T17:20:55Z

ci/README.md

+        run: echo "nightly_version=$(cat nightly-version)" >> $GITHUB_OUTPUT
+```
+
+## run_task.sh


Suggested change

## run_task.sh

## `run_task.sh`

storopoli · 2024-05-02T17:21:09Z

ci/README.md

+
+## run_task.sh
+
+Used by github actions to run a CI job, can also be run from the terminal.


Suggested change

Used by github actions to run a CI job, can also be run from the terminal.

Used by Github actions to run a CI job, can also be run from the terminal.

Sometimes I write GitHub but a little bit of me hurts at how much we rely on them so sometimes I use the slightly insulting "github", kind of like writing "god" with a small g - childish I know.

In other PRs in this "spawn" I've seen GitHub as well

lolz, "spawn" I love it. Took me almost all day yesterday to spawn this new CI setup.

storopoli · 2024-05-02T17:21:38Z

ci/README.md

+EXAMPLES=""
+```
+
+`EXAMPLES` string is of format:


Suggested change

`EXAMPLES` string is of format:

`EXAMPLES` string is of format:

empty line around fenced blocks

storopoli · 2024-05-02T17:23:23Z

ci/README.md

+Additional, crate specific, tests can be put in an optional `contrib/extra_tests.sh` file, which is
+expected to be a normal `bash` file (including exit non-zero on failure).


What "normal bash file means".

A file with a shebang like !#/usr/bin/env bash?
Or a file with .sh extension that can be called with bash file.sh?
Does it need to be chmod +x?
Can I use POSIX sh only or does it need to be a "bash" only file?

Ha ha, I meant "normal" as in the same as all the other ones in this repo - that was, now I read it, totally unclear. Will fix, thanks.

tcharding · 2024-05-02T21:31:01Z

I guess this is a draft til we nail down why it doesn't work on rust-bitcoincore-rpc?

Yes and until I fully understand pinning vs lock files.

What I don't understand is how we have a recent lock file that does not work with msrv. It seems Cargo-recent.lock include oldish versions that work with msrv because then its not that recent. Should we provide 3 lock files:

Cargo-minimal.lock (works with msrv and stable)
Cargo-recent-msrv.lock works unmodified with cargo +1.56.1 check
Cargo-recent.lock does not necessarily work with msrv but is actually recent

Or am I misunderstanding something?

tcharding · 2024-05-02T23:45:16Z

FWIW I just added CI to the new rust-chf using this branch and it was easy peasy - that's a win.

We would like to put all the CI scripts in a single place instead of copied to each repository. Add a `ci/` directory and in it a `run_task.sh` script as well as auxilary scripts required. Include a README to document the directory.

tcharding · 2024-05-03T04:38:41Z

ChatGPT is killing it today, finds bugs in bash scripts like a boss.

tcharding · 2024-05-03T04:44:56Z

Today I re-worked the docs quite a bit. And changed the script so that it now handles duplicate dependencies (for use during upgrade).

tcharding · 2024-05-03T04:58:19Z

Just quietly, this rust-bitcoin-maintainer-tools idea you had @apoelstra is f**cking sick.

storopoli · 2024-05-03T08:32:09Z

ChatGPT is killing it today, finds bugs in bash scripts like a boss.

Have you also ever experimented with shellcheck?

tcharding · 2024-05-03T19:56:12Z

Yeah I try to run that on all scripts, we don't in CI though. Perhaps we should. And with all this yaml work just now I came across yamllint which taught me a few things.

storopoli · 2024-05-03T22:01:19Z

And with all this yaml work just now I came across yamllint which taught me a few things.

The yaml document from hell

This is a fun read!

apoelstra

utACK 72e42b3

My one comment is that we should probably have nightly and stable do some checks that you are actually invoking the script with a nightly/stable compiler. Or alternately, we should combine the two of them into a single item and expect the user to choose a compiler out of band. (But I'd rather keep them separate, and eventually pull the nightly-version stuff into here.)

Either way can wait for a followup PR.

tcharding · 2024-05-07T19:55:59Z

Yeah during one iteration somewhere I was passing the toolchain to the build_and_run shell function and using it with cargo so that the cargo command would fail if it was called with the wrong toolchain. Not too hard to add, I'll keep it in mind.

6def5bc CI: Use run_task from maintainer tools (Tobin C. Harding) c5af528 CI: Add docs and document start (Tobin C. Harding) 62ba105 CI: Use correct spacing (Tobin C. Harding) 0c0e881 CI: Add README file (Tobin C. Harding) 44cb225 CI: Add sanitizer script (Tobin C. Harding) 3407257 CI: Add WASM script (Tobin C. Harding) cc14edf CI: Run the schemars job directly using cargo (Tobin C. Harding) 1fb12e1 CI: Remove recent from schemars job (Tobin C. Harding) 8d7117b CI: Use original name (Tobin C. Harding) Pull request description: First we do some clean up, then we pull the stuff that is specific to this repo out into separate tests, then as the last patch we switch over to use the new script for `rust-bitcoin-maintainer-tools` that was introduced in rust-bitcoin/rust-bitcoin-maintainer-tools#4. ACKs for top commit: apoelstra: ACK 6def5bc Tree-SHA512: 70105d455294d49deb43461958435096d074d19ea8d9adc7036e15d74dfdab466f04b1d9e934574077ca0c32a6fe77d0e5aa11068d8c4d51acef634591227d33

93602a9 CI: Use script from rust-bitcoin-maintainer-tools (Tobin C. Harding) 41c457e CI: Reduce indentation (Tobin C. Harding) Pull request description: Use the new maintainer tools test script we created in rust-bitcoin/rust-bitcoin-maintainer-tools#4 For this repo usage of the script is a big change, the coverage does not change that much except we run one example instead of just building it and we run cargo using `cargo --locked` whereas currently for stable and nightly the dependencies used are much more recent. FTR: - We do not format (exists in current ci but is not enforced because of bug is `test.sh`) - We just build the examples (same as current behaviour) - We do not lint (same as current behaviour) - We do not pin, instead we commit a lock file with working dependency versions. Please note that there are two lock files as is customary but in this repo I was unable to get a set of more recent dependencies to build so both files are the same. This replaces #338 ACKs for top commit: apoelstra: ACK 93602a9 Tree-SHA512: 67a298fddb670714fd27736341b2aa922208d45baadffefc7a9b8dc2ebd48d551b8fc36426bea06d89cddd901ae7feeaf0ebe16ec4668cc3066056f9bb6cd580

We don't want to pull the maintainer tools repo from master otherwise we cannot patch it, we should use a specific revision. This was an oversite in the original work when introducing the maintainer tools `run_task` script. Use commit: b2ac115 Merge rust-bitcoin/rust-bitcoin-maintainer-tools#4: Add CI shell scripts

dc7ed26 CI: Use specific revision of maintainer-tools (Tobin C. Harding) Pull request description: We don't want to pull the maintainer tools repo from master otherwise we cannot patch it, we should use a specific revision. This was an oversite in the original work when introducing the maintainer tools `run_task` script. Use commit: b2ac115 Merge rust-bitcoin/rust-bitcoin-maintainer-tools#4: Add CI shell scripts ACKs for top commit: sanket1729: ACK dc7ed26 apoelstra: ACK dc7ed26 successfully ran local tests; though once it stabilizes a bit we may want to start adding tags to maintainer-tools (maybe with just the date) to make it more visually obvious when we get out of date Tree-SHA512: d6e7f3713c5e0f93361fd2b9ff91c3d391b929d2bfb6e31fa0e706748b1ab63ee6115fb0c31a490003e826778f441c3a8126e63484459b2bbf7d3375f51d22fa

tcharding force-pushed the 05-02-ci branch 9 times, most recently from e8b79cc to ddc87f5 Compare May 2, 2024 02:15

This was referenced May 2, 2024

Re-write CI rust-bitcoin/rust-bitcoincore-rpc#338

Closed

CI: Use run_task from maintainer tools rust-bitcoin/rust-bitcoin#2736

Merged

tcharding force-pushed the 05-02-ci branch 6 times, most recently from 02c0f56 to 22f1579 Compare May 2, 2024 04:42

This was referenced May 2, 2024

CI: Use run_task from maintainer tools rust-bitcoin/rust-miniscript#682

Merged

Consider updating CI to be more like rust-bitcoin rust-bitcoin/rust-miniscript#667

Open

storopoli suggested changes May 2, 2024

View reviewed changes

tcharding force-pushed the 05-02-ci branch 3 times, most recently from 3d9ebbe to b831ab2 Compare May 3, 2024 01:22

Add CI shell scripts

72e42b3

We would like to put all the CI scripts in a single place instead of copied to each repository. Add a `ci/` directory and in it a `run_task.sh` script as well as auxilary scripts required. Include a README to document the directory.

tcharding force-pushed the 05-02-ci branch from b831ab2 to 72e42b3 Compare May 3, 2024 04:37

tcharding marked this pull request as ready for review May 3, 2024 04:59

tcharding changed the title ~~WIP: Add CI shell scripts~~ Add CI shell scripts May 3, 2024

storopoli mentioned this pull request May 3, 2024

ci: Consider shellcheck for bash/shell scripts linting rust-bitcoin/rust-bitcoin#2739

Closed

apoelstra approved these changes May 7, 2024

View reviewed changes

apoelstra merged commit b2ac115 into rust-bitcoin:master May 7, 2024

tcharding deleted the 05-02-ci branch May 7, 2024 20:12

tcharding mentioned this pull request May 7, 2024

Use rust-bitcoin-maintainer-tools and re-write CI rust-bitcoin/rust-bitcoincore-rpc#348

Merged

tcharding mentioned this pull request Aug 10, 2024

CI: Use specific revision of maintainer-tools rust-bitcoin/rust-miniscript#720

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add CI shell scripts #4

Add CI shell scripts #4

tcharding commented May 2, 2024 •

edited

Loading

apoelstra commented May 2, 2024

storopoli left a comment

storopoli May 2, 2024

tcharding May 2, 2024

storopoli May 2, 2024

tcharding May 3, 2024

storopoli May 2, 2024

tcharding May 3, 2024

storopoli May 2, 2024

storopoli May 2, 2024

tcharding May 2, 2024

storopoli May 2, 2024

tcharding May 2, 2024 •

edited

Loading

storopoli May 2, 2024

tcharding May 2, 2024

storopoli May 2, 2024

tcharding May 2, 2024

tcharding commented May 2, 2024

tcharding commented May 2, 2024

tcharding commented May 3, 2024

tcharding commented May 3, 2024

tcharding commented May 3, 2024 •

edited

Loading

storopoli commented May 3, 2024

tcharding commented May 3, 2024

storopoli commented May 3, 2024

apoelstra left a comment

tcharding commented May 7, 2024


		## Lock files

		All repositories MUST include a minimal and recent lock file:

	The `run_taks.sh` script invokes copies each to `Cargo.toml` and uses `cargo --locked`.
	The `run_tasks.sh` script will overwrite each of these `.lock` files to `Cargo.toml` and uses `cargo --locked`.


		## run_task.sh

		Used by github actions to run a CI job, can also be run from the terminal.

	`EXAMPLES` string is of format:
	`EXAMPLES` string is of format:

		Additional, crate specific, tests can be put in an optional `contrib/extra_tests.sh` file, which is
		expected to be a normal `bash` file (including exit non-zero on failure).

Add CI shell scripts #4

Add CI shell scripts #4

Conversation

tcharding commented May 2, 2024 • edited Loading

apoelstra commented May 2, 2024

storopoli left a comment

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tcharding May 2, 2024 • 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

Choose a reason for hiding this comment

tcharding commented May 2, 2024

tcharding commented May 2, 2024

tcharding commented May 3, 2024

tcharding commented May 3, 2024

tcharding commented May 3, 2024 • edited Loading

storopoli commented May 3, 2024

tcharding commented May 3, 2024

storopoli commented May 3, 2024

apoelstra left a comment

Choose a reason for hiding this comment

tcharding commented May 7, 2024

tcharding commented May 2, 2024 •

edited

Loading

tcharding May 2, 2024 •

edited

Loading

tcharding commented May 3, 2024 •

edited

Loading