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

workflows: macos: Add workflow for MacOS builds on PRs #9386

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

robskillington
Copy link

@robskillington robskillington commented Sep 14, 2024

Adding regular MacOS builds for PRs to avoid frequently breaking MacOS builds.

There have been several recent regressions with MacOS builds. These failures have not always effected MacoS 14 but sometimes just MacOS 12 for instance. These still should be caught before PRs are merged given Windows builds are tested before merge on each PR.

Fixes #9004 which was filed because this seems to regularly occur. I think we can close out that issue and be confident it won't come back if we merge this and perform MacOS builds before each PR is merged.

For related fixes where main was broken for some period of time, see the extra out of band work that was performed adhoc (since these failures are not checked before PR merge):


Enter [N/A] in the box, if an item is not applicable to your change.

Testing
Before we can approve your change; please submit the following in a comment:

  • Example configuration file for the change
  • Debug log output from testing the change
  • Attached Valgrind output that shows no leaks or memory corruption was found

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.

  • Run local packaging test showing all targets (including any new ones) build.
  • Set ok-package-test label to test for all targets (requires maintainer to do).

Documentation

  • Documentation required for this feature

Backporting

  • Backport to latest stable release.

Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

Copy link
Contributor

@patrick-stephens patrick-stephens left a comment

Choose a reason for hiding this comment

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

We can test a full package build already via label.

I think the concern of doing it every time is the cost and time of the builds.

@edsiper I think we should pick a subset to do always or just always do the packaging build rather than keep adding workflows?

@cosmo0920
Copy link
Contributor

We already have a macOS workflow which has unit-testing: https://github.com/fluent/fluent-bit/blob/master/.github/workflows/unit-tests.yaml#L80
Is there any lack of mechanism to detect failing of macOS builds in this workflow?
https://github.com/fluent/fluent-bit/blob/master/.github/workflows/unit-tests.yaml

@patrick-stephens
Copy link
Contributor

We already have a macOS workflow which has unit-testing: https://github.com/fluent/fluent-bit/blob/master/.github/workflows/unit-tests.yaml#L80 Is there any lack of mechanism to detect failing of macOS builds in this workflow? https://github.com/fluent/fluent-bit/blob/master/.github/workflows/unit-tests.yaml

The only thing I can see with this is we generally just ignore failing there at the moment as the tests flake. But yeah we do build for multiple macOS variants already if unit tests pass for Linux first.

I'm all for more tests around this but we should update the others I think as well, e.g. if we're doing a macOS build always then we can remove it from the larger full package test triggered by a label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build on macOS fails
3 participants