-
Notifications
You must be signed in to change notification settings - Fork 241
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
Consumed catch2 from usersim path #3973
base: main
Are you sure you want to change the base?
Consumed catch2 from usersim path #3973
Conversation
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.
Looks good to me other than needing one line deleted.
Thanks for doing this!
2505004
to
b58bef7
Compare
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.
You still need to delete the original catch2 line from dependabot.yml
b2ac67d
to
9f297ef
Compare
18efeef
to
84a7751
Compare
@dthaler few tests are failing for this build, This error is ambiguous for me, is it related catch2 registration failure as the path in catch_wrapper file is not correct? or is it something else? Would appreciate your help. |
I don't know, but it seems to be specific to netebpfext. @Alan-Jowett @shankarseal have you seen this before? |
Can you refer to the stack dump here: #3972 (comment) |
Looking at the stack trace, catch2 is still being searched at the previous path for netebpfext_unit in here : netebpfext_unit!Catch::operator<<+0x5d [D:\a\ebpf-for-windows\ebpf-for-windows\external\Catch2\src\catch2\internal\catch_lazy_expr.cpp @ 20] so, do I need to update path in catch_wrapper.hpp? as I am not sure from where is the above path picked up |
@agarwalishita I looked at the crash dump of netebpf_unit_tests from this PR.
Code in netebpfext_unit.cpp
Build logs:
|
Since the test cases were passing in the baseline, I would like you to check the below:
To rule out, can you check the versions of the Catch2 in usersim repo to compare against? (Maybe, type_traits assert can be caused by mismatch of msvc tools version). Also, netebpfext_unit.cpp has included: Hence check the test\libs\util\catch_wrapper.hpp for path and correctness |
@shpalani These have the same versions for me as well Regarding catch_wrapper, as previously mentioned, I am unsure how the library is able to fetch the Catch2 path from an external source directly without specifying its path. If I need to make a change here, what path should I use? |
The path seems right in tests/libs/util/test_util.vcxproj. From your PR code, where the path is being included for catch_all.hpp is |
I pulled a branch from this PR, and it is reproducible locally.
|
Looks like an artifact of catchorg/Catch2@f24569a |
Verified that the same problem occurred in https://github.com/microsoft/ebpf-for-windows/actions/runs/10746638325/job/29808074192 in PR #3772 trying to update catch2 on Sep. 9, after the change in catch2 went in on Aug. 13. So I think this is a regression in catch2. |
Filed catchorg/Catch2#2935 |
Thanks for your detailed analysis, please let me know when can I take these changes forward. |
I added a temporary workaround for sock_addr_invoke_concurrent1 to this PR until catchorg/Catch2#2935 is fixed. The problem is in Catch2 (in multi-threaded scenarios, maybe m_activate is not set properly, and hence activate/deactivate is not set for m_redirect?? ). |
@agarwalishita : Please resolve the conflicts, with the latest pull, to allow the pipeline to run. |
9a14c7b
to
e424861
Compare
@agarwalishita With the latest pull from this PR, build failed with the same errors locally too! Main: In this PR, the build failure is due to, ebpf-verifier does not have
Hence there is a mismatch in ebpf-verifier version, Please do the below to get the right external repo versions as stated in the 'main' https://github.com/microsoft/ebpf-for-windows/tree/main/external
|
e424861
to
5b5cd82
Compare
This is blocked on #4079 |
Description
Catch2 was getting synced at 2 paths:
external\catch2
external\usersim\external\catch2
So, the former submodule has been deleted as part of this PR
and the source path used is : external\usersim\external\catch2 for creating build file