Skip to content

Makefile: call cargo for binary targets if workspace library dependencies changed - #9358

Open
daywalker90 wants to merge 1 commit into
ElementsProject:masterfrom
daywalker90:makefile-fix-dep-invoke
Open

Makefile: call cargo for binary targets if workspace library dependencies changed#9358
daywalker90 wants to merge 1 commit into
ElementsProject:masterfrom
daywalker90:makefile-fix-dep-invoke

Conversation

@daywalker90

Copy link
Copy Markdown
Collaborator

No description provided.

@daywalker90
daywalker90 requested a review from cdecker as a code owner July 27, 2026 11:39
@daywalker90 daywalker90 added the Status::Ready for Review The work has been completed and is now awaiting evaluation or approval. label Jul 27, 2026
Comment thread plugins/Makefile Outdated
$(RUST_TARGET_DIR)/examples/cln-subscribe-wildcard: ${CLN_RPC_SOURCES} ${CLN_PLUGIN_SRC} plugins/examples/cln-subscribe-wildcard.rs
$(CARGO) build ${CARGO_OPTS} --example cln-subscribe-wildcard

$(RUST_TARGET_DIR)/cln-grpc: ${CLN_PLUGIN_SRC} ${CLN_GRPC_PLUGIN_SRC} $(MSGGEN_GENALL) $(MSGGEN_GEN_ALL)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Was cln-grpc left out on purpose,do you think $(MSGGEN_GENALL) already covers it well enough in practice? As i understood $(MSGGEN_GENALL) already pulls in cln-rpcs generated files like model.rs, notifications.rs, hooks.rs, but not the rest of cln-rpcs hand-written sources like cln-rpc/src/lib.rs. plugins/grpc-plugin/src/main.rs does use cln_rpc::notifications::Notification so maybe we need $(RUST_TARGET_DIR)/cln-grpc: ${CLN_PLUGIN_SRC} ${CLN_GRPC_PLUGIN_SRC} $(MSGGEN_GENALL) $(MSGGEN_GEN_ALL)?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

In practice cln-grpc was rebuilding in most cases, but you are right it needs ${CLN_RPC_SOURCES} as well. Fixed.

@daywalker90
daywalker90 force-pushed the makefile-fix-dep-invoke branch from bdceb59 to 7d8f0c8 Compare July 29, 2026 09:54
@madelinevibes madelinevibes added this to the v26.09 milestone Jul 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Status::Ready for Review The work has been completed and is now awaiting evaluation or approval.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants