Skip to content

Various fixups to build#1580

Merged
leighmcculloch merged 4 commits intomainfrom
build-fix-ups
Sep 30, 2025
Merged

Various fixups to build#1580
leighmcculloch merged 4 commits intomainfrom
build-fix-ups

Conversation

@leighmcculloch
Copy link
Copy Markdown
Member

@leighmcculloch leighmcculloch commented Sep 30, 2025

What

Install the msrv ahead of using it as an override. Remove making warnings declined in the Makefile. Do not rebuild test wasms when running tests. Change the default target to just run tests.

Why

Few things I wish I had done slightly differently in #1576.

Warnings are enabled in the makefile, but that needs to be set only in CI, and only on the msrv now that the CI uses the makefile. Right now warnings are enabled for both msrv and latest, when they shouldn't be. Having warnings as errors off on local is good.

The tests run by the makefile always build the test wasms, but in CI that doesn't need to happen as the test wasms are injected as an artifact.

The default target currently calls the check target, which no longer exists.

@leighmcculloch leighmcculloch requested review from a team and graydon September 30, 2025 10:52
@leighmcculloch leighmcculloch added this pull request to the merge queue Sep 30, 2025
Merged via the queue into main with commit 9f73aa5 Sep 30, 2025
34 checks passed
@leighmcculloch leighmcculloch deleted the build-fix-ups branch September 30, 2025 16:54
github-merge-queue bot pushed a commit that referenced this pull request Sep 30, 2025
### What
Add a job that expands the generated code of each test wasm both when
built locally for the host with tests, and when built for wasm, and
verifies if the committed files are up-to-date.

  ### Why
So that PRs show diffs when there are changes to generated code. To
catch unexpected and make hyper-visible the impact of changes to macros.

There's always an argument with this type of files we're committing that
it will get too noisy, too cumbersome, and that it won't be the right
balance and tradeoff in maintainence cost. This is an experiment to some
degree and I think we'll find out during Q4 if it pays off or not and
then we can retro, because we have changes planned that'll change the
generated code (e.g. #1507 #1535) and we'll see how it goes. We can
decide to remove this if the tradeoff doesn't end up being right.

For #1544

Dependent on:
- stellar/binaries#45
- #1580
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants