Commit graph

21297 commits

Author SHA1 Message Date
Junio C Hamano fb6e6e06d5 Merge branch 'jk/ort-unused-parameter-cleanups'
Code clean-up.

* jk/ort-unused-parameter-cleanups:
  merge-ort: lowercase a few error messages
  merge-ort: drop unused "opt" parameter from merge_check_renames_reusable()
  merge-ort: drop unused parameters from detect_and_process_renames()
  merge-ort: stop passing "opt" to read_oid_strbuf()
  merge-ort: drop custom err() function
2023-09-22 17:01:36 -07:00
Junio C Hamano 3c2af826a3 Merge branch 'jc/update-index-show-index-version'
"git update-index" learns "--show-index-version" to inspect
the index format version used by the on-disk index file.

* jc/update-index-show-index-version:
  test-tool: retire "index-version"
  update-index: add --show-index-version
  update-index doc: v4 is OK with JGit and libgit2
2023-09-20 10:45:16 -07:00
Junio C Hamano 767e4d68c7 Merge branch 'ob/t3404-typofix'
Code clean-up.

* ob/t3404-typofix:
  t3404-rebase-interactive.sh: fix typos in title of a rewording test
2023-09-20 10:44:58 -07:00
Junio C Hamano 671eaaac0c Merge branch 'js/diff-cached-fsmonitor-fix'
"git diff --cached" codepath did not fill the necessary stat
information for a file when fsmonitor knows it is clean and ended
up behaving as if it is not clean, which has been corrected.

* js/diff-cached-fsmonitor-fix:
  diff-lib: fix check_removed when fsmonitor is on
2023-09-20 10:44:57 -07:00
Junio C Hamano 7435d51bfd Merge branch 'pw/diff-no-index-from-named-pipes'
"git diff --no-index -R <(one) <(two)" did not work correctly,
which has been corrected.

* pw/diff-no-index-from-named-pipes:
  diff --no-index: fix -R with stdin
2023-09-20 10:44:57 -07:00
Junio C Hamano f41c5a5eec Merge branch 'js/complete-checkout-t'
The completion script (in contrib/) has been taught to treat the
"-t" option to "git checkout" and "git switch" just like the
"--track" option, to complete remote-tracking branches.

* js/complete-checkout-t:
  completion(switch/checkout): treat --track and -t the same
2023-09-18 13:53:13 -07:00
Jeff King 24c5a270d1 merge-ort: lowercase a few error messages
As noted in CodingGuidelines, error messages should not be capitalized.
Fix up a few of these that were copied verbatim from merge-recursive to
match our modern style.

We'll likewise fix up the matching ones from merge-recursive. We care a
bit less there, since the hope is that it will eventually go away. But
besides being the right thing to do in the meantime, it is necessary for
t6406 to pass both with and without GIT_TEST_MERGE_ALGORITHM set (one of
our CI jobs sets it to "recursive", which will use the merge-recursive.c
code). An alternative would be to use "grep -i" in the test to check
the message, but it's nice for the test suite to be be more exact (we'd
notice if the capitalization fix regressed).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-16 17:26:53 -07:00
Jeff King 808e83f266 merge-ort: drop custom err() function
The merge-ort code has an err() function, but it's really just error()
in disguise. It differs in two ways:

  1. It takes a "struct merge_options" argument. But the function
     completely ignores it! We can simply remove it.

  2. It formats the error string into a strbuf, prepending "error: ",
     and then feeds the result into error(). But this is wrong! The
     error() function already adds the prefix, so we end up with:

        error: error: Failed to execute internal merge

So let's just drop this function entirely and call error() directly, as
the functions are otherwise identical (note that they both always return
-1).

Presumably nobody noticed the bogus messages because they are quite hard
to trigger (they are mostly internal errors reading and writing
objects). However, one easy trigger is a custom merge driver which dies
by signal; we have a test already here, but we were not checking the
contents of stderr.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-14 12:01:29 -07:00
Junio C Hamano b995e78147 Merge branch 'pw/rebase-i-after-failure'
Various fixes to the behaviour of "rebase -i" when the command got
interrupted by conflicting changes.

* pw/rebase-i-after-failure:
  rebase -i: fix adding failed command to the todo list
  rebase --continue: refuse to commit after failed command
  rebase: fix rewritten list for failed pick
  sequencer: factor out part of pick_commits()
  sequencer: use rebase_path_message()
  rebase -i: remove patch file after conflict resolution
  rebase -i: move unlink() calls
2023-09-14 11:17:00 -07:00
Junio C Hamano f73604fabf Merge branch 'ob/revert-of-revert-is-reapply'
The default log message created by "git revert", when reverting a
commit that records a revert, has been tweaked.

* ob/revert-of-revert-is-reapply:
  git-revert.txt: add discussion
  sequencer: beautify subject of reverts of reverts
2023-09-14 11:16:59 -07:00
Junio C Hamano 86b56ff267 Merge branch 'ak/pretty-decorate-more'
"git log --format" has been taught the %(decorate) placeholder.

* ak/pretty-decorate-more:
  decorate: use commit color for HEAD arrow
  pretty: add pointer and tag options to %(decorate)
  pretty: add %(decorate[:<options>]) format
  decorate: color each token separately
  decorate: avoid some unnecessary color overhead
  decorate: refactor format_decorations()
  pretty-formats: enclose options in angle brackets
  pretty-formats: define "literal formatting code"
2023-09-14 11:16:59 -07:00
Junio C Hamano 174dfe4637 Merge branch 'jk/tree-name-and-depth-limit'
We now limit depth of the tree objects and maximum length of
pathnames recorded in tree objects.

* jk/tree-name-and-depth-limit:
  lower core.maxTreeDepth default to 2048
  tree-diff: respect max_allowed_tree_depth
  list-objects: respect max_allowed_tree_depth
  read_tree(): respect max_allowed_tree_depth
  traverse_trees(): respect max_allowed_tree_depth
  add core.maxTreeDepth config
  fsck: detect very large tree pathnames
  tree-walk: rename "error" variable
  tree-walk: drop MAX_TRAVERSE_TREES macro
  tree-walk: reduce stack size for recursive functions
2023-09-14 11:16:59 -07:00
Junio C Hamano 6a4e7440fb Merge branch 'ks/ref-filter-sort-numerically'
"git for-each-ref --sort='contents:size'" sorts the refs according
to size numerically, giving a ref that points at a blob twelve-byte
(12) long before showing a blob hundred-byte (100) long.

* ks/ref-filter-sort-numerically:
  ref-filter: sort numerically when ":size" is used
2023-09-14 11:16:59 -07:00
Junio C Hamano 877c9919d6 Merge branch 'bc/more-git-var'
Fix-up for a topic that already has graduated.

* bc/more-git-var:
  var: avoid a segmentation fault when `HOME` is unset
2023-09-13 10:07:57 -07:00
Junio C Hamano c52a02a0f0 Merge branch 'jk/unused-post-2.42-part2'
Unused parameters to functions are marked as such, and/or removed,
in order to bring us closer to -Wunused-parameter clean.

* jk/unused-post-2.42-part2:
  parse-options: mark unused parameters in noop callback
  interpret-trailers: mark unused "unset" parameters in option callbacks
  parse-options: add more BUG_ON() annotations
  merge: do not pass unused opt->value parameter
  parse-options: mark unused "opt" parameter in callbacks
  parse-options: prefer opt->value to globals in callbacks
  checkout-index: delay automatic setting of to_tempfile
  format-patch: use OPT_STRING_LIST for to/cc options
  merge: simplify parsing of "-n" option
  merge: make xopts a strvec
