Commit Graph

71399 Commits

Author SHA1 Message Date
Junio C Hamano
ece54894fe Merge branch 'ii/branch-error-messages-update'
Error message update.

* ii/branch-error-messages-update:
  builtin/branch.c: adjust error messages to coding guidelines
2023-10-31 12:57:44 +09:00
Junio C Hamano
3130c155df The twenty-second batch
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-30 07:09:59 +09:00
Junio C Hamano
3adc25a695 Merge branch 'ms/doc-push-fix'
Docfix.

* ms/doc-push-fix:
  git-push doc: more visibility for -q option
2023-10-30 07:09:59 +09:00
Junio C Hamano
5006bfc1f5 Merge branch 'jk/send-email-fix-addresses-from-composed-messages'
The codepath to handle recipient addresses `git send-email
--compose` learns from the user was completely broken, which has
been corrected.

* jk/send-email-fix-addresses-from-composed-messages:
  send-email: handle to/cc/bcc from --compose message
  Revert "send-email: extract email-parsing code into a subroutine"
  doc/send-email: mention handling of "reply-to" with --compose
2023-10-30 07:09:59 +09:00
Junio C Hamano
90c8096657 Merge branch 'ob/rebase-cleanup'
Code clean-up.

* ob/rebase-cleanup:
  rebase: move parse_opt_keep_empty() down
  rebase: handle --strategy via imply_merge() as well
  rebase: simplify code related to imply_merge()
2023-10-30 07:09:58 +09:00
Junio C Hamano
4fcbc5b94f Merge branch 'jc/commit-new-underscore-index-fix'
Message fix.

* jc/commit-new-underscore-index-fix:
  commit: do not use cryptic "new_index" in end-user facing messages
2023-10-30 07:09:58 +09:00
Junio C Hamano
9a48da7843 Merge branch 'wx/merge-ort-comment-typofix'
Typofix.

* wx/merge-ort-comment-typofix:
  merge-ort.c: fix typo 'neeed' to 'needed'
2023-10-30 07:09:58 +09:00
Junio C Hamano
39072d2496 Merge branch 'ps/git-repack-doc-fixes'
Doc updates.

* ps/git-repack-doc-fixes:
  doc/git-repack: don't mention nonexistent "--unpacked" option
  doc/git-repack: fix syntax for `-g` shorthand option
2023-10-30 07:09:57 +09:00
Junio C Hamano
64912cc023 Merge branch 'kh/pathspec-error-wo-repository-fix'
The pathspec code carelessly dereferenced NULL while emitting an
error message, which has been corrected.

* kh/pathspec-error-wo-repository-fix:
  grep: die gracefully when outside repository
2023-10-30 07:09:57 +09:00
Junio C Hamano
6597631888 Merge branch 'ni/die-message-fix-for-git-add'
Message updates.

* ni/die-message-fix-for-git-add:
  builtin/add.c: clean up die() messages
2023-10-30 07:09:57 +09:00
Junio C Hamano
030c2fba90 Merge branch 'jc/am-doc-whitespace-action-fix'
Docfix.

* jc/am-doc-whitespace-action-fix:
  am: align placeholder for --whitespace option with apply
2023-10-30 07:09:56 +09:00
Junio C Hamano
9030f85730 Merge branch 'mm/p4-symlink-with-lfs'
"git p4" tried to store symlinks to LFS when told, but has been
fixed not to do so, because it does not make sense.

* mm/p4-symlink-with-lfs:
  git-p4 shouldn't attempt to store symlinks in LFS
2023-10-30 07:09:56 +09:00
Junio C Hamano
3a5e77e346 Merge branch 'da/t7601-style-fix'
Coding style update.

* da/t7601-style-fix:
  t7601: use "test_path_is_file" etc. instead of "test -f"
2023-10-30 07:09:56 +09:00
Junio C Hamano
1551066dc5 Merge branch 'jc/update-list-references-to-lore'
Doc update.

* jc/update-list-references-to-lore:
  doc: update list archive reference to use lore.kernel.org
2023-10-30 07:09:56 +09:00
Junio C Hamano
26dd307cfa Merge branch 'jc/attr-tree-config'
The attribute subsystem learned to honor `attr.tree` configuration
that specifies which tree to read the .gitattributes files from.

* jc/attr-tree-config:
  attr: add attr.tree for setting the treeish to read attributes from
  attr: read attributes from HEAD when bare repo
2023-10-30 07:09:55 +09:00
Junio C Hamano
8183b63ff6 Merge branch 'sn/typo-grammo-phraso-fixes'
Many typos, ungrammatical sentences and wrong phrasing have been
fixed.

* sn/typo-grammo-phraso-fixes:
  t/README: fix multi-prerequisite example
  doc/gitk: s/sticked/stuck/
  git-jump: admit to passing merge mode args to ls-files
  doc/diff-options: improve wording of the log.diffMerges mention
  doc: fix some typos, grammar and wording issues
2023-10-30 07:09:55 +09:00
Junio C Hamano
2e8e77cbac The twenty-first batch
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-23 13:56:38 -07:00
Junio C Hamano
d12166d3c8 Merge branch 'en/docfixes'
Documentation typo and grammo fixes.

* en/docfixes: (25 commits)
  documentation: add missing parenthesis
  documentation: add missing quotes
  documentation: add missing fullstops
  documentation: add some commas where they are helpful
  documentation: fix whitespace issues
  documentation: fix capitalization
  documentation: fix punctuation
  documentation: use clearer prepositions
  documentation: add missing hyphens
  documentation: remove unnecessary hyphens
  documentation: add missing article
  documentation: fix choice of article
  documentation: whitespace is already generally plural
  documentation: fix singular vs. plural
  documentation: fix verb vs. noun
  documentation: fix adjective vs. noun
  documentation: fix verb tense
  documentation: employ consistent verb tense for a list
  documentation: fix subject/verb agreement
  documentation: remove extraneous words
  ...
2023-10-23 13:56:37 -07:00
Junio C Hamano
5edbcead42 Merge branch 'bc/racy-4gb-files'
The index file has room only for lower 32-bit of the file size in
the cached stat information, which means cached stat information
will have 0 in its sd_size member for a file whose size is multiple
of 4GiB.  This is mistaken for a racily clean path.  Avoid it by
storing a bogus sd_size value instead for such files.

* bc/racy-4gb-files:
  Prevent git from rehashing 4GiB files
  t: add a test helper to truncate files
2023-10-23 13:56:37 -07:00
Junio C Hamano
626f689f79 Merge branch 'jc/fail-stash-to-store-non-stash'
Feeding "git stash store" with a random commit that was not created
by "git stash create" now errors out.

* jc/fail-stash-to-store-non-stash:
  stash: be careful what we store
2023-10-23 13:56:37 -07:00
Junio C Hamano
755fb09163 Merge branch 'so/diff-merges-dd'
"git log" and friends learned "--dd" that is a short-hand for
"--diff-merges=first-parent -p".

* so/diff-merges-dd:
  completion: complete '--dd'
  diff-merges: introduce '--dd' option
  diff-merges: improve --diff-merges documentation
2023-10-23 13:56:37 -07:00
Junio C Hamano
f32af12cee Merge branch 'jk/chunk-bounds'
The codepaths that read "chunk" formatted files have been corrected
to pay attention to the chunk size and notice broken files.

* jk/chunk-bounds: (21 commits)
  t5319: make corrupted large-offset test more robust
  chunk-format: drop pair_chunk_unsafe()
  commit-graph: detect out-of-order BIDX offsets
  commit-graph: check bounds when accessing BIDX chunk
  commit-graph: check bounds when accessing BDAT chunk
  commit-graph: bounds-check generation overflow chunk
  commit-graph: check size of generations chunk
  commit-graph: bounds-check base graphs chunk
  commit-graph: detect out-of-bounds extra-edges pointers
  commit-graph: check size of commit data chunk
  midx: check size of revindex chunk
  midx: bounds-check large offset chunk
  midx: check size of object offset chunk
  midx: enforce chunk alignment on reading
  midx: check size of pack names chunk
  commit-graph: check consistency of fanout table
  midx: check size of oid lookup chunk
  commit-graph: check size of oid fanout chunk
  midx: stop ignoring malformed oid fanout chunk
  t: add library for munging chunk-format files
  ...
2023-10-23 13:56:36 -07:00
Isoken June Ibizugbe
12b99928c8 builtin/branch.c: adjust error messages to coding guidelines
As per the CodingGuidelines document, it is recommended that error messages
such as die(), error() and warning(), should start with a lowercase letter
and should not end with a period.

This patch adjusts tests to match updated messages.

