Skip to content

Commit

Permalink
Remove continue on error Windows jobs ci (#313)
Browse files Browse the repository at this point in the history
  • Loading branch information
mcbarton authored Oct 26, 2024
1 parent 920347b commit c0a4dfe
Show file tree
Hide file tree
Showing 11 changed files with 87 additions and 51 deletions.
48 changes: 5 additions & 43 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -75,19 +75,6 @@ jobs:
clang-runtime: '17'
cling: Off
cppyy: Off
- name: win2022-msvc-clang-repl-16
os: windows-2022
compiler: msvc
clang-runtime: '16'
cling: Off
cppyy: Off
- name: win2022-msvc-cling
os: windows-2022
compiler: msvc
clang-runtime: '13'
cling: On
cling-version: '1.0'
cppyy: Off
- name: osx14-arm-clang-clang-repl-19
os: macos-14
compiler: clang
Expand Down Expand Up @@ -459,7 +446,7 @@ jobs:
}
cd build
echo "Apply clang${{ matrix.clang-runtime }}-*.patch patches:"
cmake -DLLVM_ENABLE_PROJECTS="clang" `
cmake -DLLVM_ENABLE_PROJECTS="clang" `
-DLLVM_TARGETS_TO_BUILD="host;NVPTX" `
-DCMAKE_BUILD_TYPE=Release `
-DLLVM_ENABLE_ASSERTIONS=ON `
Expand Down Expand Up @@ -553,32 +540,6 @@ jobs:
# clang-runtime: '17'
# cling: Off
# cppyy: On
- name: win2022-msvc-clang-repl-16
os: windows-2022
compiler: msvc
clang-runtime: '16'
cling: Off
cppyy: Off
#- name: win2022-msvc-clang-repl-16-cppyy
# os: windows-2022
# compiler: msvc
# clang-runtime: '16'
# cling: Off
# cppyy: On
- name: win2022-msvc-cling
os: windows-2022
compiler: msvc
clang-runtime: '13'
cling: On
cling-version: '1.0'
cppyy: Off
#- name: win2022-msvc-cling-cppyy
# os: windows-2022
# compiler: msvc
# clang-runtime: '13'
# cling: On
# cling-version: '1.0'
# cppyy: On
- name: osx14-arm-clang-clang-repl-19-cppyy
os: macos-14
compiler: clang
Expand Down Expand Up @@ -821,6 +782,7 @@ jobs:
brew install eigen
brew install boost
pip install distro pytest
- name: Restore Cache LLVM/Clang runtime build directory
uses: actions/cache/restore@v4
Expand Down Expand Up @@ -898,7 +860,6 @@ jobs:
echo "CPLUS_INCLUDE_PATH=$CPLUS_INCLUDE_PATH" >> $GITHUB_ENV
- name: Build and Test/Install CppInterOp on Windows systems
continue-on-error: true
if: ${{ runner.os == 'windows' }}
run: |
#FIXME: Windows CppInterOp tests expected to fail
Expand All @@ -912,7 +873,7 @@ jobs:
$env:LLVM_BUILD_DIR="$env:PWD_DIR\llvm-project\build"
echo "LLVM_BUILD_DIR=$env:LLVM_BUILD_DIR"
echo "LLVM_BUILD_DIR=$env:LLVM_BUILD_DIR" >> $env:GITHUB_ENV
if ( "${{ matrix.cling }}" -imatch "On" )
{
$env:CLING_DIR="$env:PWD_DIR\cling"
Expand Down Expand Up @@ -966,6 +927,7 @@ jobs:
-DLLVM_DIR="$env:LLVM_BUILD_DIR\lib\cmake\llvm" `
-DLLVM_ENABLE_WERROR=On `
-DClang_DIR="$env:LLVM_BUILD_DIR\lib\cmake\clang" -DCODE_COVERAGE=${{ env.CODE_COVERAGE }} -DCMAKE_INSTALL_PREFIX="$env:CPPINTEROP_DIR" ..\
cmake --build . --config ${{ env.BUILD_TYPE }} --target googletest --parallel ${{ env.ncpus }}
}
cmake --build . --config ${{ env.BUILD_TYPE }} --target check-cppinterop --parallel ${{ env.ncpus }}
cd ..
Expand Down Expand Up @@ -1117,7 +1079,7 @@ jobs:
# When debugging increase to a suitable value!
timeout-minutes: 30

emscripten_wasm:
emscripten_wasm_CppInterOp_and_xeus_cpp:
needs: [build_cache]
name: ${{ matrix.name }}
runs-on: ${{ matrix.os }}
Expand Down
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ if (LLVM_COMPILER_IS_GCC_COMPATIBLE)
endif ()

# Fixes "C++ exception handler used, but unwind semantics are not enabled" warning Windows
if (WIN32)
if (MSVC)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /EHsc")
endif ()

Expand Down
19 changes: 18 additions & 1 deletion lib/Interpreter/Compatibility.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,24 @@
#include "clang/Basic/Version.h"
#include "clang/Config/config.h"

#ifdef _WIN32
#define dup _dup
#define dup2 _dup2
#define close _close
#define fileno _fileno
#endif

static inline char* GetEnv(const char* Var_Name) {
#ifdef _WIN32
char* Env = nullptr;
size_t sz = 0;
getenv_s(&sz, Env, sz, Var_Name);
return Env;
#else
return getenv(Var_Name);
#endif
}

#if CLANG_VERSION_MAJOR < 19
#define Template_Deduction_Result Sema::TemplateDeductionResult
#define Template_Deduction_Result_Success \
Expand Down Expand Up @@ -45,7 +63,6 @@
#if LLVM_VERSION_MAJOR < 18
#define starts_with startswith
#define ends_with endswith
#define starts_with_insensitive startswith_insensitive
#endif

#if CLANG_VERSION_MAJOR >= 18
Expand Down
17 changes: 15 additions & 2 deletions lib/Interpreter/CppInterOp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3277,7 +3277,22 @@ namespace Cpp {
int m_DupFD = -1;

public:
#ifdef _WIN32
StreamCaptureInfo(int FD)
: m_TempFile(
[]() {
FILE* stream = nullptr;
errno_t err;
err = tmpfile_s(&stream);
if (err)
printf("Cannot create temporary file!\n");
return stream;
}(),
std::fclose),
m_FD(FD) {
#else
StreamCaptureInfo(int FD) : m_TempFile(tmpfile(), std::fclose), m_FD(FD) {
#endif
if (!m_TempFile) {
perror("StreamCaptureInfo: Unable to create temp file");
return;
Expand All @@ -3289,7 +3304,6 @@ namespace Cpp {
// This seems only neccessary when piping stdout or stderr, but do it
// for ttys to avoid over complicated code for minimal benefit.
::fflush(FD == STDOUT_FILENO ? stdout : stderr);

if (dup2(fileno(m_TempFile.get()), FD) < 0)
perror("StreamCaptureInfo:");
}
Expand All @@ -3306,7 +3320,6 @@ namespace Cpp {
assert(m_DupFD != -1 && "Multiple calls to GetCapturedString");

fflush(nullptr);

if (dup2(m_DupFD, m_FD) < 0)
perror("StreamCaptureInfo:");
// Go to the end of the file.
Expand Down
3 changes: 2 additions & 1 deletion lib/Interpreter/DynamicLibraryManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
//------------------------------------------------------------------------------

#include "DynamicLibraryManager.h"
#include "Compatibility.h"
#include "Paths.h"

#include "llvm/ADT/StringSet.h"
Expand Down Expand Up @@ -52,7 +53,7 @@ namespace Cpp {
// Behaviour is to not add paths that don't exist...In an interpreted env
// does this make sense? Path could pop into existance at any time.
for (const char* Var : kSysLibraryEnv) {
if (const char* Env = ::getenv(Var)) {
if (const char* Env = GetEnv(Var)) {
SmallVector<StringRef, 10> CurPaths;
SplitPaths(Env, CurPaths, utils::kPruneNonExistant, Cpp::utils::platform::kEnvDelim);
for (const auto& Path : CurPaths)
Expand Down
5 changes: 3 additions & 2 deletions lib/Interpreter/Paths.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
//------------------------------------------------------------------------------

#include "Paths.h"
#include "Compatibility.h"

#include "clang/Basic/FileManager.h"
#include "clang/Lex/HeaderSearchOptions.h"
Expand Down Expand Up @@ -168,11 +169,11 @@ bool ExpandEnvVars(std::string& Str, bool Path) {

std::string EnvVar = Str.substr(DPos + 1, Length -1); //"HOME"
std::string FullPath;
if (const char* Tok = ::getenv(EnvVar.c_str()))
if (const char* Tok = GetEnv(EnvVar.c_str()))
FullPath = Tok;

Str.replace(DPos, Length, FullPath);
DPos = Str.find("$", DPos + 1); //search for next env variable
DPos = Str.find("$", DPos + 1); // search for next env variable
}
if (!Path)
return true;
Expand Down
9 changes: 9 additions & 0 deletions unittests/CppInterOp/CUDATest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,18 @@ TEST(DISABLED_CUDATest, Sanity) {
#else
TEST(CUDATest, Sanity) {
#endif // CLANG_VERSION_MAJOR < 16
#ifdef _WIN32
GTEST_SKIP() << "Disabled on Windows. Needs fixing.";
#endif
if (!HasCudaSDK())
GTEST_SKIP() << "Skipping CUDA tests as CUDA SDK not found";
EXPECT_TRUE(Cpp::CreateInterpreter({}, {"--cuda"}));
}

TEST(CUDATest, CUDAH) {
#ifdef _WIN32
GTEST_SKIP() << "Disabled on Windows. Needs fixing.";
#endif
if (!HasCudaSDK())
GTEST_SKIP() << "Skipping CUDA tests as CUDA SDK not found";

Expand All @@ -61,6 +67,9 @@ TEST(CUDATest, CUDAH) {
}

TEST(CUDATest, CUDARuntime) {
#ifdef _WIN32
GTEST_SKIP() << "Disabled on Windows. Needs fixing.";
#endif
if (!HasCudaRuntime())
GTEST_SKIP() << "Skipping CUDA tests as CUDA runtime not found";

Expand Down
12 changes: 12 additions & 0 deletions unittests/CppInterOp/FunctionReflectionTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -799,6 +799,9 @@ TEST(FunctionReflectionTest, IsStaticMethod) {
TEST(FunctionReflectionTest, GetFunctionAddress) {
if (llvm::sys::RunningOnValgrind())
GTEST_SKIP() << "XFAIL due to Valgrind report";
#ifdef _WIN32
GTEST_SKIP() << "Disabled on Windows. Needs fixing.";
#endif
std::vector<Decl*> Decls, SubDecls;
std::string code = "int f1(int i) { return i * i; }";

Expand Down Expand Up @@ -1075,6 +1078,10 @@ TEST(FunctionReflectionTest, GetFunctionArgDefault) {
TEST(FunctionReflectionTest, Construct) {
if (llvm::sys::RunningOnValgrind())
GTEST_SKIP() << "XFAIL due to Valgrind report";
#ifdef _WIN32
GTEST_SKIP() << "Disabled on Windows. Needs fixing.";
#endif

Cpp::CreateInterpreter();

Interp->declare(R"(
Expand Down Expand Up @@ -1111,6 +1118,11 @@ TEST(FunctionReflectionTest, Construct) {
TEST(FunctionReflectionTest, Destruct) {
if (llvm::sys::RunningOnValgrind())
GTEST_SKIP() << "XFAIL due to Valgrind report";

#ifdef _WIN32
GTEST_SKIP() << "Disabled on Windows. Needs fixing.";
#endif

Cpp::CreateInterpreter();

Interp->declare(R"(
Expand Down
12 changes: 12 additions & 0 deletions unittests/CppInterOp/InterpreterTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ TEST(InterpreterTest, DebugFlag) {
}

TEST(InterpreterTest, Evaluate) {
#ifdef _WIN32
GTEST_SKIP() << "Disabled on Windows. Needs fixing.";
#endif
if (llvm::sys::RunningOnValgrind())
GTEST_SKIP() << "XFAIL due to Valgrind report";
// EXPECT_TRUE(Cpp::Evaluate(I, "") == 0);
Expand All @@ -61,6 +64,9 @@ TEST(InterpreterTest, Evaluate) {
}

TEST(InterpreterTest, Process) {
#ifdef _WIN32
GTEST_SKIP() << "Disabled on Windows. Needs fixing.";
#endif
if (llvm::sys::RunningOnValgrind())
GTEST_SKIP() << "XFAIL due to Valgrind report";
Cpp::CreateInterpreter();
Expand Down Expand Up @@ -99,6 +105,9 @@ TEST(InterpreterTest, DetectResourceDir) {
#else
TEST(InterpreterTest, DISABLED_DetectResourceDir) {
#endif // LLVM_BINARY_DIR
#ifdef _WIN32
GTEST_SKIP() << "Disabled on Windows. Needs fixing.";
#endif
Cpp::CreateInterpreter();
EXPECT_STRNE(Cpp::DetectResourceDir().c_str(), Cpp::GetResourceDir());
llvm::SmallString<256> Clang(LLVM_BINARY_DIR);
Expand All @@ -108,6 +117,9 @@ TEST(InterpreterTest, DISABLED_DetectResourceDir) {
}

TEST(InterpreterTest, DetectSystemCompilerIncludePaths) {
#ifdef _WIN32
GTEST_SKIP() << "Disabled on Windows. Needs fixing.";
#endif
std::vector<std::string> includes;
Cpp::DetectSystemCompilerIncludePaths(includes);
EXPECT_FALSE(includes.empty());
Expand Down
3 changes: 3 additions & 0 deletions unittests/CppInterOp/JitTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ static int printf_jit(const char* format, ...) {
TEST(JitTest, InsertOrReplaceJitSymbol) {
if (llvm::sys::RunningOnValgrind())
GTEST_SKIP() << "XFAIL due to Valgrind report";
#ifdef _WIN32
GTEST_SKIP() << "Disabled on Windows. Needs fixing.";
#endif
std::vector<Decl*> Decls;
std::string code = R"(
extern "C" int printf(const char*,...);
Expand Down
8 changes: 7 additions & 1 deletion unittests/CppInterOp/VariableReflectionTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,10 @@ TEST(VariableReflectionTest, GetDatamembers) {
EXPECT_EQ(datamembers.size(), 3);
EXPECT_EQ(datamembers1.size(), 0);
}

// If on Windows disable warning due to unnamed structs/unions in defined CODE.
#ifdef _WIN32
#pragma warning(disable : 4201)
#endif
#define CODE \
struct Klass1 { \
Klass1(int i) : num(1), b(i) {} \
Expand Down Expand Up @@ -131,6 +134,9 @@ TEST(VariableReflectionTest, DatamembersWithAnonymousStructOrUnion) {
((intptr_t) & (k3.c)) - ((intptr_t) & (k3.num)));
EXPECT_EQ(Cpp::GetVariableOffset(datamembers_klass3[4]),
((intptr_t) & (k3.num2)) - ((intptr_t) & (k3.num)));
#ifdef _WIN32
#pragma warning(default : 4201)
#endif
}

TEST(VariableReflectionTest, LookupDatamember) {
Expand Down

0 comments on commit c0a4dfe

Please sign in to comment.