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

Remove building Bacalhau as part of Docker image build #3092

Merged
merged 1 commit into from
Dec 12, 2023

Conversation

simonwo
Copy link
Contributor

@simonwo simonwo commented Dec 12, 2023

This meant that our new Web UI build steps were not being picked up. This is not really necessary as we can just reuse the output that was created from our CI build step, rather than the rigmarole of trying to capture the right files and recreate the build process within the Dockerfile.

This change also means we can build the docker image on PRs as well, to check that it works.

There are some other refactors here – we now build and test as separate jobs so that we don't need to wait for tests to pass before pushing binaries to a release, and also generate all of our binaries from a single cross-compiling step (because we don't use CGO and hence can compile for all platforms from anywhere).

As a result of this refactor, the build time in CI is reduced from 17m to 14m... every little helps, I guess.

@@ -192,7 +192,7 @@ $(WEB_INSTALL_GUARD): webui/package.json
cd webui && npm install

export GENERATE_SOURCEMAP := false
$(WEB_BUILD_FILES): $(WEB_SRC_FILES) $(WEB_INSTALL_GUARD)
${WEB_BUILD_FILES} &: $(WEB_SRC_FILES) $(WEB_INSTALL_GUARD)
Copy link
Contributor

Choose a reason for hiding this comment

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

The &: was a good find!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I thought make did this by default, but it turns out it doesn't! TIL!

This meant that our new Web UI build steps were not being picked up.
This is not really necessary as we can just reuse the output that was
created from our CI build step, rather than the rigmarole of trying to
capture the right files and recreate the build process within the
Dockerfile.

This change also means we can build the docker image on PRs as well,
to check that it works.

There are some other refactors here – we now build and test as
separate jobs so that we don't need to wait for tests to pass before
pushing binaries to a release, and also generate all of our binaries
from a single cross-compiling step (because we don't use CGO and hence
can compile for all platforms from anywhere).
@simonwo simonwo merged commit 42c2d5d into main Dec 12, 2023
8 checks passed
@simonwo simonwo deleted the fix-docker-build branch December 12, 2023 18:54
@simonwo simonwo self-assigned this Dec 18, 2023
@simonwo simonwo added the type/tech-debt Type: Issues meant to address technical debt label Dec 18, 2023
BINARY_PATH = bin/${GOOS}_${GOARCH}${GOARM}/${BINARY_NAME}
BINARY_PATH = bin/${GOOS}/${GOARCH}${GOARM}/${BINARY_NAME}
Copy link
Member

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/tech-debt Type: Issues meant to address technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants