open-quantum-safe / liboqs

C library for prototyping and experimenting with quantum-resistant cryptography
https://openquantumsafe.org/
Other
1.82k stars 447 forks source link

CI: macOS build failures #1804

Open planetf1 opened 4 months ago

planetf1 commented 4 months ago

Currently macOS builds are failing

Screenshot 2024-05-31 at 11 09 48
For macOS 14 : Component Working Failed
runner 2.316.1 2.316.1
Xcode 15.0.1 15.0.1
gcc Apple 13.2.0 gcc 13.3.0
example log https://github.com/open-quantum-safe/liboqs/actions/runs/9211032819/job/25339585288 https://github.com/open-quantum-safe/liboqs/actions/runs/9309161112/job/25624155404

For example

planetf1 commented 4 months ago

I've created a DRAFT PR as a TEST which replaces gcc with 13.2.0 to validate if this is the cause.

planetf1 commented 4 months ago

Additional commit reverts the version reversion ...

This seems to prove the cause.

Also note that some annotations are added - just due to stderr when installing direct using ruby. Cosmetic, can be cleaned up if we want to go with a workaround

[macos (macos-14, -DOQS_USE_OPENSSL=OFF)](https://github.com/open-quantum-safe/liboqs/actions/runs/9317181419/job/25647067308#step:4:27)
Failed to load cask: gcc@13.rb Cask 'gcc@13' is unreadable: wrong constant name #<Class:0x000000014973f790>
SWilson4 commented 4 months ago

Thanks for looking into this, @planetf1. The doxygen error should be resolved by syncing your fork so that it includes commit a23046ffcea9b16b6bf9e2a4dc9c045316134dc2.

I am OK with merging a workaround, as this does seem like a non-liboqs problem. However, I feel that we should leave this issue (or another one) open as a reminder to remove the workaround whenever possible.

planetf1 commented 4 months ago

Thanks @SWilson4 thought I was up to date. pushed update now, so hopefully will be cleaner (except the annotations)

planetf1 commented 4 months ago

My initial reason for doing this was to at least understand why the failure. Perhaps we could discuss in the dev call next week & decide if we want the workaround. If so we could use as-is (perhaps squash) or also cleanup the warnings (redirect!)

bhess commented 4 months ago

Thanks @planetf1 for investigating and adding the workaround!

planetf1 commented 4 months ago

@bhess My current plan is to share the findings at the OQS status call so we can discuss/agree. However if there is critical mass of agreement we could merge ahead of time. Another option to consider is moving to a later gcc version.

baentsch commented 3 months ago

My current plan is to share the findings at the OQS status call so we can discuss/agree.

Sharing findings only at status calls is suboptimal for several reasons: 1) It slows down development 2) It robs non-participants of the option to check your findings 3) It eliminates the option for users to search github for "previously known issues" and learn from them

--> I'd strongly suggest always creating PRs that others can see and comment on. If they do not get merged, at least others (and posteriority) can see and learn; if they get merged, so much the better.

What I find troublesome is that these build failures did not get visible at top-level CI reporting (GH CI report not shown): Created #1827 to track.

planetf1 commented 3 months ago

@baentsch very much agree with you on ensuring we always share info through the github issues for all of those reasons - the status call should only be in addition, and as a reminder to review the comment. Hopefully the text here, did address that.

Good catch on addressing the status reporting.