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

Update action to use native bmake if possible #1452

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kevemueller
Copy link

On MacOS, native bmake works fine, refactor the environment to be able to use it.
While here, update to the latest ubuntu version, clang versions and clean-up YAML formatting.

On MacOS, native bmake works fine, refactor the environment
to be able to use it.
While here, update to the latests ubuntu version, clang
versions.
@kevemueller
Copy link
Author

Using private email now 72columns commit message.

- name: make buildkernel
run: ./tools/build/make.py --debug $EXTRA_BUILD_ARGS TARGET=${{ matrix.target }} TARGET_ARCH=${{ matrix.target_arch }} KERNCONF=GENERIC NO_MODULES=yes buildkernel -s -j$NPROC $EXTRA_MAKE_ARGS
run: ${{ matrix.bmake }} TARGET=${{ matrix.target }} TARGET_ARCH=${{ matrix.target_arch }} -s -j$NPROC buildkernel KERNCONF=GENERIC NO_MODULES=yes
Copy link
Member

@bsdimp bsdimp Dec 17, 2024

Choose a reason for hiding this comment

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

Why eliminate EXTRA_MAKE_ARGS here?

Copy link
Author

Choose a reason for hiding this comment

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

EXTRA_MAKE_ARGS was referenced in the original script, it was never set to any value, it could not be set in the call environment. I have left the echo and would remove that in a subsequent change, could be done right away.
I found it confusing to have to find what EXTRA_MAKE_ARGS in this context is actually used for, hence I removed the reference.

include:
# TODO: both Ubuntu and macOS have bmake packages, we should try them instead of bootstrapping our own copy.
- os: ubuntu-22.04
Copy link
Member

Choose a reason for hiding this comment

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

This also seems to eliminate ubuntu-22 from the matrix. Is that intentional?

Copy link
Author

Choose a reason for hiding this comment

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

Yes. ubuntu-20 will be kicked out in 4 months (actions/runner-images#11101), ubuntu-latest has started to default to ubuntu-24 (actions/runner-images#10636).
ubuntu-22 is still good for another 2 years, there is no obstacle to put it back, I have honestly not seen any added value to having it around.
Happy to change this.
If you want maximum coverage, we could also add macos-13 (Darwin/amd64).

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

Successfully merging this pull request may close these issues.

2 participants