Update macos and ubuntu ci to be more extensive #173

mcbarton · 2023-12-19T20:22:53Z

Added section that will run tests on arm osx once free tier github runner available (currently commented out)
Build cppyy and tests on macos as well as linux, and where CppInterOp uses clang-repl as well as cling
Use homebrew provided clang rather than apple provided clang
Test for both clang-repl 16 and 17 for both macos and linux
Update ubuntu runner to be consistent across all Ubuntu runners

codecov · 2023-12-19T21:24:03Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (cf18068) 72.75% compared to head (2d93347) 77.55%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #173      +/-   ##
==========================================
+ Coverage   72.75%   77.55%   +4.79%     
==========================================
  Files           8        8              
  Lines        2885     2887       +2     
==========================================
+ Hits         2099     2239     +140     
+ Misses        786      648     -138

Files	Coverage Δ
lib/Interpreter/CppInterOpInterpreter.h	`92.77% <100.00%> (+17.77%)`	⬆️

... and 5 files with indirect coverage changes

Files	Coverage Δ
lib/Interpreter/CppInterOpInterpreter.h	`92.77% <100.00%> (+17.77%)`	⬆️

... and 5 files with indirect coverage changes

mcbarton · 2023-12-19T21:32:57Z

Issues that have shown as a result of the CI running is
ubu22-gcc12-clang-repl-17 fails, but ubu22-gcc12-clang-repl-17-cppyy fails (not entirely sure why, and will look into)

cling based ci runs fails. Unsure why. Only noticed difference from quick glance is that previous run of ci was using cached llvm. This run built from scratch. Any details on cached cling and llvm-project it uses would be useful.

@alexander-penev @vgvassilev I thought from this readme that there was a smaller m1 github runner https://github.com/actions/runner-images/blob/main/images/macos/macos-13-arm64-Readme.md . Given it keeps waiting for runner then it may not exist. Based on this thread then the only github m1 runners may currently be the xl variants actions/runner-images#8439 which cost money. I can change it to this runner if the project is willing to bare the cost until there is a free one. Let me know what you want me to do

It's also worth noting that my ci update was to use macos version 13, but come January there are plans for a beta which uses macos version 14 (latest version). see actions/runner-images#7508

mcbarton · 2023-12-19T21:36:40Z

More details on the larger macos runners if you decide to proceed with that route for now https://docs.github.com/en/actions/using-github-hosted-runners/about-larger-runners/about-larger-runners . I know from issue #138 that it wasn't expected to be necessary due to llvm caching.

mcbarton · 2023-12-20T12:37:19Z

By fixing the version of cling in CI to the latest version, rather than using the latest commit I have now been able to have the ci pass for cling based jobs. It is currently hardcoded, but it future the version can be chosen against the version of clang cling is being built alongside.

ubu22-gcc12-clang-repl-17 failing doesn't appear to have anything to do with the changes I have made for this PR.

Now this PR is just waiting on a decision as to what arm based macos runner should it be run on (i.e. the xlarge ones which currently cost money to run on, or wait and see if a free variant is made available)

vgvassilev · 2023-12-20T13:16:15Z

The cache system requires one successful build of llvm and then the rest is built on top. Could we employ that to avoid the costs of M1?

mcbarton · 2023-12-20T13:22:35Z

The cache system requires one successful build of llvm and then the rest is built on top. Could we employ that to avoid the costs of M1?

I don't think you can completely avoid the costs of the M1, but you can certainly limit the costs, by using your cache (so run time minutes are low for most runs, and only high if cache needs updating), and have it only run the macos tests by manually activating them instead of automatically like currently. For example you only run it once you are satisfied that the ubuntu tests are all ok for a PR.

mcbarton · 2023-12-20T13:27:16Z

The only time I can think of that a new llvm cache would need creating for the M1 ci runs is when a new patch is added to the project, so that the ci can test with the new patch.

mcbarton · 2023-12-21T09:06:39Z