2023-09-13 10:07:56 -07:00
Oswald Buddenhagen 8aae489756 t3404-rebase-interactive.sh: fix typos in title of a rewording test
This test was introduced by commit 0c164ae7a ("rebase -i: add another
reword test", 2021-08-20). I didn't quite get what it was meant to do,
so here's an explanation from Phillip:

The purpose of the test is to ensure that

  (i) There are no uncommitted changes when the editor runs. i.e., we
      commit without running the editor and then reword by amending
      that commit. This ensures that we have the same user experience
      whether or not the commit was fast-forwarded [1].

 (ii) That the todo list is re-read after the commit has been reworded.
      This is to allow the user to update the todo list while the rebase
      is paused for editing the commit message.

[1] https://lore.kernel.org/git/20190812175046.GM20404@szeder.dev/

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-12 17:24:56 -07:00
Junio C Hamano 83708f80fc test-tool: retire "index-version"
As "git update-index --show-index-version" can do the same thing,
the 'index-version' subcommand in the test-tool lost its reason to
exist.  Remove it and replace its use with the end-user facing
'git update-index --show-index-version'.

Helped-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-12 16:21:53 -07:00
Junio C Hamano 606e088d5d update-index: add --show-index-version
"git update-index --index-version N" is used to set the index format
version to a specific version, but there was no way to query the
current version used in the on-disk index file.

Teach the command a new "--show-index-version" option, and also
teach the "--index-version N" option to report what the version was
when run with the "--verbose" option.

Helped-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-12 16:21:53 -07:00
Josip Sokcevic 6a044a2048 diff-lib: fix check_removed when fsmonitor is on
`git diff-index` may return incorrect deleted entries when fsmonitor
is used in a repository with git submodules. This can be observed on
Mac machines, but it can affect all other supported platforms too.

If fsmonitor is used, `stat *st` is not initialized if cache_entry has
CE_FSMONITOR_VALID set. But, there are three call sites that rely on stat
afterwards, which can result in incorrect results.

This change partially reverts commit 4f3d6d02 (fsmonitor: skip lstat
deletion check during git diff-index, 2021-03-17).

Signed-off-by: Josip Sokcevic <sokcevic@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-11 16:45:49 -07:00
René Scharfe 48944f214c diff --no-index: fix -R with stdin
When -R is given, queue_diff() swaps the mode and name variables of the
two files to produce a reverse diff.  1e3f26542a (diff --no-index:
support reading from named pipes, 2023-07-05) added variables that
indicate whether files are special, i.e named pipes or - for stdin.
These new variables were not swapped, though, which broke the handling
of stdin with with -R.  Swap them like the other metadata variables.

Reported-by: Martin Storsjö <martin@martin.st>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-11 12:05:37 -07:00
Johannes Schindelin 9f892830d6 completion(switch/checkout): treat --track and -t the same
When `git switch --track ` is to be completed, only remote refs are
eligible because that is what the `--track` option targets.

And when the short-hand `-t` is used instead, the same _should_ happen.
Let's make it so.

Note that the bug exists both in the completions of `switch` and
`completion`, even if it manifests in slightly different ways: While
the completion of `git switch -t ` will not even look at remote refs,
the completion of `git checkout -t ` will look at both remote _and_
local refs. Both should look only at remote refs.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-08 09:26:15 -07:00
Junio C Hamano 09684a12b0 Merge branch 'dd/format-patch-rfc-updates'
"git format-patch --rfc --subject-prefix=<foo>" used to ignore the
"--subject-prefix" option and used "[RFC PATCH]"; now we will add
"RFC" prefix to whatever subject prefix is specified.

This is a backward compatible change that may deserve a note.

* dd/format-patch-rfc-updates:
  format-patch: --rfc honors what --subject-prefix sets
2023-09-07 15:06:08 -07:00
Junio C Hamano 25ff15d108 Merge branch 'jk/unused-post-2.42'
Unused parameters to functions are marked as such, and/or removed,
in order to bring us closer to -Wunused-parameter clean.

* jk/unused-post-2.42: (22 commits)
  update-ref: mark unused parameter in parser callbacks
  gc: mark unused descriptors in scheduler callbacks
  bundle-uri: mark unused parameters in callbacks
  fetch: mark unused parameter in ref_transaction callback
  credential: mark unused parameter in urlmatch callback
  grep: mark unused parmaeters in pcre fallbacks
  imap-send: mark unused parameters with NO_OPENSSL
  worktree: mark unused parameters in noop repair callback
  negotiator/noop: mark unused callback parameters
  add-interactive: mark unused callback parameters
  grep: mark unused parameter in output function
  test-trace2: mark unused argv/argc parameters
  trace2: mark unused config callback parameter
  trace2: mark unused us_elapsed_absolute parameters
  stash: mark unused parameter in diff callback
  ls-tree: mark unused parameter in callback
  commit-graph: mark unused data parameters in generation callbacks
  worktree: mark unused parameters in each_ref_fn callback
  pack-bitmap: mark unused parameters in show_object callback
  ref-filter: mark unused parameters in parser callbacks
  ...
2023-09-07 15:06:07 -07:00
Junio C Hamano 8af5aac986 Merge branch 'tb/multi-cruft-pack'
Use of --max-pack-size to allow multiple packfiles to be created is
now supported even when we are sending unreachable objects to cruft
packs.

* tb/multi-cruft-pack:
  Documentation/gitformat-pack.txt: drop mixed version section
  Documentation/gitformat-pack.txt: remove multi-cruft packs alternative
  builtin/pack-objects.c: support `--max-pack-size` with `--cruft`
  builtin/pack-objects.c: remove unnecessary strbuf_reset()
2023-09-07 15:06:07 -07:00
Phillip Wood 203573b024 rebase -i: fix adding failed command to the todo list
When rebasing commands are moved from the todo list in "git-rebase-todo"
to the "done" file (which is used by "git status" to show the recently
executed commands) just before they are executed. This means that if a
command fails because it would overwrite an untracked file it has to be
added back into the todo list before the rebase stops for the user to
fix the problem.

Unfortunately when a failed command is added back into the todo list the
command preceding it is erroneously appended to the "done" file.  This
means that when rebase stops after "pick B" fails the "done" file
contains

	pick A
	pick B
	pick A

instead of

	pick A
	pick B

This happens because save_todo() updates the "done" file with the
previous command whenever "git-rebase-todo" is updated. When we add the
failed pick back into "git-rebase-todo" we do not want to update
"done". Fix this by adding a "reschedule" parameter to save_todo() which
prevents the "done" file from being updated when adding a failed command
back into the "git-rebase-todo" file. A couple of the existing tests are
modified to improve their coverage as none of them trigger this bug or
check the "done" file.

Reported-by: Stefan Haller <lists@haller-berlin.de>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-06 10:29:44 -07:00
Phillip Wood 405509cbd6 rebase --continue: refuse to commit after failed command
If a commit cannot be picked because it would overwrite an untracked
file then "git rebase --continue" should refuse to commit any staged
changes as the commit was not picked. This is implemented by refusing to
commit if the message file is missing. The message file is chosen for
this check because it is only written when "git rebase" stops for the
user to resolve merge conflicts.

Existing commands that refuse to commit staged changes when continuing
such as a failed "exec" rely on checking for the absence of the author
script in run_git_commit(). This prevents the staged changes from being
committed but prints

    error: could not open '.git/rebase-merge/author-script' for
    reading

before the message about not being able to commit. This is confusing to
users and so checking for the message file instead improves the user
experience. The existing test for refusing to commit after a failed exec
is updated to check that we do not print the error message about a
missing author script anymore.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-06 10:29:44 -07:00
Phillip Wood e032abd5a0 rebase: fix rewritten list for failed pick
git rebase keeps a list that maps the OID of each commit before it was
rebased to the OID of the equivalent commit after the rebase.  This list
is used to drive the "post-rewrite" hook that is called at the end of a
successful rebase. When a rebase stops for the user to resolve merge
conflicts the OID of the commit being picked is written to
".git/rebase-merge/stopped-sha". Then when the rebase is continued that
OID is added to the list of rewritten commits. Unfortunately if a commit
cannot be picked because it would overwrite an untracked file we still
write the "stopped-sha1" file. This means that when the rebase is
continued the commit is added into the list of rewritten commits even
though it has not been picked yet.

Fix this by not calling error_with_patch() for failed commands. The pick
has failed so there is nothing to commit and therefore we do not want to
set up the state files for committing staged changes when the rebase
continues. This change means we no-longer write a patch for the failed
command or display the error message printed by error_with_patch(). As
the command has failed the patch isn't really useful and in any case the
user can inspect the commit associated with the failed command by
inspecting REBASE_HEAD. Unless the user has disabled it we already print
an advice message that is more helpful than the message from
error_with_patch() which the user will still see. Even if the advice is
disabled the user will see the messages from the merge machinery
detailing the problem.

The code to add a failed command back into the todo list is duplicated
between pick_one_commit() and the loop in pick_commits(). Both sites
print advice about the command being rescheduled, decrement the current
item and save the todo list. To avoid duplicating this code
pick_one_commit() is modified to set a flag to indicate that the command
should be rescheduled in the main loop. This simplifies things as only
the remaining copy of the code needs to be modified to set REBASE_HEAD
rather than calling error_with_patch().

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-06 10:29:43 -07:00
Phillip Wood 206a78d710 rebase -i: remove patch file after conflict resolution
When a rebase stops for the user to resolve conflicts it writes a patch
for the conflicting commit to .git/rebase-merge/patch. This file has
been written since the introduction of "git-rebase-interactive.sh" in
1b1dce4bae (Teach rebase an interactive mode, 2007-06-25). I assume the
idea was to enable the user inspect the conflicting commit in the same
way as they could for the patch based rebase. This file should be
deleted when the rebase continues as if the rebase stops for a failed
"exec" command or a "break" command it is confusing to the user if there
is a stale patch lying around from an unrelated command. As the path is
now used in two different places rebase_path_patch() is added and used
to obtain the path for the patch.

To construct the path write_patch() previously used get_dir() which
returns different paths depending on whether we're rebasing or
cherry-picking/reverting. As this function is only called when
rebasing it is safe to use a hard coded string for the directory
instead. An assertion is added to make sure we don't starting calling
this function when cherry-picking in the future.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-06 10:29:43 -07:00
Johannes Schindelin 256a94ef6c var: avoid a segmentation fault when HOME is unset
The code introduced in 576a37fccb (var: add attributes files locations,
2023-06-27) paid careful attention to use `xstrdup()` for pointers known
never to be `NULL`, and `xstrdup_or_null()` otherwise.

One spot was missed, though: `git_attr_global_file()` can return `NULL`,
when the `HOME` variable is not set (and neither `XDG_CONFIG_HOME`), a
scenario not too uncommon in certain server scenarios.

Fix this, and add a test case to avoid future regressions.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Acked-by: brian m. carlson <bk2204@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-05 15:28:26 -07:00
Kousik Sanagavarapu 6d79cd8474 ref-filter: sort numerically when ":size" is used
Atoms like "raw" and "contents" have a ":size" option which can be used
to know the size of the data. Since these atoms have the cmp_type
FIELD_STR, they are sorted alphabetically from 'a' to 'z' and '0' to
'9'. Meaning, even when the ":size" option is used and what we
ultimatlely have is numbers, we still sort alphabetically.

For example, consider the the following case in a repo

refname			contents:size		raw:size
=======			=============		========
refs/heads/branch1	1130			1210
refs/heads/master	300			410
refs/tags/v1.0		140			260

Sorting with "--format="%(refname) %(contents:size) --sort=contents:size"
would give

refs/heads/branch1 1130
refs/tags/v1.0.0 140
refs/heads/master 300

which is an alphabetic sort, while what one might really expect is

refs/tags/v1.0.0 140
refs/heads/master 300
refs/heads/branch1 1130

which is a numeric sort (that is, a "$ sort -n file" as opposed to a
"$ sort file", where "file" contains only the "contents:size" or
"raw:size" info, each of which is on a newline).

Same is the case with "--sort=raw:size".

So, sort numerically whenever the sort is done with "contents:size" or
"raw:size" and do it the normal alphabetic way when "contents" or "raw"
are used with some other option (they are FIELD_STR anyways).

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-05 14:49:40 -07:00
Jeff King 9b40386586 checkout-index: delay automatic setting of to_tempfile
Using --stage=all requires writing to tempfiles, since we cannot put
multiple stages into a single file. So --stage=all implies --temp.

But we do so by setting to_tempfile in the options callback for --stage,
rather than after all options have been parsed. This leads to two bugs:

  1. If you run "checkout-index --stage=all --stage=2", this should not
     imply --temp, but it currently does. The callback cannot just unset
     to_tempfile when it sees the "2" value, because it no longer knows
     if its value was from the earlier --stage call, or if the user
     specified --temp explicitly.

  2. If you run "checkout-index --stage=all --no-temp", the --no-temp
     will overwrite the earlier implied --temp. But this mode of
     operation cannot work, and the command will fail with "<path>
     already exists" when trying to write the higher stages.

We can fix both by lazily setting to_tempfile. We'll make it a tristate,
with -1 as "not yet given", and have --stage=all enable it only after
all options are parsed. Likewise, after all options are parsed we can
detect and reject the bogus "--no-temp" case.

Note that this does technically change the behavior for "--stage=all
--no-temp" for paths which have only one stage present (which
accidentally worked before, but is now forbidden). But this behavior was
never intended, and you'd have to go out of your way to try to trigger
it.

