Skip to content

cmake: Fix stale codegen when mpgen binary changes#316

Open
ViniciusCestarii wants to merge 1 commit into
bitcoin-core:masterfrom
ViniciusCestarii:prevent-stale-codegen
Open

cmake: Fix stale codegen when mpgen binary changes#316
ViniciusCestarii wants to merge 1 commit into
bitcoin-core:masterfrom
ViniciusCestarii:prevent-stale-codegen

Conversation

@ViniciusCestarii

@ViniciusCestarii ViniciusCestarii commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Add MPGEN_BINARY to DEPENDS so target_capnp_sources() also rebuilds when mpgen binary changes. This prevents stale code gen when mpgen binary changes but capnp files don't.

I noticed this after seeing IWYU errors while building the examples, which were caused by stale generated code that no longer matched what the current mpgen produces.

@DrahtBot

DrahtBot commented Jul 22, 2026

Copy link
Copy Markdown

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline and AI policy for information on the review process.

Type Reviewers
ACK hebasto, xyzconstant

If your review is incorrectly listed, please copy-paste <!--meta-tag:bot-skip--> into the comment that the bot should ignore.

@hebasto hebasto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ACK eea9c64.

From CMake docs:

List target names with the DEPENDS option to add ... file-level dependencies.

@xyzconstant

Copy link
Copy Markdown
Contributor

ACK eea9c64

I noticed this after seeing IWYU errors while building the examples, which was caused by stale generated code that no longer matched what the current mpgen produces.

Nice catch! I experienced this too, but I ignored it thinking there was something wrong with my machine.

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.

4 participants