Signed-off-by: Isoken June Ibizugbe <isokenjune@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-23 12:22:57 -07:00
王常新
243c79fdc7 merge-ort.c: fix typo 'neeed' to 'needed'
Signed-off-by: 王常新 <wchangxin824@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-21 23:13:49 -07:00
Junio C Hamano
ceadf0f3cf The twentieth batch
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-20 16:23:11 -07:00
Junio C Hamano
4835409be1 Merge branch 'ps/rewritten-is-per-worktree-doc'
Doc update.

* ps/rewritten-is-per-worktree-doc:
  doc/git-worktree: mention "refs/rewritten" as per-worktree refs
2023-10-20 16:23:11 -07:00
Junio C Hamano
92741d83c0 Merge branch 'ak/pretty-decorate-more-fix'
Unlike "git log --pretty=%D", "git log --pretty="%(decorate)" did
not auto-initialize the decoration subsystem, which has been
corrected.

* ak/pretty-decorate-more-fix:
  pretty: fix ref filtering for %(decorate) formats
2023-10-20 16:23:11 -07:00
Junio C Hamano
6b1e2254d6 Merge branch 'vd/loose-ref-iteration-optimization'
The code to iterate over loose references have been optimized to
reduce the number of lstat() system calls.

* vd/loose-ref-iteration-optimization:
  files-backend.c: avoid stat in 'loose_fill_ref_dir'
  dir.[ch]: add 'follow_symlink' arg to 'get_dtype'
  dir.[ch]: expose 'get_dtype'
  ref-cache.c: fix prefix matching in ref iteration
2023-10-20 16:23:11 -07:00
Junio C Hamano
c662038629 Merge branch 'ty/merge-tree-strategy-options'
"git merge-tree" learned to take strategy backend specific options
via the "-X" option, like "git merge" does.

* ty/merge-tree-strategy-options:
  merge: introduce {copy|clear}_merge_options()
  merge-tree: add -X strategy option
2023-10-20 16:23:11 -07:00
Michal Suchanek
f6d83e2115 git-push doc: more visibility for -q option
The "-v" option is shown in the SYNOPSIS section near the top, but
"-q" is not shown anywhere there.

List "-q" alongside "-v".

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-20 15:13:38 -07:00
Oswald Buddenhagen
96db17352d rebase: move parse_opt_keep_empty() down
This moves it right next to parse_opt_empty(), which is a much more
logical place. As a side effect, this removes the need for a forward
declaration of imply_merge().

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-20 14:47:44 -07:00
Oswald Buddenhagen
37e80a2471 rebase: handle --strategy via imply_merge() as well
At least after the successive trimming of enum rebase_type mentioned in
the previous commit, this code did exactly what imply_merge() does, so
just call it instead.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-20 14:47:44 -07:00
Oswald Buddenhagen
a5b5740bf6 rebase: simplify code related to imply_merge()
The code's evolution left in some bits surrounding enum rebase_type that
don't really make sense any more. In particular, it makes no sense to
invoke imply_merge() if the type is already known not to be
REBASE_APPLY, and it makes no sense to assign the type after calling
imply_merge().

