-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Use GitHub Actions for CI #3808
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
PastelMobileSuit
approved these changes
Sep 6, 2019
Contributor
PastelMobileSuit
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
cygpathon the Windows Actions servers produces a short path (C:\Users\RUNNER~1instead ofC:\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.