@vgvassilev I managed to find this message actions/runner-images#8439 (comment) which is relevant. Github plan to offer a free tier for M1 sometime next year (it says the date is to be decided, but they are still ramping up their hardware acquisition until about March 2024). My suggestion is I update this PR to have extensive tests for macos x86, which should catch any ci issues with macos builds. I would comment out the arm based jobs for now until the date for the free tier of m1 runners has been announced, and the correct runner key can be added. I'd update the commit message and PR name accordingly. What do you think?

…eing, until free m1 github runner tier available)

mcbarton · 2023-12-21T17:59:01Z

@vgvassilev I managed to find this message actions/runner-images#8439 (comment) which is relevant. Github plan to offer a free tier for M1 sometime next year (it says the date is to be decided, but they are still ramping up their hardware acquisition until about March 2024). My suggestion is I update this PR to have extensive tests for macos x86, which should catch any ci issues with macos builds. I would comment out the arm based jobs for now until the date for the free tier of m1 runners has been announced, and the correct runner key can be added. I'd update the commit message and PR name accordingly. What do you think?

Implementing this suggestion has raised the following issues with ci in this PR

Forgot to change the following cppyy on this line to on
https://github.com/mcbarton/CppInterOp/blob/ec4ac34dd8fcf707e845175eea866e5012828e00/.github/workflows/ci.yml#L114
Similarly need to change cppyy on this to on
https://github.com/mcbarton/CppInterOp/blob/ec4ac34dd8fcf707e845175eea866e5012828e00/.github/workflows/ci.yml#L159C24-L159C24
Issue with Xcode is stopping the following line passing without error
https://github.com/mcbarton/CppInterOp/blob/ec4ac34dd8fcf707e845175eea866e5012828e00/.github/workflows/ci.yml#L454

Once I determine how to solve the xcode related issue I will push a commit solving these issues

… pass cppyy tests

mcbarton · 2023-12-21T18:54:12Z

@alexander-penev I consider this PR ready for review. The issue which stops osx-x86-clang-clang-repl-17-cppyy and osx-x86-clang-clang-repl-16-cppyy passing although appears to be to do with the xcode on the runner, it doesn't appear to be due to this from further inspection, since it passes when CppInterOp uses cling instead of clang-repl. I plan to remove the line
sudo xcode-select -s /Applications/Xcode_15.1.app/Contents/Developer
from the ci once any issues you raise are fixed, since this xcode isn't available yet despite being in the readme.

Add a interpreter-specific overload of operator new for C++.

github-actions · 2023-12-22T13:35:04Z

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

github-actions · 2023-12-22T13:59:05Z

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

mcbarton · 2023-12-22T15:14:39Z

@vgvassilev thanks for the suggestion of what was stopping the clang repl cppyy ci jobs from passing, and @alexander-penev for implementing this fix. Do the commits just need to be squashed now, or is this PR in need of further review?

alexander-penev · 2023-12-22T16:48:16Z

.github/workflows/ci.yml

          - name: ubu22-gcc9-clang-repl-16
-            os: ubuntu-22.04
+            os: ubuntu-latest


If not explicitly required ubuntu-latest, it is better to keep the specific version. Makes the matrix more stable for github changes.

At the time the ci had both ubuntu-22.04 and ubuntu-latest . I didn't know which one to pick. I should have realised from the name. I've now made this change.

alexander-penev · 2023-12-22T16:50:58Z

.github/workflows/ci.yml

+          #Block commented out until free tier for m1
+          #exists (expected sometime 2024) and key for os 
+          #can be replaced
+          #- name: osx-arm-clang-clang-repl-17


Maybe in the name it is better to put arm64 instead of just arm.

I've now made this change, as well as change osx to osx13 to make it clear its running on macos version 13.

…ck to ubuntu 22.04 for all cases

alexander-penev · 2023-12-22T17:15:38Z

.github/workflows/ci.yml

+          #FIXME: Need latest version of cling not hardcoded.
+          git clone https://github.com/root-project/cling.git
+          cd ./cling 
+          git checkout tags/v1.0


Instead of a hardcoded version of cling, we can add in the build matrix a new parameter for example cling-version:
- name: ubu22-gcc9-clang13-cling
os: ubuntu-22.04
compiler: gcc-9
clang-runtime: '13'
cling: On
cling-version: '1.0'
...