enum rebase_type had more values until commit a74b35081c ("rebase: drop
support for `--preserve-merges`") and commit 10cdb9f38a ("rebase: rename
the two primary rebase backends"). The latter commit also renamed
imply_interactive() to imply_merge().

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-20 14:47:43 -07:00
Jeff King
3ec6167567 send-email: handle to/cc/bcc from --compose message
If the user writes a message via --compose, send-email will pick up
various headers like "From", "Subject", etc and use them for other
patches as if they were specified on the command-line. But we don't
handle "To", "Cc", or "Bcc" this way; we just tell the user "those
aren't interpeted yet" and ignore them.

But it seems like an obvious thing to want, especially as the same
feature exists when the cover letter is generated separately by
format-patch. There it is gated behind the --to-cover option, but I
don't think we'd need the same control here; since we generate the
--compose template ourselves based on the existing input, if the user
leaves the lines unchanged then the behavior remains the same.

So let's fill in the implementation; like those other headers we already
handle, we just need to assign to the initial_* variables. The only
difference in this case is that they are arrays, so we'll feed them
through parse_address_line() to split them (just like we would when
reading a single string via prompting).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-20 14:31:39 -07:00
Jeff King
637e8944a1 Revert "send-email: extract email-parsing code into a subroutine"
This reverts commit b6049542b9.

Prior to that commit, we read the results of the user editing the
"--compose" message in a loop, picking out parts we cared about, and
streaming the result out to a ".final" file. That commit split the
reading/interpreting into two phases; we'd now read into a hash, and
then pick things out of the hash.

The goal was making the code more readable. And in some ways it did,
because the ugly regexes are confined to the reading phase. But it also
introduced several bugs, because now the two phases need to match each
other. In particular:

  - we pick out headers like "Subject: foo" with a case-insensitive
    regex, and then use the user-provided header name as the key in a
    case-sensitive hash. So if the user wrote "subject: foo", we'd no
    longer recognize it as a subject.

  - the namespace for the hash keys conflates header names with meta
    information like "body". If you put "body: foo" in your message, it
    would be misinterpreted as the actual message body (nobody is likely
    to do that in practice, but it seems like an unnecessary danger).

  - the handling for to/cc/bcc is totally broken. The behavior before
    that commit is to recognize and skip those headers, with a note to
    the user that they are not yet handled. Not great, but OK. But
    after the patch, the reading side now splits the addresses into a
    perl array-ref. But the interpreting side doesn't handle this at
    all, and blindly prints the stringified array-ref value. This leads
    to garbage like:

      (mbox) Adding to: ARRAY (0x555b4345c428) from line 'To: ARRAY(0x555b4345c428)'
      error: unable to extract a valid address from: ARRAY (0x555b4345c428)
      What to do with this address? ([q]uit|[d]rop|[e]dit):

    Probably not a huge deal, since nobody should even try to use those
    headers in the first place (since they were not implemented). But
    the new behavior is worse, and indicative of the sorts of problems
    that come from having the two layers.

The revert had a few conflicts, due to later work in this area from
15dc3b9161 (send-email: rename variable for clarity, 2018-03-04) and
d11c943c78 (send-email: support separate Reply-To address, 2018-03-04).
I've ported the changes from those commits over as part of the conflict
resolution.

The new tests show the bugs. Note the use of GIT_SEND_EMAIL_NOTTY in the
second one. Without it, the test is happy to reach outside the test
harness to the developer's actual terminal (when run with the buggy
state before this patch).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-20 14:31:32 -07:00
Jeff King
e0c7e2c326 doc/send-email: mention handling of "reply-to" with --compose
The documentation for git-send-email lists the headers handled specially
by --compose in a way that implies that this is the complete set of
headers that are special. But one more was added by d11c943c78
(send-email: support separate Reply-To address, 2018-03-04) and never
documented.

Let's add it, and reword the documentation slightly to avoid having to
specify the list of headers twice (as it is growing and will continue to
do so as we add new features).

If you read the code, you may notice that we also handle MIME-Version
specially, in that we'll avoid over-writing user-provided MIME headers.
I don't think this is worth mentioning, as it's what you'd expect to
happen (as opposed to the other headers, which are picked up to be used
in later emails). And certainly this feature existed when the
documentation was expanded in 01d3861217 (git-send-email.txt: describe
--compose better, 2009-03-16), and we chose not to mention it then.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-20 14:31:30 -07:00
Kristoffer Haugsbakk
b1688ea02d grep: die gracefully when outside repository
Die gracefully when `git grep --no-index` is run outside of a Git
repository and the path is outside the directory tree.

If you are not in a Git repository and say:

    git grep --no-index search ..

You trigger a `BUG`:

    BUG: environment.c:213: git environment hasn't been setup
    Aborted (core dumped)

Because `..` is a valid path which is treated as a pathspec. Then
`pathspec` figures out that it is not in the current directory tree. The
`BUG` is triggered when `pathspec` tries to advise the user about how the
path is not in the current (non-existing) repository.

Reported-by: ks1322 ks1322 <ks1322@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-20 11:06:45 -07:00
Matthew McClain
10c89a02b0 git-p4 shouldn't attempt to store symlinks in LFS
git-p4.py would attempt to put a symlink in LFS if its file extension
matched git-p4.largeFileExtensions.

Git LFS doesn't store symlinks because smudge/clean filters don't handle
symlinks. They never get passed to the filter process nor the
smudge/clean filters, nor could that occur without a change to the
protocol or command-line interface. Unless Git learned how to send them
to the filters, Git LFS would have a hard time using them in any useful
way.

