-
Notifications
You must be signed in to change notification settings - Fork 681
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
Bump macos version #1609
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
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
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@egecetin also bump the packaging please |
@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. |
I need help for gradle. I don't have experience with android builds unfortunately |
This reverts commit ae0fc43.
The error is caused by this:
cc @seladb |
@tigercosmos This is because new MacOS comes with Java 21 -> need to upgrade graddle |
@egecetin I would be in favor to keep both sync no? If we test under macos15 then why not releasing a package for MacOS15 ? |
@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 |
I think we just missed them. Let's fix it. |
There was a problem hiding this 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
.github/workflows/build_and_test.yml
Outdated
- 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 |
There was a problem hiding this comment.
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
?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
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.
Macos 12 is deprecated. Replace it with macos-15.
actions/runner-images#10721