Overhaul CI with Makefile #51

buffalojoec · 2025-04-22T03:11:57Z

Originally this PR overhauled the repository's CI, including scripts and pnpm workspace configurations, with cargo-make.

See discussion below, where we transitioned to a Makefile.

buffalojoec · 2025-04-22T16:47:37Z

Opening this up to see what you guys think! The diff is pretty nice. 😊

grod220

cargo-make for the win!

package.json

scripts/publish-js.sh

grod220

Think this is really great!

Makefile.toml

lorisleiva

I've got mixed feelings about this but I think it is a net improvement for this repository considering most of the bells and whistles from the zx scripts and the special Cargo.toml variables where not really used as much anyway.

The previous scripting implementation was providing a mini-framework within your repository to inject features into your workflow in a way that would automatically be picked up by CI. But on the flip side, there is decent amount of overhead in having that extra complexity on each repo and having to keep that mini-framework in sync between them.

I'd say I'm in favour of this change. I'm looking forward to see how it adapts to different repos with different needs and then perhaps we can find a good strategy for using it in the create-solana-program generator.

lorisleiva · 2025-05-02T07:52:54Z

dev.env

+RUST_TOOLCHAIN_NIGHTLY="nightly-2024-11-22"
+SOLANA_TOOLCHAIN="2.2.0"


Isn't this information also on the workspace Cargo file which used to be the source of truth for env configs like this? If this is changing that source of truth, should we consider removing that information from the Cargo file so we don't have to keep them in sync?

Yes! I probably forgot to cut it from Cargo.toml.

lorisleiva · 2025-05-02T07:54:12Z

package.json

-    "solana:check": "tsx ./scripts/helpers/check-solana-version.mts",
-    "solana:link": "tsx ./scripts/helpers/link-solana-version.mts",


I might have missed it but I don't think these command have replacements. They seem to be just removed from the repo.

That's correct, I felt like they weren't really necessary. If I'm wrong, we can add them back!

Yeah, I'm fine not having it back. It was just a useful thing to ensure your local Solana version matches the project requirements but tbh this is probably best as a user-based utility instead.

lorisleiva · 2025-05-02T07:58:35Z

package.json

-    "interface:publish": "tsx ./scripts/rust.mts publish interface",
-    "interface:test": "tsx ./scripts/rust.mts test interface",
-    "interface:wasm": "tsx ./scripts/rust.mts wasm interface",
-    "template:upgrade": "tsx ./scripts/helpers/upgrade-template.ts"


Same for this command which doesn't seem to be replaced.

lorisleiva · 2025-05-02T08:03:42Z

scripts/start-test-validator.sh

This file is not quite equivalent to its zx variant (no checking for additional account or program addresses to load from the program Cargo files) but here it's okay because the system program wasn't making use of its features.

Right yeah, I also simplified it a bit. Sorry, I should have called that out!

buffalojoec · 2025-05-02T08:18:58Z

I'd say I'm in favour of this change. I'm looking forward to see how it adapts to different repos with different needs and then perhaps we can find a good strategy for using it in the create-solana-program generator.

Just pointing out that I'm not advocating for this to be used in the template generator. It's certainly possible, but my goal here was to simplify only our stuff for the time being.

joncinque

To be honest, I think we'd almost be better off with an old-school Makefile, since we're not really using any of the special features of cargo-make.

I don't want to derail the effort too much, but this mimics most of the functionality:

nightly = +nightly-2024-11-22

clippy-%:
  cargo $(nightly) clippy --manifest-path $(subst -,/,$*)/Cargo.toml

clippy-%-fix:
  cargo $(nightly) clippy --fix --manifest-path $(subst -,/,$*)/Cargo.toml

format-%:
  cargo $(nightly) fmt --check --manifest-path $(subst -,/,$*)/Cargo.toml

format-%-fix:
  cargo $(nightly) fmt --manifest-path $(subst -,/,$*)/Cargo.toml

features-%:
  cargo $(nightly) hack check --feature-powerset --all-targets --manifest-path $(subst -,/,$*)/Cargo.toml

publish-%:
  ./scripts/publish-rust.sh $(subst -,/,$*)

build-wasm-interface:
  wasm-pack build --target nodejs --dev ./interface --features bincode

lint-docs-%:
  RUSTDOCFLAGS="--cfg docsrs -D warnings" cargo $(nightly) doc --all-features --no-deps --manifest-path $(subst -,/,$*)/Cargo.toml

test-%:
  cargo $(nightly) test --manifest-path $(subst -,/,$*)/Cargo.toml

format-js:
  cd ./clients/js && pnpm install && pnpm format

test-js:
  ./scripts/start-test-validator.sh
  cd ./clients/js && pnpnm install && pnpm build && pnpm test
  ./scripts/stop-test-validator.sh

So you can run make lint-clients-rust or make lint-interface, and then anyone can run this without additional software.

