Skip to content

Commit

Permalink
Make Git system scope nonempty in container
Browse files Browse the repository at this point in the history
This creates `/etc/gitconfig` in the container and populates it
with a single dummy configuration variable, so that the `system`
scope will exist and tests of "GitInstallation"-related
functionality will be able to pass.

This is intended to let the following tests pass in the container:

- gix-config-tests::config source::git_config_no_system
- gix-path env::git::tests::exe_info::tolerates_broken_temp
- gix-path env::git::tests::exe_info::tolerates_git_config_env_var
- gix-path env::git::tests::exe_info::tolerates_oversanitized_env
- gix-path env::git::tests::exe_info::with_unmodified_environment
- gix-path::path env::installation_config

Those tests have been failing in the container on CI, and in local
container environments that are also Debian 12 and created through
a similar procedure. But they are not among the tests that fail in
a non-container 32-bit Debian 12 system, probably because that test
system has user config in the `global` scope. When that is the
highest scope, it is currently treated as "GitInstallation", even
though it is not truly associated with the Git installation.

The change in this commit is a workaround *specific* to one CI job:

- In at least the case of `git_config_no_system`, it seems like the
  test should automatically handle the case of there being no scope
  detected as "GitInstallation". For the others, it may be
  reasonable for them to fail in a CI environment that does not
  support robustly testing the scenarios they pertain to, but there
  should probably be a way to suppress or weaken them on systems
  that have no scopes that are intended to be detected as
  "GitInstallation", while still testing "GitInstallation"
  functionality as robustly as it can be tested under those
  conditions. This commit does not fix that.

- On macOS with Apple Git, the `unknown` scope higher than `system`
  is meant to be detected as "GitInstallation" in the usual case
  that it is nonempty. On other systems, `system` is the highest
  scope that is expected to exist. On some systems (besides macOS),
  the `system` scope is often empty, in which case the `global`
  scope, if nonempty, will be detected as "GitInstallation". But
  allowing the `global` scope, which in practice is rarely if ever
  associated with the installation itself (it is "global" to the
  user, and still separate from the `system` scope in a per-user Git
  installation), to be detected as a "GitInstallation" has never
  been guaranteed, and may be changed.

  Ever since the CVE-2024-45305 fix in GitoxideLabs#1523 and follow-up in
  GitoxideLabs#1567, `local` and `worktree` scopes are never detected as
  "GitInstallation", so it looks like there would be no harm, and
  some benefit, to excluding the `global` scope as well. This
  commit does not make that change.

- If/when the change of excluding `global` from consideration for
  "GitInstallation" is made, tests that require a variable to exist
  in an installation-associated scope will start to fail regularly
  on some developers' machines, similar to how they have been
  failing in the container. This commit does not address that; it
  only adds a configuration variable in the `system` scope in the
  container.
  • Loading branch information
EliahKagan committed Nov 17, 2024
1 parent 442e9f5 commit 3ed519b
Showing 1 changed file with 2 additions and 0 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,8 @@ jobs:
- uses: taiki-e/install-action@v2
with:
tool: nextest
- name: Make `system` scope nonempty for "GitInstallation" tests
run: git config --system gitoxide.imaginary.arbitraryVariable arbitraryValue
- name: Test (nextest)
run: cargo nextest run --workspace --no-fail-fast

Expand Down

0 comments on commit 3ed519b

Please sign in to comment.