Git LFS's goal is to move large files out of the repository history, and
symlinks are functionally limited to 4 KiB or a similar size on most
systems.

Signed-off-by: Matthew McClain <mmcclain@noprivs.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-19 10:57:44 -07:00
Dorcas AnonoLitunya
5abb758118 t7601: use "test_path_is_file" etc. instead of "test -f"
Some tests in t7601 use "test -f" and "test ! -f" to see if a path
exists or is missing.

Use test_path_is_file and test_path_is_missing helper functions to
clarify these tests a bit better. This especially matters for the
"missing" case because "test ! -f F" will be happy if "F" exists as a
directory, but the intent of the test is that "F" should not exist, even
as a directory. The updated code expresses this better.

Signed-off-by: Dorcas AnonoLitunya <anonolitunya@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-18 16:57:49 -07:00
Junio C Hamano
14d569b1a7 am: align placeholder for --whitespace option with apply
`git am` passes the value given to its `--whitespace` option through
to the underlying `git apply`, and the value is called <action> over
there.  Fix the documentation for the command that calls the value
<option> to say <action> instead.

Note that the option help given by `git am -h` already calls the
value <action>, so there is no need to make a matching change there.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-18 16:35:44 -07:00
Junio C Hamano
813d9a9188 The nineteenth batch
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-18 13:25:42 -07:00
Junio C Hamano
7906b5c957 Merge branch 'jc/merge-ort-attr-index-fix'
Fix "git merge-tree" to stop segfaulting when the --attr-source
option is used.

* jc/merge-ort-attr-index-fix:
  merge-ort: initialize repo in index state
2023-10-18 13:25:42 -07:00
Junio C Hamano
cc7d7183f0 Merge branch 'sn/cat-file-doc-update'
"git cat-file" documentation updates.

* sn/cat-file-doc-update:
  doc/cat-file: make synopsis and description less confusing
2023-10-18 13:25:41 -07:00
Junio C Hamano
0bc6bff9d5 Merge branch 'xz/commit-title-soft-limit-doc'
Doc update.

* xz/commit-title-soft-limit-doc:
  doc: correct the 50 characters soft limit (+)
2023-10-18 13:25:41 -07:00
Junio C Hamano
79861babe2 Merge branch 'tb/repack-max-cruft-size'
"git repack" learned "--max-cruft-size" to prevent cruft packs from
growing without bounds.

* tb/repack-max-cruft-size:
  repack: free existing_cruft array after use
  builtin/repack.c: avoid making cruft packs preferred
  builtin/repack.c: implement support for `--max-cruft-size`
  builtin/repack.c: parse `--max-pack-size` with OPT_MAGNITUDE
  t7700: split cruft-related tests to t7704
2023-10-18 13:25:41 -07:00
Junio C Hamano
a060705d94 commit: do not use cryptic "new_index" in end-user facing messages
These error messages say "new_index" as if that spelling has some
significance to the end users (e.g. the file "$GIT_DIR/new_index"
has some issues), but that is not the case at all.  The i18n folks
were made to include the word literally in the translated messages,
which was not a good idea at all.  Spell it "new index", as we are
just telling the users that we failed to create a new index file.
The term is expected to be translated to the end-users' languages,
not left as if it were a literal file name.

This dates all the way back to the first re-implemenation of "git
commit" command in C (the scripted version did not have such wording
in its error messages), in f5bbc322 (Port git commit to C.,
2007-11-08).

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-17 22:09:54 -07:00
Naomi Ibe
48399e9cf0 builtin/add.c: clean up die() messages
As described in the CodingGuidelines document, a single line message
given to die() and its friends should not capitalize its first word,
and should not add full-stop at the end.

Signed-off-by: Naomi Ibe <naomi.ibeh69@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-17 12:41:55 -07:00
Patrick Steinhardt
ca3285dd69 doc/git-repack: don't mention nonexistent "--unpacked" option
The documentation for geometric repacking mentions a "--unpacked" option
that supposedly changes how loose objects are rolled up. This option has
never existed, and the implied behaviour, namely to include all unpacked
objects into the resulting packfile, is in fact the default behaviour.

Correct the documentation to not mention this option.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-16 14:21:59 -07:00
Patrick Steinhardt
e9cc3a027b doc/git-repack: fix syntax for -g shorthand option
The `-g` switch is a shorthand for `--geometric=` and allows the user to
specify the geometric. The documentation is wrong though and indicates
that the syntax for the shorthand is `-g=<factor>`. In fact though, the
option must be specified without the equals sign via `-g<factor>`.

