search

compiler-research / CppInterOp

A Clang-based C++ Interoperability Library
Other
38 stars 20 forks source link

[CI] Update CI to include a Windows build of CppInterOp + other general CI improvements #180

Closed mcbarton closed 5 months ago

mcbarton commented 5 months ago

PR to add CI build infrastructure + fixes https://github.com/compiler-research/CppInterOp/issues/182

codecov[bot] commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (16f1c4b) 77.56% compared to head (e765d75) 78.68%.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/compiler-research/CppInterOp/pull/180/graphs/tree.svg?width=650&height=150&src=pr&token=7UWTYSVVT5&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=compiler-research)](https://app.codecov.io/gh/compiler-research/CppInterOp/pull/180?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=compiler-research) ```diff @@ Coverage Diff @@ ## main #180 +/- ## ========================================== + Coverage 77.56% 78.68% +1.12% ========================================== Files 8 8 Lines 2897 3050 +153 ========================================== + Hits 2247 2400 +153 Misses 650 650 ``` [see 7 files with indirect coverage changes](https://app.codecov.io/gh/compiler-research/CppInterOp/pull/180/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=compiler-research) [see 7 files with indirect coverage changes](https://app.codecov.io/gh/compiler-research/CppInterOp/pull/180/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=compiler-research)
mcbarton commented 5 months ago

@alexander-penev @vgvassilev Currently I've got the Windows ci build upto the point where it needs to build CppInterOp , but it fails to find the llvm config file. I've tried a few different definitions of LLVM_DIR without success. Any ideas what the folder location should be on Windows?

A separate issue which also needs to be fixed is that currently for the Windows ci build LLVM_HASH and CLING_HASH are currently hardcoded (I have an idea for a fix, but have yet to test). After these 2 issues have been fixed then this project should have a working ci which shows that CppInterOp builds on Windows, assuming no other issues arise once LLVM_DIR issue is resolved.

mcbarton commented 5 months ago

@alexander-penev @vgvassilev Currently I've got the Windows ci build upto the point where it needs to build CppInterOp , but it fails to find the llvm config file. I've tried a few different definitions of LLVM_DIR without success. Any ideas what the folder location should be on Windows?

A separate issue which also needs to be fixed is that currently for the Windows ci build LLVM_HASH and CLING_HASH are currently hardcoded (I have an idea for a fix, but have yet to test). After these 2 issues have been fixed then this project should have a working ci which shows that CppInterOp builds on Windows, assuming no other issues arise once LLVM_DIR issue is resolved.

Update I have now solved the LLVM_DIR and Clang_DIR issue. Now I need to solve the following issue with the cmake configure stage of building CppInterOp

-- Looking for os_signpost_interval_begin
CMake Error at D:/a/CppInterOp/CppInterOp/build/CMakeFiles/CMakeScratch/TryCompile-j9evml/CMakeLists.txt:2 (set):
  Syntax error in cmake code at

    D:/a/CppInterOp/CppInterOp/build/CMakeFiles/CMakeScratch/TryCompile-j9evml/CMakeLists.txt:2

  when parsing string

    D:/a/CppInterOp/CppInterOp/cmake;D:/a/CppInterOp/CppInterOp/cmake/modules;D:/a/CppInterOp/CppInterOp/llvm-project/build/lib/cmake/llvm;D:/a/CppInterOp/CppInterOp/llvm-project/build/lib/cmake/llvm;D:\a\CppInterOp\CppInterOp\llvm-project\build\lib\cmake\llvm

  Invalid character escape '\a'.
vgvassilev commented 5 months ago

@alexander-penev @vgvassilev Currently I've got the Windows ci build upto the point where it needs to build CppInterOp , but it fails to find the llvm config file. I've tried a few different definitions of LLVM_DIR without success. Any ideas what the folder location should be on Windows? A separate issue which also needs to be fixed is that currently for the Windows ci build LLVM_HASH and CLING_HASH are currently hardcoded (I have an idea for a fix, but have yet to test). After these 2 issues have been fixed then this project should have a working ci which shows that CppInterOp builds on Windows, assuming no other issues arise once LLVM_DIR issue is resolved.

Update I have now solved the LLVM_DIR and Clang_DIR issue. Now I need to solve the following issue with the cmake configure stage of building CppInterOp

-- Looking for os_signpost_interval_begin
CMake Error at D:/a/CppInterOp/CppInterOp/build/CMakeFiles/CMakeScratch/TryCompile-j9evml/CMakeLists.txt:2 (set):
  Syntax error in cmake code at

    D:/a/CppInterOp/CppInterOp/build/CMakeFiles/CMakeScratch/TryCompile-j9evml/CMakeLists.txt:2

  when parsing string

    D:/a/CppInterOp/CppInterOp/cmake;D:/a/CppInterOp/CppInterOp/cmake/modules;D:/a/CppInterOp/CppInterOp/llvm-project/build/lib/cmake/llvm;D:/a/CppInterOp/CppInterOp/llvm-project/build/lib/cmake/llvm;D:\a\CppInterOp\CppInterOp\llvm-project\build\lib\cmake\llvm

  Invalid character escape '\a'.

Don't we need to use backslashes? Additionally, we can increase the 1 minute tmate session to more time here so that you can ssh into that machine and try things interactively.

mcbarton commented 5 months ago

@alexander-penev @vgvassilev Currently I've got the Windows ci build upto the point where it needs to build CppInterOp , but it fails to find the llvm config file. I've tried a few different definitions of LLVM_DIR without success. Any ideas what the folder location should be on Windows? A separate issue which also needs to be fixed is that currently for the Windows ci build LLVM_HASH and CLING_HASH are currently hardcoded (I have an idea for a fix, but have yet to test). After these 2 issues have been fixed then this project should have a working ci which shows that CppInterOp builds on Windows, assuming no other issues arise once LLVM_DIR issue is resolved.

Update I have now solved the LLVM_DIR and Clang_DIR issue. Now I need to solve the following issue with the cmake configure stage of building CppInterOp

-- Looking for os_signpost_interval_begin
CMake Error at D:/a/CppInterOp/CppInterOp/build/CMakeFiles/CMakeScratch/TryCompile-j9evml/CMakeLists.txt:2 (set):
  Syntax error in cmake code at

    D:/a/CppInterOp/CppInterOp/build/CMakeFiles/CMakeScratch/TryCompile-j9evml/CMakeLists.txt:2

  when parsing string

    D:/a/CppInterOp/CppInterOp/cmake;D:/a/CppInterOp/CppInterOp/cmake/modules;D:/a/CppInterOp/CppInterOp/llvm-project/build/lib/cmake/llvm;D:/a/CppInterOp/CppInterOp/llvm-project/build/lib/cmake/llvm;D:\a\CppInterOp\CppInterOp\llvm-project\build\lib\cmake\llvm

  Invalid character escape '\a'.

Don't we need to use backslashes? Additionally, we can increase the 1 minute tmate session to more time here so that you can ssh into that machine and try things interactively.

Truth be told I don't know. I never build on Windows, so most of this PR has been based on what I've read online about building on Windows. My guess is that its not supposed to be \ purely because it has no issue with any of the other path directories.

mcbarton commented 5 months ago

@alexander-penev @vgvassilev Currently I've got the Windows ci build upto the point where it needs to build CppInterOp , but it fails to find the llvm config file. I've tried a few different definitions of LLVM_DIR without success. Any ideas what the folder location should be on Windows? A separate issue which also needs to be fixed is that currently for the Windows ci build LLVM_HASH and CLING_HASH are currently hardcoded (I have an idea for a fix, but have yet to test). After these 2 issues have been fixed then this project should have a working ci which shows that CppInterOp builds on Windows, assuming no other issues arise once LLVM_DIR issue is resolved.

Update I have now solved the LLVM_DIR and Clang_DIR issue. Now I need to solve the following issue with the cmake configure stage of building CppInterOp

-- Looking for os_signpost_interval_begin
CMake Error at D:/a/CppInterOp/CppInterOp/build/CMakeFiles/CMakeScratch/TryCompile-j9evml/CMakeLists.txt:2 (set):
  Syntax error in cmake code at

    D:/a/CppInterOp/CppInterOp/build/CMakeFiles/CMakeScratch/TryCompile-j9evml/CMakeLists.txt:2

  when parsing string

    D:/a/CppInterOp/CppInterOp/cmake;D:/a/CppInterOp/CppInterOp/cmake/modules;D:/a/CppInterOp/CppInterOp/llvm-project/build/lib/cmake/llvm;D:/a/CppInterOp/CppInterOp/llvm-project/build/lib/cmake/llvm;D:\a\CppInterOp\CppInterOp\llvm-project\build\lib\cmake\llvm

  Invalid character escape '\a'.

Don't we need to use backslashes? Additionally, we can increase the 1 minute tmate session to more time here so that you can ssh into that machine and try things interactively.

Looking at the cmake configure stage for llvm on windows they have / not \ (example from ci shown below)

-- The C compiler identification is MSVC 19.37.32826.1
-- The CXX compiler identification is MSVC 19.37.32826.1
-- The ASM compiler identification is MSVC
-- Found assembler: C:/Program Files/Microsoft Visual Studio/2022/Enterprise/VC/Tools/MSVC/14.37.32822/bin/HostX64/x64/cl.exe
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: C:/Program Files/Microsoft Visual Studio/2022/Enterprise/VC/Tools/MSVC/14.37.32822/bin/HostX64/x64/cl.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: C:/Program Files/Microsoft Visual Studio/2022/Enterprise/VC/Tools/MSVC/14.37.32822/bin/HostX64/x64/cl.exe - skipped
mcbarton commented 5 months ago

I did a little research and found out that although Windows paths have \ as far as cmake as is concerned the paths should always use / , as in Windows a \ is likely to interpreted as escape sequence. So to solve the current issue it needs to be determined where in the cmake files the \ path is being included, and change it to use / (references below) https://discourse.cmake.org/t/window-env-variable-and-cmake-path/5410 https://stackoverflow.com/questions/28070810/cmake-generate-error-on-windows-as-it-uses-as-escape-seq

mcbarton commented 5 months ago

@vgvassilev I have now got past the parsing issue for the clang-repl builds of CppInterOp on Windows (worked around it by replacing \ with / in LLVM_DIR within cmakelists.txt). The issue with them now seems to be a few header files it can't find, and a few other errors . I think some more familiar with the CppInterOp code and Clang needs to fix those issues.

The cling build still seems to be having issues with finding the cling config file. Not entirely sure what is causing that issue.

vgvassilev commented 5 months ago

@vgvassilev I have now got past the parsing issue for the clang-repl builds of CppInterOp on Windows (worked around it by replacing \ with / in LLVM_DIR within cmakelists.txt). The issue with them now seems to be a few header files it can't find, and a few other errors . I think some more familiar with the CppInterOp code and Clang needs to fix those issues.

The cling build still seems to be having issues with finding the cling config file. Not entirely sure what is causing that issue.

We might need to include llvm/Support/Casting.h for the isa to work?

mcbarton commented 5 months ago

@vgvassilev can you delete the llvm cache from the windows build where cling was on by hand? Looking at the ci when it was last built the cling directory was incorrect, so it was never built, so that's why it can't be found.

vgvassilev commented 5 months ago

Done.

fsfod commented 5 months ago

I don't know how much use this, but been experimenting with getting windows to build. I have it successfully building with few things commented out. For dlfcn\dlopen i just used some of llvm support API's. You see my changes here https://github.com/fsfod/CppInterOp/commits/windows/

vgvassilev commented 5 months ago

@fsfod, this looks quite nice. Are you interested in moving some of that work upstream?

mcbarton commented 5 months ago

I don't know how much use this, but been experimenting with getting windows to build. I have it successfully building with few things commented out. For dlfcn\dlopen i just used some of llvm support API's. You see my changes here https://github.com/fsfod/CppInterOp/commits/windows/

Thanks. I would also appreciate if your work is moved upstream, like @vgvassilev asks.

vgvassilev commented 5 months ago

What’s the plan here? Do we want to merge that PR and move to the other one with the windows changes in the codebase?

mcbarton commented 5 months ago

What’s the plan here? Do we want to merge that PR and move to the other one with the windows changes in the codebase?

@vgvassilev Yes. That is the plan. I thought if this PR gets merged then the changes in the other PR can be checked. I've started the process of setting myself up a build environment on a local Windows machine, so I can try and help with the other PR. This PR will help to determine when the other PR is ready for merging I believe.

mcbarton commented 5 months ago

What’s the plan here? Do we want to merge that PR and move to the other one with the windows changes in the codebase?

@vgvassilev Yes. That is the plan. I thought if this PR gets merged then the changes in the other PR can be checked. I've started the process of setting myself up a build environment on a local Windows machine, so I can try and help with the other PR. This PR will help to determine when the other PR is ready for merging I believe.

Assuming your ok for this idea then this PR is ready for review.

mcbarton commented 5 months ago

Just realised looking at the ci that the patches are not applying on Windows. Looking into a possible solution. Not sure why clang-repl-16 jobs have suddenly started failing. Looking into this too.

mcbarton commented 5 months ago

@alexander-penev This PR is ready for review. Applying the patches to clang is currently hardcoded for Windows, but I will fix this in a later PR. I can't explain the spaces in the patch file, except for without them the patch will fail to apply on Windows (it will output that it finds the patch file corrupt every time it meets a blank line without them). The windows ci build is not mean to pass. This PR is supposed to put in a place a ci build for Windows, with the codebase changes in https://github.com/compiler-research/CppInterOp/pull/181 meant to help it pass.