Skip to content

Conversation

@bk2204
Copy link
Member

@bk2204 bk2204 commented Sep 4, 2019

Currently, we have three different CI systems that handle our CI: Travis for Linux, CircleCI for macOS, and AppVeyor for Windows. This results in widely varying performance across systems and the need to maintain code that works differently across different CI systems.

In addition, we'd like to use GitHub Actions to automate the release process, so it makes sense to use it for CI as well. Switch over by adding a CI workflow that runs our existing jobs.

There are several causes of flaky tests that are also fixed in this series, plus some workarounds for the fact that cygpath on the Windows Actions servers produces a short path (C:\Users\RUNNER~1 instead of C:\Users\runneradmin). This latter issue is heavily exacerbated by the fact that we cannot canonicalize a Windows-style path from the shell since the shell uses Unix-style MSYS paths.

This does not remove the existing CI providers, since AppVeyor falls back to trying to build a standard MSBuild project when there's no configuration file and it fails when trying to do so. I'll follow up with an additional PR to remove them once this has merged to master to allow topics in flight to settle on the new system.

@bk2204 bk2204 marked this pull request as ready for review September 4, 2019 21:21
Copy link
Contributor

@PastelMobileSuit PastelMobileSuit left a comment

Choose a reason for hiding this comment

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

This looks great, thanks for doing this! ✨

When we're trying to make a path in the repository relative to the
current working directory, the path to the repository that we use will
already have been canonicalized.  Ensure that the path to the current
working directory is also canonicalized, so that we don't end up
creating a long chain of ".." components when the repository path and
the working directory differ in canonicalization.
The credential helper can fail in some cases when it gets data that
doesn't contain a colon.  In such a case, make it print a useful error
instead of panicking.
When we run tests in parallel, we run the setup code each time.  Since
we share some test data between tests, such as credential files, it's
important that we not overwrite them when running tests in parallel.

If we do, it's possible for the credential helper to find a truncated
file with no credentials, thereby causing the test to fail.  Ensure this
doesn't occur by checking to see if the file exists before overwriting
it.

This doesn't erase the race window entirely, but it decreases it
significantly and limits it to the first two tests to run when we're
running in parallel, since the files will already exist when other tests
are invoked.
It's possible that the final "Downloading objects" message might not be
printed in some cases due to a race condition.  (The corresponding
upload message, on the other hand, will always be printed because it
runs before the actual push.)

To avoid spurious failures, don't look for this message in several tests
where this race condition commonly occurs.  Instead, use "git lfs fsck"
to verify that we downloaded all the objects we expected to get.

In addition, several of the tests also check that the expected number of
objects is correct, which provides additional confidence that our test
did in fact succeed.
We set some variables for the test harness when running in CI on
Windows.  However, if these variables leak into the environment of the
Git LFS process, they'll cause the test to fail, since they'll be
printed in the env output.  Make sure that we unset them in the tests
that care about this to ensure that they don't cause failures.
On Actions, our Windows build root has a path with a short name.  This
causes significant grief, since our tests expect one version of the path
and we output another, causing our tests for equality to fail.  Since
Windows lacks a realpath(1) utility, provide a small wrapper around the
Go primitives to provide a limited version of this utility for use only
in our tests.
There are several places in our testsuite where we want to produce a
canonical path.  For example, in t/t-env.sh, we compare the expected
output, which we synthesize, with the output from the git-lfs binary
we're testing.  The binary will have canonicalized the path, and
therefore so should we.  This is especially noticeable on Windows on
Actions, since cygpath -w produces a noncanonical path.

Introduce a test helper to produce a canonical native path that combines
the functionality of our existing native_path helper and the
lfstest-realpath binary we introduced in an earlier commit.  Add a form
that handles escaped paths as well.
There are several places in our testsuite where the git-lfs binary we're
testing produces canonical paths.  Ensure that our test framework also
uses canonical paths so that our comparisons for equality do not fail
spuriously.
Currently, we have three different CI systems that handle our CI: Travis
for Linux, CircleCI for macOS, and AppVeyor for Windows.  This results
in widely varying performance across systems and the need to maintain
code that works differently across different CI systems.

In addition, we'd like to use GitHub Actions to automate the release
process, so it makes sense to use it for CI as well.  Switch over by
adding a CI workflow that runs our existing jobs.  Ensure that we filter
out the environment variables that GitHub Actions provides, since they
will cause tests that run "git lfs env" to fail.

Add a script for those jobs where we build a custom Git and install the
appropriate dependencies.  In the cibuild script, hoist the Windows
handling to the top, set a specific environment variable for us to
remember that we're on Windows, and then disable locking, which fails on
Windows and causes the testsuite to abort.  These same environment
variables were set for AppVeyor and are also needed on Windows
development systems.
@bk2204 bk2204 merged commit e614648 into master Sep 9, 2019
@bk2204 bk2204 deleted the actions-ci branch September 9, 2019 15:33
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Jun 27, 2024
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 added a commit to chrisd8088/git-lfs that referenced this pull request Aug 24, 2024
In commit 9b73c80 of PR git-lfs#3125 the
GIT_LFS_LOCK_ACQUIRE_DISABLED environment variable was introduced
for use with our test suites.  When this variable was set to the value
"1", it indicated to the lfstest-count-tests test utility that it
should skip acquiring a file lock before updating the test_count
file we use to track the number of active test suites.

This was done because, at the time, the AppVeyor service was used
to run our CI jobs on the Windows platform, and according to the
commit description, the lfstest-count-tests program was not able
to acquire the test_count.lock file successfully.  Rather than
resolve this issue, because only one instance of lfstest-count-tests
was expected to be run at a time, locking was simply disabled
by setting the GIT_LFS_LOCK_ACQUIRE_DISABLED variable to "1" in
the appveyor.yml configuration file.

However, since PR git-lfs#3808 we have used GitHub Actions for our CI
jobs, including our Windows CI jobs, and the appveyor.yml file
was later removed in commit 95d6d2d
of PR git-lfs#4390 as it was no longer needed.  Therefore there the
GIT_LFS_LOCK_ACQUIRE_DISABLED environment variable is never set, and we
can remove the remaining references to it, along with the support for
disabling lock acquisition in the lfstest-count-tests program that was
added in commit 9b73c80 but is now
unused.

(Only one instance of lfstest-count-tests was expected to be run at
a time because the GIT_LFS_NO_TEST_COUNT environment variable was also
set to "1" in the appveyor.yml file, which caused the individual test
suites to skip running lfstest-count-tests entirely.  Instead, the
program was run just once in the "test" Makefile target before executing
the test suites with the "prove" command, and then once again afterwards.
These two invocations would cause the lfstest-count-tests program to
in turn start and then stop the lfstest-gitserver program.  We will
remove this unused legacy environment variable in a subsequent commit
in this PR as well.)
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Aug 24, 2024
In commit c591ff7 of PR git-lfs#3125 the
GIT_LFS_NO_TEST_COUNT environment variable was introduced for use
with our test suites.  When this variable was set to a non-empty
value, it indicated to the setup() and shutdown() test helper
shell functions that they should skip running the lfstest-count-tests
utility program entirely.

This was done because, at the time, the AppVeyor service was used
to run our CI jobs on the Windows platform, and apparently there
were issues with running multiple instances of the lfstest-count-tests
program at the same time.  (According to the description of commit
9b73c80 in the same PR, the
lfstest-count-tests program was not able to acquire the test_count.lock
file successfully on Windows; perhaps this was the root cause of
the problem.)

The GIT_LFS_NO_TEST_COUNT variable was set to "1" in the appveyor.yml
configuration file, which caused the individual test suites to skip
running lfstest-count-tests entirely.  Instead, the program was run
just once in the "test" Makefile target before executing the test
suites with the "prove" command, and then once again afterwards,
both times by explicitly resetting the GIT_LFS_NO_TEST_COUNT variable
to an empty string value.  These two invocations would cause the
lfstest-count-tests program to start the lfstest-gitserver program
before all the test suites were executed and then stop it once they
had all finished.

However, since PR git-lfs#3808 we have used GitHub Actions for our CI
jobs, including our Windows CI jobs, and the appveyor.yml file
was later removed in commit 95d6d2d
of PR git-lfs#4390 as it was no longer needed.  Therefore there the
GIT_LFS_NO_TEST_COUNT environment variable is never set, and we
can remove the remaining references to it, including the conditionals
in the setup() and shutdown() test helper shell functions which
check for an empty or unset value in this variable before calling
the lfstest-count-tests program.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Aug 25, 2024
Since commit c591ff7 of PR git-lfs#3125 we
have installed Strawberry Perl before running our CI test suite on
Windows.  This was necessary when we used AppVeyor to run our test
suite on Windows, and subsequently also when we converted our CI jobs
to GitHub Actions in PR git-lfs#3808.  We require Perl because we use the
"prove" command, which runs all of our t/t-*.sh test scripts and
collects and summarizes the results.

However, since commit 8818630 of
PR git-lfs#5666 we have installed the Git for Windows SDK before running
our test suite, and it includes a full Perl distribution, which means
we no longer need to install Strawberry Perl separately.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Aug 25, 2024
In commit 9b73c80 of PR git-lfs#3125 the
GIT_LFS_LOCK_ACQUIRE_DISABLED environment variable was introduced
for use with our test suites.  When this variable was set to the value
"1", it indicated to the lfstest-count-tests test utility that it
should skip acquiring a file lock before updating the test_count
file we use to track the number of active test suites.

This was done because, at the time, the AppVeyor service was used
to run our CI jobs on the Windows platform, and according to the
commit description, the lfstest-count-tests program was not able
to acquire the test_count.lock file successfully.  Rather than
resolve this issue, because only one instance of lfstest-count-tests
was expected to be run at a time, locking was simply disabled
by setting the GIT_LFS_LOCK_ACQUIRE_DISABLED variable to "1" in
the appveyor.yml configuration file.

However, since PR git-lfs#3808 we have used GitHub Actions for our CI
jobs, including our Windows CI jobs, and the appveyor.yml file
was later removed in commit 95d6d2d
of PR git-lfs#4390 as it was no longer needed.  Therefore there the
GIT_LFS_LOCK_ACQUIRE_DISABLED environment variable is never set, and we
can remove the remaining references to it, along with the support for
disabling lock acquisition in the lfstest-count-tests program that was
added in commit 9b73c80 but is now
unused.

(Only one instance of lfstest-count-tests was expected to be run at
a time because the GIT_LFS_NO_TEST_COUNT environment variable was also
set to "1" in the appveyor.yml file, which caused the individual test
suites to skip running lfstest-count-tests entirely.  Instead, the
program was run just once in the "test" Makefile target before executing
the test suites with the "prove" command, and then once again afterwards.
These two invocations would cause the lfstest-count-tests program to
in turn start and then stop the lfstest-gitserver program.  We will
remove this unused legacy environment variable in a subsequent commit
in this PR as well.)
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Aug 25, 2024
Since commit c591ff7 of PR git-lfs#3125 we
have installed Strawberry Perl before running our CI test suite on
Windows.  This was necessary when we used AppVeyor to run our test
suite on Windows, and subsequently also when we converted our CI jobs
to GitHub Actions in PR git-lfs#3808.  We require Perl because we use the
"prove" command, which runs all of our t/t-*.sh test scripts and
collects and summarizes the results.

However, since commit 8818630 of
PR git-lfs#5666 we have installed the Git for Windows SDK before running
our test suite, and it includes a full Perl distribution, which means
we no longer need to install Strawberry Perl separately.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Aug 25, 2024
In commit 9b73c80 of PR git-lfs#3125 the
GIT_LFS_LOCK_ACQUIRE_DISABLED environment variable was introduced
for use with our test suites.  When this variable was set to the value
"1", it indicated to the lfstest-count-tests test utility that it
should skip acquiring a file lock before updating the test_count
file we use to track the number of active test suites.

This was done because, at the time, the AppVeyor service was used
to run our CI jobs on the Windows platform, and according to the
commit description, the lfstest-count-tests program was not able
to acquire the test_count.lock file successfully.  Rather than
resolve this issue, because only one instance of lfstest-count-tests
was expected to be run at a time, locking was simply disabled
by setting the GIT_LFS_LOCK_ACQUIRE_DISABLED variable to "1" in
the appveyor.yml configuration file.

However, since PR git-lfs#3808 we have used GitHub Actions for our CI
jobs, including our Windows CI jobs, and the appveyor.yml file
was later removed in commit 95d6d2d
of PR git-lfs#4390 as it was no longer needed.  Therefore there the
GIT_LFS_LOCK_ACQUIRE_DISABLED environment variable is never set, and we
can remove the remaining references to it, along with the support for
disabling lock acquisition in the lfstest-count-tests program that was
added in commit 9b73c80 but is now
unused.

(Only one instance of lfstest-count-tests was expected to be run at
a time because the GIT_LFS_NO_TEST_COUNT environment variable was also
set to "1" in the appveyor.yml file, which caused the individual test
suites to skip running lfstest-count-tests entirely.  Instead, the
program was run just once in the "test" Makefile target before executing
the test suites with the "prove" command, and then once again afterwards.
These two invocations would cause the lfstest-count-tests program to
in turn start and then stop the lfstest-gitserver program.  We will
remove this unused legacy environment variable in a subsequent commit
in this PR as well.)
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Aug 25, 2024
In commit 9b73c80 of PR git-lfs#3125 the
GIT_LFS_LOCK_ACQUIRE_DISABLED environment variable was introduced
for use with our test suites.  When this variable was set to the value
"1", it indicated to the lfstest-count-tests test utility that it
should skip creating a test_count.lock file before updating the
test_count file we use to track the number of active test suites.

This was done because the lfstest-count-tests program had a bug
which prevented it from deleting the lock file, causing subsequent
invocations of the program to be unable to exclusively create it
again.  As this bug was not resolved at the time, the choice was
made to work around the problem by skipping the creation of a
lock file altogether on Windows.

This change was also made in conjunction with the addition of another
environment variable, GIT_LFS_NO_TEST_COUNT, in commit
c591ff7 of the same PR.  That variable
is also set only on Windows, and stops the setup() and shutdown()
shell functions defined in our t/testhelpers.sh script from calling
the lfstest-count-tests program.

As we have now resolved the underlying problem in the lfstest-count-tests
utility, in a prior commit in this PR, we can now remove both of
these environment variables and the test suite features they control.

We start by removing support for the GIT_LFS_LOCK_ACQUIRE_DISABLED
environment variable from the lfstest-count-tests utility, and
changing our script/cibuild script so it no longer sets the variable.

We can also remove the environment variable from the list of those
we unset before running certain tests.  We had to introduce this
behaviour in commit aca1aff of
PR git-lfs#3808, when we migrated our CI test suite to GitHub Actions.
However, we will no longer need to unset these variables as we will
now never set them at all, and they will have no effect if they
were set.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Aug 25, 2024
In commit c591ff7 of PR git-lfs#3125 the
GIT_LFS_NO_TEST_COUNT environment variable was introduced for use
with our test suites.  When this variable was set to a non-empty
value, it indicated to the setup() and shutdown() test helper
shell functions that they should skip running the lfstest-count-tests
utility program entirely.

This was done because the lfstest-count-tests program had a bug
which prevented it from deleting the lock file, causing subsequent
invocations of the program to be unable to exclusively create it
again.  As this bug was not resolved at the time, the choice was
made to work around the problem in part by not counting the number
of running test scripts on Windows.  (We also introduced the
GIT_LFS_LOCK_ACQUIRE_DISABLED environment variable, in commit
9b73c80 of the same PR, to further
address the problem.)

When running our test suite on Windows, we set the GIT_LFS_NO_TEST_COUNT
to "1", which causes the individual test suites to skip invoking
lfstest-count-tests entirely.  Instead, the program is run just once
in the "test" t/Makefile target before executing the test suites with
the "prove" command, and then once again afterwards, both times by
explicitly resetting the GIT_LFS_NO_TEST_COUNT variable to an empty
string value.  These two invocations cause the lfstest-count-tests
program to start the lfstest-gitserver program before all the test
suites were executed and then stop it once they had all finished.

As we have now resolved the underlying problem in the lfstest-count-tests
utility, in a prior commit in this PR, we can remove this environment
variable and the test suite features it controls.

We change our script/cibuild script to no longer set the
GIT_LFS_NO_TEST_COUNT variable, and we revise the conditionals
in the setup() and shutdown() test helper shell functions so they
always call the lfstest-count-tests program, rather than only doing
so if the GIT_LFS_NO_TEST_COUNT variable is unset or empty.  We
also change the "test" target's recipe in our t/Makefile file to
not unset GIT_LFS_NO_TEST_COUNT when running lfstest-count-tests
before and after it runs the "prove" command.

Finally, as we have already removed the GIT_LFS_LOCK_ACQUIRE_DISABLED
environment variable and all support for it, in a previous commit in
this PR, that allows us now to also eliminate the unset_vars() shell
functions in some of our t/t-*.sh test scripts, which was used to unset
both that variable and the GIT_LFS_NO_TEST_COUNT variable before
running the tests in those scripts.  We added this behaviour in commit
aca1aff of PR git-lfs#3808, when we migrated
our CI test suite to GitHub Actions.  However, we will no longer need
to unset these variables as we will now never set them at all, and they
will have no effect if they were set.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Aug 30, 2024
Since commit c591ff7 of PR git-lfs#3125 we
have installed Strawberry Perl before running our CI test suite on
Windows.  This was necessary when we used AppVeyor to run our test
suite on Windows, and subsequently also when we converted our CI jobs
to GitHub Actions in PR git-lfs#3808.  We require Perl because we use the
"prove" command, which runs all of our t/t-*.sh test scripts and
collects and summarizes the results.

However, since commit 8818630 of
PR git-lfs#5666 we have installed the Git for Windows SDK before running
our test suite, and it includes a full Perl distribution, which means
we no longer need to install Strawberry Perl separately.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Aug 30, 2024
In commit 9b73c80 of PR git-lfs#3125 the
GIT_LFS_LOCK_ACQUIRE_DISABLED environment variable was introduced
for use with our test suites.  When this variable was set to the value
"1", it indicated to the lfstest-count-tests test utility that it
should skip creating a test_count.lock file before updating the
test_count file we use to track the number of active test suites.

This was done because the lfstest-count-tests program had a bug
which prevented it from deleting the lock file, causing subsequent
invocations of the program to be unable to exclusively create it
again.  As this bug was not resolved at the time, the choice was
made to work around the problem by skipping the creation of a
lock file altogether on Windows.

This change was also made in conjunction with the addition of another
environment variable, GIT_LFS_NO_TEST_COUNT, in commit
c591ff7 of the same PR.  That variable
is also set only on Windows, and stops the setup() and shutdown()
shell functions defined in our t/testhelpers.sh script from calling
the lfstest-count-tests program.

As we have now resolved the underlying problem in the lfstest-count-tests
utility, in a prior commit in this PR, we can now remove both of
these environment variables and the test suite features they control.

We start by removing support for the GIT_LFS_LOCK_ACQUIRE_DISABLED
environment variable from the lfstest-count-tests utility, and
changing our script/cibuild script so it no longer sets the variable.

We can also remove the environment variable from the list of those
we unset before running certain tests.  We had to introduce this
behaviour in commit aca1aff of
PR git-lfs#3808, when we migrated our CI test suite to GitHub Actions.
However, we will no longer need to unset these variables as we will
now never set them at all, and they will have no effect if they
were set.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Aug 30, 2024
In commit c591ff7 of PR git-lfs#3125 the
GIT_LFS_NO_TEST_COUNT environment variable was introduced for use
with our test suites.  When this variable was set to a non-empty
value, it indicated to the setup() and shutdown() test helper
shell functions that they should skip running the lfstest-count-tests
utility program entirely.

This was done because the lfstest-count-tests program had a bug
which prevented it from deleting the lock file, causing subsequent
invocations of the program to be unable to exclusively create it
again.  As this bug was not resolved at the time, the choice was
made to work around the problem in part by not counting the number
of running test scripts on Windows.  (We also introduced the
GIT_LFS_LOCK_ACQUIRE_DISABLED environment variable, in commit
9b73c80 of the same PR, to further
address the problem.)

When running our test suite on Windows, we set the GIT_LFS_NO_TEST_COUNT
to "1", which causes the individual test suites to skip invoking
lfstest-count-tests entirely.  Instead, the program is run just once
in the "test" t/Makefile target before executing the test suites with
the "prove" command, and then once again afterwards, both times by
explicitly resetting the GIT_LFS_NO_TEST_COUNT variable to an empty
string value.  These two invocations cause the lfstest-count-tests
program to start the lfstest-gitserver program before all the test
suites were executed and then stop it once they had all finished.

As we have now resolved the underlying problem in the lfstest-count-tests
utility, in a prior commit in this PR, we can remove this environment
variable and the test suite features it controls.

