Skip to content

gitaly: Stop writing a default gitconfig

Patrick Steinhardt requested to merge pks-git-drop-deprecated-fsyncobjectfiles-conig into master

What does this MR do?

The default gitconfig as written by CNG is quite misleading: Gitaly has been ignoring all of its current config entries it contains because it is injecting mandatory defaults on its own nowadays, and this has been true for a long time already. One ommission had been the Ruby sidecar, which still required the gitconfig to be around. But this has recently been fixed in Gitaly, where the sidecar now uses the same configuration as the main Go server.

Stop writing a default gitconfig to fix the confusion. Please note that technically, Gitaly still does read the gitconfig. But it will only ever use config entries whose values Gitaly doesn't overwrite itself. Admins who require the ability to override keys set by Gitaly by default should instead add [[git.config]] entries to its config.toml.

This also fixes an issue we have when different Git versions are in use. While Git versions previous to v2.36.0 accepted core.fsyncObjectFiles to enable the use of fsync(3P), this was changed to a new more flexible core.fsync config entry. So starting with Git v2.36.0, it would print a warning whenever it sees the old Git configuration due to it being deprecated. This means we need to conditionally inject gitconfig, which is an easy thing to do in Gitaly itself. But it's hard to do with the current model where we ship static gitconig files.

This change also prepares us for an upcoming change where Gitaly will start to ignore any gitconfig files altogether, where the only way to change Git configuration will then be the config.toml.

Related issues

Checklist

See Definition of done.

For anything in this list which will not be completed, please provide a reason in the MR discussion

Required

  • Merge Request Title, and Description are up to date, accurate, and descriptive
  • MR targeting the appropriate branch
  • MR has a green pipeline on GitLab.com

Expected (please provide an explanation if not completing)

  • Test plan indicating conditions for success has been posted and passes
  • Documentation created/updated
  • Integration tests added to GitLab QA
  • The impact any change in container size has should be evaluated
Edited by Hossein Pursultani

Merge request reports