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

feat: allow providing options to bluebuild build command #63

Merged
merged 4 commits into from
Nov 11, 2024

Conversation

dkolb
Copy link
Contributor

@dkolb dkolb commented Nov 9, 2024

This creates a build_opts input along with some basic validation to ensure that duplicate options are not passed to buildbuild build.

  • Add a new input for the action called build_opts
    • Pass that as the BUILD_OPTS environment variable to the image build step
    • Remove the line BUILD_OPTS="" (or it might need to be made conditional depending on whether the input has been added)
      • The input defaults to a single space. This allows us to pass it into the environment unconditionally.
      • Validation step prevents the action running with duplicate arguments.
      • For remaining arguments, bluebuild build errors should mention arguments explicitly added by the user as being the problem. So they should not be caught completely unawares that --nonexistent-arg is the problem.
  • Do testing based on your custom branch, at least these two scenarios
    • Is BUILD_OPTS empty if you pass nothing into the build_opts input? (Build)[
    • Is it possible to break the build by adding some opts that conflict with what is automatically configured?
      • Yes. The build_opts and squash values are passed into a validation script that will exit with a non-zero status should the combination of inputs result in duplicate arguments. It also checks for -p or --push since the action provides that option by default.
    • Maybe we'd want to do an error message that points out that your custom build opts might be the issue in case of a build failure.
    • (of course try testing whether the input works for its actual use case too) Successful build using zstd compression

@dkolb dkolb requested a review from xynydev as a code owner November 9, 2024 20:26
xynydev
xynydev previously approved these changes Nov 10, 2024
@xynydev
Copy link
Member

xynydev commented Nov 10, 2024

Thanks a lot! This is a fantastic PR! 💙

I'll let this be open for a moment in case other maintaines want to have a say, but will do a full merge + release tomorrow at the latest.

action.yml Outdated Show resolved Hide resolved
build_opts_check.sh Outdated Show resolved Hide resolved
@xynydev xynydev requested a review from gmpinder November 11, 2024 17:13
@xynydev xynydev merged commit acae198 into blue-build:main Nov 11, 2024
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.

3 participants