-
Notifications
You must be signed in to change notification settings - Fork 595
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cmake: Add support for extra dependencies to messages ids check function and make odb and cts use it #6468
Conversation
Signed-off-by: Christian Costa <[email protected]>
Signed-off-by: Christian Costa <[email protected]>
Signed-off-by: Christian Costa <[email protected]>
clang-tidy review says "All clean, LGTM! 👍" |
Any feedback on this PR? |
src/cts/src/CMakeLists.txt
Outdated
@@ -102,6 +102,7 @@ target_link_libraries(cts | |||
messages( | |||
TARGET cts | |||
OUTPUT_DIR .. | |||
DEPENDS cts_lib |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this be needed on every tool?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then we should automate it in the cmake so the clients don't need to specify it or add it as needed in the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to automate it for #5704 but didn't end up with something very clean.
I'll give another try. Thanks for the review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a patch to detect dependencies within tools module.
Here are the detected dependencies for all tools:
odb: db;cdl;defin;defout;lefin;lefout;zutil;gdsin;gdsout
dbSta: dbSta_lib
rsz: rsz_lib
stt: stt_lib
dpl: dpl_lib
ppl: Munkres
cts: cts_lib
grt: grt_lib;FastRoute4.1
mpl: ParquetFP
rcx: rcx_lib
ant: ant_lib
utl: utl_lib
dft: dft_cells_lib;dft_architect_lib;dft_config_lib;dft_replace_lib;dft_clock_domain_lib;dft_utils_lib;dft_stitch_lib
All the related files triggers the messages ids checking now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any news on this PR?
src/cmake/messages.cmake
Outdated
COMMAND ${CMAKE_SOURCE_DIR}/etc/find_messages.py | ||
${local} | ||
> ${OUTPUT_DIR}/messages.txt | ||
&& touch ${OUTPUT_BUILD_DIR}/messages_checked |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need messages_checked ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is needed for odb as it is an interface library without physical target file as explained in #5704 (comment).
Maybe it's not needed for non interface library. I'll check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't get dependencies to work for a non interface library. Adding a DEPENDS argument does not change anything.
add_custom_command(
TARGET ${ARG_TARGET}
POST_BUILD
COMMAND ${CMAKE_SOURCE_DIR}/etc/find_messages.py
${local}
> ${OUTPUT_DIR}/messages.txt
WORKING_DIRECTORY ${SOURCE_DIR}
DEPENDS ${ARG_DEPENDS}
)
So we have to rely on the intermediate file messages_check as done for interface libraries like odb.
Also put messages.checked at the same location as messages.txt as using bin dir does not work in some cases for an unknown reason. Signed-off-by: Christian Costa <[email protected]>
clang-tidy review says "All clean, LGTM! 👍" |
No description provided.