And then this is written as:
git checkout tags/v${{ matrix.cling-version }}

Done this change.

github-actions · 2023-12-22T17:20:21Z

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

github-actions · 2023-12-22T17:26:31Z

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

alexander-penev · 2023-12-22T17:29:12Z

.github/workflows/ci.yml

        cd cppyy-backend
        mkdir -p $CPPINTEROP_DIR/lib build && cd build
        # Install CppInterOp
        (cd $CPPINTEROP_BUILD_DIR && cmake --build . --target install --parallel $(nproc --all))
        # Build and Install cppyy-backend
        cmake -DCppInterOp_DIR=$CPPINTEROP_DIR ..
        cmake --build . --parallel $(nproc --all)
+        OS=$(uname -s)
+        if [[ "$OS" == "Darwin" ]]; then
+        cp libcppyy-backend.dylib $CPPINTEROP_DIR/lib/


Please indent the command by 2 spaces.

alexander-penev · 2023-12-22T17:29:23Z

.github/workflows/ci.yml

+        OS=$(uname -s)
+        if [[ "$OS" == "Darwin" ]]; then
+        cp libcppyy-backend.dylib $CPPINTEROP_DIR/lib/
+        else
        cp libcppyy-backend.so $CPPINTEROP_DIR/lib/


Please indent the command by 2 spaces.

github-actions · 2023-12-22T17:37:45Z

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

mcbarton · 2023-12-22T18:58:38Z

@alexander-penev I just tried squashing the commits on my local machine, but it seems to remove you as a contributor when I do so. Can you tell me how I do this while keeping all the contributors in the squashed commit?

kgantchev · 2024-01-15T17:46:12Z

. I would comment out the arm based jobs for now until the date for the free tier of m1 runners has been announced, and the correct runner key can be added. I'd update the commit message and PR name accordingly. What do you think?

Might I make a suggestion here? Feel free to use FlyCI's M1 and M2 runners. Our runners are on average 2x faster and 2x cheaper than GitHub's AND we have a free tier for OSS projects (see below).

Install Instructrions

Easily replace your M1 runners:

jobs:
 ci:
-    runs-on: macos-latest
+    runs-on: flyci-macos-large-latest-m1
   steps:
   - name: 👀 Checkout repo
     uses: actions/checkout@v4

Or try the M2 runners:

jobs:
  ci:
-    runs-on: macos-latest
+    runs-on: flyci-macos-large-latest-m2
    steps:
      - name: 👀 Checkout repo
        uses: actions/checkout@v4

Pricing

Processor	vCPU	RAM (GB)	Storage	Label	Price on FlyCI	Price on GitHub
M1	4	7	28 GB	flyci-macos-large-latest-m1	$0.06	-
M1	8	14	28 GB	flyci-macos-xlarge-latest-m1	$0.12	$0.16
M2	4	7	28 GB	flyci-macos-large-latest-m2	$0.08	-
M2	8	14	28 GB	flyci-macos-xlarge-latest-m2	$0.16	-

500 mins/month Free for Public Repos

If your repo is public, then FlyCI offers 500 mins/month of free M1 runner usage with the flyci-macos-large-latest-m1 runner.

Best Regards,
Kiril Gantchev
CEO and co-founder of FlyCI

vgvassilev requested a review from alexander-penev December 19, 2023 20:53

Update CI to run on arm based macos system + run extra tests in CI

c797ab2

Update to have extensive osx x86 tests (turned off arm for the time b…

ec4ac34

…eing, until free m1 github runner tier available)

Fix cppyy issue for cling based runs + try latest version of xcode to…

bb40e41

… pass cppyy tests

mcbarton changed the title ~~Update CI to run on arm based macos system + run extra tests in CI~~ Update macos and ubuntu ci to be more extensive Dec 21, 2023

vgvassilev mentioned this pull request Dec 22, 2023

[clang-repl] Add a interpreter-specific overload of operator new for C++ llvm/llvm-project#76218

Merged

alexander-penev added 4 commits December 22, 2023 12:31

