Skip to content

Conversation

@mcbarton
Copy link
Collaborator

@mcbarton mcbarton commented Jan 28, 2024

@vgvassilev This PR allows for those trying to solve Windows build issues to use a different compiler to MSVC . With these changes I was able to compile CppInterOp and its tests using the Clang compiler. Currently I'm not passing the tests as the header files cannot be found, but I believe this has something to do with the environment variables I have set, and am investigating.

Fixes #175 (provides proof that earlier PR fixed it)

@codecov
Copy link

codecov bot commented Jan 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (e61e7ca) 78.83% compared to head (30c91dc) 78.83%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #195   +/-   ##
=======================================
  Coverage   78.83%   78.83%           
=======================================
  Files           8        8           
  Lines        3048     3048           
=======================================
  Hits         2403     2403           
  Misses        645      645           
Files Coverage Δ
lib/Interpreter/CppInterOp.cpp 86.31% <ø> (ø)
Files Coverage Δ
lib/Interpreter/CppInterOp.cpp 86.31% <ø> (ø)

@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@vgvassilev
Copy link
Contributor

Can we use continue-on-error in the yml file instead?

@mcbarton
Copy link
Collaborator Author

mcbarton commented Jan 28, 2024

@vgvassilev So just to be certain before I make the change, you want me to reactivate the other Windows builds, but add continue-on-error: true to the 'Build and Test/Install CppInterOp on Windows systems' part of the jobs?

@vgvassilev
Copy link
Contributor

Yes, assuming we still get some green/ or different than X build sign.

@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@mcbarton
Copy link
Collaborator Author

mcbarton commented Jan 28, 2024

@vgvassilev After that change the Windows builds now get a green tick. I have added a comment to this section that tests are expected to fail until #188 is resolved.

@mcbarton
Copy link
Collaborator Author

@vgvassilev Can this PR be merged now that is gets a green tick for all the jobs?

@github-actions
Copy link
Contributor

github-actions bot commented Feb 2, 2024

clang-tidy review says "All clean, LGTM! 👍"

1 similar comment
@github-actions
Copy link
Contributor

github-actions bot commented Feb 2, 2024

clang-tidy review says "All clean, LGTM! 👍"

@github-actions
Copy link
Contributor

github-actions bot commented Feb 2, 2024

clang-tidy review says "All clean, LGTM! 👍"

@mcbarton
Copy link
Collaborator Author

mcbarton commented Feb 2, 2024

@alexander-penev can you review this PR for me? The purpose of the PR is to do 3 things

  1. Enable the ability to build CppInterOp on Windows with a different compiler to MSVC
  2. Add 'continue on error' for CppInterOp build on Windows until codebase can be changed for tests to pass
  3. Add osx 14 + osx arm build to CI (cc: @vgvassilev new addition)

Copy link
Contributor

@vgvassilev vgvassilev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thank you!

@vgvassilev vgvassilev merged commit 5953058 into compiler-research:main Feb 2, 2024
@mcbarton mcbarton deleted the windows branch February 12, 2024 19:29
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

Successfully merging this pull request may close these issues.

Clang-repl=ON issue for both clang 16 & 17 on Apple Silicon for cppyy

2 participants