The new tests cover both cases, as well the general "--stage=all implies
--temp", as most of the other tests explicitly say "--temp". Ironically,
the test "checkout --temp within subdir" is the only one that _doesn't_
use "--temp", and so was implicitly covering this case. But it seems
reasonable to have a more explicit test alongside the other related
ones.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-05 14:47:29 -07:00
Junio C Hamano 4241eece79 Merge branch 'jk/test-lsan-denoise-output'
Tests with LSan from time to time seem to emit harmless message
that makes our tests unnecessarily flakey; we work it around by
filtering the uninteresting output.

* jk/test-lsan-denoise-output:
  test-lib: ignore uninteresting LSan output
2023-09-05 14:38:56 -07:00
Junio C Hamano 8cc32c6b37 Merge branch 'tb/mark-more-tests-as-leak-free'
Tests that are known to pass with LSan are now marked as such.

* tb/mark-more-tests-as-leak-free:
  leak tests: mark t5583-push-branches.sh as leak-free
  leak tests: mark t3321-notes-stripspace.sh as leak-free
  leak tests: mark a handful of tests as leak-free
2023-09-05 14:38:56 -07:00
Oswald Buddenhagen 883cb1b8f8 sequencer: beautify subject of reverts of reverts
Instead of generating a silly-looking `Revert "Revert "foo""`, make it
a more humane `Reapply "foo"`.

