ci: Generate protocol_autogen only once in CI - #7918
Conversation
There was a problem hiding this comment.
Pull request overview
This PR streamlines CI verification of the generated protocol wrapper classes by introducing a lightweight, standalone CMake “codegen-only” project and a dedicated reusable GitHub Actions workflow, so protocol autogen is generated/checked once per CI run instead of per build variant.
Changes:
- Add a standalone
cmake/codegenCMake project that exposessetup_code_genandcode_genwithout configuring the full build/dependencies. - Introduce a reusable CI workflow to run code generation and fail with a helpful diff/message when generated files are out of date (including newly added generated files).
- Update CI orchestration to run the new check once, and update docs to describe the standalone codegen flow.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| include/xrpl/protocol_autogen/README.md | Documents the new standalone CMake codegen invocation for regenerating protocol autogen files. |
| cmake/XrplProtocolAutogen.cmake | Makes codegen targets relocatable by deriving repo root from the module location and using it for paths/working directory. |
| cmake/codegen/CMakeLists.txt | Adds a minimal standalone CMake project to expose setup_code_gen / code_gen without the full build. |
| BUILD.md | Documents the standalone codegen project and clarifies CI verifies generated files are up-to-date. |
| .github/workflows/reusable-check-autogen.yml | New reusable workflow that configures/runs standalone codegen and checks for diffs (including new untracked generated files). |
| .github/workflows/reusable-build-test-config.yml | Removes the per-build autogen verification step now handled by the dedicated workflow. |
| .github/workflows/on-trigger.yml | Adds the new reusable autogen check job to scheduled/push/manual trigger workflow. |
| .github/workflows/on-pr.yml | Adds the new reusable autogen check job to PR workflow and includes it in the final dependency aggregation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # so that classes generated for a newly added transaction or ledger | ||
| # entry type show up in the diff below rather than silently as an | ||
| # empty one. | ||
| git add --intent-to-add . |
There was a problem hiding this comment.
Since checking for new differences is done in multiple workflows, should their logic be identical to each as well?
I'm seeing a new git add --intent-to-add ., which I actually have never seen before. Either way, if this is better than what's done in the other workflows, I'd suggest to make that same change to the other workflows as well.
There was a problem hiding this comment.
Other workflows don't possibly create new files though, so I don't think we should add it anywhere else
|
|
||
| The regenerated files should be committed alongside your changes. | ||
| The same targets are also available as a standalone project, which does not | ||
| need the dependencies to be configured first: |
There was a problem hiding this comment.
In terms of configuring dependencies, when a proxy is configured for accessing the Python libraries (as some of us have who work from an office) & connecting to a VPN is required, does that change when using the standalone project approach? Here it says that dependencies do not need to be configured first, so I'm wondering what is really meant by this sentence.
There was a problem hiding this comment.
It works the same way as before, because essentually setup_code_gen implementation hasn't changed.
By dependencies it means C++ dependencies - the standalone project requires 0 knowledge of C++/Conan/compiler and stuff like that
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
High Level Overview of Change
Instead of generating multiple times, let's do it once.
Also, made it so it doesn't require all the tooling, so it's done much faster, and doesn't require using heavy runners / nor our image.
And fixed a bug, where newly added files will trigger an error, if their generated things weren't added to a repo.
Context of Change
API Impact
libxrplchange (any change that may affectlibxrplor dependents oflibxrpl)