If you think that's a bit too much though, we can go with this approach.

dev.env

scripts/publish-rust.sh

scripts/start-test-validator.sh

scripts/stop-test-validator.sh

buffalojoec · 2025-05-05T04:58:45Z

To be honest, I think we'd almost be better off with an old-school Makefile, since we're not really using any of the special features of cargo-make.

Nice. I'm honestly cool with a Makefile as well. That eliminates the requirement to install the cargo-make binary and it also doesn't add any extra terminal output, which is an annoying feature of cargo-make. The generic aliasing (%) is also a nice touch!

joncinque · 2025-05-05T12:05:21Z

The generic aliasing (%) is also a nice touch!

I'm glad that I didn't waste too much time figuring out how to make it work then 😅 shall I put up a PR with the Makefile?

buffalojoec · 2025-05-05T13:56:38Z

The generic aliasing (%) is also a nice touch!

I'm glad that I didn't waste too much time figuring out how to make it work then 😅 shall I put up a PR with the Makefile?

If you want, you can add it to this PR, and I'll just address the bash feedback?

joncinque · 2025-05-05T14:33:50Z

Works for me!

This reverts commit 6340769.

buffalojoec

I can't approve, since I wrote it, but the Makefile changes LGTM!

joncinque

Looks great!

buffalojoec force-pushed the cargo-make branch 7 times, most recently from fa557e4 to 8e88243 Compare April 22, 2025 16:36

buffalojoec requested review from febo, grod220 and joncinque April 22, 2025 16:46

buffalojoec marked this pull request as ready for review April 22, 2025 16:46

grod220 reviewed Apr 23, 2025

View reviewed changes

package.json Show resolved Hide resolved

scripts/publish-js.sh Show resolved Hide resolved

buffalojoec added 4 commits April 24, 2025 10:25

scripts: replace simple jobs with cargo-make

ec5778e

scripts: replace js test job with cargo-make

5a7be3e

scripts: bashify publish scripts

343ad38

scripts: drop everything unused

38a661d

buffalojoec force-pushed the cargo-make branch 2 times, most recently from 1ec9fa4 to 6699269 Compare April 24, 2025 11:08

move all aliases to Makefile.toml

1928202

buffalojoec force-pushed the cargo-make branch from 6699269 to e3aaa61 Compare April 24, 2025 11:12

ci: make pnpm install optional

e3aaa61

grod220 previously approved these changes Apr 24, 2025

View reviewed changes

Makefile.toml Outdated Show resolved Hide resolved

lorisleiva reviewed May 2, 2025

View reviewed changes

joncinque reviewed May 2, 2025

View reviewed changes

Use a Makefile

9edb2c0

joncinque dismissed grod220’s stale review via 9edb2c0 May 5, 2025 23:28

joncinque added 4 commits May 6, 2025 01:31

Spell pnpm correctly, parallelize more

c2f176e

Remove dev.env file

6340769

Revert "Remove dev.env file"

d397cf3

This reverts commit 6340769.

Use / rename vars file

bbcabb4

buffalojoec changed the title ~~Overhaul CI with cargo-make~~ Overhaul CI with Makefile May 6, 2025

buffalojoec added 3 commits May 6, 2025 01:27

shebang

072014a

cli version name

9f544da

cargo release dry run

4dda2d7

buffalojoec commented May 6, 2025

View reviewed changes

restart-test-validator.sh

e96682b

joncinque approved these changes May 6, 2025

View reviewed changes

buffalojoec merged commit 960949f into solana-program:main May 6, 2025
8 checks passed

buffalojoec deleted the cargo-make branch May 6, 2025 10:34

buffalojoec mentioned this pull request May 6, 2025

overhaul CI with Makefile solana-program/config#52

Merged

		RUST_TOOLCHAIN_NIGHTLY="nightly-2024-11-22"
		SOLANA_TOOLCHAIN="2.2.0"

		"solana:check": "tsx ./scripts/helpers/check-solana-version.mts",
		"solana:link": "tsx ./scripts/helpers/link-solana-version.mts",

Overhaul CI with Makefile #51

Overhaul CI with Makefile #51

Uh oh!

Conversation

buffalojoec commented Apr 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

buffalojoec commented Apr 22, 2025

Uh oh!

grod220 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

grod220 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lorisleiva left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

buffalojoec commented May 2, 2025

Uh oh!

joncinque left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

buffalojoec commented May 5, 2025

Uh oh!

joncinque commented May 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

buffalojoec commented May 5, 2025

Uh oh!

joncinque commented May 5, 2025

Uh oh!

buffalojoec left a comment

Choose a reason for hiding this comment

Uh oh!

joncinque left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

buffalojoec commented Apr 22, 2025 •

edited

Loading

joncinque left a comment •

edited

Loading

joncinque commented May 5, 2025 •

edited

Loading