Create clang17-1-NewOperator.patch

a1e9a9c

Add a interpreter-specific overload of operator new for C++.

Update clang17-1-NewOperator.patch

c7f2345

Update CppInterOpInterpreter.h

ceda9e5

Fix typo in CppInterOpInterpreter.h

959d0e3

alexander-penev added 2 commits December 22, 2023 15:52

Temporary disable valgrind error

5a51735

Update clang17-1-NewOperator.patch

8ab6599

alexander-penev reviewed Dec 22, 2023

View reviewed changes

Update ci.yml to have better names for jobs, and reverse ubuntu os ba…

e0a013b

…ck to ubuntu 22.04 for all cases

alexander-penev reviewed Dec 22, 2023

View reviewed changes

Update ci.yml to include cling-version for jobs that require it

8b7ce8a

alexander-penev reviewed Dec 22, 2023

View reviewed changes

Update ci.yml to properly indent cp cppyy-backend library command

2d93347

alexander-penev approved these changes Dec 22, 2023

View reviewed changes

alexander-penev merged commit 7fc1874 into compiler-research:main Dec 22, 2023
16 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update macos and ubuntu ci to be more extensive #173

Update macos and ubuntu ci to be more extensive #173

mcbarton commented Dec 19, 2023 •

edited

Loading

codecov bot commented Dec 19, 2023 •

edited

Loading

mcbarton commented Dec 19, 2023 •

edited

Loading

mcbarton commented Dec 19, 2023

mcbarton commented Dec 20, 2023

vgvassilev commented Dec 20, 2023

mcbarton commented Dec 20, 2023

mcbarton commented Dec 20, 2023

mcbarton commented Dec 21, 2023

mcbarton commented Dec 21, 2023 •

edited

Loading

mcbarton commented Dec 21, 2023 •

edited

Loading

github-actions bot commented Dec 22, 2023

github-actions bot commented Dec 22, 2023

mcbarton commented Dec 22, 2023

alexander-penev Dec 22, 2023

mcbarton Dec 22, 2023

alexander-penev Dec 22, 2023

mcbarton Dec 22, 2023

alexander-penev Dec 22, 2023 •

edited

Loading

mcbarton Dec 22, 2023

github-actions bot commented Dec 22, 2023

github-actions bot commented Dec 22, 2023

alexander-penev Dec 22, 2023

mcbarton Dec 22, 2023

alexander-penev Dec 22, 2023

mcbarton Dec 22, 2023

github-actions bot commented Dec 22, 2023

mcbarton commented Dec 22, 2023

kgantchev commented Jan 15, 2024

Update macos and ubuntu ci to be more extensive #173

Update macos and ubuntu ci to be more extensive #173

Conversation

mcbarton commented Dec 19, 2023 • edited Loading

codecov bot commented Dec 19, 2023 • edited Loading

Codecov Report

mcbarton commented Dec 19, 2023 • edited Loading

mcbarton commented Dec 19, 2023

mcbarton commented Dec 20, 2023

vgvassilev commented Dec 20, 2023

mcbarton commented Dec 20, 2023

mcbarton commented Dec 20, 2023

mcbarton commented Dec 21, 2023

mcbarton commented Dec 21, 2023 • edited Loading

mcbarton commented Dec 21, 2023 • edited Loading

github-actions bot commented Dec 22, 2023

github-actions bot commented Dec 22, 2023

mcbarton commented Dec 22, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexander-penev Dec 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Dec 22, 2023

github-actions bot commented Dec 22, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Dec 22, 2023

mcbarton commented Dec 22, 2023

kgantchev commented Jan 15, 2024

Install Instructrions

Pricing

500 mins/month Free for Public Repos

mcbarton commented Dec 19, 2023 •

edited

Loading

codecov bot commented Dec 19, 2023 •

edited

Loading

mcbarton commented Dec 19, 2023 •

edited

Loading

mcbarton commented Dec 21, 2023 •

edited

Loading

mcbarton commented Dec 21, 2023 •

edited

Loading

alexander-penev Dec 22, 2023 •

edited

Loading