[PATCH] fetch-pack: defer .gitattributes checks for packfile URIs - #77
Open
friel-openai wants to merge 37 commits into
Open
[PATCH] fetch-pack: defer .gitattributes checks for packfile URIs#77friel-openai wants to merge 37 commits into
friel-openai wants to merge 37 commits into
Conversation
The default-branch trampoline must not start from pull_request_review: that event loads workflow code from the PR merge ref. Replace it with a trusted default-branch scanner. On a five-minute schedule, or an explicit scan dispatch, it pins meta, checks it out read-only, preflights approved candidates with propose-plan --no-push, and calls the App-backed producer only for a valid exact-head review. Stale or policy-invalid approvals are skipped instead of blocking later topics. Plan admission stays pull_request_target with trusted meta code; remove and reorder remain explicit dispatch-only policy operations.
Commit 9bc1518 (builtin/maintenance: introduce "geometric-repack" task, 2025-10-24) added a new maintenance task to perform either a geometric repack, or an all-into-one repack if the geometric repack would itself produce a single pack. Some time later, commit dcc9c7e (builtin/repack: handle promisor packs with geometric repacking, 2026-01-05) taught the geometric repacking machinery to separate promisor packs from ordinary ones, but did not update the maintenance task accordingly. As a consequence, the geometric-repack maintenance task only considers the non-promisor pack progression. It falls back to all-into-one whenever a geometric repack would roll up all non-promisor packs into a single pack, even if the promisor progression would keep a large pack and roll up only smaller ones. Check both progressions before choosing the repack mode. If either leaves a pack above its split, geometric repack still avoids rewriting that pack, whereas the all-into-one fallback would rewrite it. Use the fallback only when neither progression leaves a pack behind. That preserves the reason for the fallback: let the all-into-one repack handle unreachable objects when it is not rewriting more packs than the geometric repack. Signed-off-by: Taylor Blau <ttaylorr@openai.com>
Commit 9bc1518 (builtin/maintenance: introduce "geometric-repack" task, 2025-10-24) added an auto condition for the geometric-repack task. It runs the task when ordinary packs need to be combined or when the number of loose objects crosses the configured threshold. Later on in commit dcc9c7e (builtin/repack: handle promisor packs with geometric repacking, 2026-01-05), the geometric repack machinery started handling promisor packs separately, but did not correspondingly update the auto condition. As a result, a repository can have promisor packs ready to combine while its non-promisor packs and loose object count require no work. In that case, `--auto` skips the task even though a geometric repack would combine at least two promisor packs. Check `geometry.promisor_split` alongside `geometry.split`. There is some fallout in t5331: the new condition makes a filtered clone eligible for auto-maintenance before the test inspects its promisor packs. Disable auto-maintenance in that fixture so it continues to test `--stdin-packs`, not the maintenance task. Signed-off-by: Taylor Blau <ttaylorr@openai.com>
Topic pull requests merge into codex before the controller rebuilds its recorded output. Treating every codex push as a release would run the full build matrix and publish artifacts for an intermediate tree. Compare each pushed commit with the output recorded in meta:codex.config before starting version detection. Skip the build and release jobs unless the controller published that exact commit; a later pending merge must not suppress its release.
The release workflow currently listens only to codex and checks every push against codex.output-tip. The controller records and atomically publishes codex-unstable.output-tip separately, so preview output pushes need an independent gate. Listen to both generated branches, skip deletion events, and select the recorded output from the exact pushed ref. Unknown refs and missing state fail closed. The existing version, build, and release chain still runs only when the selected output matches GITHUB_SHA.
Codex and codex-unstable releases currently share the same prerelease shape. Their target SHA differs, but the GitHub release object does not record which output ref triggered it. Consumers must therefore join release data with Actions runs to distinguish the lanes. Carry github.ref into the publish step and put source_ref and source_sha at the start of the release notes. This leaves tag and asset names unchanged while giving release-API consumers a machine-readable lane marker.
In the following commit(s), some tests will need to distinguish between `REF_DELTA`s and `OFS_DELTA`s to exercise a new '--no-ref-delta' option for 'pack-objects'. Existing tools report delta relationships, but not how their bases are represented in the pack. Teach 'test-tool pack-deltas' a '--list-deltas' mode. For each delta entry, print the object ID, its REF_DELTA or OFS_DELTA type, and the base object ID or pack offset, respectively. This lets tests inspect pack headers without open-coding a parser. Signed-off-by: Taylor Blau <ttaylorr@openai.com>
Some consumers of 'pack-objects' may wish to avoid packs which contain `REF_DELTA` entries. For instance, a 'receive-pack' implementation which retains the resulting pack without building an index of object IDs may prefer every delta base to be discoverable from an earlier entry in the same pack. Teach 'pack-objects' a new `--no-ref-delta` option to avoid writing `REF_DELTA` entries, without changing whether `OFS_DELTA` is allowed. When used without `--delta-base-offset`, no delta representation remains, so avoid delta search entirely. Otherwise, allow new deltas whose bases appear earlier in the same pack. For now, disable delta- and bitmap-reuse under `--no-ref-delta`, since either may copy an existing `REF_DELTA` entry. This is overly pessimistic, but simplifies the changes in this commit. The next commit re-enables reuse in the cases which do not require `REF_DELTA`. Signed-off-by: Taylor Blau <ttaylorr@openai.com>
The previous commit disables delta- and bitmap-reuse entirely whenever pack-objects is given '--no-ref-delta' for the sake of simplicity. This is overly pessimistic. When '--delta-base-offset' is also given, delta reuse can remain enabled. A reused delta whose base is written earlier in the output can be encoded as an `OFS_DELTA`, even when its source copy was encoded as a `REF_DELTA`. Preferred bases and external thin-pack bases are different: neither appears in the output, so deltas against either still require encoding the object as a `REF_DELTA`, and thus cannot be reused. Without '--delta-base-offset', delta reuse remains disabled, since no delta representation remains. Bitmap reuse follows a different path, since selected entries may be copied without passing through the code which chooses a delta representation. When given '--no-ref-delta', we must inspect candidate objects individually, and leave `REF_DELTA` entries to the normal object path outside of pack-reuse. We must likewise avoid the special-case for reusing either the single or preferred pack corresponding to the bitmap by whole `eword_t`'s at a time. Signed-off-by: Taylor Blau <ttaylorr@openai.com>
Add a 'no-ref-delta' receive-pack capability and teach send-pack to pass '--no-ref-delta' to 'pack-objects' when the server advertises it. Keep this separate from 'ofs-delta' so that a server may request that `send-pack` omit `REF_DELTA` without also accepting `OFS_DELTA`. Signed-off-by: Taylor Blau <ttaylorr@openai.com>
A client can rely on a server no-ref-delta promise only if it checks every received pack. Add --no-ref-delta to reject a REF_DELTA while parsing the pack. OFS_DELTA remains accepted. By definition, an OFS_DELTA refers to an earlier entry in the same pack, independently of this option. Test file and stdin input containing REF_DELTA. Retain coverage that accepts OFS_DELTA. Signed-off-by: Friel <friel@openai.com>
The existing no-ref-delta capability lets a receive-pack server ask send-pack not to emit REF_DELTA entries. Protocol v2 fetch needs the same guarantee in the other direction before a client can treat packfile URI responses as independent packs. Add uploadpack.allowNoRefDelta and advertise no-ref-delta as a fetch feature when it is enabled. When a client requests it, pass --no-ref-delta to the inline pack-objects process. The same promise covers every configured packfile URI; upload-pack cannot inspect those packs, so the administrator must create or verify them accordingly. The promise remains independent of ofs-delta. Test advertisement, rejection of an unadvertised request, and the pack-objects argument. Signed-off-by: Friel <friel@openai.com>
Packfile URI responses are indexed in advertised order because a later URI pack can contain a REF_DELTA whose base is installed by an earlier response pack. That ordering prevents independent URI packs from using more than one indexer. When upload-pack advertises no-ref-delta and more than one URI job is configured, request it with packfile-uris. Pass --no-ref-delta to the inline indexer and every URI indexer, then run URI packs concurrently. Keep each URI indexer at one thread so the configured job count also bounds indexer threads. Precreate each expected keep path before starting its http-fetch child, record newly-created paths in the existing pack-lock list, and omit the child's --keep argument. Existing fetch cleanup then owns keeps created by this process while pre-existing keeps remain untouched. Mark both helper process layers for exit cleanup. Use a small poll loop to refill a finished slot until all URI packs are indexed. One job, one URI, or callers without pack-lock ownership retain the serial path. A promised URI pack that contains a REF_DELTA fails closed in index-pack. Test the negotiated request, inline verifier, and three URI indexers. Signed-off-by: Friel <friel@openai.com>
The release workflow cross-compiles Linux arm64 on an x64 runner and skips the smoke test for arm64 POSIX bundles. That prevents the workflow from executing the Linux artifact it just produced. Run Linux arm64 on GitHub's arm64 runner and install native development packages rather than configuring a foreign dpkg architecture. All matrix entries can then run the existing distribution smoke test.
Codex consumes Git release artifacts built with the Makefile's default -O2 flags. The release job compiles each artifact without link-time optimization. Add a release-only config.mak.openai and copy it into Git's ignored config.mak slot before building. Use thin LTO for Clang targets and automatic LTO for GCC targets, then check GIT-CFLAGS records the selected flag in every distribution job. Keeping the setting in config.mak.openai avoids carrying release-only policy in the upstream Makefile.
LTO can optimize across translation units, but the release job has no execution profile for the status, diff, clone, fetch, and repack paths Codex invokes frequently. Git's built-in profile target runs the 1,048-script test suite serially. That is too expensive for every release target and weights test-harness paths more heavily than the local workload. Extend config.mak.openai with GCC and LLVM profile modes. Gate GIT-CFLAGS on an instrumented build, run a short offline trainer, merge LLVM raw profiles when needed, and rebuild with profile-use flags. Each matrix entry runs on its target architecture, so it can execute the instrumented binary. Check that final GIT-CFLAGS includes a profile-use flag and increase the timeout for the second compilation pass. The focused trainer took about 30 seconds locally; the full macOS build/install validation completed with thin LTO and LLVM profile-use enabled.
Integrate the current tb/codex/automation topic into the internally distributed codex branch. Codex-Integration: tb/codex/automation@9b7652c87e369dafb6a50e97d24670ccd512f1bc
Integrate the current tb/codex/geometric-maintenance-promisor topic into the internally distributed codex branch. Codex-Integration: tb/codex/geometric-maintenance-promisor@bccc1fd882f27143f7331192a75c6281ba76e2f7
Integrate the current tb/codex/release topic into the internally distributed codex branch. Codex-Integration: tb/codex/release@2bb6c1c18aa3170efff362eb3fb8f096134955f7
Integrate the current dr/codex/dugite topic into the internally distributed codex branch. Codex-Integration: dr/codex/dugite@5b75aebc9071c08a87ddf7bf00b173e05a5bfc96
Integrate the current tb/codex/lto-pgo topic into the internally distributed codex branch. Codex-Integration: tb/codex/lto-pgo@67ba20645b1792f43c8c8ea69c716687bda87ec3
Integrate the current tb/codex/packfile-uri-concurrency topic into the internally distributed codex branch. Codex-Integration: tb/codex/packfile-uri-concurrency@2c3adbb2c475981e340c79fdc5e7f4f9b5d9054e
Integrate the current af/codex/pack-bytes topic into the internally distributed codex branch. Codex-Integration: af/codex/pack-bytes@a6bb1bb89133e5eccf6e3ca3aaa58f560eb3744c
Integrate the current tb/codex/parallel-packfile-uris topic into the internally distributed codex branch. Codex-Integration: tb/codex/parallel-packfile-uris@6f0edce407d5e8c1d212514281f49c9bc433ee3c
friel-openai
added
the
git
ChatGPT Codex Connector
With fetch.fsckObjects or transfer.fsckObjects enabled, index-pack checks each pack as soon as it has been installed. Some checks need objects from another pack, though. A tree in the inline pack can name a .gitattributes blob that arrives in a packfile URI response. index-pack then reports the missing blob before fetch-pack has downloaded the URI pack. The same split already works for .gitmodules. index-pack writes any unresolved .gitmodules object IDs after its pack result. fetch-pack collects those IDs and runs fsck_finish() after all packs have been installed. Extend that handoff to .gitattributes. Prefix .gitattributes records so fetch-pack can distinguish them while leaving the existing .gitmodules output unchanged. The existing fsck_finish() call then checks both sets after every packfile URI has been indexed. Test that two concurrently indexed URI packs accept a valid split .gitattributes file and reject an overlong one. Signed-off-by: Friel <friel@openai.com>
friel-openai
force-pushed
the
dev/friel/defer-packfile-uri-fsck
branch
from
August 29, 2026 23:22
9901b53 to
d20dd4f
Compare
friel-openai
marked this pull request as ready for review
August 30, 2026 00:14
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d20dd4fd44
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+1881
to
+1882
| fsck_options_init(&fsck_options, the_repository, | ||
| FSCK_OPTIONS_MISSING_GITMODULES_AND_GITATTRIBUTES); |
There was a problem hiding this comment.
When packfile URIs split a .gitattributes blob from its tree, the final fsck_finish() uses this strictly initialized options object without applying fsck_msg_types. Consequently settings such as fetch.fsck.gitattributesLineLength=ignore and fetch.fsck.skipList are ignored for the newly deferred check, so a fetch configured to tolerate or skip that object still fails with fsck failed; populate these options from the fetch fsck configuration before accumulating deferred IDs.
Useful? React with 👍 / 👎.
friel-openai
changed the title
ttaylorr-oai
force-pushed
the
codex
branch
6 times, most recently
from
September 11, 2026 17:07
af892c1 to
45d6d20
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
With fetch.fsckObjects or transfer.fsckObjects enabled, index-pack checks
each pack as soon as it has been installed. Some checks need objects from
another pack, though. A tree in the inline pack can name a .gitattributes
blob that arrives in a packfile URI response. index-pack then reports the
missing blob before fetch-pack has downloaded the URI pack.
The same split already works for .gitmodules. index-pack writes any
unresolved .gitmodules object IDs after its pack result. fetch-pack
collects those IDs and runs fsck_finish() after all packs have been
installed.
Extend that handoff to .gitattributes. Prefix .gitattributes records so
fetch-pack can distinguish them while leaving the existing .gitmodules
output unchanged. The existing fsck_finish() call then checks both sets
after every packfile URI has been indexed.
Test that two concurrently indexed URI packs accept a valid split
.gitattributes file and reject an overlong one.
Signed-off-by: Friel friel@openai.com
Documentation/git-index-pack.adoc | 8 +++---
builtin/index-pack.c | 3 ++-
fetch-pack.c | 57 ++++++++++++++++++++++-----------------
fsck.c | 23 +++++++++-------
fsck.h | 11 ++++----
t/t5702-protocol-v2.sh | 45 +++++++++++++++++++++++++++++++
6 files changed, 103 insertions(+), 44 deletions(-)