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 github.com/libgit2/git2go to v31.6.1 #437

Merged
merged 10 commits into from
Oct 8, 2021
Merged

Update github.com/libgit2/git2go to v31.6.1 #437

merged 10 commits into from
Oct 8, 2021

Conversation

hiddeco
Copy link
Member

@hiddeco hiddeco commented Sep 10, 2021

This PR updates github.com/libgit2/git2go to v31.6.1 (with libgit2 1.1.1), and changes the container image build process so that it makes use of ghcr.io/fluxcd/golang-with-libgit2.

This image provides a Makefile with specific build instructions to compile libgit2 in such a way that it is linked against OpenSSL and LibSSH2 (without gcrypt) (see rationale and usage for more detailed information).

The linked set of dependencies should solve most known issues around unsupported private key types, but does not resolve the issues with ECDSA* and ED25519 hostkeys yet. Solving this requires a newer version of libgit2 (>=1.2.0), which currently does not seem to work properly with git2go/v32.

In addition, to support the new image and improve the contributor experience, a couple of changes have been made to the various make targets:

  • Detection of the system version of libgit2 is attempted using pkg-config. If this fails, or does not match the version as defined in the Makefile (or configured using LIBGIT2_VERSION=1.x.y), the library is compiled using the instructions from the LIBGIT2_IMG, and installed to REPOSITORY_ROOT/hack/libgit2.
  • Where libgit2 is required as a dependency, LD_LIBRARY_PATH and/or PKG_CONFIG_PATH instructions are added as a prefix to the command. This works for system libraries as well, because the paths are ignored if they do not exist.
  • docker-build is now always making use of docker buildx. It allows configuration over the target platform(s) and additional build arguments using BUILD_PLATFORMS and BUILD_ARGS.

@hiddeco hiddeco force-pushed the update-libgit2 branch 3 times, most recently from d2d8682 to e8808e4 Compare September 16, 2021 15:24
Makefile Outdated Show resolved Hide resolved
@hiddeco hiddeco force-pushed the update-libgit2 branch 4 times, most recently from 9cfd0be to 04ab5b9 Compare September 17, 2021 21:57
@hiddeco hiddeco changed the title Update github.com/libgit2/git2go to v31.6.1 Update github.com/libgit2/git2go to v32 Sep 17, 2021
@hiddeco hiddeco force-pushed the update-libgit2 branch 8 times, most recently from 8fbc69a to c1a094d Compare September 21, 2021 08:43
Makefile Show resolved Hide resolved
@hiddeco hiddeco force-pushed the update-libgit2 branch 12 times, most recently from 522ceba to 045a648 Compare September 27, 2021 21:14
@hiddeco hiddeco added the area/git Git related issues and pull requests label Sep 30, 2021
@hiddeco hiddeco changed the title Update github.com/libgit2/git2go to v32 Update github.com/libgit2/git2go to v31.6.1 Sep 30, 2021
To provide a better (contributing) experience to those with Apple
machines, as determining the correct paths there is a bit harder.

Signed-off-by: Hidde Beydals <hello@hidde.co>
@hiddeco hiddeco force-pushed the update-libgit2 branch 2 times, most recently from b3bb41e to 94480e6 Compare October 4, 2021 13:27
This can be useful on machines where libgit2 is installed due to other
applications depending on it, but where the composition of this
installation does not properly work with the controller.

Reason the system version is still preferred, is because this lowers the
barrier for drive-by contributors, as a working set of (Git) dependencies
should only really be required if you are going to perform work in that
domain.

Signed-off-by: Hidde Beydals <hello@hidde.co>
Signed-off-by: Hidde Beydals <hello@hidde.co>
Signed-off-by: Hidde Beydals <hello@hidde.co>
@hiddeco hiddeco force-pushed the update-libgit2 branch 5 times, most recently from cefc96a to 1ddb895 Compare October 7, 2021 19:30
This moves the `libgit2` compilation to the image, to ensure it
can be build on builders that aren't backed by AMD64.

The image is structured in such a way that e.g. running nightly
builds targeting a different Go version, or targeting a different
OS vendor would be possible in the future via build arguments.

Signed-off-by: Hidde Beydals <hello@hidde.co>
This ensures the Dockerfile used for testing is making use of the
same scratch image to compile `libgit2` as the actual application
image.

In a future iteration we should restructure our GitHub Action
workflows to re-use the application image, saving us an additional
Dockerfile and a duplicate build. Inspiration for this (which makes
use of a local registry for the duration of the build) can be found
at: https://github.com/fluxcd/golang-with-libgit2/blob/main/.github/workflows/build.yaml

Signed-off-by: Hidde Beydals <hello@hidde.co>
As this isn't available on Darwin by default, unlike on most Linux
distributions.

Signed-off-by: Hidde Beydals <hello@hidde.co>
Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @hiddeco for saving the day 🏅

@hiddeco
Copy link
Member Author

hiddeco commented Oct 8, 2021

@squaremo as I really want to progress this I am going to merge it although I know your machine was having a difficult time. It has confirmed to be working on both @stefanprodan and my own 🍏 machines.

If it continues to be stubborn, you know where to find me so we can do another round of debugging.

@hiddeco hiddeco merged commit eb167bc into main Oct 8, 2021
@hiddeco hiddeco mentioned this pull request Oct 8, 2021
hiddeco added a commit that referenced this pull request Oct 8, 2021
Chery picked from `main`.
Update github.com/libgit2/git2go to v31.6.1
@hiddeco hiddeco deleted the update-libgit2 branch October 8, 2021 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ci CI related issues and pull requests area/git Git related issues and pull requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants