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
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.

2 participants