libfetchers: Don't have a single shared tarball cache

[xokdvium]

Motivation

This partially reverts commit bc6b9ce.

This transformation is unsound and thread unsafe. Internal libgit2
structures must never be shared between threads. This causes
internal odb corruption with e.g.:

nix flake prefetch-inputs:

error:
       … while fetching the input 'github:nixos/nixpkgs/89c2b2330e733d6cdb5eae7b899326930c2c0648?narHash=sha256-Stk9ZYRkGrnnpyJ4eqt9eQtdFWRRIvMxpNRf4sIegnw%3D'

       error: adding a file to a tree builder: failed to insert entry: invalid object specified - upload-image.sh
error:
       … while fetching the input 'github:NixOS/nixpkgs/a8d610af3f1a5fb71e23e08434d8d61a466fc942?narHash=sha256-v5afmLjn/uyD9EQuPBn7nZuaZVV9r%2BJerayK/4wvdWA%3D'

       error: adding a file to a tree builder: failed to insert entry: invalid object specified - outline.nix
double free or corruption (!prev)

Thread 21 "nix" received signal SIGABRT, Aborted.

Context

---

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

[xokdvium]

Root cause triaged with @roberth

[roberth]

libgit2 does implement a global packfile cache that should make this revert not the end of the world.

The git_mwindow__pack_cache shares the actual pack file handles, so you won't have duplicate file descriptors or duplicate memory mappings.

It does still perform:

• Full directory listing of objects/pack/ (via direach)
• Potentially loading multi-pack-index file

This has some overhead that would be nice to avoid somehow, but we have some added constraints

• The mempack odb needs to be 1:1 with the packfile creation process
• Not refreshing indexes might be a problem if we keep an increasingly stale copy of the pack index around (e.g. if we keep a pool of git_repository instances around or something)

More sharing is probably best achieved within the libgit2 codebase.

[roberth]

Maybe leave a comment that GitRepo is not thread safe

[xokdvium]

Added a comment. Hopefully we won't step on this landmine any more in the future.

[Ericson2314]

I don't get how making it a global again prevents multi-threaded access? Also what's ODB corruption? My Google search did not answer that question.

[xokdvium]

don't get how making it a global again prevents multi-threaded access?

It's not a global. openRepo returns a fresh GitRepo object. The only global there is the path to the tarball cache.

[xokdvium]

@Ericson2314 has convinced me that rather than a full revert we can do a smaller diff change by just making getTarballCache open a new repo.