Upgrade libgit2 and fix static builds #311
Conversation
aryan9600
force-pushed
the
aryan9600/static-build
branch
from
45ad744
to
cdfc80f
Compare
There was a problem hiding this comment.
Nice one, @aryan9600. Just a few nits from me, apart from that really good stuff. 👍
.github/workflows/build.yaml
Outdated
| GOBIN="${GITHUB_WORKSPACE}/build/gobin" go install sigs.k8s.io/controller-tools/cmd/controller-gen@v0.7.0 | ||
| GOBIN="${GITHUB_WORKSPACE}/build/gobin" go install github.com/ahmetb/gen-crd-api-reference-docs@v0.3.0 |
There was a problem hiding this comment.
When building against the arm64 runners for SC, only the setup-envtest did not work via Makefile. Did you also have issues with controller-gen and gen-crd-api-reference-docs?
There was a problem hiding this comment.
There was a problem hiding this comment.
Ca we set GOBIN for Make and get rid of the duplicate install? The issue with this is that the controller-gen version will diverge from the one in the Makefile.
There was a problem hiding this comment.
We do set GOBIN in the Makefile which works for linux-amd64 and darwin-amd64 but it mysteriously fails on linux-arm64.
Makefile
Outdated
| @@ -148,7 +192,7 @@ docker-build: ## Build the Docker image | |||
| docker buildx build \ | |||
| --build-arg LIBGIT2_IMG=$(LIBGIT2_IMG) \ | |||
| --build-arg LIBGIT2_TAG=$(LIBGIT2_TAG) \ | |||
| --platform=$(BUILD_PLATFORMS) \ | |||
| --platform=linux/amd64 \ | |||
There was a problem hiding this comment.
We probably need to revert this:
| --platform=linux/amd64 \ | |
| --platform=$(BUILD_PLATFORMS) \ |
hack/install-libraries.sh
Outdated
| if [[ $OSTYPE == 'darwin'* ]]; then | ||
| # For MacOS development environments, download the amd64 static libraries released from from golang-with-libgit2. | ||
|
|
||
| #TODO: update URL with official URL + TAG: |
There was a problem hiding this comment.
| #TODO: update URL with official URL + TAG: |
aryan9600
force-pushed
the
aryan9600/static-build
branch
from
cdfc80f
to
f615b79
Compare
aryan9600
force-pushed
the
aryan9600/static-build
branch
5 times, most recently
from
5e0562e
to
cdcccff
Compare
There was a problem hiding this comment.
It is looking really good, thank you @aryan9600. The only change we must do before merging is to reset the img/tag on the Dockerfile, otherwise this will cause the release workflow to fail.
Dockerfile
Outdated
| ARG LIBGIT2_IMG=ghcr.io/fluxcd/golang-with-libgit2 | ||
| ARG LIBGIT2_TAG=libgit2-1.1.1-4 |
There was a problem hiding this comment.
This is required for release purposes xref: fluxcd/source-controller#574
Makefile
Outdated
| ENVTEST_KUBERNETES_VERSION?=latest | ||
| install-envtest: setup-envtest | ||
| mkdir -p ${ENVTEST_ASSETS_DIR} | ||
| $(ENVTEST) use $(ENVTEST_KUBERNETES_VERSION) --arch=$(ENVTEST_ARCH) --bin-dir=$(ENVTEST_ASSETS_DIR) | ||
| chmod -R u+w $(BUILD_DIR) |
There was a problem hiding this comment.
This may conflict with fuzzing, which creates files that are owned by root. I would rather just reset chmod of things that are set to 0555 by setenvtest:
| chmod -R u+w $(BUILD_DIR) | |
| chmod -R u+w $(BUILD_DIR)/testbin |
* Bump to golang-with-libgit2:1.1.1.6 to speed up build time when cross compiling. Previous version was compiling in emulation mode instead, which added +10x overhead.
* Ensure that make test is executed against the exact same libraries that will be shipped on the built image.
* Simplify Makefile to reduce its complexity.
* Libgit2 behaviour:
linux-amd64 download static libraries from the official container image.
linux-arm64 on top of the above, requires static musl tool chain (automatically downloaded).
darwin-amd64 and darwin-arm64 download universal static libraries for darwin from https://github.com/fluxcd/golang-with-libgit2 releases.
Co-authored-by: Paulo Gomes <paulo.gomes@weave.works>
Signed-off-by: Sanskar Jaiswal <sanskar.jaiswal@weave.works>
aryan9600
force-pushed
the
aryan9600/static-build
branch
from
cdcccff
to
a348d9f
Compare
linux-amd64 download static libraries from the official container image.
linux-arm64 on top of the above, requires static musl tool chain (automatically downloaded).
darwin-amd64 and darwin-arm64 download universal static libraries for darwin from https://github.com/fluxcd/golang-with-libgit2 releases.
Fixes: #309
Co-authored-by: Paulo Gomes paulo.gomes@weave.works
Signed-off-by: Sanskar Jaiswal sanskar.jaiswal@weave.works