-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
base: main
Are you sure you want to change the base?
Conversation
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.
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 |
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.
Why eliminate EXTRA_MAKE_ARGS here?
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.
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 |
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.
This also seems to eliminate ubuntu-22 from the matrix. Is that intentional?
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.
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).
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.
We'll have to ask @jrtc27 to see which ones we can drop, since she added this originally.
I think that ubuntu-20 is safe to drop, but I think she (or some other CheriBSD person) had said that they still needed to support ubuntu-22.
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.
We (the CheriBSD project) depend on being able to build on ubuntu-22. We see build regressions specific to older Ubuntu versions with some regularity so it's useful to keep them around so we catch issues before they hit CheriBSD some time later (typically weeks to months).
I don't think the native bmake is configured correctly. Parts of the build, in particular release/, rely on it being configured with .../share/mk as the first item in its path (see tools/build/make.py's configure line). The comment in the workflow predates that configuration and is likely no longer accurate. I would be very concerned with doing anything other than using tools/build/make.py to wrap bootstrapping the in-tree bmake, and do not see the benefit of it when it takes a fraction of the total build time to bootstrap, only the downside of more headaches from subtle configuration differences. |
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.