Skip to content
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

[CI] Add Windows runner for automated testing #132

Closed
gruenich opened this issue Nov 25, 2023 · 6 comments
Closed

[CI] Add Windows runner for automated testing #132

gruenich opened this issue Nov 25, 2023 · 6 comments

Comments

@gruenich
Copy link
Contributor

We are testing Ubuntu to build and test SuperLU for every pull request. We should add a Windows runner to detect issues that only occur on Windows.

@gruenich
Copy link
Contributor Author

#133 provides much of required code changes, but it does currently fail compilation without an error message.

@wo80
Copy link
Contributor

wo80 commented Nov 25, 2023

Of course there's an error message:

D:\a\superlu\superlu\SRC\util.c(482,19): error C2057: expected constant expression [D:\a\superlu\superlu\build\SRC\superlu.vcxproj]
D:\a\superlu\superlu\SRC\util.c(482,25): error C2466: cannot allocate an array of constant size 0 [D:\a\superlu\superlu\build\SRC\superlu.vcxproj]
D:\a\superlu\superlu\SRC\util.c(482,12): error C2133: 'bucket': unknown size [D:\a\superlu\superlu\build\SRC\superlu.vcxproj]

And if you had read my comment #112 (comment) carefully, you would know. This does not work with VC compiler, so revert it: e87529f

BTW: thank you for finally addressing this.

@wo80
Copy link
Contributor

wo80 commented Nov 25, 2023

Adding the WinGetOpt dependency to actually run the tests on Windows shouldn't be hard, too. Find inspiration here: https://github.com/wo80/vs-arpack/blob/593ae85a26fbccd9e000cc32acdf55def643ea1b/.github/workflows/build.yml#L70-L79

Of course, you'll have to replace SuperLU with the WinGetOpt repo. You might also have to play around with the install prefix.

@wo80
Copy link
Contributor

wo80 commented Nov 26, 2023

The fact that you overlooked the error message is probably due to an abundance of warnings given by the VC compiler. So while you're at it, you might at least disable the pretty useless C4996. Just add _CRT_SECURE_NO_WARNINGS here

superlu/CMakeLists.txt

Lines 121 to 123 in 8f4f993

if(MSVC)
add_compile_definitions(NOMINMAX _COMPLEX_DEFINED)
endif()

@gruenich
Copy link
Contributor Author

Thanks for the hint! I will include this, but I want to investigate whether fixing some of the warnings, e.g., the ones with insecure print functions.

@gruenich
Copy link
Contributor Author

Fixed by #133 and #138.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants