-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Use expected version of Git on macOS in CI jobs #5813
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
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
Possibly since we first switched to the use of GitHub Actions for our CI jobs, in PR git-lfs#3808, the version of Git we've used on macOS in our build-earliest and build-latest jobs has actually been the one installed by Homebrew rather than the one we've custom-built for the purpose. This is due to the fact that, by default, the PATH environment variable supplied in GitHub Actions runners on macOS has the /opt/homebrew/bin location listed ahead of the /usr/local/bin location where we install our custom version of Git. We can address this by ensuring that our build-git script installs the custom version of Git into a unique location, when running on macOS, and then placing this location ahead of all others in the PATH environment variable. On Linux we can just continue to install our version of Git into /usr/local/bin, as that takes precedence over any other installed version of Git on the GitHub Actions runners.
larsxschneider
approved these changes
Jul 1, 2024
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
Aug 30, 2024
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 which is pre-installed on the runners using Homebrew. However, our custom Git version was still linked against the curl dynamic library installed by macOS itself in /usr/lib, rather than the one our script/build-git script installs using Homebrew. (Actually, there is already a version of curl installed by Homebrew which is pre-installed on the macOS Actions runners, so our script's request to install it just reports that curl is installed and up-to-date.) Until recently this has not caused problems, but at the moment, the version of curl shipped with macOS has a regression which affects the programs used by git-http-backend(1). The consequence is that some of our tests fail because they happen to trigger this bug; 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 happens to trigger the curl regression, 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", and then pass this to the Makefile configuration we use when building our custom Git. As our macOS runners have the Homebrew installation paths ahead of the default /usr/bin path in the list of paths in PATH, this means the Homebrew-installed curl-config program will be found, and so we will then link its dynamic library into the Git programs we build. On Linux runners, this should cause no change from the current behaviour, as we expect the curl programs and libraries to be installed in standard locations, so "curl-config --prefix" will just return /usr.
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
Aug 30, 2024
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 which is pre-installed on the runners using Homebrew. --> DEBUG - for build-earliest only! However, our custom Git version was still linked against the curl dynamic library installed by macOS itself in /usr/lib, rather than the one our script/build-git script installs using Homebrew. (Actually, there is already a version of curl installed by Homebrew which is pre-installed on the macOS Actions runners, so our script's request to install it just reports that curl is installed and up-to-date.) Until recently this has not caused problems, but at the moment, the version of curl shipped with macOS has a regression which affects the programs used by git-http-backend(1). --> DEBUG add curl issue link The consequence is that some of our tests fail because they happen to trigger this bug; 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 happens to trigger the curl regression, resulting in a Git push failure and a test failure. This problem only appears when we test with Git version 2.0.0 in our build-earliest CI jobs, because as of Git version 2.20.0, the change in commit git/git@23c4bbe -> DEBUG 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", and then pass this to the Makefile configuration we use when building our custom Git. As our macOS runners have the Homebrew installation paths ahead of the default /usr/bin path in the list of paths in PATH, this means the Homebrew-installed curl-config program will be found, and so we will then link its dynamic library into the Git programs we build. On Linux runners, this should cause no change from the current behaviour, as we expect the curl programs and libraries to be installed in the standard system locations, so "curl-config --prefix" will just return /usr.
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
Aug 30, 2024
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 which is pre-installed on the runners using Homebrew. --> DEBUG - for build-earliest only! However, our custom Git version was still linked against the curl dynamic library installed by macOS itself in /usr/lib, rather than the one our script/build-git script installs using Homebrew. (Actually, there is already a version of curl installed by Homebrew which is pre-installed on the macOS Actions runners, so our script's request to install it just reports that curl is installed and up-to-date.) Until recently this has not caused problems, but at the moment, the version of curl shipped with macOS has a regression which affects the programs used by git-http-backend(1). --> DEBUG add curl issue link The consequence is that some of our tests fail because they happen to trigger this bug; 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 happens to trigger the curl regression, resulting in a Git push failure and a test failure. This problem only appears when we test with Git version 2.0.0 in our build-earliest CI jobs, because as of Git version 2.20.0, the change in commit git/git@23c4bbe -> DEBUG 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", and then pass this to the Makefile configuration we use when building our custom Git. As our macOS runners have the Homebrew installation paths ahead of the default /usr/bin path in the list of paths in PATH, this means the Homebrew-installed curl-config program will be found, and so we will then link its dynamic library into the Git programs we build. On Linux runners, this should cause no change from the current behaviour, as we expect the curl programs and libraries to be installed in the standard system locations, so "curl-config --prefix" will just return /usr.
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
Aug 30, 2024
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 which is pre-installed on the runners using Homebrew. However, in our build-earliest jobs, our custom build of Git was still linked against the curl 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.) This is because the Makefile used to build Git, prior to commit git/git@23c4bbe of 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 from Homebrew. Until recently this has not caused problems, but at the moment, the 8.7.x versions of curl shipped with macOS has a regression which affects the programs used by git-http-backend(1), including git-http-push. The consequence is that some of our tests fail because they happen to trigger this bug; 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 happens to trigger the curl regression, 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.
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
Sep 15, 2024
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.
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.
Possibly since we first switched to the use of GitHub Actions for our CI jobs, in PR #3808, the version of Git we've used on macOS in our
build-earliestandbuild-latestjobs has actually been the one installed by Homebrew rather than the one we've custom-built for the purpose.This is due to the fact that, by default, the
PATHenvironment variable supplied in GitHub Actions runners on macOS has the/opt/homebrew/binlocation listed ahead of the/usr/local/binlocation where we install our custom version of Git.We can address this by ensuring that our
build-gitscript installs the custom version of Git into a unique location, when running on macOS, and then placing this location ahead of all others in thePATHenvironment variable.(We could consider using
$HOME/.local/bininstead of$HOME/git/bin, which has the advantage that at present$HOME/.local/binis already listed ahead of/opt/homebrew/bininPATHon the macOS runners, so we wouldn't need to modifyPATHat all. However, that order is likely not guaranteed, and if it changed in the future we could see another silent regression in the version of Git used in our macOS CI jobs. Using$HOME/git/binand explicitly placing it at the start ofPATHshould avoid this problem, and it is also aligned with the$HOME/go/binlocation used by Go.)On Linux we can just continue to install our version of Git into
/usr/local/bin, as that takes precedence over any other installed version of Git on the GitHub Actions Linux runners.In this PR's
build-earliestjob run andbuild-latestjob run we can see the expected version of Git is now used in thescript/cibuildstep:Fixes #5790.
/cc @bk2204 as reporter.