Always build Git against custom libcurl in CI workflows on macOS #5866
+7
−1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In commit b9f7f5f of PR git-lfs#5813 we updated our script/build-git script to install the version of Git it builds into the location provided by the GIT_INSTALL_DIR environment variable, if one is set; otherwise, /usr/local is used as a default. At the same time, we also changed the GitHub Actions workflow for our CI jobs so that on macOS, when running our build-latest and build-earliest jobs, we set the GIT_INSTALL_DIR environment variable to a path of our choosing, and then also place that path at the start of the list of paths in the PATH environment variable. This ensures that on macOS Actions runners, our custom version of Git is used in preference to the one pre-installed on the runners by Homebrew. However, in our build-earliest jobs, our custom build of Git is still linked against the libcurl dynamic library installed by macOS in /usr/lib, rather than the one our script/build-git script installs and links using Homebrew. (Technically, macOS Actions runners now come with the Homebrew version installed but not linked, so our script actually only performs the latter step.) The reason our build of Git is linked against the default macOS version of libcurl in our build-earliest CI jobs is that the Makefile used to build Git, prior to commit git/git@23c4bbe of Git version 2.20.0, does not use the output of "curl-config --libs" to populate its CURL_LIBCURL variable but just adds the -lcurl argument directly. (Note that if the CURLDIR variable is defined in the build configurations settings, its value takes precedence over these defaults.) The CURL_LIBCURL variable is then used when building certain programs like git-http-push(1), so these end up linked against the macOS version of libcurl in /usr/lib, rather than the one we have installed with Homebrew. Until recently this has not caused problems, but at the moment, the 8.7.1 version of libcurl currently shipped with macOS 13 and 14 (Ventura and Sonoma) has a regression which affects the programs used by git-http-backend(1), including git-http-push, as detailed in curl/curl#13229. The consequence is that some of our tests fail because they happen to encounter this regression. In particular, the "does not look in current directory for git with credential helper" test in our t/t-path.sh test script commits a compiled Go binary file to a Git repository and tries to push it, and when our lfstest-gitserver program passes the HTTP POST request body on the git-http-backend program, the large binary data in the request body includes byte sequences which trigger the bug, resulting in a Git push failure and a test failure. To resolve this problem and avoid similar ones in the future, we revise our script/build-git script to set the CURLDIR variable with the output of "curl-config --prefix" when running on macOS, and then include this setting in the Makefile configuration we use to build our custom Git. As we have already run "brew link" to ensure "curl-config" will be located in the /opt/homebrew/bin directory, which by default is ahead of /usr/bin in the list of paths in PATH on our macOS Actions runners, this means the Homebrew-installed "curl-config" program will be found, and so we will then always link its dynamic library into the Git programs we build on macOS. These changes have no effect in our build-latest CI jobs, because Git's Makefile already uses "curl-config" to set its CURL_LIBCURL variable, but will resolve the problems we see at the moment in our build-earliest CI jobs on macOS.
larsxschneider
approved these changes
Sep 17, 2024
This was referenced Sep 19, 2024
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
Dec 15, 2024
Since we migrated our CI jobs to GitHub Actions in PR git-lfs#3808, our "build-latest" and "build-earliest" jobs, when running on macOS, have explicitly installed and linked the "curl", "openssl", "pcre2", and "zlib" Homebrew formulae before building a custom version of Git. However, we have always set the NO_OPENSSL environment variable when building custom versions of Git for our CI jobs, since at least commit d874af9 of PR git-lfs#1617 in 2016. This implies that our Git builds do not make use of OpenSSL at all, either for SHA hashing or for SSL support in the git-imap-send(1) command. (Further, since Git version 7.34.0 Git does not require OpenSSL to make IMAP connections over SSL, and in any case our test suite does not use IMAP at all.) Hence we do not need to install the "openssl" Homebrew formula, since this library will never be linked into the Git binaries we build. Meanwhile, we do not set the USE_LIBPCRE environment variable when building Git, so the PCRE2 library is not linked into our custom Git binaries and we can thus skip installing the "pcre2" Homebrew formula. (Also, on the Ubuntu Linux runners provided by GitHub Actions, the "libpcre2-8-0" package is not available by default, so if we wanted to set the USE_LIBPCRE variable we would need to install that package for our Linux CI jobs.) In addition,the modified version of zlib 1.2.12 supplied by macOS 14 (Sonoma) on the GitHub Actions runners we are using at present is is sufficient to build Git, so we do not have to install the "zlib" Homebrew formula either. As described in commit f3cd1ef of PR git-lfs#5866, for the moment we must continue to utilize the version of libcurl from the Homebrew "curl" formula rather than the one supplied by macOS itself, because the 8.7.1 version of libcurl currently shipped with macOS 13 and 14 (Ventura and Sonoma) has a regression which affects the programs used by git-http-backend(1). However, the macOS 14 runners provided by GitHub Actions are provisioned with a pre-installed version of the Homebrew "curl" formula, so we do not need to actually install this formula ourselves. We can therefore simply drop the "brew install" command entirely, since there are no Homebrew formulae we have to install any more. Note, though, that by default Homebrew does not create the symlinks that would make its version of libcurl take precedence over the one supplied by macOS. As commit f3cd1ef of PR git-lfs#5866 describes, we have to ensure the CURLDIR variable is set properly so that our "build-earliest" CI jobs will link the Git programs they build against the Homebrew version of libcurl rather than the one supplied by macOS. For this reason we currently run the "brew link" command to make sure the Homebrew version of the curl-config(1) command is used to set the value of the CURLDIR variable. Using the "brew link --force" command with "keg-only" libraries such as those from the "curl" formula is not recommended practice, though, so in a subsequent commit in this PR we will further adjust our script to set the value of the CURLDIR variable as we require, but without running the "brew link" command first.
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
Dec 15, 2024
Since we migrated our CI jobs to GitHub Actions in PR git-lfs#3808, our "build-latest" and "build-earliest" jobs, when running on macOS, have explicitly installed and linked the "curl", "openssl", "pcre2", and "zlib" Homebrew formulae before building a custom version of Git. In a prior commit in this PR we removed the "brew install" command from our script/build-git script entirely, as we no longer need to install any Homebrew formulae, for the reasons described in that commit. However, we left an invocation of the "brew link" command for the "curl" formula in place in order to ensure that in our "build-earliest" CI job the CURLDIR variable is set such that when we build Git version 2.0.0 in that job, the Git binaries are linked against the Homebrew instance of the libcurl library rather than the one provided by macOS. We introduced the CURLDIR variable in commit f3cd1ef of PR git-lfs#5866 so as to prevent our Git builds from linking with the libcurl library supplied by macOS, because at present macOS 13 and 14 (Ventura and Sonoma) provide version 8.7.1 of libcurl and it has a regression that affects the programs used by git-http-backend(1). The Homebrew project recommends against using the "brew link --force" command, though, for "keg-only" libraries like those from the "curl" formula: https://docs.brew.sh/How-to-Build-Software-Outside-Homebrew-with-Homebrew-keg-only-Dependencies Fortunately, we can achieve our desired result of linking our custom Git binaries against the Homebrew version of libcurl just by setting the CURLDIR variable using the "brew --prefix curl" command, so we do that now.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR resolves a problem which is causing our
Build with earliest GitCI jobs to fail at present on macOS. The failures are due to a regression in the system-installed version 8.7.1 of libcurl currently available on macOS.We therefore revise our scripts to ensure we build Git so it links against the newer version of libcurl we install with Homebrew. The regression in the 8.7.x versions of libcurl is described in curl/curl#13229 and affects programs such as
git-http-push(1)andgit-http-backend(1), which are utilized by ourlfstest-gitservertest utility.Details
The
does not look in current directory for git with credential helpertest in ourt/t-path.shtest script commits a compiled Go binary file to a Git repository and tries to push it, and when ourlfstest-gitserverprogram passes the HTTPPOSTrequest body on thegit-http-backendprogram, the large binary data in the request body includes byte sequences which just happen to trigger the bug in libcurl, resulting in a Git push failure and a test failure.The reason this problem only affects our
Build with earliest GitCI job is that as of commit git/git@23c4bbe, which was included in Git version 2.20.0, Git's ownMakefileuses the output ofcurl-config --libsto populate itsCURL_LIBCURLvariable, and on our GitHub Actions macOS runners, the Homebrew-installed version ofcurl-configis found first inPATH. Therefore that version ofcurl-configis run, and since it returns linker arguments referencing the correspondinglibcurl.4.dylibdynamic library, in the majority of our CI jobs all the Git programs which utilize libcurl are linked against the newer libcurl library.However, we build Git version 2.0.0 in our
Build with earliest GitCI job, and in itsMakefile, theCURL_LIBCURLvariable is simply set to-lcurl(unlessCURLDIRis defined). As a result, the default version of libcurl in/usr/libis linked into Git's programs, rather than the one installed under Homebrew's paths.To resolve this problem and avoid similar ones in the future, we revise our
script/build-gitscript to set theCURLDIRvariable with the output ofcurl-config --prefixwhen running on macOS, and then includeCURLDIRin theMakefileconfiguration we use to build our custom Git. This works with both old and new versions of Git, since whenCURLDIRis defined, its value is used to set an appropriate path for the-Llinker argument, ensuring that our custom Git is built against the Homebrew-installed libcurl dynamic library.Further details are provided in the description of the sole commit in this PR.
Expected and Unrelated CI Failures
Note that we expect our
Build with specific GoCI job to fail at the moment, due to the recent release of Go version 1.23. We install the latest version of thegoimportspackage, and the installation fails in theBuild with specific GoCI job because thex/toolsmodule requires Go 1.22 as of commit golang/tools@70f5626, and we are still using Go 1.21 for that CI job. We will address this issue in a subsequent PR.