-
Notifications
You must be signed in to change notification settings - Fork 79
Fix failing 3C test for Windows #956
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
Conversation
This should resolve tests failing on windows due to null type location
Making sure to notify @mgrang that this has been submitted. |
I have fired off Windows and Linux validations for this PR. Will approve once those pass. |
Windows X64 build still fails with this.
|
It's odd if this failure did not manifest when you originally filed correctcomputation#345. But based on ea1edad, I'm guessing the culprit is the loss of the |
Hope this fixes a Windows build failure reported by Mandeep.
Several unit tests seem to be still failing.
We are currently discussing ways to enable you to validate your changes on Windows. Because this PR seems to be taking more than a couple of iterations I would suggest we keep the tests disabled until you are able to run your validations. |
We have some guesses on what the problem what might be, so we could try to push those. But if you can provide us a way to test on Windows, we can do that first. Let us know what you'd prefer. |
This avoids an assertion fail on some windows header files.
Windows doesn't have sigaction, and we couldn't find a reasonable alternative.
This is the same workaround used in many other tests for a Windows-specific problem where 3C thinks it isn't allowed to write a file because (1) it isn't under the base dir (which defaults to something under the build directory) and (2) the backslash in the canonicalized file path doesn't match the hard-coded forward slash in the RUN command. We should probably fix this silly problem a better way, but first we may as well apply the workaround we already have. While I'm here, replace --output-postfix with -output-postfix for consistency with the other tests.
`#include "rewrite_header.checked.h"` will work just fine to include a file in the same directory, and %S caused a problem on Windows when it expanded to a file path including "\3C" and the \3 was interpreted as a regular expression backreference.
The GnuWin32 `sed -i` has a bug where it leaves its temporary file behind, and there's no known solution other than to not use it.
A conditional in Constraints.cpp checked if the first character of a file path was "/" to determine if the path was absolute, but on windows, an absolute path will start with "C:\".
@mgrang , the latest batch of commits fixes all the errors we found when testing 3C on a windows VM. |
Thanks for fixing this. I verified that the X64 Windows build passes with your latest commit set. LGTM. |
Could you please also validate the 32bit compiler on Windows? I see that one test is still failing with the 32bit compiler on Windows:
|
The reason that I could not validate Windows X86 build before merging this PR is because that build never seems to complete if run manually. This is because the build uses 32 bit MSVC linker for building clang. This should get fixed if we switch it to use the 64 bit MSVC linker. But we would only be able to do that after our source upgrade is complete. |
@mgrang If you could not complete a manual build on Windows x86, I doubt we will be able to either. (John said he tried and ran into a problem; I didn't ask what it was.) Wouldn't it be more reasonable to mark the failing test unsupported on Windows x86 (or if that isn't easy, all Windows) until you fix the linker issue you mentioned, and then we can retry the manual build on Windows x86? As with the previous test failures, I think it will be more efficient if you iterate on getting the right |
AFAIK, there is no way to disable a test only for Windows X86. We need to disable for all Windows. You can refer to this (the current) PR for the correct UNSUPPORTED directive (it is the one that John reverted when he re-enabled the tests). |
Sure: #961 |
Our previous PR #930 left some tests failing on windows (correctcomputation#345) . These changes should will hopefully fix those tests. We haven't been able to test these a windows build.