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

refactor(eventsub): generate entire files #5897

Merged
merged 5 commits into from
Feb 5, 2025

Conversation

Nerixyz
Copy link
Contributor

@Nerixyz Nerixyz commented Feb 2, 2025

This refactors the generation of the JSON parsing to happen at build time and at whole file level. There are two advantages: (1) we can utilize modern processor capabilities and generate multiple files simultaneously (multithreading) and (2) we can make our build system happy, which only knows about whole files (and their timestamps).

One important goal is that building the deserializers is optional (i.e. if someone doesn't have libclang, or it can't parse a TU, the build should still work).

When configuring, we do the following:

  • Check if python3 is available (not required)
    • Set up a virtual environment (venv) in the build directory and install the required dependencies
    • Test that clang is able to parse a simple translation unit that includes boost, Qt, and a standard library header.
  • For each header that is specified to generate_json_impls, the corresponding source and include paths are computed
  • If the generation is supported
    • Add a target for each source file that generates that file (add_custom_command)
  • Add all source files to the twitch-eventsub-ws target (in both cases, however, if generation is supported, these files will have a dependency on the previously generated target)

In CI, we generate on Ubuntu-24.04 and Windows (where FORCE_JSON_GENERATION=On). I'm not sure why this fails on macOS, even with llvm installed (seems like it can't find Qt headers).

Copy link
Member

@pajlada pajlada left a comment

Choose a reason for hiding this comment

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

Small note, but this seems like a good approach

I don't like reading the extra cmake code but it is what it is

@Nerixyz Nerixyz marked this pull request as ready for review February 5, 2025 13:00
Copy link
Member

@pajlada pajlada left a comment

Choose a reason for hiding this comment

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

Works as expected on my machine, thank you!

@pajlada pajlada merged commit 449aefc into Chatterino:master Feb 5, 2025
19 checks passed
@Nerixyz Nerixyz deleted the refactor/json-gen branch February 5, 2025 16:28
@camporter
Copy link
Contributor

After rebasing on top of master, the freebsd builds are very slow. It seems like this is because there's no freebsd wheels for clang-format and other requirements?

@pajlada
Copy link
Member

pajlada commented Feb 6, 2025

After rebasing on top of master, the freebsd builds are very slow. It seems like this is because there's no freebsd wheels for clang-format and other requirements?

I added an option to skip it entirely SKIP_JSON_GENERATION, should be fixed with #5904

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.

3 participants