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

Test compilers on MacOs and Windows #53

Closed
wants to merge 9 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ env:
# Modify these variables for upstream package versions - do NOT hardcode the version
# anywhere else!
OPENFHE_VERSION: '1.1.4'
LIBCXXWRAP_JULIA_VERSION: '0.12.2'
FORCE_CXXWRAP_JL_VERSION: '0.15' # Use only briefly during transition (default: '')
LIBCXXWRAP_JULIA_VERSION: '0.13.0'
FORCE_CXXWRAP_JL_VERSION: '0.16' # Use only briefly during transition (default: '')

jobs:
test:
Expand Down Expand Up @@ -63,10 +63,10 @@ jobs:
steps:
- name: Checkout repository
uses: actions/checkout@v4
- uses: msys2/setup-msys2@v2
- uses: msys2/setup-msys2@v2.22.0
Copy link
Collaborator Author

@ArseniyKholod ArseniyKholod Jun 26, 2024

Choose a reason for hiding this comment

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

Hi, @sloede,
I try to downgrade setup-msys2 here to get older version of msys2, because it is not possible to choose gcc version within msys2, only by installing older version of msys2 itself. I'm not really good in CI stuff, I used msys2/[email protected], but it still uses newest version of gcc. If you have experience with downgrading GitHub Action version, am I doing it right? If you haven't, do not worry, I will find out this

Copy link
Member

Choose a reason for hiding this comment

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

using msys2/[email protected] will only change the version of the GitHub action script, i.e., the version of https://github.com/msys2/setup-msys2. What you really want is a specific version of mingw-w64-x86_64-toolchain in line 70 below

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

older msys2/[email protected] installs older msys2 with older gcc, I recognized, that there is an update: true below, now I was able to install older version of msys with gcc 13.2, should work

Copy link
Member

Choose a reason for hiding this comment

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

However, if this only affects Windows testing, I'd be ok with accepting a failing windows test, if it only concerns the self-built version of openfhe and not the Yggdrasil-provided ones

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Am I right, that we are not choosing gcc version for Yggdrasil, is it done automatically?

Copy link
Member

Choose a reason for hiding this comment

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

older msys2/[email protected] installs older msys2 with older gcc, I recognized, that there is an update: true below, now I was able to install older version of msys with gcc 13.2, should work

Wow, this is great! In that case, maybe just do not update for now, but add a TODO comment that we should fix this once OpenFHE 1.2.1 is available and hopefully fixes this issue.

Copy link
Member

Choose a reason for hiding this comment

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

Am I right, that we are not choosing gcc version for Yggdrasil, is it done automatically?

Yes. I think we specify a rather old version usually to ensure compatibility the greatest possible variety of GCC versions.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

older msys2/[email protected] installs older msys2 with older gcc, I recognized, that there is an update: true below, now I was able to install older version of msys with gcc 13.2, should work

Wow, this is great! In that case, maybe just do not update for now, but add a TODO comment that we should fix this once OpenFHE 1.2.1 is available and hopefully fixes this issue.

Okay! I'll do!

if: ${{ matrix.os == 'windows-latest' }}
with:
update: true
update: false
install: git base-devel mingw-w64-x86_64-toolchain mingw-w64-x86_64-cmake
- name: Install OpenMP
if: ${{ matrix.os == 'macos-latest' }}
Expand Down