-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Fix broken build when using system zxcvbn #10717
Fix broken build when using system zxcvbn #10717
Conversation
2d50624
to
abb7832
Compare
Which system does this break on? We build using system zxcvbn on our Linux CI. |
Linux is actually affected. The reason you don't see this in CI is simply because file |
I see, so the provided header is used even though we are linking to the system library. |
Exactly, I think the CI job should be updated to remove Also, with the CMake changes I have made it should be possible to just move zxcvbn to |
I don't agree with tampering with the src folder for CI. But I do agree with moving to thirdparty. Can you bundle that in this PR? |
7627cb5
to
75c5248
Compare
Fixup of zxcvbn include statement added in 5513ff5. A zxcvbn/ directory prefix breaks building with system zxcvbn. Remove this prefix to align this include statement with ones present in other files. Add zxcvbn libraries as dependency to CliTest.
75c5248
to
e93cfbc
Compare
* Fix broken build when using system zxcvbn Fixup of zxcvbn include statement added in 5513ff5. A zxcvbn/ directory prefix breaks building with system zxcvbn. Remove this prefix to align this include statement with ones present in other files. Add zxcvbn libraries as dependency to CliTest. * Move src/zxcvbn/ to src/thirdparty/zxcvbn
* Fix broken build when using system zxcvbn Fixup of zxcvbn include statement added in 5513ff5. A zxcvbn/ directory prefix breaks building with system zxcvbn. Remove this prefix to align this include statement with ones present in other files. Add zxcvbn libraries as dependency to CliTest. * Move src/zxcvbn/ to src/thirdparty/zxcvbn
Fixes #10779.
This change is a fixup of the zxcvbn include statement added in
CliTest.cpp
in commit 5513ff5 . A zxcvbn/ directory prefix breaks building with system zxcvbn. Remove this prefix to align this include statement with ones present in other files. Furthermore, add zxcvbn libraries as dependency to CliTest.Move src/zxcvbn to src/thirdparty/zxcvbn.