Fix the syntax accordingly.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-16 14:21:59 -07:00
Jeff King
7538f9d89b t5319: make corrupted large-offset test more robust
The test t5319.88 ("reader bounds-checks large offset table") can fail
intermittently. The failure mode looks like this:

  1. An earlier test sets up "objects64", a directory that can be used
     to produce a midx with a corrupted large-offsets table. To get the
     large offsets, it corrupts the normal ".idx" file to have a fake
     large offset, and then builds a midx from that.

     That midx now has a large offset table, which is what we want. But
     we also have a .idx on disk that has a corrupted entry. We'll call
     the object with the corrupted large-offset "X".

  2. In t5319.88, we further corrupt the midx by reducing the size of
     the large-offset chunk (because our goal is to make sure we do not
     do an out-of-bounds read on it).

  3. We then enumerate all of the objects with "cat-file --batch-check
     --batch-all-objects", expecting to see a complaint when we try to
     show object X. We use --batch-all-objects because our objects64
     repo doesn't actually have any refs (but if we check them all, one
     of them will be the failing one). The default batch-check format
     includes %(objecttype) and %(objectsize), both of which require us
     to access the actual pack data (and thus requires looking at the
     offset).

  4a. Usually, this succeeds. We try to output object X, do a lookup via
      the midx for the type/size lookup, and run into the corrupt
      large-offset table.

  4b. But sometimes we hit a different error. If another object points
      to X as a delta base, then trying to find the type of that object
      requires walking the delta chain to the base entry (since only the
      base has the concrete type; deltas themselves are either OFS_DELTA
      or REF_DELTA).

      Normally this would not require separate offset lookups at all, as
      deltas are usually stored as OFS_DELTA, specifying the relative
      offset to the base. But the corrupt idx created in step 1 is done
      directly with "git pack-objects" and does not pass the
      --delta-base-offset option, meaning we have REF_DELTA entries!
      Those do have to consult an index to find the location of the base
      object, and they use the pack .idx to do this. The same pack .idx
      that we know is corrupted from step 1!

      Git does notice the error, but it does so by seeing the corrupt
      .idx file, not the corrupt midx file, and the error it reports is
      different, causing the test to fail.

The set of objects created in the test is deterministic. But the delta
selection seems not to be (which is not too surprising, as it is
multi-threaded). I have seen the failure in Windows CI but haven't
reproduced it locally (not even with --stress). Re-running a failed
Windows CI job tends to work. But when I download and examine the trash
directory from a failed run, it shows a different set of deltas than I
get locally. But the exact source of non-determinism isn't that
important; our test should be robust against any order.

There are a few options to fix this:

  a. It would be OK for the "objects64" setup to "unbreak" the .idx file
     after generating the midx. But then it would be hard for subsequent
     tests to reuse it, since it is the corrupted idx that forces the
     midx to have a large offset table.

  b. The "objects64" setup could use --delta-base-offset. This would fix
     our problem, but earlier tests have many hard-coded offsets. Using
     OFS_DELTA would change the locations of objects in the pack (this
     might even be OK because I think most of the offsets are within the
     .idx file, but it seems brittle and I'm afraid to touch it).

  c. Our cat-file output is in oid order by default. Since we store
     bases before deltas, if we went in pack order (using the
     "--unordered" flag), we'd always see our corrupt X before any delta
     which depends on it. But using "--unordered" means we skip the midx
     entirely. That makes sense, since it is just enumerating all of
     the packs, using the offsets found in their .idx files directly.
     So it doesn't work for our test.

  d. We could ask directly about object X, rather than enumerating all
     of them. But that requires further hard-coding of the oid (both
     sha1 and sha256) of object X. I'd prefer not to introduce more
     brittleness.

  e. We can use a --batch-check format that looks at the pack data, but
     doesn't have to chase deltas. The problem in this case is
     %(objecttype), which has to walk to the base. But %(objectsize)
     does not; we can get the value directly from the delta itself.
     Another option would be %(deltabase), where we report the REF_DELTA
     name but don't look at its data.

I've gone with option (e) here. It's kind of subtle, but it's simple and
has no side effects.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-14 10:17:25 -07:00