walterzhaoJR commented on code in PR #3417:
URL: https://github.com/apache/brpc/pull/3417#discussion_r3671884006
##########
BUILD.bazel:
##########
@@ -370,6 +370,23 @@ cc_library(
}),
)
+cc_library(
+ # Header-only interface so :bvar can use RecursiveMutex without depending
on
+ # :bthread (which itself depends on :bvar). Implementation lives in
:bthread.
+ name = "bthread_recursive_mutex",
+ hdrs = [
+ "src/bthread/recursive_mutex.h",
+ "src/bthread/types.h",
+ ],
+ includes = [
+ "src/",
+ ],
+ visibility = ["//visibility:public"],
+ deps = [
+ ":butil",
+ ],
+)
Review Comment:
OK,this is a valid concern. The current //:bthread_recursive_mutex target
exports only the headers, while the bthread_recursive_mutex_* implementations
remain in //:bthread. As a result, a target depending only on //:bvar can
compile but fail to link. The current bvar tests do not expose this because
they explicitly link bvar_bthread_stubs.cpp.
Moving only the recursive-mutex functions into a separate library would not
fully solve the cycle: they still depend on bthread_self() and
bthread_mutex_*(), while //:bthread already depends on //:bvar.
A clean solution would require either:
splitting a bvar-independent mutex/runtime core out of //:bthread; or
combining the Bazel bvar and bthread targets, at the cost of losing
standalone //:bvar.
Is standalone //:bvar considered a compatibility requirement? I would
appreciate guidance on the preferred dependency boundary before restructuring
this part.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
