Skip to content

Conversation

@chrisd8088
Copy link
Member

@chrisd8088 chrisd8088 commented Jun 27, 2024

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-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.

(We could consider using $HOME/.local/bin instead of $HOME/git/bin, which has the advantage that at present $HOME/.local/bin is already listed ahead of /opt/homebrew/bin in PATH on the macOS runners, so we wouldn't need to modify PATH at 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/bin and explicitly placing it at the start of PATH should avoid this problem, and it is also aligned with the $HOME/go/bin location 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-earliest job run and build-latest job run we can see the expected version of Git is now used in the script/cibuild step:

# Git LFS: /home/runner/work/git-lfs/git-lfs/bin/git-lfs
# git-lfs/3.5.0 (GitHub; linux amd64; go 1.22.4; git 693728f)
# git version 2.0.0
# Git LFS: /Users/runner/work/git-lfs/git-lfs/bin/git-lfs
# git-lfs/3.5.0 (GitHub; darwin arm64; go 1.22.4; git 693728f3)
# git version 2.45.2.648.g1e1586e4ed

Fixes #5790.
/cc @bk2204 as reporter.

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.
@chrisd8088 chrisd8088 added github_actions Pull requests that update GitHub Actions code macos labels Jun 27, 2024
@chrisd8088 chrisd8088 marked this pull request as ready for review June 28, 2024 02:34
@chrisd8088 chrisd8088 requested a review from a team as a code owner June 28, 2024 02:34
@chrisd8088 chrisd8088 merged commit fc61feb into git-lfs:main Jul 1, 2024
@chrisd8088 chrisd8088 deleted the macos-ci-custom-git branch July 1, 2024 15:45
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

Labels

github_actions Pull requests that update GitHub Actions code macos

Projects

None yet

Development

Successfully merging this pull request may close these issues.

macOS earliest and latest CI jobs don't run earliest or latest Git

2 participants