This is done for two reasons:
- To cover the actually common case of just a double revert.
- To encourage people to rewrite summaries of recursive reverts by
  setting an example (a subsequent commit will also do this explicitly
  in the documentation).

To achieve these goals, the mechanism does not need to be particularly
sophisticated. Therefore, more complicated alternatives which would
"compress more efficiently" have not been implemented.

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-02 15:20:43 -07:00
Junio C Hamano 3b4e395cb3 Merge branch 'ob/format-patch-description-file'
"git format-patch" learns a way to feed cover letter description,
that (1) can be used on detached HEAD where there is no branch
description available, and (2) also can override the branch
description if there is one.

* ob/format-patch-description-file:
  format-patch: add --description-file option
2023-09-01 11:26:28 -07:00
Junio C Hamano f137bd4358 Merge branch 'jk/diff-result-code-cleanup'
"git diff --no-such-option" and other corner cases around the exit
status of the "diff" command has been corrected.

* jk/diff-result-code-cleanup:
  diff: drop useless "status" parameter from diff_result_code()
  diff: drop useless return values in git-diff helpers
  diff: drop useless return from run_diff_{files,index} functions
  diff: die when failing to read index in git-diff builtin
  diff: show usage for unknown builtin_diff_files() options
  diff-files: avoid negative exit value
  diff: spell DIFF_INDEX_CACHED out when calling run_diff_index()
