Skip to content

Conversation

@coolreader18
Copy link
Collaborator

Description of Changes

In preparation for adding another host type, I realized it would make sense to have it so we don't have to compile huge runtimes for spacetimedb-cli in addition to spacetimedb-standalone. Now, spacetime generate shells out to spacetimedb-standalone, which uses host_controller.check_module_validity() to load the module and get the ModuleDef.

Expected complexity level and risk

2: Introducing another way that the binaries interact with each other, but this is altogether pretty small and straightforward.

Testing

  • Ensured spacetime generate still works.

@coolreader18 coolreader18 force-pushed the noa/shell-out-for-schema branch 4 times, most recently from 0cb9645 to c9f886b Compare April 21, 2025 16:47
@bfops bfops added the release-any To be landed in any release window label Apr 21, 2025
@coolreader18 coolreader18 force-pushed the noa/shell-out-for-schema branch 3 times, most recently from 01d902c to cafb0d8 Compare April 21, 2025 17:52
Copy link
Contributor

@cloutiertyler cloutiertyler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems quite reasonable, and minimal. I like this quite a bit. Two things before I approve:

  • Left one comment that we should mark the new command as unstable
  • Need to fix the private repo:
#14 66.46 Caused by:
#14 66.46     0: failed to spawn /build/public/target/debug/spacetimedb-standalone
#14 66.46     1: No such file or directory (os error 2)
#14 66.47 Error: failed to generate control-client

@coolreader18 coolreader18 force-pushed the noa/shell-out-for-schema branch from cafb0d8 to 9aed4cb Compare April 29, 2025 19:03
@coolreader18 coolreader18 force-pushed the noa/shell-out-for-schema branch from 9aed4cb to a3f59eb Compare April 29, 2025 19:11
@coolreader18
Copy link
Collaborator Author

Apologies for the rebase; there was a big conflict with #2593 and it didn't really make sense to do a merge.

Copy link
Contributor

@cloutiertyler cloutiertyler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm approving this. Note that I spoke with Noa and we can't print the unstable warning in standalone extract-schema, because it'll be printed from spacetime generate

@coolreader18 coolreader18 added this pull request to the merge queue Apr 29, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 29, 2025
@CLAassistant
Copy link

CLAassistant commented May 3, 2025

CLA assistant check
All committers have signed the CLA.

@coolreader18 coolreader18 added this pull request to the merge queue May 7, 2025
Merged via the queue into master with commit c5f1d8d May 7, 2025
19 of 20 checks passed
@coolreader18 coolreader18 deleted the noa/shell-out-for-schema branch May 7, 2025 22:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-any To be landed in any release window

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants