Introduce CodeChecker toolchain#226
Conversation
17263c7 to
d51ce3b
Compare
1007067 to
2f2f858
Compare
nettle
left a comment
There was a problem hiding this comment.
Good start! I think this is right way forward!
| """ | ||
|
|
||
| CodeCheckerInfo = provider( | ||
| doc = "This provider provides the executable path for CodeChecker", |
There was a problem hiding this comment.
I guess we should list all parameters including clang, clang-tidy and maybe more
There was a problem hiding this comment.
I have added a way to define clang and clang-tidy.
For everything else, we can extend it later.
| content = "\n".join(ctx.attr.skip), | ||
| ) | ||
|
|
||
| info = ctx.toolchains["//src:toolchain_type"].codecheckerinfo |
There was a problem hiding this comment.
Yes, we discussed that to get rid of "src" you may try alias. (I havent tried myself though :) )
|
|
||
| codechecker_toolchain( | ||
| name = "codechecker_local", | ||
| analyzer_binary = "CodeChecker", # Find it in PATH ? |
There was a problem hiding this comment.
We should find a way how to make it flexible. At least we should be able to pass CodeChecker binary by absolute path, try to find in the PATH (current implementation), as a Label, maybe via env var, maybe somehow else...
There was a problem hiding this comment.
I'm working on this.
I want to create targets with the repository rule and have those be the label() targets.
There was a problem hiding this comment.
Fixed.
All binary targets are now labels.
8c9fc81 to
93dd1e4
Compare
nettle
left a comment
There was a problem hiding this comment.
I think this is the right direction!
| clang_tidy_binary = "@my_tools//:clang-tidy", | ||
| clangsa_binary = "@my_tools//:clang", | ||
| codechecker_binary = "@my_tools//:CodeChecker", |
There was a problem hiding this comment.
The description and examples are missing for @my_tools//:CodeChecker etc
| CodeCheckerInfo = provider( | ||
| doc = "This provider provides the executable path for CodeChecker and its related tools", | ||
| fields = { | ||
| "clang_tidy_bin": "clang-tidy executable", |
There was a problem hiding this comment.
Do we need suffix "_bin"? Does it have any meaning?
| EXECUTION_MODE = "{Mode}" | ||
| VERBOSITY = "{Verbosity}" | ||
| CODECHECKER_PATH = "{codechecker_bin}" | ||
| CODECHECKER_PATH = os.path.realpath("{codechecker_bin}") |
There was a problem hiding this comment.
So... we call realpath() only for CodeChecker?
| codechecker_toolchain = rule( | ||
| implementation = _codechecker_toolchain_impl, | ||
| attrs = { | ||
| "clang_tidy_binary": attr.label( |
There was a problem hiding this comment.
The same question: "_binary" - do we need?
| use_repo(codechecker_extension, "default_codechecker_tools") | ||
|
|
||
| register_toolchains( | ||
| "//src:codechecker_local_toolchain", |
There was a problem hiding this comment.
I wonder what is the meaning of "locat"?
Is there a "global" alternative?
| @@ -0,0 +1,46 @@ | |||
| """ | |||
There was a problem hiding this comment.
As we discussed, probably we do not need prefix "codechecker" in the file name.
Why:
We want to use toolchains to determine which CodeChecker binary to use.
What:
Addresses:
none
Notes:
//codechecker:toolchain_typeinstead of//src:toolchain_type.