2023-09-01 11:26:28 -07:00
Jeff King 7b61bd18b1 tree-diff: respect max_allowed_tree_depth
When diffing trees, we recurse to handle subtrees. That means we may run
out of stack space and segfault. Let's teach this code path about
core.maxTreeDepth in order to fail more gracefully.

As with the previous patch, we have no way to return an error (and other
tree-loading problems would just cause us to die()). So we'll likewise
call die() if we exceed the maximum depth.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-31 15:51:08 -07:00
Jeff King 670a1dadc7 list-objects: respect max_allowed_tree_depth
The tree traversal in list-objects.c, which is used by "rev-list
--objects", etc, uses recursion and may run out of stack space. Let's
teach it about the new core.maxTreeDepth config option.

We unfortunately can't return an error here, as this code doesn't
produce an error return at all. We'll die() instead, which matches the
behavior when we see an otherwise broken tree.

Note that this will also generally reject such deep trees from entering
the repository from a fetch or push, due to the use of rev-list in the
connectivity check. But it's not foolproof! We stop traversing when we
see an UNINTERESTING object, and the connectivity check marks existing
ref tips as UNINTERESTING. So imagine commit X has a tree
with maximum depth N. If you then create a new commit Y with a tree
entry "Y:subdir" that points to "X^{tree}", then the depth of Y will be
N+1. But a connectivity check running "git rev-list --objects Y --not X"
won't realize that; it will stop traversing at X^{tree}, since that was
already reachable.

