-
Notifications
You must be signed in to change notification settings - Fork 530
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
Fix broken build on Alpine #4264
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4264 +/- ##
==========================================
- Coverage 85.77% 83.51% -2.27%
==========================================
Files 56 56
Lines 15382 15382
==========================================
- Hits 13194 12846 -348
- Misses 2188 2536 +348 ☔ View full report in Codecov by Sentry. |
This is still not enough. Since the build kind of assumes Ubuntu as the only distro and enables XDP everywhere except Ubuntu 20.04: msquic/src/platform/CMakeLists.txt Line 38 in 5052f34
Which leads to:
on Alpine. |
4dcaa92
to
146d3a0
Compare
Thanks for the change. As far as I remember, our docker environment doesn't have /etc/os-release. let me double check |
Exists!😵💫 |
89d05b0
to
aff5c5f
Compare
lsb-release is not present on Alpine, whereas os-release is on most distributions.
I renamed the parameter, updated the pipeline (hopefully in the right place) and removed |
What should we do for the Official, OneBranch builds? Enabled or disabled? |
rerunning BVT xdp test... |
Could you update Linux XDP documentation? I believe build.ps1 requires additional parameter https://github.com/microsoft/msquic/blob/main/docs/BUILD.md#linux-xdp |
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.
LGTM
BTW, I don't have rights to merge this, so unless there is more feedback to address (let me know if there still something is), feel free to merge on my behalf. And thanks all for help with this! |
lsb-release is not present on Alpine, whereas os-release is on most distributions.