We change our script/cibuild script to no longer set the
GIT_LFS_NO_TEST_COUNT variable, and we revise the conditionals
in the setup() and shutdown() test helper shell functions so they
always call the lfstest-count-tests program, rather than only doing
so if the GIT_LFS_NO_TEST_COUNT variable is unset or empty.  We
also change the "test" target's recipe in our t/Makefile file to
not unset GIT_LFS_NO_TEST_COUNT when running lfstest-count-tests
before and after it runs the "prove" command.

Finally, as we have already removed the GIT_LFS_LOCK_ACQUIRE_DISABLED
environment variable and all support for it, in a previous commit in
this PR, that allows us now to also eliminate the unset_vars() shell
functions in some of our t/t-*.sh test scripts, which was used to unset
both that variable and the GIT_LFS_NO_TEST_COUNT variable before
running the tests in those scripts.  We added this behaviour in commit
aca1aff of PR git-lfs#3808, when we migrated
our CI test suite to GitHub Actions.  However, we will no longer need
to unset these variables as we will now never set them at all, and they
will have no effect if they were set.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Sep 6, 2024
In commit 9b73c80 of PR git-lfs#3125 the
GIT_LFS_LOCK_ACQUIRE_DISABLED environment variable was introduced
for use with our test suites.  When this variable was set to the value
"1", it indicated to the lfstest-count-tests test utility that it
should skip creating a test_count.lock file before updating the
test_count file we use to track the number of active test suites.

This was done because the lfstest-count-tests program had a bug
which prevented it from deleting the lock file, causing subsequent
invocations of the program to be unable to exclusively create it
again.  As this bug was not resolved at the time, the choice was
made to work around the problem by skipping the creation of a
lock file altogether on Windows.

This change was also made in conjunction with the addition of another
environment variable, GIT_LFS_NO_TEST_COUNT, in commit
c591ff7 of the same PR.  That variable
is also set only on Windows, and stops the setup() and shutdown()
shell functions defined in our t/testhelpers.sh script from calling
the lfstest-count-tests program.

As we have now resolved the underlying problem in the lfstest-count-tests
utility, in a prior commit in this PR, we can now remove both of
these environment variables and the test suite features they control.

We start by removing support for the GIT_LFS_LOCK_ACQUIRE_DISABLED
environment variable from the lfstest-count-tests utility, and
changing our script/cibuild script so it no longer sets the variable.

We can also remove the environment variable from the list of those
we unset before running certain tests.  We had to introduce this
behaviour in commit aca1aff of
PR git-lfs#3808, when we migrated our CI test suite to GitHub Actions.
However, we will no longer need to unset these variables as we will
now never set them at all, and they will have no effect if they
were set.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Sep 6, 2024
In commit c591ff7 of PR git-lfs#3125 the
GIT_LFS_NO_TEST_COUNT environment variable was introduced for use
with our test suites.  When this variable was set to a non-empty
value, it indicated to the setup() and shutdown() test helper
shell functions that they should skip running the lfstest-count-tests
utility program entirely.

This was done because the lfstest-count-tests program had a bug
which prevented it from deleting the lock file, causing subsequent
invocations of the program to be unable to exclusively create it
again.  As this bug was not resolved at the time, the choice was
made to work around the problem in part by not counting the number
of running test scripts on Windows.  (We also introduced the
GIT_LFS_LOCK_ACQUIRE_DISABLED environment variable, in commit
9b73c80 of the same PR, to further
address the problem.)

When running our test suite on Windows, we set the GIT_LFS_NO_TEST_COUNT
to "1", which causes the individual test suites to skip invoking
lfstest-count-tests entirely.  Instead, the program is run just once
in the "test" t/Makefile target before executing the test suites with
the "prove" command, and then once again afterwards, both times by
explicitly resetting the GIT_LFS_NO_TEST_COUNT variable to an empty
string value.  These two invocations cause the lfstest-count-tests
program to start the lfstest-gitserver program before all the test
suites were executed and then stop it once they had all finished.

As we have now resolved the underlying problem in the lfstest-count-tests
utility, in a prior commit in this PR, we can remove this environment
variable and the test suite features it controls.

We change our script/cibuild script to no longer set the
GIT_LFS_NO_TEST_COUNT variable, and we revise the conditionals
in the setup() and shutdown() test helper shell functions so they
always call the lfstest-count-tests program, rather than only doing
so if the GIT_LFS_NO_TEST_COUNT variable is unset or empty.  We
also change the "test" target's recipe in our t/Makefile file to
not unset GIT_LFS_NO_TEST_COUNT when running lfstest-count-tests
before and after it runs the "prove" command.

