Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Embed default rust config in binary #216

Merged
merged 8 commits into from
Apr 23, 2024
Merged

Embed default rust config in binary #216

merged 8 commits into from
Apr 23, 2024

Conversation

Ottatop
Copy link
Collaborator

@Ottatop Ottatop commented Apr 22, 2024

This PR embeds the default Rust config into the binary, meaning that Pinnacle no longer needs to run a separate process for the default config. It removes the hard dependency on Luarocks. This is halfway to resolving #214 and unblocking #215; the other half is to create a Makefile or some other external runner to copy over protobuf definitions and default configs for pinnacle config gen, but that's for another PR/commit.

This does mean that users who want to use the Lua API will need to manually cd into api/lua and run luarocks make [--local]. Edit: Or stick that in the Makefile maybe idk my brain's fried rn

Also build times went up by like 9% :P. I considered locking the embedding behind a feature flag to mitigate this for Lua users, but I think it's cleaner if Pinnacle doesn't hard require a separate process and library to run.

TODO:

  • Fix how the Rust API handles tokio task panics
    • Currently all tokio tasks panic if something goes wrong. Also the shutdown watcher task panics as well because everything needs to have the same return value. Goal is to make all futures return a Result or something instead of panicking.
      • Nvm lol we ballin', turns out there was an issue with a tokio task not exiting correctly
  • The layout receiver keeps erroring when the config is reloaded, should probably Option::take the receiver when reloading the config to drop it ignore the error? idk
  • Remove the Luarocks make from build.rs
    • Removing the rest of the build script is for the future PR/commit
  • Test running various configs and --no-config

Need to clean up how the Rust API handles shutdown signals
@Ottatop Ottatop marked this pull request as ready for review April 23, 2024 02:00
@Ottatop Ottatop marked this pull request as draft April 23, 2024 02:05
@Ottatop Ottatop marked this pull request as ready for review April 23, 2024 02:23
@Ottatop Ottatop merged commit 478b9f7 into main Apr 23, 2024
5 checks passed
@Ottatop Ottatop deleted the embed_rust_config branch April 23, 2024 02:24
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.

1 participant