{"thread":{"id":"66476","subject":"[BUG] push resends common history after repack during pre-push (2.54.0, 2.56.0)","startedAt":"2026-10-06T17:37:47Z","lastAt":"2026-10-07T09:03:38Z","messageCount":4,"participants":["Jens Röcker","D. Ben Knoble","Patrick Steinhardt"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"554311","messageId":"CA+tGzvYYKm=Yo88knZb4oavG9dH5smUCXnoqa-RR9-7YEBycVA@mail.gmail.com","threadId":"66476","inReplyTo":null,"subject":"[BUG] push resends common history after repack during pre-push (2.54.0, 2.56.0)","fromName":"Jens Röcker","fromEmail":"jens.roecker@gmail.com","sentAt":"2026-10-06T17:37:47Z","receivedAt":"2026-10-06T17:37:47Z","isPatch":false,"sender":{"key":"jens.roecker@gmail.com","avatar":null},"body":"Hello Git developers,\n\nA push can resend common history if its pre-push hook repacks the local\nobject database and removes previously loose common objects. I reproduced\nthis with Apple Git 2.54.0 (Apple Git-157) and an unmodified build of the\ncurrent upstream Git 2.56.0 release on macOS 27.0 / arm64.\n\nThe attached inline Python script creates fresh local repositories, seeds\na bare receiver with a deterministic, incompressible 4-MiB historical blob,\nand pushes one tiny text-file commit. The common base is initially loose.\nThe receiver uses receive.unpackLimit=1 so the added pack is measurable.\nEach case starts from a separate fresh repository pair. All pushes succeed\nand the receiver ends at the expected tip.\n\nObserved added receiver pack sizes, in bytes:\n\n                        Apple Git 2.54.0    upstream Git 2.56.0\n  no hook                      300                  300\n  repack in pre-push      4,196,026            4,196,026\n  repack + negotiate     4,196,026            4,196,026\n\nThe repacking hook is simply:\n\n  #!/bin/sh\n  set -eu\n  cat >/dev/null\n  git repack -adq\n  git prune-packed\n\nExpected: the already-advertised common history should still be excluded\nwhen its storage moves from loose objects to a newly created pack.\nActual: the historical blob is transmitted again. The receiver stores a\nnew pack roughly the size of the historical blob. Enabling\npush.negotiate=true does not prevent the redundant transfer in this test.\n\nPossible mechanism, based on source inspection:\n\nIn 2.54.0, send-pack.c:feed_object() drops negative OIDs when\nodb_has_object(..., 0) returns false. In 2.56.0, the same quick check is in\nappend_negative_object(). In both versions, odb_has_object() uses\nOBJECT_INFO_QUICK unless ODB_HAS_OBJECT_RECHECK_PACKED is set. A parent\nprocess with a stale pack catalogue may therefore miss the base after the\nhook removes the loose copy; the fresh pack generator then sees the new\npack and walks history without that excluded base. This is a proposed\nexplanation of the measured effect, not an instrumented proof of the\nparent process's in-memory state.\n\nRelevant release source:\nhttps://github.com/git/git/blob/v2.54.0/send-pack.c\nhttps://github.com/git/git/blob/v2.56.0/send-pack.c\nhttps://github.com/git/git/blob/v2.56.0/odb.c\n\nThe upstream 2.56.0 binary was built from the kernel.org release tarball\nwith optional gettext, curl, Tcl/Tk, Perl, Python and Rust components\ndisabled. Neither global Git configuration nor the installed system Git\nwas changed. The script isolates system/global Git configuration and uses\nonly local transport. Measurements are receiver pack-file sizes, rather\nthan network-byte counters. A separate large-repository incident motivated\nthis test, but this report includes only synthetic fixtures.\n\nTo reproduce, save the inline script as reproduce.py and run:\n\n  python3 reproduce.py --git /path/to/git\n\nFor a Git binary built in-place, add:\n\n  --exec-path /path/to/git/build/directory\n\nThe script requires Python 3 and Git; its default mode removes only its\nown temporary test repositories when the run completes. To retain all\nfixture repositories and push logs, pass --output with a new directory.\n\nMinimal reproducer follows:\n\n#!/usr/bin/env python3\n\"\"\"Reproduce redundant push history after loose objects move into a new pack.\n\nUses fresh local test repositories only. Requires Python 3 and Git.\nExample: python3 reproduce.py --git /path/to/git --output /new/results/path\nFor an uninstalled Git build, add --exec-path /path/to/build/directory.\n\"\"\"\n\nimport argparse\nimport hashlib\nimport json\nimport os\nfrom pathlib import Path\nimport platform\nimport shutil\nimport subprocess\nimport tempfile\nimport time\n\n\ndef main():\n    parser = argparse.ArgumentParser(description=__doc__)\n    parser.add_argument(\"--git\", default=shutil.which(\"git\"))\n    parser.add_argument(\"--exec-path\")\n    parser.add_argument(\"--output\", type=Path)\n    args = parser.parse_args()\n    binary = str(Path(args.git).resolve())\n    temporary = None\n    if args.output:\n        root = args.output.resolve()\n        root.mkdir(parents=True, exist_ok=False)\n    else:\n        temporary = tempfile.TemporaryDirectory(prefix=\"git-push-repack-repro-\")\n        root = Path(temporary.name)\n\n    env = {key: value for key, value in os.environ.items()\n           if not key.startswith(\"GIT_\")}\n    env.update(GIT_CONFIG_NOSYSTEM=\"1\", GIT_CONFIG_GLOBAL=os.devnull,\n               GIT_AUTHOR_DATE=\"2001-01-01T00:00:00+0000\",\n               GIT_COMMITTER_DATE=\"2001-01-01T00:00:00+0000\")\n    env[\"PATH\"] = str(Path(binary).parent) + os.pathsep + env.get(\"PATH\", \"\")\n    if args.exec_path:\n        env[\"GIT_EXEC_PATH\"] = str(Path(args.exec_path).resolve())\n\n    def git(cwd, *words):\n        return subprocess.run([binary, *words], cwd=cwd, env=env,\n                              text=True, capture_output=True, check=True,\n                              timeout=60)\n\n    results = []\n    for mode in (\"no-hook\", \"repack\", \"repack-negotiate\"):\n        case = root / mode\n        case.mkdir()\n        repo, remote = case / \"repo\", case / \"remote.git\"\n        git(case, \"init\", \"-q\", \"-b\", \"main\", str(repo))\n        git(case, \"init\", \"-q\", \"--bare\", \"-b\", \"main\", str(remote))\n        for key, value in ((\"user.name\", \"Git bug reproduction\"),\n                           (\"user.email\", \"test@example.invalid\"),\n                           (\"commit.gpgsign\", \"false\"), (\"gc.auto\", \"0\"),\n                           (\"core.hooksPath\", str(repo / \".git/hooks\"))):\n            git(repo, \"config\", key, value)\n        git(remote, \"config\", \"gc.auto\", \"0\")\n        git(remote, \"config\", \"receive.unpackLimit\", \"1\")\n        (repo / \"history.bin\").write_bytes(\n            hashlib.shake_256(b\"historical fixture\").digest(4 * 1024 * 1024))\n        git(repo, \"add\", \"--\", \"history.bin\")\n        git(repo, \"commit\", \"-q\", \"-m\", \"historical seed\", \"--\", \"history.bin\")\n        base = git(repo, \"rev-parse\", \"HEAD\").stdout.strip()\n        git(repo, \"push\", \"-q\", str(remote), \"HEAD:refs/heads/main\")\n        (repo / \"change.txt\").write_text(\"tiny change\\n\")\n        git(repo, \"add\", \"--\", \"change.txt\")\n        git(repo, \"commit\", \"-q\", \"-m\", \"tiny change\", \"--\", \"change.txt\")\n        tip = git(repo, \"rev-parse\", \"HEAD\").stdout.strip()\n        assert (repo / \".git/objects\" / base[:2] / base[2:]).is_file()\n        if mode != \"no-hook\":\n            hook = repo / \".git/hooks/pre-push\"\n            hook.parent.mkdir(parents=True, exist_ok=True)\n            hook.write_text(\"#!/bin/sh\\nset -eu\\ncat >/dev/null\\n\"\n                            \"git repack -adq\\ngit prune-packed\\n\")\n            hook.chmod(0o700)\n        before = set((remote / \"objects/pack\").glob(\"*.pack\"))\n        config = [\"-c\", \"push.negotiate=true\"] if mode ==\n\"repack-negotiate\" else []\n        start = time.monotonic()\n        result = git(repo, *config, \"push\", \"--progress\", str(remote),\n                     \"HEAD:refs/heads/main\")\n        elapsed = time.monotonic() - start\n        (case / \"push.log\").write_text(result.stdout + result.stderr)\n        packs = set((remote / \"objects/pack\").glob(\"*.pack\")) - before\n        remote_tip = git(remote, \"rev-parse\", \"refs/heads/main\").stdout.strip()\n        assert remote_tip == tip\n        results.append({\"case\": mode, \"push_rc\": result.returncode,\n                        \"new_remote_pack_bytes\": sum(p.stat().st_size\nfor p in packs),\n                        \"elapsed_seconds\": round(elapsed, 6),\n                        \"remote_tip_matches\": True})\n    report = {\"git_version\": git(root, \"version\",\n\"--build-options\").stdout.strip(),\n              \"platform\": {\"system\": platform.system(), \"machine\":\nplatform.machine(),\n                           \"macos\": platform.mac_ver()[0]},\n              \"fixture_bytes\": 4 * 1024 * 1024,\n              \"transport\": \"local bare repository\", \"results\": results}\n    encoded = json.dumps(report, indent=2) + \"\\n\"\n    (root / \"results.json\").write_text(encoded)\n    print(encoded, end=\"\")\n    if temporary:\n        temporary.cleanup()\n\n\nif __name__ == \"__main__\":\n    main()\n\nThank you.\n\n"},{"id":"554313","messageId":"CA+tGzva9Pzn=+zcVr9hkKa7HJEfHDQaB8wkYV7WBwjMchC_4xg@mail.gmail.com","threadId":"66476","inReplyTo":"CA+tGzvYYKm=Yo88knZb4oavG9dH5smUCXnoqa-RR9-7YEBycVA@mail.gmail.com","subject":"Re: [BUG] push resends common history after repack during pre-push (2.54.0, 2.56.0)","fromName":"Jens Röcker","fromEmail":"jens.roecker@gmail.com","sentAt":"2026-10-06T17:47:48Z","receivedAt":"2026-10-06T17:47:48Z","isPatch":false,"sender":{"key":"jens.roecker@gmail.com","avatar":null},"body":"Hello Git developers,\n\nGmail hard-wrapped several long lines in the inline Python script in my\nprevious report. Please use the attached reproduce.txt instead. It is\nthe same tested script, supplied as a text/plain attachment to preserve\nits exact contents.\n\nRun it with:\n\n  python3 reproduce.txt --git /path/to/git\n\nFor an uninstalled Git build, also pass:\n\n  --exec-path /path/to/git/build/directory\n\nAll reported measurements remain unchanged: 300 bytes without repacking,\nand 4,196,026 bytes with repacking, with or without push.negotiate=true,\nfor both Apple Git 2.54.0 and upstream Git 2.56.0.\n\nSorry for the formatting issue.\n\nAttachment SHA-256:\n932b10a426aca11eb5831cd338ad5f61d83db1301e334764a2d9df17ab4e6b76\n\nAm Di., 6. Okt. 2026 um 19:37 Uhr schrieb Jens Röcker <jens.roecker@gmail.com>:\n>\n> Hello Git developers,\n>\n> A push can resend common history if its pre-push hook repacks the local\n> object database and removes previously loose common objects. I reproduced\n> this with Apple Git 2.54.0 (Apple Git-157) and an unmodified build of the\n> current upstream Git 2.56.0 release on macOS 27.0 / arm64.\n>\n> The attached inline Python script creates fresh local repositories, seeds\n> a bare receiver with a deterministic, incompressible 4-MiB historical blob,\n> and pushes one tiny text-file commit. The common base is initially loose.\n> The receiver uses receive.unpackLimit=1 so the added pack is measurable.\n> Each case starts from a separate fresh repository pair. All pushes succeed\n> and the receiver ends at the expected tip.\n>\n> Observed added receiver pack sizes, in bytes:\n>\n>                         Apple Git 2.54.0    upstream Git 2.56.0\n>   no hook                      300                  300\n>   repack in pre-push      4,196,026            4,196,026\n>   repack + negotiate     4,196,026            4,196,026\n>\n> The repacking hook is simply:\n>\n>   #!/bin/sh\n>   set -eu\n>   cat >/dev/null\n>   git repack -adq\n>   git prune-packed\n>\n> Expected: the already-advertised common history should still be excluded\n> when its storage moves from loose objects to a newly created pack.\n> Actual: the historical blob is transmitted again. The receiver stores a\n> new pack roughly the size of the historical blob. Enabling\n> push.negotiate=true does not prevent the redundant transfer in this test.\n>\n> Possible mechanism, based on source inspection:\n>\n> In 2.54.0, send-pack.c:feed_object() drops negative OIDs when\n> odb_has_object(..., 0) returns false. In 2.56.0, the same quick check is in\n> append_negative_object(). In both versions, odb_has_object() uses\n> OBJECT_INFO_QUICK unless ODB_HAS_OBJECT_RECHECK_PACKED is set. A parent\n> process with a stale pack catalogue may therefore miss the base after the\n> hook removes the loose copy; the fresh pack generator then sees the new\n> pack and walks history without that excluded base. This is a proposed\n> explanation of the measured effect, not an instrumented proof of the\n> parent process's in-memory state.\n>\n> Relevant release source:\n> https://github.com/git/git/blob/v2.54.0/send-pack.c\n> https://github.com/git/git/blob/v2.56.0/send-pack.c\n> https://github.com/git/git/blob/v2.56.0/odb.c\n>\n> The upstream 2.56.0 binary was built from the kernel.org release tarball\n> with optional gettext, curl, Tcl/Tk, Perl, Python and Rust components\n> disabled. Neither global Git configuration nor the installed system Git\n> was changed. The script isolates system/global Git configuration and uses\n> only local transport. Measurements are receiver pack-file sizes, rather\n> than network-byte counters. A separate large-repository incident motivated\n> this test, but this report includes only synthetic fixtures.\n>\n> To reproduce, save the inline script as reproduce.py and run:\n>\n>   python3 reproduce.py --git /path/to/git\n>\n> For a Git binary built in-place, add:\n>\n>   --exec-path /path/to/git/build/directory\n>\n> The script requires Python 3 and Git; its default mode removes only its\n> own temporary test repositories when the run completes. To retain all\n> fixture repositories and push logs, pass --output with a new directory.\n>\n> Minimal reproducer follows:\n>\n> #!/usr/bin/env python3\n> \"\"\"Reproduce redundant push history after loose objects move into a new pack.\n>\n> Uses fresh local test repositories only. Requires Python 3 and Git.\n> Example: python3 reproduce.py --git /path/to/git --output /new/results/path\n> For an uninstalled Git build, add --exec-path /path/to/build/directory.\n> \"\"\"\n>\n> import argparse\n> import hashlib\n> import json\n> import os\n> from pathlib import Path\n> import platform\n> import shutil\n> import subprocess\n> import tempfile\n> import time\n>\n>\n> def main():\n>     parser = argparse.ArgumentParser(description=__doc__)\n>     parser.add_argument(\"--git\", default=shutil.which(\"git\"))\n>     parser.add_argument(\"--exec-path\")\n>     parser.add_argument(\"--output\", type=Path)\n>     args = parser.parse_args()\n>     binary = str(Path(args.git).resolve())\n>     temporary = None\n>     if args.output:\n>         root = args.output.resolve()\n>         root.mkdir(parents=True, exist_ok=False)\n>     else:\n>         temporary = tempfile.TemporaryDirectory(prefix=\"git-push-repack-repro-\")\n>         root = Path(temporary.name)\n>\n>     env = {key: value for key, value in os.environ.items()\n>            if not key.startswith(\"GIT_\")}\n>     env.update(GIT_CONFIG_NOSYSTEM=\"1\", GIT_CONFIG_GLOBAL=os.devnull,\n>                GIT_AUTHOR_DATE=\"2001-01-01T00:00:00+0000\",\n>                GIT_COMMITTER_DATE=\"2001-01-01T00:00:00+0000\")\n>     env[\"PATH\"] = str(Path(binary).parent) + os.pathsep + env.get(\"PATH\", \"\")\n>     if args.exec_path:\n>         env[\"GIT_EXEC_PATH\"] = str(Path(args.exec_path).resolve())\n>\n>     def git(cwd, *words):\n>         return subprocess.run([binary, *words], cwd=cwd, env=env,\n>                               text=True, capture_output=True, check=True,\n>                               timeout=60)\n>\n>     results = []\n>     for mode in (\"no-hook\", \"repack\", \"repack-negotiate\"):\n>         case = root / mode\n>         case.mkdir()\n>         repo, remote = case / \"repo\", case / \"remote.git\"\n>         git(case, \"init\", \"-q\", \"-b\", \"main\", str(repo))\n>         git(case, \"init\", \"-q\", \"--bare\", \"-b\", \"main\", str(remote))\n>         for key, value in ((\"user.name\", \"Git bug reproduction\"),\n>                            (\"user.email\", \"test@example.invalid\"),\n>                            (\"commit.gpgsign\", \"false\"), (\"gc.auto\", \"0\"),\n>                            (\"core.hooksPath\", str(repo / \".git/hooks\"))):\n>             git(repo, \"config\", key, value)\n>         git(remote, \"config\", \"gc.auto\", \"0\")\n>         git(remote, \"config\", \"receive.unpackLimit\", \"1\")\n>         (repo / \"history.bin\").write_bytes(\n>             hashlib.shake_256(b\"historical fixture\").digest(4 * 1024 * 1024))\n>         git(repo, \"add\", \"--\", \"history.bin\")\n>         git(repo, \"commit\", \"-q\", \"-m\", \"historical seed\", \"--\", \"history.bin\")\n>         base = git(repo, \"rev-parse\", \"HEAD\").stdout.strip()\n>         git(repo, \"push\", \"-q\", str(remote), \"HEAD:refs/heads/main\")\n>         (repo / \"change.txt\").write_text(\"tiny change\\n\")\n>         git(repo, \"add\", \"--\", \"change.txt\")\n>         git(repo, \"commit\", \"-q\", \"-m\", \"tiny change\", \"--\", \"change.txt\")\n>         tip = git(repo, \"rev-parse\", \"HEAD\").stdout.strip()\n>         assert (repo / \".git/objects\" / base[:2] / base[2:]).is_file()\n>         if mode != \"no-hook\":\n>             hook = repo / \".git/hooks/pre-push\"\n>             hook.parent.mkdir(parents=True, exist_ok=True)\n>             hook.write_text(\"#!/bin/sh\\nset -eu\\ncat >/dev/null\\n\"\n>                             \"git repack -adq\\ngit prune-packed\\n\")\n>             hook.chmod(0o700)\n>         before = set((remote / \"objects/pack\").glob(\"*.pack\"))\n>         config = [\"-c\", \"push.negotiate=true\"] if mode ==\n> \"repack-negotiate\" else []\n>         start = time.monotonic()\n>         result = git(repo, *config, \"push\", \"--progress\", str(remote),\n>                      \"HEAD:refs/heads/main\")\n>         elapsed = time.monotonic() - start\n>         (case / \"push.log\").write_text(result.stdout + result.stderr)\n>         packs = set((remote / \"objects/pack\").glob(\"*.pack\")) - before\n>         remote_tip = git(remote, \"rev-parse\", \"refs/heads/main\").stdout.strip()\n>         assert remote_tip == tip\n>         results.append({\"case\": mode, \"push_rc\": result.returncode,\n>                         \"new_remote_pack_bytes\": sum(p.stat().st_size\n> for p in packs),\n>                         \"elapsed_seconds\": round(elapsed, 6),\n>                         \"remote_tip_matches\": True})\n>     report = {\"git_version\": git(root, \"version\",\n> \"--build-options\").stdout.strip(),\n>               \"platform\": {\"system\": platform.system(), \"machine\":\n> platform.machine(),\n>                            \"macos\": platform.mac_ver()[0]},\n>               \"fixture_bytes\": 4 * 1024 * 1024,\n>               \"transport\": \"local bare repository\", \"results\": results}\n>     encoded = json.dumps(report, indent=2) + \"\\n\"\n>     (root / \"results.json\").write_text(encoded)\n>     print(encoded, end=\"\")\n>     if temporary:\n>         temporary.cleanup()\n>\n>\n> if __name__ == \"__main__\":\n>     main()\n>\n> Thank you.\n\n\n\n-- \nMit freundlichen Grüßen\nJens Röcker\n\n\n#!/usr/bin/env python3\n\"\"\"Reproduce redundant push history after loose objects move into a new pack.\n\nUses fresh local test repositories only. Requires Python 3 and Git.\nExample: python3 reproduce.py --git /path/to/git --output /new/results/path\nFor an uninstalled Git build, add --exec-path /path/to/build/directory.\n\"\"\"\n\nimport argparse\nimport hashlib\nimport json\nimport os\nfrom pathlib import Path\nimport platform\nimport shutil\nimport subprocess\nimport tempfile\nimport time\n\n\ndef main():\n    parser = argparse.ArgumentParser(description=__doc__)\n    parser.add_argument(\"--git\", default=shutil.which(\"git\"))\n    parser.add_argument(\"--exec-path\")\n    parser.add_argument(\"--output\", type=Path)\n    args = parser.parse_args()\n    binary = str(Path(args.git).resolve())\n    temporary = None\n    if args.output:\n        root = args.output.resolve()\n        root.mkdir(parents=True, exist_ok=False)\n    else:\n        temporary = tempfile.TemporaryDirectory(prefix=\"git-push-repack-repro-\")\n        root = Path(temporary.name)\n\n    env = {key: value for key, value in os.environ.items()\n           if not key.startswith(\"GIT_\")}\n    env.update(GIT_CONFIG_NOSYSTEM=\"1\", GIT_CONFIG_GLOBAL=os.devnull,\n               GIT_AUTHOR_DATE=\"2001-01-01T00:00:00+0000\",\n               GIT_COMMITTER_DATE=\"2001-01-01T00:00:00+0000\")\n    env[\"PATH\"] = str(Path(binary).parent) + os.pathsep + env.get(\"PATH\", \"\")\n    if args.exec_path:\n        env[\"GIT_EXEC_PATH\"] = str(Path(args.exec_path).resolve())\n\n    def git(cwd, *words):\n        return subprocess.run([binary, *words], cwd=cwd, env=env,\n                              text=True, capture_output=True, check=True,\n                              timeout=60)\n\n    results = []\n    for mode in (\"no-hook\", \"repack\", \"repack-negotiate\"):\n        case = root / mode\n        case.mkdir()\n        repo, remote = case / \"repo\", case / \"remote.git\"\n        git(case, \"init\", \"-q\", \"-b\", \"main\", str(repo))\n        git(case, \"init\", \"-q\", \"--bare\", \"-b\", \"main\", str(remote))\n        for key, value in ((\"user.name\", \"Git bug reproduction\"),\n                           (\"user.email\", \"test@example.invalid\"),\n                           (\"commit.gpgsign\", \"false\"), (\"gc.auto\", \"0\"),\n                           (\"core.hooksPath\", str(repo / \".git/hooks\"))):\n            git(repo, \"config\", key, value)\n        git(remote, \"config\", \"gc.auto\", \"0\")\n        git(remote, \"config\", \"receive.unpackLimit\", \"1\")\n        (repo / \"history.bin\").write_bytes(\n            hashlib.shake_256(b\"historical fixture\").digest(4 * 1024 * 1024))\n        git(repo, \"add\", \"--\", \"history.bin\")\n        git(repo, \"commit\", \"-q\", \"-m\", \"historical seed\", \"--\", \"history.bin\")\n        base = git(repo, \"rev-parse\", \"HEAD\").stdout.strip()\n        git(repo, \"push\", \"-q\", str(remote), \"HEAD:refs/heads/main\")\n        (repo / \"change.txt\").write_text(\"tiny change\\n\")\n        git(repo, \"add\", \"--\", \"change.txt\")\n        git(repo, \"commit\", \"-q\", \"-m\", \"tiny change\", \"--\", \"change.txt\")\n        tip = git(repo, \"rev-parse\", \"HEAD\").stdout.strip()\n        assert (repo / \".git/objects\" / base[:2] / base[2:]).is_file()\n        if mode != \"no-hook\":\n            hook = repo / \".git/hooks/pre-push\"\n            hook.parent.mkdir(parents=True, exist_ok=True)\n            hook.write_text(\"#!/bin/sh\\nset -eu\\ncat >/dev/null\\n\"\n                            \"git repack -adq\\ngit prune-packed\\n\")\n            hook.chmod(0o700)\n        before = set((remote / \"objects/pack\").glob(\"*.pack\"))\n        config = [\"-c\", \"push.negotiate=true\"] if mode == \"repack-negotiate\" else []\n        start = time.monotonic()\n        result = git(repo, *config, \"push\", \"--progress\", str(remote),\n                     \"HEAD:refs/heads/main\")\n        elapsed = time.monotonic() - start\n        (case / \"push.log\").write_text(result.stdout + result.stderr)\n        packs = set((remote / \"objects/pack\").glob(\"*.pack\")) - before\n        remote_tip = git(remote, \"rev-parse\", \"refs/heads/main\").stdout.strip()\n        assert remote_tip == tip\n        results.append({\"case\": mode, \"push_rc\": result.returncode,\n                        \"new_remote_pack_bytes\": sum(p.stat().st_size for p in packs),\n                        \"elapsed_seconds\": round(elapsed, 6),\n                        \"remote_tip_matches\": True})\n    report = {\"git_version\": git(root, \"version\", \"--build-options\").stdout.strip(),\n              \"platform\": {\"system\": platform.system(), \"machine\": platform.machine(),\n                           \"macos\": platform.mac_ver()[0]},\n              \"fixture_bytes\": 4 * 1024 * 1024,\n              \"transport\": \"local bare repository\", \"results\": results}\n    encoded = json.dumps(report, indent=2) + \"\\n\"\n    (root / \"results.json\").write_text(encoded)\n    print(encoded, end=\"\")\n    if temporary:\n        temporary.cleanup()\n\n\nif __name__ == \"__main__\":\n    main()\n"},{"id":"554328","messageId":"CALnO6CAAGgKK=cQ6Gycn9Y4K7rW8_vxkzpgFUYQANY=yg3Y17A@mail.gmail.com","threadId":"66476","inReplyTo":"CA+tGzvYYKm=Yo88knZb4oavG9dH5smUCXnoqa-RR9-7YEBycVA@mail.gmail.com","subject":"Re: [BUG] push resends common history after repack during pre-push (2.54.0, 2.56.0)","fromName":"D. Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2026-10-06T20:03:48Z","receivedAt":"2026-10-06T20:03:48Z","isPatch":false,"sender":{"key":"ben.knoble@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22802209?v=4"},"body":"I'm out of my depth here, but maybe others will have the same question…\n\nOn Tue, Oct 6, 2026 at 1:39 PM Jens Röcker <jens.roecker@gmail.com> wrote:\n>\n> Hello Git developers,\n>\n> A push can resend common history if its pre-push hook repacks the local\n> object database and removes previously loose common objects. I reproduced\n> this with Apple Git 2.54.0 (Apple Git-157) and an unmodified build of the\n> current upstream Git 2.56.0 release on macOS 27.0 / arm64.\n>\n> The attached inline Python script creates fresh local repositories, seeds\n> a bare receiver with a deterministic, incompressible 4-MiB historical blob,\n> and pushes one tiny text-file commit. The common base is initially loose.\n> The receiver uses receive.unpackLimit=1 so the added pack is measurable.\n> Each case starts from a separate fresh repository pair. All pushes succeed\n> and the receiver ends at the expected tip.\n>\n> Observed added receiver pack sizes, in bytes:\n>\n>                         Apple Git 2.54.0    upstream Git 2.56.0\n>   no hook                      300                  300\n>   repack in pre-push      4,196,026            4,196,026\n>   repack + negotiate     4,196,026            4,196,026\n>\n> The repacking hook is simply:\n>\n>   #!/bin/sh\n>   set -eu\n>   cat >/dev/null\n>   git repack -adq\n>   git prune-packed\n>\n> Expected: the already-advertised common history should still be excluded\n> when its storage moves from loose objects to a newly created pack.\n> Actual: the historical blob is transmitted again. The receiver stores a\n> new pack roughly the size of the historical blob. Enabling\n> push.negotiate=true does not prevent the redundant transfer in this test.\n\n…I've lost the main idea at this point. Is the problem that you see\nobjects sent from pusher to receiver more than once because of the\nrepack hook? Or something else?\n\n-- \nD. Ben Knoble\n\n"},{"id":"554360","messageId":"asYK6ld53e8lJ4Ir@pks.im","threadId":"66476","inReplyTo":"CA+tGzvYYKm=Yo88knZb4oavG9dH5smUCXnoqa-RR9-7YEBycVA@mail.gmail.com","subject":"Re: [BUG] push resends common history after repack during pre-push (2.54.0, 2.56.0)","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-10-07T09:03:38Z","receivedAt":"2026-10-07T09:03:38Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Oct 06, 2026 at 07:37:47PM +0200, Jens Röcker wrote:\n[snip]\n> Possible mechanism, based on source inspection:\n> \n> In 2.54.0, send-pack.c:feed_object() drops negative OIDs when\n> odb_has_object(..., 0) returns false. In 2.56.0, the same quick check is in\n> append_negative_object(). In both versions, odb_has_object() uses\n> OBJECT_INFO_QUICK unless ODB_HAS_OBJECT_RECHECK_PACKED is set. A parent\n> process with a stale pack catalogue may therefore miss the base after the\n> hook removes the loose copy; the fresh pack generator then sees the new\n> pack and walks history without that excluded base. This is a proposed\n> explanation of the measured effect, not an instrumented proof of the\n> parent process's in-memory state.\n\nRight, that makes sense, `odb_has_object()` can have false negatives by\ndefault. So in case the object database has been concurrently repacked\nwe'll potentially end up thinking that the object does not exist at all.\nAnd in `append_negative_object()` (which is the modern equivalent to\n`feed_object()`) we'll then silently skip such objects:\n\n\tstatic void append_negative_object(struct repository *r,\n\t\t\t\t\t   struct oid_array *haves,\n\t\t\t\t\t   const struct object_id *oid)\n\t{\n\t\t/*\n\t\t * The remote end may have advertised objects that we do not have in\n\t\t * our object database. Skip those, as we cannot use them as boundary.\n\t\t */\n\t\tif (!odb_has_object(r->objects, oid, 0))\n\t\t\treturn;\n\t\toid_array_append(haves, oid);\n\t}\n\nConsequently, we won't mark the object as negative boundary for the graph\nwalk and thus end up pushing too many objects.\n\nThe question is how to fix this. The obvious fix is of course to just\npass `ODB_HAS_OBJECT_RECHECK_PACKED`. But as the comment above explains,\nit is expected that we will receive potentially-many object IDs that we\ndon't even have. And we certainly don't want to reload the object\ndatabase every single time we see an object that we truly don't have at\nall, as that may be somewhat expensive.\n\nI wonder whether we could maybe batch this check: instead of checking\neach negative object separately, we could gather all of them and then\ncheck them for existence. And if any of them are missing, we reload the\nobject database once and then re-check only those.\n\nThat'd be more efficient for sure compared to potentially reloading on\nevery single missing object. We still have the chance of racing with a\nconcurrent repack in that case. But maybe that's good enough?\n\nSomething like the below (untested) patch.\n\nThanks!\n\nPatrick\n\ndiff --git a/send-pack.c b/send-pack.c\nindex f20460fbf4..aecc73209e 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -42,17 +42,46 @@ int option_parse_push_signed(const struct option *opt,\n \tdie(\"bad %s argument: %s\", opt->long_name, arg);\n }\n \n-static void append_negative_object(struct repository *r,\n-\t\t\t\t   struct oid_array *haves,\n-\t\t\t\t   const struct object_id *oid)\n+static void append_negative_objects(struct repository *r,\n+\t\t\t\t    struct oid_array *haves,\n+\t\t\t\t    const struct oidset *oids)\n {\n+\tstruct oidset missing = OIDSET_INIT;\n+\tconst struct object_id *oid;\n+\tstruct oidset_iter it;\n+\n+\toidset_iter_init(oids, &it);\n+\twhile ((oid = oidset_iter_next(&it))) {\n+\t\t/*\n+\t\t * The remote end may have advertised objects that we do not have in\n+\t\t * our object database. Skip those, as we cannot use them as boundary.\n+\t\t */\n+\t\tif (!odb_has_object(r->objects, oid, 0)) {\n+\t\t\toidset_insert(&missing, oid);\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\toid_array_append(haves, oid);\n+\t}\n+\n+\tif (!oidset_size(&missing))\n+\t\treturn;\n+\n \t/*\n-\t * The remote end may have advertised objects that we do not have in\n-\t * our object database. Skip those, as we cannot use them as boundary.\n+\t * A concurrent process may have repacked objects. Reprepare the object\n+\t * database once and re-try. Note that we explicitly batch this check\n+\t * so that we don't reload the object database for every truly-missing\n+\t * object.\n \t */\n-\tif (!odb_has_object(r->objects, oid, 0))\n-\t\treturn;\n-\toid_array_append(haves, oid);\n+\todb_reprepare(r->objects);\n+\n+\toidset_iter_init(&missing, &it);\n+\twhile ((oid = oidset_iter_next(&it))) {\n+\t\tif (odb_has_object(r->objects, oid, 0))\n+\t\t\toid_array_append(haves, oid);\n+\t}\n+\n+\toidset_clear(&missing);\n }\n \n /*\n@@ -64,6 +93,7 @@ static int pack_objects(struct repository *r,\n \t\t\tstruct send_pack_args *args)\n {\n \tstruct odb_generate_pack_options opts = ODB_GENERATE_PACK_OPTIONS_INIT;\n+\tstruct oidset negative_oids = OIDSET_INIT;\n \tstruct odb_pack_generator *generator;\n \tint rc;\n \n@@ -84,18 +114,20 @@ static int pack_objects(struct repository *r,\n \topts.pack_fd = args->stateless_rpc ? -1 : fd;\n \n \tfor (size_t i = 0; i < advertised->nr; i++)\n-\t\tappend_negative_object(r, &opts.haves, &advertised->oid[i]);\n+\t\toidset_insert(&negative_oids, &advertised->oid[i]);\n \tfor (size_t i = 0; i < negotiated->nr; i++)\n-\t\tappend_negative_object(r, &opts.haves, &negotiated->oid[i]);\n+\t\toidset_insert(&negative_oids, &negotiated->oid[i]);\n \n \twhile (refs) {\n \t\tif (!is_null_oid(&refs->old_oid))\n-\t\t\tappend_negative_object(r, &opts.haves, &refs->old_oid);\n+\t\t\toidset_insert(&negative_oids, &refs->old_oid);\n \t\tif (!is_null_oid(&refs->new_oid))\n \t\t\toid_array_append(&opts.wants, &refs->new_oid);\n \t\trefs = refs->next;\n \t}\n \n+\tappend_negative_objects(r, &opts.haves, &negative_oids);\n+\n \tif (odb_generate_pack(r->objects, &generator, &opts))\n \t\tdie(\"git pack-objects failed\");\n \todb_generate_pack_options_release(&opts);\n@@ -114,6 +146,7 @@ static int pack_objects(struct repository *r,\n \n \trc = odb_pack_generator_finish(generator);\n \ttrace2_region_leave(\"send_pack\", \"pack_objects\", r);\n+\toidset_clear(&negative_oids);\n \treturn rc;\n }\n \n\n"}]}