So this will stop naive pushes of too-deep trees, but not carefully
crafted malicious ones. Doing it robustly and efficiently would require
caching the maximum depth of each tree (i.e., the longest path to any
leaf entry). That's much more complex and not strictly needed. If each
recursive algorithm limits itself already, then that's sufficient.
Blocking the objects from entering the repo would be a nice
belt-and-suspenders addition, but it's not worth the extra cost.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-31 15:51:08 -07:00
Jeff King 1ee7a5c388 read_tree(): respect max_allowed_tree_depth
The read_tree() function reads trees recursively (via its read_tree_at()
helper). This can cause it to run out of stack space on very deep trees.
Let's teach it about the new core.maxTreeDepth option.

The easiest way to demonstrate this is via "ls-tree -r", which the test
covers. Note that I needed a tree depth of ~30k to trigger a segfault on
my Linux system, not the 4100 used by our "big" test in t6700. However,
that test still tells us what we want: that the default 4096 limit is
enough to prevent segfaults on all platforms. We could bump it, but that
increases the cost of the test setup for little gain.

As an interesting side-note: when I originally wrote this patch about 4
years ago, I needed a depth of ~50k to segfault. But porting it forward,
the number is much lower. Seemingly little things like cf0983213c (hash:
add an algo member to struct object_id, 2021-04-26) take it from 32,722
to 29,080.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-31 15:51:08 -07:00
Jeff King f1f63a481b traverse_trees(): respect max_allowed_tree_depth
The tree-walk.c code walks trees recursively, and may run out of stack
space. The easiest way to see this is with git-archive; on my 64-bit
Linux system it runs out of stack trying to generate a tarfile with a
tree depth of 13,772.

I've picked 4100 as the depth for our "big" test. I ran it with a much
higher value to confirm that we do get a segfault without this patch.
But really anything over 4096 is sufficient for its stated purpose,
which is to find out if our default limit of 4096 is low enough to
prevent segfaults on all platforms. Keeping it small saves us time on
the test setup.

The tree-walk code that's touched here underlies unpack_trees(), so this
protects any programs which use it, not just git-archive (but archive is
easy to test, and was what alerted me to this issue in a real-world
case).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-31 15:51:08 -07:00
Jeff King 0fbcaef6b4 fsck: detect very large tree pathnames
In general, Git tries not to arbitrarily limit what it will store, and
there are currently no limits at all on the size of the path we find in
a tree. In theory you could have one that is gigabytes long.

But in practice this freedom is not really helping anybody, and is
potentially harmful:

  1. Most operating systems have much lower limits for the size of a
     single pathname component (e.g., on Linux you'll generally get
     ENAMETOOLONG for anything over 255 bytes). And while you _can_ use
     Git in a way that never touches the filesystem (manipulating the
     index and trees directly), it's still probably not a good idea to
     have gigantic tree names. Many operations load and traverse them,
     so any clever Git-as-a-database scheme is likely to perform poorly
     in that case.

  2. We still have a lot of code which assumes strings are reasonably
     sized, and I won't be at all surprised if you can trigger some
     interesting integer overflows with gigantic pathnames. Stopping
     malicious trees from entering the repository provides an extra line
     of defense, protecting downstream code.

This patch implements an fsck check so that such trees can be rejected
by transfer.fsckObjects. I've picked a reasonably high maximum depth
here (4096) that hopefully should not bother anybody in practice. I've
also made it configurable, as an escape hatch.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-31 15:51:07 -07:00
Drew DeVault e0d7db7423 format-patch: --rfc honors what --subject-prefix sets
Rather than replacing the configured subject prefix (either through the
git config or command line) entirely with "RFC PATCH", this change
prepends RFC to whatever subject prefix was already in use.

This is useful, for example, when a user is working on a repository that
has a subject prefix considered to disambiguate patches:

	git config format.subjectPrefix 'PATCH my-project'

Prior to this change, formatting patches with --rfc would lose the
'my-project' information.