Finally, as we have already removed the GIT_LFS_LOCK_ACQUIRE_DISABLED
environment variable and all support for it, in a previous commit in
this PR, that allows us now to also eliminate the unset_vars() shell
functions in some of our t/t-*.sh test scripts, which was used to unset
both that variable and the GIT_LFS_NO_TEST_COUNT variable before
running the tests in those scripts.  We added this behaviour in commit
aca1aff of PR git-lfs#3808, when we migrated
our CI test suite to GitHub Actions.  However, we will no longer need
to unset these variables as we will now never set them at all, and they
will have no effect if they were set.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 7, 2024
Due to the issue described in git-lfs#5658, the "cloneSSL" and "clone ClientCert"
tests in our t/t-clone.sh test script do not run to completion but exit
early, as a consequence of an improper check of the TRAVIS variable (which
is no longer used since we migrated our test suite to GitHub Actions in
PR git-lfs#3808).

We expect to address this issue in subsequent commit in this PR, at which
point the tests will run fully, exposing a number of long-standing problems.
One of these occurs on the legacy CentOS 7 Linux system where the libcurl
library used by Git still depends on the libnss3 library, which has several
restrictions in the types of TLS certificates it accepts.  Specifically,
in our current builds on CentOS 7, libcurl version 7.29.0 is linked
against version 3.90 of libnss3.

The libnss3 library rejects certificates which have both Extended Key
Usage attributes for TLS Web Server Authentication, and the Basic
Constraint setting "CA:TRUE", meaning the certificate is intended
to be used as a Certificate Authority:

  https://security.stackexchange.com/questions/176177/why-does-curl-nss-encryption-library-not-allow-a-ca-with-the-extended-key-usage

However, certificates with both these settings are accepted by most
modern TLS libraries, and the self-signed certificate used by our
lfstest-gitserver utility program has both attributes.  Note that this
certificate is the one provided by the Go language's net/http/httptest
package, specifically the one from the net/http/internal/testcert package:

  https://github.com/golang/go/blob/7529d09a11496a77ccbffe245607fbd256200991/src/net/http/internal/testcert/testcert.go#L10-L33

Thus when we enable our "clone ClientCert" test, the second part of the
test, where it checks the use of an encrypted client key, will fail
on CentOS 7 because Git is dependent on the libnss3 library for TLS
certificate validation.

Moreover, we would also like to reverse the change made in commit
1cf7d6b of PR git-lfs#1893, when the
"http.<url>.sslVerify" Git configuration option was set to "false"
in the setup() shell function of our t/testhelpers.sh library.  This
was done, according to the commit description, to try to get the
"clone ClientCert" test working in Travis CI.  We no longer use
Travis, as noted above, and we can fully enable this test with
"http.<url>.sslVerify" set to its default value of "true", at least
on the majority of the systems we use in our GitHub Actions CI jobs.

When we make this change as well, three tests in the t/t-clone.sh
script (the "cloneSSL", "clone ClientCert", and "clone ClientCert with
homedir certs" tests) will fail because libnss3 rejects the certifcate
provided by the Go language test server, for the reason described
above.  In all three tests, we see libnss3 return a "Certificate type not
approved for application" SEC_ERROR_INADEQUATE_CERT_TYPE error.

We expect to drop support for CentOS 7 soon, given that it has
reached the end of offical support in all but one distribution
(SUSE Linux Enterprise Server 12 SP5, for which general support
ends on October 31 of 2024).  As well, CentOS 8 and more recent
related distributions have switched away from building libcurl
against libnss3; in fact, support for that TLS library has been
dropped by the curl project entirely as of v8.3.0, per commit
curl/curl@7c8bae0.

Therefore, rather than try to find a solution to this problem
by generating a different certificate for our lfstest-gitserver
utility program, we simply skip these three tests if we detect
that they are running on a Linux platform where the git-remote-https
binary depends on libnss3.  We can remove these exceptions later
once we drop support for CentOS 7.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 7, 2024
Due to the issue described in git-lfs#5658, the "cloneSSL" and "clone ClientCert"
tests in our t/t-clone.sh test script do not run to completion but exit
early, as a consequence of an improper check of the TRAVIS variable (which
is no longer used since we migrated our test suite to GitHub Actions in
PR git-lfs#3808).

As well, since the "clone ClientCert with homedir certs" test was added
in commit c54c9da, as part of PR git-lfs#5657,
it has skipped executing on Windows due to an error that was seen when
Git attempted to retrieve the passphrase for the encrypted key of the
TLS certificate used in that test from our git-credential-lfstest
helper program.

We expect to address the first issue in subsequent commit in this PR,
at which point the tests will run fully, exposing a number of
long-standing problems.

In the case of the "cloneSSL" test, Schannel's CertGetNameString() function
will reject the TLS certificate of our lfstest-gitserver program because
we connect to the server on the local IP address 127.0.0.1, and although
the certificate lists that address as an IPAddress field among its Subject
Alternative Name (SAN) attributes, the Schannel library only looks for
hostname matches with the DNSName fields and ignores any IPAddress fields.
As a result, Git returns a "failed to match connection hostname (127.0.0.1)
against server certificate names" error, as produced by libcurl.  As of
version 8.9.0 of libcurl, the CertGetNameString() function is called
with the CERT_NAME_DNS_TYPE option, which is documented to have this
behaviour:

  https://github.com/curl/curl/blob/5040f7e94cd01decbe7ba8fdacbf489182d503dc/lib/vtls/schannel_verify.c#L577-L582
  https://github.com/curl/curl/blob/5040f7e94cd01decbe7ba8fdacbf489182d503dc/lib/vtls/schannel_verify.c#L373-L378
  https://learn.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-certgetnamestringw#cert_name_dns_type

The TLS certificate presented by our lfstest-gitserver program is the
one provided by the Go language's net/http/httptest package, specifically
the one from the net/http/internal/testcert package:

  https://github.com/golang/go/blob/7529d09a11496a77ccbffe245607fbd256200991/src/net/http/internal/testcert/testcert.go#L10-L33

While it does include a DNSName field in its SAN list with the hostname
example.com, attempting to use this in our tests would involve forcing
both Git and Git LFS to override the available system DNS resolution.
For Git LFS in particular, there does not appear to be, at the moment,
a straightforward way of overriding the Go language's "net" package's
DNS resolution in a cross-platform and convenient manner.

Instead, with recent versions of Git and libcurl, we can bypass this
problem by setting the "http.sslBackend" Git configuration option
to "openssl", and thereby forcing the use of OpenSSL in place of
Schannel in libcurl.  This option has been available since commit
git/git@21084e8 in Git version 2.20.0,
and is supported so long as the version of libcurl is at 7.56.0.
Since our CI jobs on Windows are executed on GitHub Actions where the
pre-installed Git for Windows releases are kept current, we can
make use of this option in our tests where TLS certificate valiation
is required.

Note that this option does not affect how Git LFS behaves, because
it uses the TLS validation provided by the Go net/http and crypto/tls
packages, and because the intent of our tests is to confirm correct
behaviour of Git LFS and not Git, using OpenSSL instead of Schannel
to ensure Git accepts our test server's TLS certificate on Windows
does not compromise the integrity of our tests.  (Moreover, having
the tests actually run to completion is a big step forward over the
current state where they pass in all circumstances due to the bug
in our conditional check of the TRAVIS variable.)

As for the problem observed when the "clone ClientCert with homedir certs"
test was added, Git reports the error "refusing to work with credential
missing host field" when libcurl uses the default of the Schannel
library on Windows, and so for this reason, the test has always
been skipped on Windows.  When we enable the "clone ClientCert" test
as well by removing the check on the TRAVIS variable, it too will
report the same error.

However, we can again resolve this problem by switching to the OpenSSL
backend, as noted a discussion on the Git mailing list:

  https://lore.kernel.org/git/TY0PR06MB54426B92E745B3035F0CC2DFD197A@TY0PR06MB5442.apcprd06.prod.outlook.com/

So for both of these tests as well, we set the "http.sslBackend"
Git configuration option to "openssl" when they run on Windows, and
we can then allow the "clone ClientCert with homedir certs" to
execute on that platform for the first time since it was introduced.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 7, 2024
In commit caaa0f9 of PR git-lfs#2647 and
number of tests of the "git lfs clone" and "git lfs pull" commands
were enhanced so as check that after those commands are invoked,
a "git status" command returns a "working tree clean" message.
To perform these checks, a call to our assert_clean_status() shell
function was added to the tests.

In the case of the "cloneSSL" test, an assert_clean_status() call
was added, but left commented out; it was then uncommented in commit
e0eede1 of the same PR.  Unfortunately,
the call is made when the current working directory has not yet been
changed to that of the newly cloned repository's working tree, and
so will fail as it stands now.

However, as described in git-lfs#5658, the "cloneSSL" test and the "clone
ClientCert" tests in our t/t-clone.sh test script do not actually
run to completion, as a consequence of an improper check of the TRAVIS
variable (which is no longer used since we migrated our test suite to
GitHub Actions in PR git-lfs#3808).  This bug was already in place at the time
of PR git-lfs#2647, and so the assertions are never actually performed in
these tests.

We expect to address this problem in a subsequent commit in this PR,
and when we do so, the test "cloneSSL" test will fail because the
assert_clean_status() call is made without having changed the current
working directory to that of the cloned repository's working tree.

Therefore we move the assertion to the end of the block of checks
performed after the "pushd" shell command is used to change the
directory to that of the new clone's working tree.

We also take the opportunity to add the assert_clean_status() calls
to a number of other tests in the t/t-clone.sh script, so they are
all performing similar sets of checks.  This will help keep our
tests in closer aligment with each other.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 7, 2024
In commit d2c1c0f of PR git-lfs#1067 the
"cloneSSL" test in what is now our t/t-clone.sh test script was changed
with the intent that the test should be skipped when it was run in the
Travis CI environment.  The commit description from that time notes that
we could not determine why the test was failing only on that platform,
so the decision was made to just ignore the test in that case.

Subsequently, similar checks were added to three other tests.  First,
the "clone ClientCert" test was introduced in PR git-lfs#1893, and as of
commit b471566 in that PR it has
included an initial check for a Travis CI environment with the intent
of skipping the test when it is run there.

Next, in commits 9da65c0 of PR git-lfs#2500
and e53e34e of PR git-lfs#2609 the "askpass:
push with core.askPass" and "askpass: push with SSH_ASKPASS" tests
in what is now our t/t-askpass.sh tests script were amended to skip
execution in the Travis CI context.

In the case of the latter two tests, this check works as designed;
however, in the case of the two tests in t/t-clone.sh, the check is
written as a simple "if $TRAVIS" shell statement, rather than one
which uses a shell test command or builtin ("test" or "[" or "[[").
As described in git-lfs#5658, the result is that when the TRAVIS variable
is undefined, these statements always succeed, and so their conditional
blocks run, meaning these tests are skipped on all platforms, at
least since we migrated our CI jobs to GitHub Actions in PR git-lfs#3808.

In previous commits in this PR we have addressed all the latent
bugs that have accumulated in these tests since this problem was
first introduced, and so we are now in a position to remove both
the valid and invalid checks for the TRAVIS environment variable
from all of our tests.  This should ensure that the full set of
tests in both t/t-askpass.sh and t/t-clone.sh run on all our CI
and build platforms.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 7, 2024
In PR git-lfs#1893 we introduced support for TLS client certificates, and
while that was largely successful, some of our CI test jobs ran on the
Travis CI platform, and those encountered problems when validating the
TLS certificate generated by our lfstest-gitserver test utility program.
Several attempts were made to resolve this issue, and ultimately in
commit 1cf7d6b the "http.<url>.sslVerify"
Git configuration value was set to "false" to allow the "clone ClientCert"
test to pass in the Travis CI environment.

However, we no longer use the Travis CI platform, as we migrated our
CI jobs to GitHub Actions in PR git-lfs#3808.  Further, in a prior commit in
this PR we have revised several tests in our t/t-clone.sh test script
so they do not run on the legacy CentOS 7 platform, where the libcurl
library used by Git depends on the libnss3 library.  The libnss3 library
does not accept the TLS certificate generated by our lfstest-gitserver
program, so these tests would otherwise fail on CentOS 7 if
"http.<url>.sslVerify" is set to "true".

As well, in another commit in this PR we have altered our t/t-clone.sh
test script to require the use of OpenSSL on Windows instead of the
default Schannel backend.

With these changes to our test suite, all our tests should now pass
if we allow the "http.<url>.sslVerify" Git configuration option to
default to "true", so we now remove the call which explicitly set
that option to "false".
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 7, 2024
In commit 32244d9 of PR git-lfs#1825 the
APPVEYOR_REPO_COMMIT_MESSAGE variable was added to our CI test suite
environment in order to demonstrate that a problem exposed by the prior
commit 0951c69 in the same PR had been
addresssed.

Specifically, the earlier commit had the string "GIT_SSH" in its
message, and the AppVeyor CI service automatically provided the message
from the most recent commit in a Git branch under test in the
APPVEYOR_REPO_COMMIT_MESSAGE environment variable.  At the time,
the Environ() function in our "lfs" package would populate map it
returns with the names and values of any environment variables whose
names or values contained the string "GIT_".  The result was that
the APPVEYOR_REPO_COMMIT_MESSAGE variable was included in the returned
environment, which would cause the tests in what is now our t/t-env.sh
test script to fail.

The problem was resolved by adjusting the Environ() function to only
include environment variables in its map when their name contained
the string "GIT_".

To validate this fix, our test script environment was updated to
write a fixed value into the APPVEYOR_REPO_COMMIT_MESSAGE environment
variable (overriding whatever the AppVeyor service set) with an
example string containing the string "GIT_".  The example string used
was the commit message from the commit which exposed the problem.

Subsequently, we further revised the Environ() function in commit
f4f8fae of PR git-lfs#4269 so it only
includes environment variables whose names start with "GIT_",
excluding those whose names just include that string.

As we no longer use the AppVeyor service since we migrated our CI jobs
to GitHub Actions in PR git-lfs#3808, and because the intent of the
APPVEYOR_REPO_COMMIT_MESSAGE variable is now somewhat obscure, we
replace it with a dedicated TEST_GIT_EXAMPLE environment variable
in our test scripts where the output of "git lfs env" is checked.
We ensure the "GIT_" string is included in both the name and value
of this variable, and we preface it with some comments to clarify
its purpose.

If a regression were ever to be introduced into the Environ() function
such that it did not exclude environment variables with "GIT_" in their
names or values, the tests in the t/t-env.sh and t/t-worktree.sh scripts
should still catch the problem.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 9, 2024
Due to the issue described in git-lfs#5658, the "clone ClientCert" test in our
t/t-clone.sh test script does not actually run to completion, because
it exits early due to an improper check of the TRAVIS variable (which
is, itself, no longer used since we migrated our test suite to GitHub
Actions in PR git-lfs#3808).

We will address this issue in a subsequent commit in this PR, at which
point the test will run fully, exposing a number of long-standing problems
with the test.  One of these occurs on Windows when the test checks the
use of a TLS/SSL client certificate with an encrypted client key file.
This check was added in commit 706beca
of PR git-lfs#3270 in 2018, but may never have been fully validated since then.

Our Windows CI jobs run in the Git Bash environment provided by the
Git for Windows project, which in turn is based on the MSYS2 environment.
In this context, when we set the "http.<url>.sslCert" Git configuration
value in our setup() shell function in t/testhelpers.sh, the command-line
argument with the path to the certificate file is converted by the MSYS2
environment into a Windows-style path with a leading volume identifier.

When Git later makes a request to our credential helper test program to
retrieve the passphrase for the encrypted key, it passes this path in the
input "path" field.  Our helper program then converts this path into a
filename by replacing slash characters with dashes, and looks for a
matching credential record file.  However, we have created this file
in the setup_creds() shell function using the Unix-style path in the
LFS_CLIENT_KEY_FILE_ENCRYPTED environment variable we populate in
our t/testenv.sh script.  As a result, our credential helper program
is unable to find the record file with the passphrase for the encrypted
key, and Git then attempts to prompt the user for it, which also fails
because our CI jobs run in a context without a terminal prompt, and
the "clone ClientCert" test fails as a consequence.

In theory we might work around this problem by setting the
MSYS2_ARG_CONV_EXCL environment variable with a "*" value before
calling "git config" to set the "http.<url>.sslCert" configuration
value, as this would prevent MSYS2 from rewriting command-line arguments
which look like Unix-style paths.  Unfortunately, this introduces
other failures into our test suite on Windows.

Instead, we work around the problem by performing a simple
Windows-to-Unix path translation in our credential helper test program.
As we note in a code comment, a more comprehensive solution might
invoke the "cygpath" utility to perform the path conversion.  But
as we only need to perform this translation step in a single test
in our Windows CI jobs, for the deprecated "git lfs clone" command,
for now we just look for paths which start with an uppercase volume
identifier such as "D:" and replace them with a lowercase version
which starts with a leading slash, like "/d".  This is sufficient to
resolve the problem in our current Windows CI jobs on GitHub Actions
runners.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 9, 2024
Due to the issue described in git-lfs#5658, the "clone ClientCert" test in our
t/t-clone.sh test script does not actually run to completion, because
it exits early due to an improper check of the TRAVIS variable (which
is, itself, no longer used since we migrated our test suite to GitHub
Actions in PR git-lfs#3808).

We will address this issue in a subsequent commit in this PR, at which
point the test will run fully, exposing a number of long-standing
problems with the test.  One of these occurs on Debian and Ubuntu Linux
systems where the libcurl library used by Git depends on a version of
the GnuTLS library older than v3.8.0, which at the moment, includes all
of our Debian build target platforms (Debian 10, 11, and 12).

Version 3.8.0 of GnuTLS contains a bug fix which allows it to parse and
accept DEK-Info PEM headers with lowercase hexadecimal strings.  Prior
to this change, it would reject these with an "invalid request" error:

  gnutls/gnutls@4604bbd
  https://gitlab.com/gnutls/gnutls/-/merge_requests/1655

As it happens, the encrypted TLS certificate key generated by our
libtest-gitserver test utility has its DEK-Info header formatted with
lowercase "a" through "f" letters, as this is what the EncodeToString()
function of the Go language "encoding/hex" package generates, and
that function is used by the (deprecated) EncryptPEMBlock() function
of the "crypto/x509" package which our test server calls to create the
encrypted key:

  https://github.com/golang/go/blob/7529d09a11496a77ccbffe245607fbd256200991/src/encoding/hex/hex.go#L17
  https://github.com/golang/go/blob/7529d09a11496a77ccbffe245607fbd256200991/src/crypto/x509/pem_decrypt.go#L228

When our "clone ClientCert" test runs on a Debian system, GnuTLS rejects
this key, which means Git in turn rejects it, and the test fails.
Our "clone ClientCert" test is the only one which validates the use
of the encrypted key, so it alone is affected by this problem, and because
the test always exits early at the moment (due to the issue described
in git-lfs#5658), we have not noticed the failure until now.

We can work around the problem simply by rewriting the DEK-Info header
returned by the EncryptPEMBlock() function to use uppercase "A" through
"F" letters, which in turns allows GnuTLS to validate the salt, so
Git proceeds and our test succeeds.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 9, 2024
Due to the issue described in git-lfs#5658, the "cloneSSL" and "clone ClientCert"
tests in our t/t-clone.sh test script do not run to completion but exit
early, as a consequence of an improper check of the TRAVIS variable (which
is no longer used since we migrated our test suite to GitHub Actions in
PR git-lfs#3808).

We will address this issue in a subsequent commit in this PR, at which
point the tests will run fully, exposing a number of long-standing
problems.  One of these occurs on the legacy CentOS 7 Linux system where
the libcurl library used by Git still depends on the libnss3 library,
which has several restrictions in the types of TLS certificates it
accepts.  Specifically, in our current builds on CentOS 7, libcurl
version 7.29.0 is linked against version 3.90 of libnss3.

The libnss3 library rejects certificates which have both an Extended Key
Usage attribute for TLS Web Server Authentication and a Basic Constraint
setting of "CA:TRUE", meaning the certificate is intended to be used as
a Certificate Authority:

  https://security.stackexchange.com/questions/176177/why-does-curl-nss-encryption-library-not-allow-a-ca-with-the-extended-key-usage

However, certificates with both these settings are accepted by most
modern TLS libraries, and the self-signed certificate used by our
lfstest-gitserver utility program has both attributes.  Note that this
certificate is the one provided by the Go language's "net/http/httptest"
package, specifically the one from the "net/http/internal/testcert"
package:

  https://github.com/golang/go/blob/7529d09a11496a77ccbffe245607fbd256200991/src/net/http/internal/testcert/testcert.go#L10-L33

Thus when we enable our "clone ClientCert" test, the second part of the
test, where it checks the use of an encrypted client key, will fail
on CentOS 7 because Git on that platform is dependent on the libnss3
library for TLS certificate validation.

Moreover, we would also like to reverse the change made in commit
1cf7d6b of PR git-lfs#1893, when the
"http.<url>.sslVerify" Git configuration option was set to "false"
in the setup() shell function of our t/testhelpers.sh library.  This
was done, according to the commit description, to try to get the
"clone ClientCert" test working in the Travis CI environment.  We no
longer use the Travis CI service, as noted above, but independent of
that, we want to be able to run all our tests with "http.<url>.sslVerify"
set to its default value of "true".

When we set the "http.<url>.sslVerify" Git configuration option to
"true", however, three tests in the t/t-clone.sh test script (the
"cloneSSL", "clone ClientCert", and "clone ClientCert with homedir
certs" tests) will fail on CentOS 7 because libnss3 rejects the TLS
certificate provided by the Go language test server, for the reasons
described above.  In these tests, libnss3 will return a "Certificate
type not approved for application" SEC_ERROR_INADEQUATE_CERT_TYPE error.

We expect to drop support for CentOS 7 soon, given that it has
reached the end of official support in all but one distribution
(SUSE Linux Enterprise Server 12 SP5, for which general support
ends on October 31 of 2024).  As well, CentOS 8 and more recent
related distributions have switched away from building libcurl
against libnss3; in fact, support for that TLS library has been
dropped by the curl project entirely as of v8.3.0, per commit
curl/curl@7c8bae0.

Therefore, rather than try to generate a different TLS certificate
for our lfstest-gitserver utility program, we simply skip the three
affected tests when we detect that they are running on a Linux platform
where the git-remote-https binary depends on libnss3.  We can remove
these exceptions later once we drop support for CentOS 7.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 9, 2024
Due to the issue described in git-lfs#5658, the "cloneSSL" and "clone ClientCert"
tests in our t/t-clone.sh test script do not run to completion but exit
early, as a consequence of an improper check of the TRAVIS variable (which
is no longer used since we migrated our test suite to GitHub Actions in
PR git-lfs#3808).

As well, since the "clone ClientCert with homedir certs" test was added
in commit c54c9da, as part of PR git-lfs#5657,
we have skipped executing it on Windows.  We do this to avoid an error
when Git attempts to retrieve the encrypted key of the test's client
TLS certificate from our git-credential-lfstest test utility program.

We will address the first issue in subsequent commit in this PR, at
which point the tests will run fully, exposing a number of long-standing
problems with the "cloneSSL" and "clone ClientCert" tests.

One of these problems occurs on Windows in the "cloneSSL" test.  By
default, Git on Windows uses the Secure Channel (SChannel) backend for
TLS/SSL verification in libcurl.  However, Schannel's CertGetNameString()
function will reject the TLS certificate of our lfstest-gitserver program
because we connect to the server on the local IP address 127.0.0.1, and
although the certificate lists that address in an IPAddress field among
its Subject Alternative Name (SAN) attributes, the Schannel library only
looks for hostname matches with the DNSName fields and ignores any
IPAddress fields.  As a result, Git returns a "failed to match connection
hostname (127.0.0.1) against server certificate names" error, as produced
by libcurl.  Since version 8.9.0 of libcurl the CertGetNameString()
function is called with the CERT_NAME_DNS_TYPE option, which is
documented to have this behaviour:

  https://github.com/curl/curl/blob/5040f7e94cd01decbe7ba8fdacbf489182d503dc/lib/vtls/schannel_verify.c#L577-L582
  https://github.com/curl/curl/blob/5040f7e94cd01decbe7ba8fdacbf489182d503dc/lib/vtls/schannel_verify.c#L373-L378
  https://learn.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-certgetnamestringw#cert_name_dns_type

The TLS certificate presented by our lfstest-gitserver program is the
one provided by the Go language's "net/http/httptest" package, specifically
the one from the "net/http/internal/testcert" package:

  https://github.com/golang/go/blob/7529d09a11496a77ccbffe245607fbd256200991/src/net/http/internal/testcert/testcert.go#L10-L33

While it does include a DNSName field in its SAN list with the hostname
"example.com", attempting to use this in our tests would involve forcing
both Git and Git LFS to override the available system DNS resolution.
For Git LFS in particular, there does not appear to be, at the moment,
a straightforward way of overriding the DNS resolution of the Go
language's "net" package in a cross-platform and convenient manner.

Instead, with recent versions of Git and libcurl, we can bypass this
problem by setting the "http.sslBackend" Git configuration option
to "openssl", and thereby forcing the use of OpenSSL in place of
Schannel in libcurl.  This option has been available since commit
git/git@21084e8 in Git version 2.20.0,
and is supported so long as libcurl's version is 7.56.0 or above.
Since our CI jobs on Windows are executed on GitHub Actions, where the
pre-installed Git for Windows releases are kept current, we can make
use of this Git option in our tests when TLS certificate validation
is required.

Note that this option does not affect how Git LFS behaves, because it
uses the TLS validation provided by the Go "net/http" and "crypto/tls"
packages.  Also, because the intent of our tests is to confirm correct
behaviour of Git LFS and not Git, using OpenSSL instead of Schannel
to ensure Git accepts our test server's TLS certificate on Windows
does not compromise the integrity of our tests.  (Further, having our
tests actually run to completion will be a significant improvement over
the current state, where they trivially pass in all circumstances due
to the bug in our conditional check of the TRAVIS variable.)

As for the problem mentioned earlier regarding the reason we skip the
"clone ClientCert with homedir certs" test on Windows, this also stems
from the use of the Schannel backend.  Specifically, Git reports the
error "refusing to work with credential missing host field" when
libcurl uses the Schannel library, and so we have always skipped this
test on Windows.

When we enable the "clone ClientCert" test, by removing the check on
the TRAVIS variable, it too will report the same error.

We can again resolve this problem for both tests by switching to the
OpenSSL backend, as noted in a discussion on the Git mailing list:

  https://lore.kernel.org/git/TY0PR06MB54426B92E745B3035F0CC2DFD197A@TY0PR06MB5442.apcprd06.prod.outlook.com/

So for these tests we also set the "http.sslBackend" Git configuration
option to "openssl" when executing them on Windows.  Notably, this will
allow the "clone ClientCert with homedir certs" test to run successfully
on that platform for the first time since it was introduced.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 9, 2024
In commit caaa0f9 of PR git-lfs#2647 a
number of tests of the "git lfs clone" and "git lfs pull" commands
were enhanced so as to check that after those commands are invoked,
the "git status" command returns a "working tree clean" message.
To perform these checks, a call to our assert_clean_status()
shell function was added to the tests.

In the case of the "cloneSSL" test, an assert_clean_status() call
was added, but left commented out; it was then uncommented in commit
e0eede1 of the same PR.  Unfortunately,
the call is made when the current working directory has not yet been
changed to that of the newly cloned repository's working tree, and
so should fail as it stands now.

However, as described in git-lfs#5658, the "cloneSSL" test and the "clone
ClientCert" tests in our t/t-clone.sh test script do not actually
run to completion, a consequence of an improper check of the TRAVIS
variable (which is no longer used since we migrated our test suite to
GitHub Actions in PR git-lfs#3808).  Instead, the tests exit early and
always declare success.

We will address this problem in a subsequent commit in this PR, and when
we do so, the "cloneSSL" test will fail because the misplaced assertion.
Therefore we move the assertion into the block of checks performed
after a "pushd" shell command changes the current directory to that
of the new clone's working tree, which will allow the test to pass
when we resolve the bug with the TRAVIS variable check.

We also take the opportunity to add assert_clean_status() calls to a
number of other tests in the t/t-clone.sh test script, so the tests are
all performing similar sets of checks.  This will help us keep the
tests in that script more consistent with each other in the future.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 9, 2024
In PR git-lfs#1893 we introduced support for TLS client certificates, and
while that was largely successful, some of our CI test jobs ran on the
Travis CI platform, and those encountered problems when validating the
TLS certificate generated by our lfstest-gitserver test utility program.
Several attempts were made to resolve this issue, but ultimately in
commit 1cf7d6b the "http.<url>.sslVerify"
Git configuration value was set to "false" for our entire test suite,
just to allow the "clone ClientCert" test to pass in the Travis CI
environment.

However, we no longer use the Travis CI service, as we migrated our
CI jobs to GitHub Actions in PR git-lfs#3808.  Further, in a prior commit in
this PR we revised several tests in our t/t-clone.sh test script so
they do not run on the legacy CentOS 7 platform, where the libcurl
library used by Git depends on the libnss3 library.  The libnss3 library
does not accept the TLS certificate generated by our lfstest-gitserver
program, so these tests would otherwise fail on CentOS 7 if the
"http.<url>.sslVerify" option is set to "true".

As well, in another commit in this PR we altered the same tests in our
t/t-clone.sh test script to require that Git and libcurl use OpenSSL for
TLS verification on Windows, rather than the default Schannel backend.
This will prevent the tests from failing if TLS verification is enabled,
since Schannel rejects the TLS certificate that we use in our
lfstest-gitserver test utility, but OpenSSL accepts it.

With these changes, all our tests should now pass if we allow the
"http.<url>.sslVerify" Git configuration option to default to "true".
Therefore we now remove the command which set that option to "false"
for our whole test suite.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 9, 2024
Due to the issue described in git-lfs#5658, the "cloneSSL" and "clone ClientCert"
tests in our t/t-clone.sh test script do not run to completion but exit
early, as a consequence of an improper check of the TRAVIS variable (which
is no longer used since we migrated our test suite to GitHub Actions in
PR git-lfs#3808).

As well, since the "clone ClientCert with homedir certs" test was added
in commit c54c9da, as part of PR git-lfs#5657,
we have skipped executing it on Windows.  We do this to avoid an error
when Git attempts to retrieve the encrypted key of the test's client
TLS certificate from our git-credential-lfstest test utility program.

We will address the first issue in subsequent commit in this PR, at
which point the tests will run fully, exposing a number of long-standing
problems with the "cloneSSL" and "clone ClientCert" tests.

One of these problems occurs on Windows, where by default, Git uses the
Secure Channel (SChannel) backend for TLS/SSL verification in libcurl.
However, Schannel's CertGetNameString() function rejects the TLS
certificate of our lfstest-gitserver program because we connect to the
server on the local IP address 127.0.0.1, and although the certificate
lists that address in an IPAddress field among its Subject Alternative
Name (SAN) attributes, the Schannel library only looks for hostname
matches with the DNSName fields and ignores any IPAddress fields.
As a result, Git returns a "failed to match connection hostname
(127.0.0.1) against server certificate names" error, as produced by
libcurl.  Since version 8.9.0 of libcurl the CertGetNameString() function
is called with the CERT_NAME_DNS_TYPE option, which is documented to have
this behaviour:

  https://github.com/curl/curl/blob/5040f7e94cd01decbe7ba8fdacbf489182d503dc/lib/vtls/schannel_verify.c#L577-L582
  https://github.com/curl/curl/blob/5040f7e94cd01decbe7ba8fdacbf489182d503dc/lib/vtls/schannel_verify.c#L373-L378
  https://learn.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-certgetnamestringw#cert_name_dns_type

The TLS certificate presented by our lfstest-gitserver program is the
one provided by the Go language's "net/http/httptest" package, specifically
the one from the "net/http/internal/testcert" package:

  https://github.com/golang/go/blob/7529d09a11496a77ccbffe245607fbd256200991/src/net/http/internal/testcert/testcert.go#L10-L33

While it does include a DNSName field in its SAN list with the hostname
"example.com", attempting to use this in our tests would involve forcing
both Git and Git LFS to override the available system DNS resolution.
For Git LFS in particular, there does not appear to be, at the moment,
a straightforward way of overriding the DNS resolution of the Go
language's "net" package in a cross-platform and convenient manner.

Instead, with recent versions of Git and libcurl, we can bypass this
problem by setting the "http.sslBackend" Git configuration option
to "openssl", and thereby forcing the use of OpenSSL in place of
Schannel in libcurl.  This option has been available since commit
git/git@21084e8 in Git version 2.20.0,
and is supported so long as libcurl's version is 7.56.0 or above.
Since our CI jobs on Windows are executed on GitHub Actions, where the
pre-installed Git for Windows releases are kept current, we can make
use of this Git option in our tests when TLS certificate validation
is required.

Note that this option does not affect how Git LFS behaves, because it
uses the TLS validation provided by the Go "net/http" and "crypto/tls"
packages.  Also, because the intent of our tests is to confirm correct
behaviour of Git LFS and not Git, using OpenSSL instead of Schannel
to ensure Git accepts our test server's TLS certificate on Windows
does not compromise the integrity of our tests.  (Further, having our
tests actually run to completion will be a significant improvement over
the current state, where they trivially pass in all circumstances due
to the bug in our conditional check of the TRAVIS variable.)

As for the problem mentioned earlier regarding the reason we skip the
"clone ClientCert with homedir certs" test on Windows, this also stems
from the use of the Schannel backend.  Specifically, Git reports the
error "refusing to work with credential missing host field" when
libcurl uses the Schannel library, and so we have always skipped this
test on Windows.

When we enable the "clone ClientCert" test, by removing the check on
the TRAVIS variable, it too will report the same error.

We can again resolve this problem for both tests by switching to the
OpenSSL backend, as noted in a discussion on the Git mailing list:

  https://lore.kernel.org/git/TY0PR06MB54426B92E745B3035F0CC2DFD197A@TY0PR06MB5442.apcprd06.prod.outlook.com/

So for these tests we also set the "http.sslBackend" Git configuration
option to "openssl" when executing them on Windows.  Notably, this will
allow the "clone ClientCert with homedir certs" test to run successfully
on that platform for the first time since it was introduced.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 9, 2024
In commit caaa0f9 of PR git-lfs#2647 a
number of tests of the "git lfs clone" and "git lfs pull" commands
were enhanced so as to check that after those commands are invoked,
the "git status" command returns a "working tree clean" message.
To perform these checks, a call to our assert_clean_status()
shell function was added to the tests.

In the case of the "cloneSSL" test, an assert_clean_status() call
was added, but left commented out; it was then uncommented in commit
e0eede1 of the same PR.  Unfortunately,
the call is made when the current working directory has not yet been
changed to that of the newly cloned repository's working tree, and
so should fail as it stands now.

However, as described in git-lfs#5658, the "cloneSSL" test and the "clone
ClientCert" tests in our t/t-clone.sh test script do not actually
run to completion, a consequence of an improper check of the TRAVIS
variable (which is no longer used since we migrated our test suite to
GitHub Actions in PR git-lfs#3808).  Instead, the tests exit early and
always declare success.

We will address this problem in a subsequent commit in this PR, and when
we do so, the "cloneSSL" test will fail because the misplaced assertion.
Therefore we move the assertion into the block of checks performed
after a "pushd" shell command changes the current directory to that
of the new clone's working tree, which will allow the test to pass
when we resolve the bug with the TRAVIS variable check.

We also take the opportunity to add assert_clean_status() calls to a
number of other tests in the t/t-clone.sh test script, so the tests are
all performing similar sets of checks.  This will help us keep the
tests in that script more consistent with each other in the future.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 9, 2024
In PR git-lfs#1893 we introduced support for TLS client certificates, and
while that was largely successful, some of our CI test jobs ran on the
Travis CI platform, and those encountered problems when validating the
TLS certificate generated by our lfstest-gitserver test utility program.
Several attempts were made to resolve this issue, but ultimately in
commit 1cf7d6b the "http.<url>.sslVerify"
Git configuration value was set to "false" for our entire test suite,
just to allow the "clone ClientCert" test to pass in the Travis CI
environment.

However, we no longer use the Travis CI service, as we migrated our
CI jobs to GitHub Actions in PR git-lfs#3808.  Further, in a prior commit in
this PR we revised several tests in our t/t-clone.sh test script so
they do not run on the legacy CentOS 7 platform, where the libcurl
library used by Git depends on the libnss3 library.  The libnss3 library
does not accept the TLS certificate generated by our lfstest-gitserver
program, so these tests would otherwise fail on CentOS 7 if the
"http.<url>.sslVerify" option is set to "true".

As well, in another commit in this PR we altered the same tests in our
t/t-clone.sh test script to require that Git and libcurl use OpenSSL for
TLS verification on Windows, rather than the default Schannel backend.
This will prevent the tests from failing if TLS verification is enabled,
since Schannel rejects the TLS certificate that we use in our
lfstest-gitserver test utility, but OpenSSL accepts it.

With these changes, all our tests should now pass if we allow the
"http.<url>.sslVerify" Git configuration option to default to "true".
Therefore we now remove the command which set that option to "false"
for our whole test suite.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 29, 2024
Due to the issue described in git-lfs#5658, the "clone ClientCert" test in our
t/t-clone.sh test script does not actually run to completion, because
it exits early due to an improper check of the TRAVIS variable (which
is, itself, no longer used since we migrated our test suite to GitHub
Actions in PR git-lfs#3808).

We will address this issue in a subsequent commit in this PR, at which
point the test will run fully, exposing a number of long-standing
problems with the test.  One of these occurs on Debian and Ubuntu Linux
systems where the libcurl library used by Git depends on a version of
the GnuTLS library older than v3.8.0, which at the moment, includes all
of our Debian build target platforms (Debian 10, 11, and 12).

Version 3.8.0 of GnuTLS contains a bug fix which allows it to parse and
accept DEK-Info PEM headers with lowercase hexadecimal strings.  Prior
to this change, it would reject these with an "invalid request" error:

  gnutls/gnutls@4604bbd
  https://gitlab.com/gnutls/gnutls/-/merge_requests/1655

As it happens, the encrypted TLS certificate key generated by our
lfstest-gitserver test utility has its DEK-Info header formatted with
lowercase "a" through "f" letters, as this is what the EncodeToString()
function of the Go language "encoding/hex" package generates, and
that function is used by the (deprecated) EncryptPEMBlock() function
of the "crypto/x509" package which our test server calls to create the
encrypted key:

  https://github.com/golang/go/blob/7529d09a11496a77ccbffe245607fbd256200991/src/encoding/hex/hex.go#L17
  https://github.com/golang/go/blob/7529d09a11496a77ccbffe245607fbd256200991/src/crypto/x509/pem_decrypt.go#L228

When our "clone ClientCert" test runs on a Debian system, GnuTLS rejects
this key, which means Git in turn rejects it, and the test fails.
Our "clone ClientCert" test is the only one which validates the use
of the encrypted key, so it alone is affected by this problem, and because
the test always exits early at the moment (due to the issue described
in git-lfs#5658), we have not noticed the failure until now.

We can work around the problem simply by rewriting the DEK-Info header
returned by the EncryptPEMBlock() function to use uppercase "A" through
"F" letters, which in turns allows GnuTLS to validate the salt, so
Git proceeds and our test succeeds.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 29, 2024
Due to the issue described in git-lfs#5658, the "cloneSSL" and "clone ClientCert"
tests in our t/t-clone.sh test script do not run to completion but exit
early, as a consequence of an improper check of the TRAVIS variable (which
is no longer used since we migrated our test suite to GitHub Actions in
PR git-lfs#3808).

We will address this issue in a subsequent commit in this PR, at which
point the tests will run fully, exposing a number of long-standing
problems.  One of these occurs on the legacy CentOS 7 Linux system where
the libcurl library used by Git still depends on the libnss3 library,
which has several restrictions in the types of TLS certificates it
accepts.  Specifically, in our current builds on CentOS 7, libcurl
version 7.29.0 is linked against version 3.90 of libnss3.

The libnss3 library rejects certificates which have both an Extended Key
Usage attribute for TLS Web Server Authentication and a Basic Constraint
setting of "CA:TRUE", meaning the certificate is intended to be used as
a Certificate Authority:

  https://security.stackexchange.com/questions/176177/why-does-curl-nss-encryption-library-not-allow-a-ca-with-the-extended-key-usage

However, certificates with both these settings are accepted by most
modern TLS libraries, and the self-signed certificate used by our
lfstest-gitserver utility program has both attributes.  Note that this
certificate is the one provided by the Go language's "net/http/httptest"
package, specifically the one from the "net/http/internal/testcert"
package:

  https://github.com/golang/go/blob/7529d09a11496a77ccbffe245607fbd256200991/src/net/http/internal/testcert/testcert.go#L10-L33

Thus when we enable our "clone ClientCert" test, the second part of the
test, where it checks the use of an encrypted client key, will fail
on CentOS 7 because Git on that platform is dependent on the libnss3
library for TLS certificate validation.

Moreover, we would also like to reverse the change made in commit
1cf7d6b of PR git-lfs#1893, when the
"http.<url>.sslVerify" Git configuration option was set to "false"
in the setup() shell function of our t/testhelpers.sh library.  This
was done, according to the commit description, to try to get the
"clone ClientCert" test working in the Travis CI environment.  We no
longer use the Travis CI service, as noted above, but independent of
that, we want to be able to run all our tests with "http.<url>.sslVerify"
set to its default value of "true".

When we set the "http.<url>.sslVerify" Git configuration option to
"true", however, three tests in the t/t-clone.sh test script (the
"cloneSSL", "clone ClientCert", and "clone ClientCert with homedir
certs" tests) will fail on CentOS 7 because libnss3 rejects the TLS
certificate provided by the Go language test server, for the reasons
described above.  In these tests, libnss3 will return a "Certificate
type not approved for application" SEC_ERROR_INADEQUATE_CERT_TYPE error.

We expect to drop support for CentOS 7 soon, given that it has
reached the end of official support in all but one distribution
(SUSE Linux Enterprise Server 12 SP5, for which general support
ends on October 31 of 2024).  As well, CentOS 8 and more recent
related distributions have switched away from building libcurl
against libnss3; in fact, support for that TLS library has been
dropped by the curl project entirely as of v8.3.0, per commit
curl/curl@7c8bae0.

Therefore, rather than try to generate a different TLS certificate
for our lfstest-gitserver utility program, we simply skip the three
affected tests when we detect that they are running on a Linux platform
where the git-remote-https binary depends on libnss3.  We can remove
these exceptions later once we drop support for CentOS 7.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 29, 2024
Due to the issue described in git-lfs#5658, the "cloneSSL" and "clone ClientCert"
tests in our t/t-clone.sh test script do not run to completion but exit
early, as a consequence of an improper check of the TRAVIS variable (which
is no longer used since we migrated our test suite to GitHub Actions in
PR git-lfs#3808).

As well, since the "clone ClientCert with homedir certs" test was added
in commit c54c9da, as part of PR git-lfs#5657,
we have skipped executing it on Windows.  We do this to avoid an error
when Git attempts to retrieve the encrypted key of the test's client
TLS certificate from our git-credential-lfstest test utility program.

We will address the first issue in subsequent commit in this PR, at
which point the tests will run fully, exposing a number of long-standing
problems with the "cloneSSL" and "clone ClientCert" tests.

One of these problems occurs on Windows, where by default, Git uses the
Secure Channel (SChannel) backend for TLS/SSL verification in libcurl.
However, Schannel's CertGetNameString() function rejects the TLS
certificate of our lfstest-gitserver program because we connect to the
server on the local IP address 127.0.0.1, and although the certificate
lists that address in an IPAddress field among its Subject Alternative
Name (SAN) attributes, the Schannel library only looks for hostname
matches with the DNSName fields and ignores any IPAddress fields.
As a result, Git returns a "failed to match connection hostname
(127.0.0.1) against server certificate names" error, as produced by
libcurl.  Since version 8.9.0 of libcurl the CertGetNameString() function
is called with the CERT_NAME_DNS_TYPE option, which is documented to have
this behaviour:

  https://github.com/curl/curl/blob/5040f7e94cd01decbe7ba8fdacbf489182d503dc/lib/vtls/schannel_verify.c#L577-L582
  https://github.com/curl/curl/blob/5040f7e94cd01decbe7ba8fdacbf489182d503dc/lib/vtls/schannel_verify.c#L373-L378
  https://learn.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-certgetnamestringw#cert_name_dns_type

The TLS certificate presented by our lfstest-gitserver program is the
one provided by the Go language's "net/http/httptest" package, specifically
the one from the "net/http/internal/testcert" package:

  https://github.com/golang/go/blob/7529d09a11496a77ccbffe245607fbd256200991/src/net/http/internal/testcert/testcert.go#L10-L33

While it does include a DNSName field in its SAN list with the hostname
"example.com", attempting to use this in our tests would involve forcing
both Git and Git LFS to override the available system DNS resolution.
For Git LFS in particular, there does not appear to be, at the moment,
a straightforward way of overriding the DNS resolution of the Go
language's "net" package in a cross-platform and convenient manner.

Instead, with recent versions of Git and libcurl, we can bypass this
problem by setting the "http.sslBackend" Git configuration option
to "openssl", and thereby forcing the use of OpenSSL in place of
Schannel in libcurl.  This option has been available since commit
git/git@21084e8 in Git version 2.20.0,
and is supported so long as libcurl's version is 7.56.0 or above.
Since our CI jobs on Windows are executed on GitHub Actions, where the
pre-installed Git for Windows releases are kept current, we can make
use of this Git option in our tests when TLS certificate validation
is required.

Note that this option does not affect how Git LFS behaves, because it
uses the TLS validation provided by the Go "net/http" and "crypto/tls"
packages.  Also, because the intent of our tests is to confirm correct
behaviour of Git LFS and not Git, using OpenSSL instead of Schannel
to ensure Git accepts our test server's TLS certificate on Windows
does not compromise the integrity of our tests.  (Further, having our
tests actually run to completion will be a significant improvement over
the current state, where they trivially pass in all circumstances due
to the bug in our conditional check of the TRAVIS variable.)

As for the problem mentioned earlier regarding the reason we skip the
"clone ClientCert with homedir certs" test on Windows, this also stems
from the use of the Schannel backend.  Specifically, Git reports the
error "refusing to work with credential missing host field" when
libcurl uses the Schannel library, and so we have always skipped this
test on Windows.

When we enable the "clone ClientCert" test, by removing the check on
the TRAVIS variable, it too will report the same error.

We can again resolve this problem for both tests by switching to the
OpenSSL backend, as noted in a discussion on the Git mailing list:

  https://lore.kernel.org/git/TY0PR06MB54426B92E745B3035F0CC2DFD197A@TY0PR06MB5442.apcprd06.prod.outlook.com/

So for these tests we also set the "http.sslBackend" Git configuration
option to "openssl" when executing them on Windows.  Notably, this will
allow the "clone ClientCert with homedir certs" test to run successfully
on that platform for the first time since it was introduced.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 29, 2024
In commit caaa0f9 of PR git-lfs#2647 a
number of tests of the "git lfs clone" and "git lfs pull" commands
were enhanced so as to check that after those commands are invoked,
the "git status" command returns a "working tree clean" message.
To perform these checks, a call to our assert_clean_status()
shell function was added to the tests.

In the case of the "cloneSSL" test, an assert_clean_status() call
was added, but left commented out; it was then uncommented in commit
e0eede1 of the same PR.  Unfortunately,
the call is made when the current working directory has not yet been
changed to that of the newly cloned repository's working tree, and
so should fail as it stands now.

However, as described in git-lfs#5658, the "cloneSSL" test and the "clone
ClientCert" tests in our t/t-clone.sh test script do not actually
run to completion, a consequence of an improper check of the TRAVIS
variable (which is no longer used since we migrated our test suite to
GitHub Actions in PR git-lfs#3808).  Instead, the tests exit early and
always declare success.

We will address this problem in a subsequent commit in this PR, and when
we do so, the "cloneSSL" test will fail because the misplaced assertion.
Therefore we move the assertion into the block of checks performed
after a "pushd" shell command changes the current directory to that
of the new clone's working tree, which will allow the test to pass
when we resolve the bug with the TRAVIS variable check.

We also take the opportunity to add assert_clean_status() calls to a
number of other tests in the t/t-clone.sh test script, so the tests are
all performing similar sets of checks.  This will help us keep the
tests in that script more consistent with each other in the future.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 29, 2024
In PR git-lfs#1893 we introduced support for TLS client certificates, and
while that was largely successful, some of our CI test jobs ran on the
Travis CI platform, and those encountered problems when validating the
TLS certificate generated by our lfstest-gitserver test utility program.
Several attempts were made to resolve this issue, but ultimately in
commit 1cf7d6b the "http.<url>.sslVerify"
Git configuration value was set to "false" for our entire test suite,
just to allow the "clone ClientCert" test to pass in the Travis CI
environment.

However, we no longer use the Travis CI service, as we migrated our
CI jobs to GitHub Actions in PR git-lfs#3808.  Further, in a prior commit in
this PR we revised several tests in our t/t-clone.sh test script so
they do not run on the legacy CentOS 7 platform, where the libcurl
library used by Git depends on the libnss3 library.  The libnss3 library
does not accept the TLS certificate generated by our lfstest-gitserver
program, so these tests would otherwise fail on CentOS 7 if the
"http.<url>.sslVerify" option is set to "true".

As well, in another commit in this PR we altered the same tests in our
t/t-clone.sh test script to require that Git and libcurl use OpenSSL for
TLS verification on Windows, rather than the default Schannel backend.
This will prevent the tests from failing if TLS verification is enabled,
since Schannel rejects the TLS certificate that we use in our
lfstest-gitserver test utility, but OpenSSL accepts it.

With these changes, all our tests should now pass if we allow the
"http.<url>.sslVerify" Git configuration option to default to "true".
Therefore we now remove the command which set that option to "false"
for our whole test suite.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 30, 2024
In commit 32244d9 of PR git-lfs#1825 the
APPVEYOR_REPO_COMMIT_MESSAGE variable was added to our CI test suite
environment in order to demonstrate that a problem exposed by the prior
commit 0951c69 in the same PR had been
addressed.

Specifically, the earlier commit had the string "GIT_SSH" in its
message, and the AppVeyor CI service automatically provided the message
from the most recent commit in a Git branch under test in the
APPVEYOR_REPO_COMMIT_MESSAGE environment variable.  At the time, the
Environ() function in our "lfs" package would populate the map it
returns with the names and values of any environment variables whose
names or values contained the string "GIT_".  The result was that
the APPVEYOR_REPO_COMMIT_MESSAGE variable was included in the returned
environment, which would cause the tests in what is now our t/t-env.sh
test script to fail.

The problem was resolved by adjusting the Environ() function to only
include environment variables in its map when their name contained
the string "GIT_".

To validate this fix, our test script environment was updated to
write a fixed value into the APPVEYOR_REPO_COMMIT_MESSAGE environment
variable (overriding whatever the AppVeyor service set) with an
example string containing the string "GIT_".  The example string used
was the commit message from the commit which exposed the problem.

Subsequently, we further revised the Environ() function in commit
f4f8fae of PR git-lfs#4269 so it only
includes environment variables whose names start with "GIT_",
excluding those whose names just include that string.

As we no longer use the AppVeyor service since we migrated our CI jobs
to GitHub Actions in PR git-lfs#3808, and because the intent of the
APPVEYOR_REPO_COMMIT_MESSAGE variable is now somewhat obscure, we
replace it with a dedicated TEST_GIT_EXAMPLE environment variable
in our test scripts where the output of "git lfs env" is checked.
We ensure the "GIT_" string is included in both the name and value
of this variable, and we preface it with some comments to clarify
its purpose.

If a regression were ever to be introduced into the Environ() function
such that it did not exclude environment variables with "GIT_" in their
names or values, the tests in the t/t-env.sh and t/t-worktree.sh scripts
should still catch the problem.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Nov 5, 2024
In commit 32244d9 of PR git-lfs#1825 the
APPVEYOR_REPO_COMMIT_MESSAGE variable was added to our CI test suite
environment in order to demonstrate that a problem exposed by the prior
commit 0951c69 in the same PR had been
addressed.

Specifically, the earlier commit had the string "GIT_SSH" in its
message, and the AppVeyor CI service automatically provided the message
from the most recent commit in a Git branch under test in the
APPVEYOR_REPO_COMMIT_MESSAGE environment variable.  At the time, the
Environ() function in our "lfs" package would populate the map it
returns with the names and values of any environment variables whose
names or values contained the string "GIT_".  The result was that
the APPVEYOR_REPO_COMMIT_MESSAGE variable was included in the returned
environment data, which would cause the tests in what is now our
t/t-env.sh test script to fail.

This problem was resolved by adjusting the Environ() function to only
include environment variables in its map when their name contained
the string "GIT_".

To validate this fix, our test script environment was updated to
write a fixed value into the APPVEYOR_REPO_COMMIT_MESSAGE environment
variable (overriding whatever the AppVeyor service set) with an
example string containing the string "GIT_".  The example string used
was the commit message from the commit which exposed the problem.

Subsequently, we further revised the Environ() function in commit
f4f8fae of PR git-lfs#4269 so it only
includes environment variables whose names start with "GIT_",
excluding those whose names merely contain that string.

As we no longer use the AppVeyor service since we migrated our CI
jobs to GitHub Actions in PR git-lfs#3808, and because the intent of the
APPVEYOR_REPO_COMMIT_MESSAGE variable is now somewhat obscure, we
replace it with a dedicated TEST_GIT_EXAMPLE environment variable
that contains the "GIT_" string in both its name and value.

We define this TEST_GIT_EXAMPLE variable only in the test scripts where
the output of "git lfs env" is checked, and we preface its definition
with some comments to clarify its purpose.

If a regression were ever to be introduced into the Environ() function
such that it did not exclude environment variables with "GIT_" in their
names or values, the tests in the t/t-env.sh and t/t-worktree.sh scripts
should detect the problem due to the presence of the TEST_GIT_EXAMPLE
variable.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Nov 24, 2024
Our README file contains a brief note in its "Example Usage" section
stating that Git LFS requires a Git version higher than 1.8.2 on
Linux and 1.8.5 on macOS.  This statement dates from commit
59a49b0 in PR git-lfs#412 in 2015, and so
is relatively out of date.

In particular, when we added support for the "git lfs migrate" command
in PR git-lfs#2353, the actual minimum supported version of Git was changed
from 1.8.x to 1.9.0 (in commit 1d0e834)
and then to 2.0.0 (in commit 5aea841).

These changes were made to the Travis CI configuration in use at the time,
and later migrated to our current GitHub Actions CI workflow in commit
c32820806229c3f42364d989f7a8597f73cb107ba of PR git-lfs#3808.  This workflow
continues to run our Git LFS test suite using Git 2.0.0.

We therefore now update our README file to remove the outdated note
about Git 1.8.x versions, and add a paragraph to the "Limitations"
section which documents the current minimum supported Git version of
2.0.0 but also strongly advises the use of a more recent Git version.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Dec 10, 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 installation of Git.

At present, the macOS runners provided by GitHub Actions are always
provisioned with pre-installed versions of recent releases of the curl,
OpenSSL, and PCRE2 libraries, as packaged by Homebrew, so we no longer
need to install the Homebrew formulae for them.

We also do not need to link the "openssl" and "pcre2" formulae, but
because macOS includes its own default installation of curl and its
library, we continue to run "brew link" for the "curl" formula (as
well as "zlib") so that Homebrew's version of curl is used in
preference to the one installed by macOS.
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.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Apr 3, 2025
Since commit a343a11 of PR git-lfs#1461,
a number of our commands, including "git lfs pull", "git lfs push",
and "git lfs track", have checked the version of the currently
available Git program and reported an error if it was not at least
version 1.8.2.

However, when we added support for the "git lfs migrate" command
in PR git-lfs#2353, the actual minimum supported version of Git was changed
from 1.8.x to 1.9.0 (in commit 1d0e834)
and then to 2.0.0 (in commit 5aea841).

These changes were made to the Travis CI configuration in use at the time,
and later migrated to our current GitHub Actions CI workflow in commit
c32820806229c3f42364d989f7a8597f73cb107ba of PR git-lfs#3808.  This workflow
continues to run our Git LFS test suite using Git 2.0.0.

More recently, in commit 1501265 of
PR git-lfs#5921, we updated our README file to document that the current
minimum supported version of Git we require is v2.0.0.

We therefore now update the minimum Git version required by the Git LFS
client to 2.0.0 by adjusting the version string defined in the
requireGitVersion() function of our "commands" package.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Apr 3, 2025
Since commit a343a11 of PR git-lfs#1461,
a number of our commands, including "git lfs pull", "git lfs push",
and "git lfs track", have checked the version of the currently
available Git program and reported an error if it was not at least
version 1.8.2.

However, when we added support for the "git lfs migrate" command
in PR git-lfs#2353, the actual minimum supported version of Git was changed
from 1.8.x to 1.9.0 (in commit 1d0e834)
and then to 2.0.0 (in commit 5aea841).

These changes were made to the Travis CI configuration in use at the time,
and later migrated to our current GitHub Actions CI workflow in commit
c32820806229c3f42364d989f7a8597f73cb107ba of PR git-lfs#3808.  This workflow
continues to run our Git LFS test suite using Git 2.0.0.

More recently, in commit 1501265 of
PR git-lfs#5921, we updated our README file to document that the current
minimum supported version of Git we require is v2.0.0.

We therefore now update the minimum Git version required by the Git LFS
client to 2.0.0 by adjusting the version string defined in the
requireGitVersion() function of our "commands" package.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants