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

Bump macos version #1609

Merged
merged 14 commits into from
Oct 14, 2024
Merged

Bump macos version #1609

merged 14 commits into from
Oct 14, 2024

Conversation

egecetin
Copy link
Collaborator

@egecetin egecetin commented Oct 5, 2024

Macos 12 is deprecated. Replace it with macos-15.

actions/runner-images#10721

@egecetin egecetin requested a review from seladb as a code owner October 5, 2024 14:24
@egecetin egecetin added the CI label Oct 5, 2024
Copy link

codecov bot commented Oct 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.18%. Comparing base (db905ad) to head (70e26c3).
Report is 1 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #1609      +/-   ##
==========================================
- Coverage   83.23%   83.18%   -0.05%     
==========================================
  Files         276      276              
  Lines       46883    46880       -3     
  Branches     9298     9280      -18     
==========================================
- Hits        39023    38998      -25     
- Misses       7008     7059      +51     
+ Partials      852      823      -29     
Flag Coverage Δ
fedora40 74.87% <ø> (+<0.01%) ⬆️
macos-12 ?
macos-13 80.62% <ø> (ø)
macos-14 80.62% <ø> (+0.07%) ⬆️
macos-15 80.59% <ø> (?)
mingw32 70.82% <ø> (-0.05%) ⬇️
mingw64 70.80% <ø> (-0.03%) ⬇️
npcap 85.20% <ø> (-0.02%) ⬇️
rhel94 74.72% <ø> (-0.01%) ⬇️
ubuntu2004 57.99% <ø> (ø)
ubuntu2004-zstd 58.11% <ø> (-0.03%) ⬇️
ubuntu2204 74.61% <ø> (-0.04%) ⬇️
ubuntu2204-icpx 58.92% <ø> (+0.03%) ⬆️
ubuntu2404 74.87% <ø> (-0.04%) ⬇️
unittest 83.18% <ø> (-0.05%) ⬇️
windows-2019 85.24% <ø> (-0.02%) ⬇️
windows-2022 85.26% <ø> (-0.02%) ⬇️
winpcap 85.22% <ø> (ø)
xdp 49.73% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@clementperon
Copy link
Collaborator

@egecetin also bump the packaging please

@egecetin
Copy link
Collaborator Author

egecetin commented Oct 5, 2024

@clementperon Packaging already runs on MacOS 14. There is no need to change at least for now. But looks like Android runs on MacOS which i forgot to update.

@egecetin
Copy link
Collaborator Author

egecetin commented Oct 5, 2024

I need help for gradle. I don't have experience with android builds unfortunately

This reverts commit ae0fc43.
@tigercosmos
Copy link
Collaborator

The error is caused by this:

  General error during conversion: Unsupported class file major version 65
  
  java.lang.IllegalArgumentException: Unsupported class file major version 65

cc @seladb

@clementperon
Copy link
Collaborator

clementperon commented Oct 6, 2024

The error is caused by this:

  General error during conversion: Unsupported class file major version 65
  
  java.lang.IllegalArgumentException: Unsupported class file major version 65

cc @seladb

@tigercosmos This is because new MacOS comes with Java 21 -> need to upgrade graddle

@clementperon
Copy link
Collaborator

clementperon commented Oct 6, 2024

@clementperon Packaging already runs on MacOS 14. There is no need to change at least for now. But looks like Android runs on MacOS which i forgot to update.

@egecetin I would be in favor to keep both sync no? If we test under macos15 then why not releasing a package for MacOS15 ?

@egecetin
Copy link
Collaborator Author

egecetin commented Oct 6, 2024

@clementperon Sure, sync is better I'll update it. But is there any idea why older versions (12, 13) removed earlier or not added, @seladb maybe for some reason ? I mean only move 14 to 15 or add 15

@tigercosmos
Copy link
Collaborator

maybe for some reason ?

I think we just missed them. Let's fix it.

Copy link
Owner

@seladb seladb left a comment

Choose a reason for hiding this comment

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

Please see one comment, otherwise LGTM

Comment on lines 635 to 639
- run-on-os: macos-15
target: arm64-v8a
cmake_configure: "-DCMAKE_OSX_ARCHITECTURES=arm64"
api-version: 30
- run-on-os: macos-12
- run-on-os: macos-15
Copy link
Owner

Choose a reason for hiding this comment

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

To unblock this PR until we figure out the Android build, can we change it to a working version, maybe macos-13 or macos-14?

Copy link
Collaborator Author

@egecetin egecetin Oct 13, 2024

Choose a reason for hiding this comment

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

@seladb No luck even macos-13 fails but I just noticed, android packaging only runs with ubuntu-latest for different target architectures. What about just testing with ubuntu and targeting archs also?

Copy link
Owner

@seladb seladb Oct 13, 2024

Choose a reason for hiding this comment

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

So maybe move it back to macos 12 until we figure it out?

Copy link
Collaborator

@clementperon clementperon Oct 13, 2024

Choose a reason for hiding this comment

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

@seladb I have open a MR to bump gradle https://github.com/seladb/ToyVpn-PcapPlusPlus/pull/10/files

This is the latest gradle version that support Android SDK 30.
But I think we should consider bumping ToyVpn SDK target version

Copy link
Owner

Choose a reason for hiding this comment

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

@clementperon unfortunately ToyVpn doesn't have CI so the only way to know if your PR works is use the PR branch in PcapPlusPlus CI. It should be pretty easy to do using this change in build_and_test.yml:

- name: Checkout ToyVpn-PcapPlusPlus
   uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # master
   with:
     repository: clementperon/ToyVpn-PcapPlusPlus
     ref: bump_gradle
     path: ./ToyVpn-PcapPlusPlus
     submodules: true

BTW, I'm considering re-writing the ToyVpn example using modern Kotlin and build tools, but this will take time because I'm not familiar with Android development...

Copy link
Collaborator Author

@egecetin egecetin Oct 14, 2024

Choose a reason for hiding this comment

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

@seladb Sure I'll move it to MacOS-12 for now. We still have approx two months for MacOS-12 until completely removed. Only reminder is beware of brownouts. It might fail during some runs.

@egecetin egecetin merged commit 2eb3fc6 into seladb:dev Oct 14, 2024
71 checks passed
@egecetin egecetin deleted the bump-macos branch October 14, 2024 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants