-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Simplify macOS CI jobs and specify Go toolchain version #5931
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
Merged
Conversation
This file contains hidden or 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
larsxschneider
approved these changes
Dec 10, 2024
Member
larsxschneider
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
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.
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.
As suggested by larsxschneider on PR review, to make it easier to diagnose problems with our "build-earliest" and "build-latest" CI jobs we can log the build options and dynamic library dependencies of the custom versions of Git that we compile in those jobs. The --build-options argument to the git-version(1) command was introduced with Git v2.46.0, but will be silently ignored by earlier versions, so we can use it directly in the script/build-git script called by our "build-earliest" and "build-latest" CI jobs, including the former, which builds Git version 2.0.0. In addition, we can also list the dynamic library dependencies of the "git-http-fetch" binary we build, since this is one of the Git programs which must be linked with the libcurl library and we depend on this Git program in our test suite. We therefore use either the ldd(1) command on Linux or the otool(1) command on macOS to display the dependencies of the "git-http-fetch" binary after we have built and installed it.
Since at least commit d874af9 of PR git-lfs#1617 we have built the custom versions of Git used in our CI suite with --jobs=2 or -j2 arguments to the make(1) command, which means we run no more than two build processes simultaneously. We may slightly improve the speed of our CI jobs, though, if we use a higher number of parallel build processes, since we typically now run our CI suite on hardware with 3 or 4 CPU cores: https://github.com/github/docs/blob/7c3c8e7c936fbe1d5d71080098eeccf5cb37bf43/data/reusables/actions/supported-github-runners.md#standard--data-variablesproductprodname_dotcom--hosted-runners-for-public-repositories Note, too, that our script/cibuild script currently sets the number of tests run in parallel by the prove(1) command to nine, so using four parallel build jobs for Git is not excessive by comparison.
Since commit 6285716 in PR git-lfs#4729 we have installed the gettext library and utilities in our CI and release GitHub Actions jobs, on both macOS and Ubuntu Linux platforms, while on Windows we rely on the library and utilities being installed in advance as part of the Git for Windows environment. At present, though, the macOS runners provided by GitHub Actions are always provisioned with a pre-installed version of a recent release of the gettext library, as packaged by Homebrew, which includes the msgfmt(1) program used by our Makefile, so we no longer need to install the "gettext" Homebrew formula ourselves.
Since Go version 1.21, Go supports the designation of a specific version of the Go toolchain to use when building a module, as described in: https://go.dev/doc/toolchain If the "go" line in a "go.mod" file contains a full version, i.e., one in the M.N.P format, then this will suffice to define the minimum required Go toolchain to use when building the module. Otherwise, Go will look for a "toolchain" line in the "go.mod" file. As we do not yet provide either a "toolchain" line or a "go" line with the full version, running "go get" and other Go utilities may attempt to insert a "toolchain" line with a recent Go version, such as 1.23.3. We also receive warnings from CodeQL that our "go.mod" file contains an invalid toolchain version. To avoid these issues, we specify a full Go version in the "go" line of our "go.mod" file, which suffices to define the minimum toolchain version as well. We then also run "go mod tidy" to update our module requirements, which removes a number of entries that have become unnecessary.
a0d7087 to
1332573
Compare
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.
The macOS runners provided by GitHub Actions are now provisioned with pre-installed versions of several Homebrew formulae that we currently re-install, which leads to annotation warnings in our CI job summaries. We can therefore simplify our CI jobs on macOS slightly by eliminating the unnecessary Homebrew invocations.
As well, we now also have a warning from CodeQL that our
go.modfile does not specify a Go toolchain, which we can resolve most easily by providing a full Go version in thegoline of that file.This PR will be most easily reviewed on a commit-by-commit basis.