-
Notifications
You must be signed in to change notification settings - Fork 25.3k
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
reftable: JGit compatibility tests #1696
Conversation
7284e86
to
221f7cf
Compare
There are issues in commit de39855: |
24f824a
to
1869ddb
Compare
GitLab pipeline: https://gitlab.com/gitlab-org/git/-/merge_requests/123 |
160b026
to
fed2121
Compare
There are issues in commit cf9d79b: |
fed2121
to
a9cd20e
Compare
There are issues in commit ef61b57: |
The "runs_on_pool" environment variable is used by our CI scripts to distinguish the different kinds of operating systems. It is quite specific to GitHub Actions though and not really a descriptive name. Rename the variable to "distro" to clarify its intent. Signed-off-by: Patrick Steinhardt <ps@pks.im>
Expose a distro name in dockerized jobs. This will be used in a subsequent commit where we merge the installation scripts for dockerized and non-dockerized jobs. Signed-off-by: Patrick Steinhardt <ps@pks.im>
Our "install-dependencies.sh" script is executed by non-dockerized jobs to install dependencies. These jobs don't run with "root" permissions, but with a separate user. Consequently, we need to use sudo(8) there to elevate permissions when installing packages. We're about to merge "install-docker-dependencies.sh" into that script though, and our Docker containers do run as "root". Using sudo(8) is thus unnecessary there, even though it would be harmless. On some images like Alpine Linux though there is no sudo(8) available by default, which would consequently break the build. Adapt the script to make "sudo" a no-op when running as "root" user. This allows us to easily reuse the script for our dockerized jobs. Signed-off-by: Patrick Steinhardt <ps@pks.im>
The "linux-gcc-default" job installs common Ubuntu packages. This is already done in the distro-specific switch, so we basically duplicate the effort here. Drop the duplicate package installations and inline the variable that contains those common packages. Signed-off-by: Patrick Steinhardt <ps@pks.im>
We're about to merge the "install-docker-dependencies.sh" script into "install-dependencies.sh". This will also move our Alpine-based jobs over to use the latter script. This script uses the Bash shell though, which is not available by default on Alpine Linux. Refactor "install-dependencies.sh" to use "/bin/sh" instead of Bash. This requires us to get rid of the pushd/popd invocations, which are replaced by some more elaborate commands that download or extract executables right to where they are needed. Signed-off-by: Patrick Steinhardt <ps@pks.im>
We're downloading various executables required by our tests. Each of these executables goes into its own directory, which is then appended to the PATH variable. Consequently, whenever we add a new dependency and thus a new directory, we would have to adapt to this change in several places. Refactor this to instead put all binaries into a single directory. Signed-off-by: Patrick Steinhardt <ps@pks.im>
Part of "install-dependencies.sh" is to install some binaries required for tests into a custom directory that gets added to the PATH. This directory is located at "$HOME/path" and thus depends on the current user that the script executes as. This creates problems for GitLab CI, which installs dependencies as the root user, but runs tests as a separate, unprivileged user. As their respective home directories are different, we will end up using two different custom path directories. Consequently, the unprivileged user will not be able to find the binaries that were set up as root user. Fix this issue by allowing CI to override the custom path, which allows GitLab to set up a constant value that isn't derived from "$HOME". Signed-off-by: Patrick Steinhardt <ps@pks.im>
We have two different scripts which install dependencies, one for dockerized jobs and one for non-dockerized ones. Naturally, these scripts have quite some duplication. Furthermore, either of these scripts is missing some test dependencies that the respective other script has, thus reducing test coverage. Merge those two scripts such that there is a single source of truth for test dependencies, only. Signed-off-by: Patrick Steinhardt <ps@pks.im>
The Perforce binaries are only made executable for the current user. On GitLab CI though we execute tests as a different user than "root", and thus these binaries may not be executable by that test user at all. This has gone unnoticed so far because those binaries are optional -- in case they don't exist we simply skip over tests requiring them. Fix the setup so that we set the executable bits for all users. Signed-off-by: Patrick Steinhardt <ps@pks.im>
We have some tests in t5310 that use JGit to verify that bitmaps can be read both by Git and by JGit. We do not execute these tests in our CI jobs though because we don't make JGit available there. Consequently, the tests basically bitrot because almost nobody is ever going to have JGit in their path. Install JGit to plug this test gap. Signed-off-by: Patrick Steinhardt <ps@pks.im>
The tests in t06xx exercise specific ref formats. Next to probing some basic functionality, these tests also exercise other low-level details specific to the format. Those tests are only executed though in case `GIT_TEST_DEFAULT_REF_FORMAT` is set to the ref format of the respective backend-under-test. Ideally, we would run the full test matrix for ref formats such that our complete test suite is executed with every supported format on every supported platform. This is quite an expensive undertaking though, and thus we only execute e.g. the "reftable" tests on macOS and Linux. As a result, we basically have no test coverage for the "reftable" format at all on other platforms like Windows. Adapt these tests so that they override `GIT_TEST_DEFAULT_REF_FORMAT`, which means that they'll always execute. This increases test coverage on platforms that don't run the full test matrix, which at least gives us some basic test coverage on those platforms for the "reftable" format. This of course comes at the cost of running those tests multiple times on platforms where we do run the full test matrix. But arguably, this is a good thing because it will also cause us to e.g. run those tests with the address sanitizer and other non-standard parameters. Signed-off-by: Patrick Steinhardt <ps@pks.im>
Older versions of the Dash shell fail to parse `local var=val` assignments in some cases when `val` is unquoted. Such failures can be observed e.g. with Ubuntu 20.04 and older, which has a Dash version that still has this bug. Such an assignment has been introduced in t0610. The issue wasn't detected for a while because this test used to only run when the GIT_TEST_DEFAULT_REF_FORMAT environment variable was set to "reftable". We have dropped that requirement now though, meaning that it runs unconditionally, including on jobs which use such older versions of Ubuntu. We have worked around such issues in the past, e.g. in ebee558 (parallel-checkout: avoid dash local bug in tests, 2021-06-06), by quoting the `val` side. Apply the same fix to t0610. Signed-off-by: Patrick Steinhardt <ps@pks.im>
While the reftable format is a recent introduction in Git, JGit already knows to read and write reftables since 2017. Given the complexity of the format there is a very real risk of incompatibilities between those two implementations, which is something that we really want to avoid. Add some basic tests that verify that reftables written by Git and JGit can be read by the respective other implementation. For now this test suite is rather small, only covering basic functionality. But it serves as a good starting point and can be extended over time. Signed-off-by: Patrick Steinhardt <ps@pks.im>
a9cd20e
to
218c694
Compare
Closing. I was able to fix the issue. |
This pull request only exists to exercise the CI changes I'm doing.