The data flow for the subject-prefix was that rev.subject_prefix
were to be kept the authoritative version of the subject prefix even
while parsing command line options, and sprefix variable was used as
a temporary area to futz with it.  Now, the parsing code has been
refactored to build the subject prefix into the sprefix variable and
assigns its value at the end to rev.subject_prefix, which makes the
flow easier to grasp.

Signed-off-by: Drew DeVault <sir@cmpwn.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-31 15:02:21 -07:00
Junio C Hamano 967bfc5894 Merge branch 'ch/t6300-verify-commit-test-cleanup'
Test clean-up.

* ch/t6300-verify-commit-test-cleanup:
  t/t6300: drop magic filtering
  t/lib-gpg: forcibly run a trustdb update
2023-08-31 14:31:42 -07:00
Junio C Hamano cc48906c3b Merge branch 'ts/unpacklimit-config-fix'
transfer.unpackLimit ought to be used as a fallback, but overrode
fetch.unpackLimit and receive.unpackLimit instead.

* ts/unpacklimit-config-fix:
  transfer.unpackLimit: fetch/receive.unpackLimit takes precedence
2023-08-30 13:50:41 -07:00
Junio C Hamano 74a2e88700 Merge branch 'jc/diff-exit-code-with-w-fixes'
"git diff -w --exit-code" with various options did not work
correctly, which is being addressed.

* jc/diff-exit-code-with-w-fixes:
  diff: the -w option breaks --exit-code for --raw and other output modes
  t4040: remove test that succeeded for a wrong reason
  diff: teach "--stat -w --exit-code" to notice differences
  diff: mode-only change should be noticed by "--patch -w --exit-code"
  diff: move the fallback "--exit-code" code down
2023-08-30 13:50:41 -07:00
Junio C Hamano 5ba560ba76 Merge branch 'tb/commit-graph-verify-fix'
The commit-graph verification code that detects mixture of zero and
non-zero generation numbers has been updated.

* tb/commit-graph-verify-fix:
  commit-graph: avoid repeated mixed generation number warnings
  t/t5318-commit-graph.sh: test generation zero transitions during fsck
  commit-graph: verify swapped zero/non-zero generation cases
  commit-graph: introduce `commit_graph_generation_from_graph()`
2023-08-30 13:50:40 -07:00
Jeff King 51bf8676c0 test-trace2: mark unused argv/argc parameters
The trace2 test helper uses function pointers to dispatch to individual
tests. Not all tests bother looking at their argv/argc parameters. We
could tighten this up (e.g., complaining when seeing unexpected
parameters), but for internal test code it's not worth worrying about.

This is similar in spirit to 126e3b3d2a (t/helper: mark unused argv/argc
arguments, 2023-03-28).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-29 17:56:25 -07:00
Junio C Hamano 19cb1fc37b Merge branch 'ds/scalar-updates'
Scalar updates.

* ds/scalar-updates:
  scalar reconfigure: help users remove buggy repos
  setup: add discover_git_directory_reason()
  scalar: add --[no-]src option
2023-08-29 13:51:44 -07:00
Junio C Hamano 354356feff Merge branch 'sl/sparse-check-attr'
Teach "git check-attr" work better with sparse-index.

* sl/sparse-check-attr:
  check-attr: integrate with sparse-index
  attr.c: read attributes in a sparse directory
  t1092: add tests for 'git check-attr'
2023-08-29 13:51:43 -07:00
Taylor Blau 61568efa95 builtin/pack-objects.c: support --max-pack-size with --cruft
When pack-objects learned the `--cruft` option back in b757353676
(builtin/pack-objects.c: --cruft without expiration, 2022-05-20), we
explicitly forbade `--cruft` with `--max-pack-size`.

At the time, there was no specific rationale given in the patch for not
supporting the `--max-pack-size` option with `--cruft`. (As best I can
remember, it's because we were trying to push users towards only ever
having a single cruft pack, but I cannot be sure).

However, `--max-pack-size` is flexible enough that it already works with
`--cruft` and can shard unreachable objects across multiple cruft packs,
creating separate ".mtimes" files as appropriate. In fact, the
`--max-pack-size` option worked with `--cruft` as far back as
b757353676!

This is because we overwrite the `written_list`, and pass down the
appropriate length, i.e. the number of objects written in each pack
shard.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-29 11:58:06 -07:00