Commit graph

10609 commits

Author SHA1 Message Date
Junio C Hamano c68bd3ec22 Merge branch 'rs/gc-pack-refs-simplify'
Code clean-up.

* rs/gc-pack-refs-simplify:
  gc: simplify maintenance_task_pack_refs()
2022-10-11 10:36:12 -07:00
Junio C Hamano 38bb92cf46 Merge branch 'hn/parse-worktree-ref'
Code and semantics cleaning.

* hn/parse-worktree-ref:
  refs: unify parse_worktree_ref() and ref_type()
2022-10-10 10:08:43 -07:00
Junio C Hamano 2e6c1b59fd Merge branch 'ah/branch-autosetupmerge-grammofix'
Fix grammar of a message introduced in previous round.

* ah/branch-autosetupmerge-grammofix:
  push: improve grammar of branch.autoSetupMerge advice
2022-10-10 10:08:40 -07:00
Junio C Hamano 67bf4a83e9 Merge branch 'sy/sparse-grep'
"git grep" learned to expand the sparse-index more lazily and on
demand in a sparse checkout.

* sy/sparse-grep:
  builtin/grep.c: integrate with sparse index
2022-10-10 10:08:40 -07:00
Junio C Hamano 4b4d97cfda Merge branch 'ds/scalar-unregister-idempotent'
"scalar unregister" in a repository that is already been
unregistered reported an error.

* ds/scalar-unregister-idempotent:
  string-list: document iterator behavior on NULL input
  gc: replace config subprocesses with API calls
  scalar: make 'unregister' idempotent
  maintenance: add 'unregister --force'
2022-10-10 10:08:40 -07:00
Junio C Hamano 20a5dd670c Merge branch 'jk/remote-rename-without-fetch-refspec'
"git remote rename" failed to rename a remote without fetch
refspec, which has been corrected.

* jk/remote-rename-without-fetch-refspec:
  remote: handle rename of remote without fetch refspec
2022-10-10 10:08:39 -07:00
Junio C Hamano 7aeb0d4c47 Merge branch 'jk/clone-allow-bare-and-o-together'
"git clone" did not like to see the "--bare" and the "--origin"
options used together without a good reason.

* jk/clone-allow-bare-and-o-together:
  clone: allow "--bare" with "-o"
2022-10-10 10:08:39 -07:00
Junio C Hamano fdbfac60fd Merge branch 'jk/fsck-on-diet'
"git fsck" failed to release contents of tree objects already used
from the memory, which has been fixed.

* jk/fsck-on-diet:
  parse_object_buffer(): respect save_commit_buffer
  fsck: turn off save_commit_buffer
  fsck: free tree buffers after walking unreachable objects
2022-10-10 10:08:39 -07:00
Junio C Hamano ab26e44d98 Merge branch 'ah/fsmonitor-daemon-usage-non-l10n'
Fix messages incorrectly marked for translation.

* ah/fsmonitor-daemon-usage-non-l10n:
  fsmonitor--daemon: don't translate literal commands
2022-10-10 10:08:39 -07:00
René Scharfe b004c90282 gc: simplify maintenance_task_pack_refs()
Pass a constant string array directly to run_command_v_opt() instead of
copying it into a strvec first.  This shortens the code and avoids heap
allocations.

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-10-05 12:46:27 -07:00
Alex Henrie 2a905f8fa8 push: improve grammar of branch.autoSetupMerge advice
"upstream branches" is plural but "name" and "local branch" are
singular. Make them all singular. And because we're talking about a
hypothetical branch that doesn't exist yet, use the future tense.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-09-28 19:03:10 -07:00
Derrick Stolee 50a044f1e4 gc: replace config subprocesses with API calls
The 'git maintenance [un]register' commands set or unset the multi-
valued maintenance.repo config key with the absolute path of the current
repository. These are set in the global config file.

Instead of calling a subcommand and creating a new process, create the
proper API calls to git_config_set_multivar_in_file_gently(). It
requires loading the filename for the global config file (and erroring
out if now $HOME value is set). We also need to be careful about using
CONFIG_REGEX_NONE when adding the value and using
CONFIG_FLAGS_FIXED_VALUE when removing the value. In both cases, we
check that the value already exists (this check already existed for
'unregister').

Also, remove the transparent translation of the error code from the
config API to the exit code of 'git maintenance'. Instead, use die() to
recover from failures at that level. In the case of 'unregister
--force', allow the CONFIG_NOTHING_SET error code to be a success. This
allows a possible race where another process removes the config value.
The end result is that the config value is not set anymore, so we can
treat this as a success.

Reported-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-09-27 09:32:26 -07:00
Derrick Stolee 1ebe6b0297 maintenance: add 'unregister --force'
The 'git maintenance unregister' subcommand has a step that removes the
current repository from the multi-valued maitenance.repo config key.
This fails if the repository is not listed in that key. This makes
running 'git maintenance unregister' twice result in a failure in the
second instance.

This failure exit code is helpful, but its message is not. Add a new
die() message that explicitly calls out the failure due to the
repository not being registered.

In some cases, users may want to run 'git maintenance unregister' just
to make sure that background jobs will not start on this repository, but
they do not want to check to see if it is registered first. Add a new
'--force' option that will siltently succeed if the repository is not
already registered.

Also add an extra test of 'git maintenance unregister' at a point where
there are no registered repositories. This should fail without --force.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-09-27 09:32:25 -07:00
Shaoxuan Yuan 7cae7627c4 builtin/grep.c: integrate with sparse index
Turn on sparse index and remove ensure_full_index().

Before this patch, `git-grep` utilizes the ensure_full_index() method to
expand the index and search all the entries. Because this method
requires walking all the trees and constructing the index, it is the
slow part within the whole command.

To achieve better performance, this patch uses grep_tree() to search the
sparse directory entries and get rid of the ensure_full_index() method.

Why grep_tree() is a better choice over ensure_full_index()?

1) grep_tree() is as correct as ensure_full_index(). grep_tree() looks
   into every sparse-directory entry (represented by a tree) recursively
   when looping over the index, and the result of doing so matches the
   result of expanding the index.

2) grep_tree() utilizes pathspecs to limit the scope of searching.
   ensure_full_index() always expands the index, which means it will
   always walk all the trees and blobs in the repo without caring if
   the user only wants a subset of the content, i.e. using a pathspec.
   On the other hand, grep_tree() will only search the contents that
   match the pathspec, and thus possibly walking fewer trees.

3) grep_tree() does not construct and copy back a new index, while
   ensure_full_index() does. This also saves some time.

----------------
Performance test

- Summary:

p2000 tests demonstrate a ~71% execution time reduction for
`git grep --cached bogus -- "f2/f1/f1/*"` using tree-walking logic.
However, notice that this result varies depending on the pathspec
given. See below "Command used for testing" for more details.

Test                              HEAD~   HEAD
-------------------------------------------------------
2000.78: git grep ... (full-v3)   0.35    0.39 (≈)
2000.79: git grep ... (full-v4)   0.36    0.30 (≈)
2000.80: git grep ... (sparse-v3) 0.88    0.23 (-73.8%)
2000.81: git grep ... (sparse-v4) 0.83    0.26 (-68.6%)

- Command used for testing:

	git grep --cached bogus -- "f2/f1/f1/*"

The reason for specifying a pathspec is that, if we don't specify a
pathspec, then grep_tree() will walk all the trees and blobs to find the
pattern, and the time consumed doing so is not too different from using
the original ensure_full_index() method, which also spends most of the
time walking trees. However, when a pathspec is specified, this latest
logic will only walk the area of trees enclosed by the pathspec, and the
time consumed is reasonably a lot less.

Generally speaking, because the performance gain is acheived by walking
less trees, which are specified by the pathspec, the HEAD time v.s.
HEAD~ time in sparse-v[3|4], should be proportional to
"pathspec enclosed area" v.s. "all area", respectively. Namely, the
wider the <pathspec> is encompassing, the less the performance
difference between HEAD~ and HEAD, and vice versa.

That is, if we don't specify a pathspec, the performance difference [1]
is indistinguishable: both methods walk all the trees and take generally
same amount of time (even with the index construction time included for
ensure_full_index()).

[1] Performance test result without pathspec (hence walking all trees):

	Command used:

		git grep --cached bogus

	Test                                HEAD~  HEAD
	---------------------------------------------------
	2000.78: git grep ... (full-v3)     6.17   5.19 (≈)
	2000.79: git grep ... (full-v4)     6.19   5.46 (≈)
	2000.80: git grep ... (sparse-v3)   6.57   6.44 (≈)
	2000.81: git grep ... (sparse-v4)   6.65   6.28 (≈)

--------------------------
NEEDSWORK about submodules

There are a few NEEDSWORKs that belong to improvements beyond this
topic. See the NEEDSWORK in builtin/grep.c::grep_submodule() for
more context. The other two NEEDSWORKs in t1092 are also relative.

Suggested-by: Derrick Stolee <derrickstolee@github.com>
Helped-by: Derrick Stolee <derrickstolee@github.com>
Helped-by: Victoria Dye <vdye@github.com>
Helped-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-09-23 09:41:27 -07:00
Jeff King 5a97b38109 remote: handle rename of remote without fetch refspec
We return an error when trying to rename a remote that has no fetch
refspec:

  $ git config --unset-all remote.origin.fetch
  $ git remote rename origin foo
  fatal: could not unset 'remote.foo.fetch'

To make things even more confusing, we actually _do_ complete the config
modification, via git_config_rename_section(). After that we try to
rewrite the fetch refspec (to say refs/remotes/foo instead of origin).
But our call to git_config_set_multivar() to remove the existing entries
fails, since there aren't any, and it calls die().

We could fix this by using the "gently" form of the config call, and
checking the error code. But there is an even simpler fix: if we know
that there are no refspecs to rewrite, then we can skip that part
entirely.

Reported-by: John A. Leuenhagen <john@zlima12.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-09-22 12:59:52 -07:00
Jeff King 3b910d6e29 clone: allow "--bare" with "-o"
We explicitly forbid the combination of "--bare" with "-o", but there
doesn't seem to be any good reason to do so. The original logic came as
part of e6489a1bdf (clone: do not accept more than one -o option.,
2006-01-22), but that commit does not give any reason.

Furthermore, the equivalent combination via config is allowed:

  git -c clone.defaultRemoteName=foo clone ...

and works as expected. It may be that this combination was considered
useless, because a bare clone does not set remote.origin.fetch (and
hence there is no refs/remotes/origin hierarchy). But it does set
remote.origin.url, and that name is visible to the user via "git fetch
origin", etc.

Let's allow the options to be used together, and switch the "forbid"
test in t5606 to check that we use the requested name. That test came
much later in 349cff76de (clone: add tests for --template and some
disallowed option pairs, 2020-09-29), and does not offer any logic
beyond "let's test what the code currently does".

Reported-by: John A. Leuenhagen <john@zlima12.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-09-22 12:57:03 -07:00
Jeff King 51b27747e5 parse_object_buffer(): respect save_commit_buffer
If the global variable "save_commit_buffer" is set to 0, then
parse_commit() will throw away the commit object data after parsing it,
rather than sticking it into a commit slab. This goes all the way back
to 60ab26de99 ([PATCH] Avoid wasting memory in git-rev-list,
2005-09-15).

But there's another code path which may similarly stash the buffer:
parse_object_buffer(). This is where we end up if we parse a commit via
parse_object(), and it's used directly in a few other code paths like
git-fsck.

The original goal of 60ab26de99 was avoiding extra memory usage for
rev-list. And there it's not all that important to catch parse_object().
We use that function only for looking at the tips of the traversal, and
the majority of the commits are parsed by following parent links, where
we use parse_commit() directly. So we were wasting some memory, but only
a small portion.

It's much easier to see the effect with fsck. Since we now turn off
save_commit_buffer by default there, we _should_ be able to drop the
freeing of the commit buffer in fsck_obj(). But if we do so (taking the
first hunk of this patch without the rest), then the peak heap of "git
fsck" in a clone of git.git goes from 136MB to 194MB. Teaching
parse_object_buffer() to respect save_commit_buffer brings that down to
134.5MB (it's hard to tell from massif's output, but I suspect the
savings comes from avoiding the overhead of the mostly-empty commit
slab).

Other programs should see a small improvement. Both "rev-list --all" and
"fsck --connectivity-only" improve by a few hundred kilobytes, as they'd
avoid loading the tip objects of their traversals.

Most importantly, no code should be hurt by doing this. Any program that
turns off save_commit_buffer is already making the assumption that any
commit it sees may need to have its object data loaded on demand, as it
doesn't know which ones were parsed by parse_commit() versus
parse_object(). Not to mention that anything parsed by the commit graph
may be in the same boat, even if save_commit_buffer was not disabled.

This should be the only spot that needs to be fixed. Grepping for
set_commit_buffer() shows that this and parse_commit() are the only
relevant calls.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-09-22 11:40:47 -07:00
Jeff King 069e445256 fsck: turn off save_commit_buffer
When parsing a commit, the default behavior is to stuff the original
buffer into a commit_slab (which takes ownership of it). But for a tool
like fsck, this isn't useful. While we may look at the buffer further as
part of fsck_commit(), we'll always do so through a separate pointer;
attaching the buffer to the slab doesn't help.

Worse, it means we have to remember to free the commit buffer in all
call paths. We do so in fsck_obj(), which covers a regular "git fsck".
But with "--connectivity-only", we forget to do so in both
traverse_one_object(), which covers reachable objects, and
mark_unreachable_referents(), which covers unreachable ones. As a
result, that mode ends up storing an uncompressed copy of every commit
on the heap at once.

We could teach the code paths for --connectivity-only to also free
commit buffers. But there's an even easier fix: we can just turn off the
save_commit_buffer flag, and then we won't attach them to the commits in
the first place.

This reduces the peak heap of running "git fsck --connectivity-only" in
a clone of linux.git from ~2GB to ~1GB. According to massif, the
remaining memory goes where you'd expect: the object structs themselves,
the obj_hash containing them, and the delta base cache.

Note that we'll leave the call to free commit buffers in fsck_obj() for
now; it's not quite redundant because of a related bug that we'll fix in
a subsequent commit.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-09-22 11:40:11 -07:00
Jeff King fbce4fa9ae fsck: free tree buffers after walking unreachable objects
After calling fsck_walk(), a tree object struct may be left in the
parsed state, with the full tree contents available via tree->buffer.
It's the responsibility of the caller to free these when it's done with
the object to avoid having many trees allocated at once.

In a regular "git fsck", we hit fsck_walk() only from fsck_obj(), which
does call free_tree_buffer(). Likewise for "--connectivity-only", we see
most objects via traverse_one_object(), which makes a similar call.

The exception is in mark_unreachable_referents(). When using both
"--connectivity-only" and "--dangling" (the latter of which is the
default), we walk all of the unreachable objects, and there we forget to
free. Most cases would not notice this, because they don't have a lot of
unreachable objects, but you can make a pathological case like this:

  git clone --bare /path/to/linux.git repo.git
  cd repo.git
  rm packed-refs ;# now everything is unreachable!
  git fsck --connectivity-only

That ends up with peak heap usage ~18GB, which is (not coincidentally)
close to the size of all uncompressed trees in the repository. After
this patch, the peak heap is only ~2GB.

A few things to note:

  - it might seem like fsck_walk(), if it is parsing the trees, should
    be responsible for freeing them. But the situation is quite tricky.
    In the non-connectivity mode, after we call fsck_walk() we then
    proceed with fsck_object() which actually does the type-specific
    sanity checks on the object contents. We do pass our own separate
    buffer to fsck_object(), but there's a catch: our earlier call to
    parse_object_buffer() may have attached that buffer to the object
    struct! So by freeing it, we leave the rest of the code with a
    dangling pointer.

    Likewise, the call to fsck_walk() in index-pack is subtle. It
    attaches a buffer to the tree object that must not be freed! And
    so rather than calling free_tree_buffer(), it actually detaches it
    by setting tree->buffer to NULL.

    These cases would _probably_ be fixable by having fsck_walk() free
    the tree buffer only when it was the one who allocated it via
    parse_tree(). But that would still leave the callers responsible for
    freeing other cases, so they wouldn't be simplified. While the
    current semantics for fsck_walk() make it easy to accidentally leak
    in new callers, at least they are simple to explain, and it's not a
    function that's likely to get a lot of new call-sites.

    And in any case, it's probably sensible to fix the leak first with
    this simple patch, and try any more complicated refactoring
    separately.

  - a careful reader may notice that fsck_obj() also frees commit
    buffers, but neither the call in traverse_one_object() nor the one
    touched in this patch does so. And indeed, this is another problem
    for --connectivity-only (and accounts for most of the 2GB heap after
    this patch), but it's one we'll fix in a separate commit.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-09-22 11:30:06 -07:00
Junio C Hamano 04cc66fe8c Merge branch 'sg/parse-options-subcommand'
Fix messages incorrectly marked for translation.

* sg/parse-options-subcommand:
  gc: don't translate literal commands
2022-09-21 15:27:03 -07:00
Junio C Hamano 86c108a8a2 Merge branch 'vd/scalar-generalize-diagnose'
Portability fix.

* vd/scalar-generalize-diagnose:
  builtin/diagnose.c: don't translate the two mode values
  diagnose.c: refactor to safely use 'd_type'
2022-09-21 15:27:01 -07:00
Alex Henrie 02cb8b9ee3 fsmonitor--daemon: don't translate literal commands
These commands have no placeholders to be translated.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-09-21 11:56:42 -07:00
Alex Henrie d956fa8082 builtin/diagnose.c: don't translate the two mode values
These strings are not translatable in the diagnose_options array in
diagnose.c. Don't translate them in builtin/diagnose.c either.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-09-21 11:53:35 -07:00
Alex Henrie 8b74492135 gc: don't translate literal commands
The command you type is still "git maintenance" even in other languages.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-09-21 10:43:10 -07:00
Junio C Hamano 42bf77c7d0 Merge branch 'vd/scalar-to-main'
Hoist the remainder of "scalar" out of contrib/ to the main part of
the codebase.

* vd/scalar-to-main:
  Documentation/technical: include Scalar technical doc
  t/perf: add 'GIT_PERF_USE_SCALAR' run option
  t/perf: add Scalar performance tests
  scalar-clone: add test coverage
  scalar: add to 'git help -a' command list
  scalar: implement the `help` subcommand
  git help: special-case `scalar`
  scalar: include in standard Git build & installation
  scalar: fix command documentation section header
2022-09-19 14:35:25 -07:00
Junio C Hamano 298a958224 Merge branch 'jk/list-objects-filter-cleanup'
A couple of bugfixes with code clean-up.

* jk/list-objects-filter-cleanup:
  list-objects-filter: convert filter_spec to a strbuf
  list-objects-filter: add and use initializers
  list-objects-filter: handle null default filter spec
  list-objects-filter: don't memset after releasing filter struct
2022-09-19 14:35:24 -07:00
Junio C Hamano f876b5a686 Merge branch 'zh/ls-files-format'
Typofix in the UI of a topic that has graduated to 'master'.

* zh/ls-files-format:
  ls-files: fix black space in error message
2022-09-19 14:35:24 -07:00
Junio C Hamano 339517b035 Merge branch 'sy/mv-out-of-cone'
"git mv A B" in a sparsely populated working tree can be asked to
move a path from a directory that is "in cone" to another directory
that is "out of cone".  Handling of such a case has been improved.

* sy/mv-out-of-cone:
  builtin/mv.c: fix possible segfault in add_slash()
  mv: check overwrite for in-to-out move
  advice.h: add advise_on_moving_dirty_path()
  mv: cleanup empty WORKING_DIRECTORY
  mv: from in-cone to out-of-cone
  mv: remove BOTH from enum update_mode
  mv: check if <destination> is a SKIP_WORKTREE_DIR
  mv: free the with_slash in check_dir_in_index()
  mv: rename check_dir_in_index() to empty_dir_has_sparse_contents()
  t7002: add tests for moving from in-cone to out-of-cone
2022-09-19 14:35:23 -07:00
Han-Wen Nienhuys 71e5473493 refs: unify parse_worktree_ref() and ref_type()
The logic to handle worktree refs (worktrees/NAME/REF and
main-worktree/REF) existed in two places:

* ref_type() in refs.c

* parse_worktree_ref() in worktree.c

Collapse this logic together in one function parse_worktree_ref():
this avoids having to cross-check the result of parse_worktree_ref()
and ref_type().

Introduce enum ref_worktree_type, which is slightly different from
enum ref_type. The latter is a misleading name (one would think that
'ref_type' would have the symref option).

Instead, enum ref_worktree_type only makes explicit how a refname
relates to a worktree. From this point of view, HEAD and
refs/bisect/abc are the same: they specify the current worktree
implicitly.

The files-backend must avoid packing refs/bisect/* and friends into
packed-refs, so expose is_per_worktree_ref() separately.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-09-19 11:11:11 -07:00
Junio C Hamano ca20a44bc5 Merge branch 'jk/proto-v2-ref-prefix-fix'
"git fetch" over protocol v2 sent an incorrect ref prefix request
to the server and made "git pull" with configured fetch refspec
that does not cover the remote branch to merge with fail, which has
been corrected.

* jk/proto-v2-ref-prefix-fix:
  fetch: add branch.*.merge to default ref-prefix extension
  fetch: stop checking for NULL transport->remote in do_fetch()
2022-09-15 16:09:47 -07:00
Junio C Hamano b563638d2c Merge branch 'ab/submodule-helper-leakfix'
Plugging leaks in submodule--helper.

* ab/submodule-helper-leakfix:
  submodule--helper: fix a configure_added_submodule() leak
  submodule--helper: free rest of "displaypath" in "struct update_data"
  submodule--helper: free some "displaypath" in "struct update_data"
  submodule--helper: fix a memory leak in print_status()
  submodule--helper: fix a leak in module_add()
  submodule--helper: fix obscure leak in module_add()
  submodule--helper: fix "reference" leak
  submodule--helper: fix a memory leak in get_default_remote_submodule()
  submodule--helper: fix a leak with repo_clear()
  submodule--helper: fix "sm_path" and other "module_cb_list" leaks
  submodule--helper: fix "errmsg_str" memory leak
  submodule--helper: add and use *_release() functions
  submodule--helper: don't leak {run,capture}_command() cp.dir argument
  submodule--helper: "struct pathspec" memory leak in module_update()
  submodule--helper: fix most "struct pathspec" memory leaks
  submodule--helper: fix trivial get_default_remote_submodule() leak
  submodule--helper: fix a leak in "clone_submodule"
2022-09-14 12:56:40 -07:00
Junio C Hamano dd407f1c7c Merge branch 'ab/unused-annotation'
Undoes 'jk/unused-annotation' topic and redoes it to work around
Coccinelle rules misfiring false positives in unrelated codepaths.

* ab/unused-annotation:
  git-compat-util.h: use "deprecated" for UNUSED variables
  git-compat-util.h: use "UNUSED", not "UNUSED(var)"
2022-09-14 12:56:39 -07:00
Junio C Hamano a6b42ec0c6 Merge branch 'jk/unused-annotation'
Annotate function parameters that are not used (but cannot be
removed for structural reasons), to prepare us to later compile
with -Wunused warning turned on.

* jk/unused-annotation:
  is_path_owned_by_current_uid(): mark "report" parameter as unused
  run-command: mark unused async callback parameters
  mark unused read_tree_recursive() callback parameters
  hashmap: mark unused callback parameters
  config: mark unused callback parameters
  streaming: mark unused virtual method parameters
  transport: mark bundle transport_options as unused
  refs: mark unused virtual method parameters
  refs: mark unused reflog callback parameters
  refs: mark unused each_ref_fn parameters
  git-compat-util: add UNUSED macro
2022-09-14 12:56:39 -07:00
Junio C Hamano 2c75b3255b Merge branch 'en/merge-unstash-only-on-clean-merge' into maint
The auto-stashed local changes created by "git merge --autostash"
was mixed into a conflicted state left in the working tree, which
has been corrected.

* en/merge-unstash-only-on-clean-merge:
  merge: only apply autostash when appropriate
2022-09-13 12:21:11 -07:00
Junio C Hamano de1fee2f1e Merge branch 'ow/rev-parse-parseopt-fix'
The parser in the script interface to parse-options in "git
rev-parse" has been updated to diagnose a bogus input correctly.

* ow/rev-parse-parseopt-fix:
  rev-parse --parseopt: detect missing opt-spec
2022-09-13 11:38:25 -07:00
Junio C Hamano 76ffa818c7 Merge branch 'sg/parse-options-subcommand'
The codepath for the OPT_SUBCOMMAND facility has been cleaned up.

* sg/parse-options-subcommand:
  notes, remote: show unknown subcommands between `'
  notes: simplify default operation mode arguments check
  test-parse-options.c: fix style of comparison with zero
  test-parse-options.c: don't use for loop initial declaration
  t0040-parse-options: remove leftover debugging
2022-09-13 11:38:24 -07:00
Junio C Hamano f322e9f51b Merge branch 'ab/submodule-helper-prep'
Code clean-up of "git submodule--helper".

* ab/submodule-helper-prep: (33 commits)
  submodule--helper: fix bad config API usage
  submodule--helper: libify even more "die" paths for module_update()
  submodule--helper: libify more "die" paths for module_update()
  submodule--helper: check repo{_submodule,}_init() return values
  submodule--helper: libify "must_die_on_failure" code paths (for die)
  submodule--helper update: don't override 'checkout' exit code
  submodule--helper: libify "must_die_on_failure" code paths
  submodule--helper: libify determine_submodule_update_strategy()
  submodule--helper: don't exit() on failure, return
  submodule--helper: use "code" in run_update_command()
  submodule API: don't handle SM_..{UNSPECIFIED,COMMAND} in to_string()
  submodule--helper: don't call submodule_strategy_to_string() in BUG()
  submodule--helper: add missing braces to "else" arm
  submodule--helper: return "ret", not "1" from update_submodule()
  submodule--helper: rename "int res" to "int ret"
  submodule--helper: don't redundantly check "else if (res)"
  submodule--helper: refactor "errmsg_str" to be a "struct strbuf"
  submodule--helper: add "const" to passed "struct update_data"
  submodule--helper: add "const" to copy of "update_data"
  submodule--helper: add "const" to passed "module_clone_data"
  ...
2022-09-13 11:38:23 -07:00
ZheNing Hu 746aae3dd1 ls-files: fix black space in error message
ce74de9(ls-files: introduce "--format" option) miss
a space between two words incorrectly, it leads to
wrong i10n messages. So fix it by adding a space at
the end of the error message.

Signed-off-by: ZheNing Hu <adlternative@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-09-12 09:25:40 -07:00
Jeff King 2a01bdedf8 list-objects-filter: add and use initializers
In 7e2619d8ff (list_objects_filter_options: plug leak of filter_spec
strings, 2022-09-08), we noted that the filter_spec string_list was
inconsistent in how it handled memory ownership of strings stored in the
list. The fix there was a bit of a band-aid to set the "strdup_strings"
variable right before adding anything.

That works OK, and it lets the users of the API continue to
zero-initialize the struct. But it makes the code a bit hard to follow
and accident-prone, as any other spots appending the filter_spec need to
think about whether to set the strdup_strings value, too (there's one
such spot in partial_clone_get_default_filter_spec(), which is probably
a possible memory leak).

So let's do that full cleanup now. We'll introduce a
LIST_OBJECTS_FILTER_INIT macro and matching function, and use them as
appropriate (though it is for the "_options" struct, this matches the
corresponding list_objects_filter_release() function).

This is harder than it seems! Many other structs, like
git_transport_data, embed the filter struct. So they need to initialize
it themselves even if the rest of the enclosing struct is OK with
zero-initialization. I found all of the relevant spots by grepping
manually for declarations of list_objects_filter_options. And then doing
so recursively for structs which embed it, and ones which embed those,
and so on.

I'm pretty sure I got everything, but there's no change that would alert
the compiler if any topics in flight added new declarations. To catch
this case, we now double-check in the parsing function that things were
initialized as expected and BUG() if appropriate.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-09-12 08:38:59 -07:00
Shaoxuan Yuan 7ead46810b builtin/mv.c: fix possible segfault in add_slash()
A possible segfault was introduced in c08830de41 (mv: check if
<destination> is a SKIP_WORKTREE_DIR, 2022-08-09).

When running t7001 with SANITIZE=address, problem appears when running:

	git mv path1/path2/ .
or
	git mv directory ../
or
	any <destination> that makes dest_path[0] an empty string.

The add_slash() call could segfault when path argument to it is an empty
string, because it makes an out-of-bounds read to decide if an extra
slash '/' needs to be appended to it.

As add_slash() is used to make sure that a valid pathname to a file in
the given directory can be made by appending a filename after the value
returned from it, if path is an empty string, we want to return it
as-is.  The path to a file "F" in the top-level of the working tree
(i.e. path=="") is formed by appending "F" after "" (i.e. path) without
any slash in between.

So, just like the case where a non-empty path already ends with a slash,
return an empty path as-is.

Reported-by: Jeff King <peff@peff.net>
Helped-by: Jeff King <peff@peff.net>
Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-09-09 15:49:53 -07:00
Junio C Hamano 0e2a4764ed Merge branch 'jc/format-patch-force-in-body-from'
"git format-patch --from=<ident>" can be told to add an in-body
"From:" line even for commits that are authored by the given
<ident> with "--force-in-body-from"option.

* jc/format-patch-force-in-body-from:
  format-patch: learn format.forceInBodyFrom configuration variable
  format-patch: allow forcing the use of in-body From: header
  pretty: separate out the logic to decide the use of in-body from
2022-09-09 12:02:25 -07:00
Junio C Hamano 428dce9f4d Merge branch 'js/range-diff-with-pathspec'
Allow passing a pathspec to "git range-diff".

* js/range-diff-with-pathspec:
  range-diff: optionally accept pathspecs
  range-diff: consistently validate the arguments
  range-diff: reorder argument handling
2022-09-09 12:02:25 -07:00
Øystein Walle f20b9c36d0 rev-parse --parseopt: detect missing opt-spec
After 2d893dff4c (rev-parse --parseopt: allow [*=?!] in argument hints,
2015-07-14) updated the parser, a line in parseopts's input can start
with one of the flag characters and be erroneously parsed as a opt-spec
where the short name of the option is the flag character itself and the
long name is after the end of the string. This makes Git want to
allocate SIZE_MAX bytes of memory at this line:

    o->long_name = xmemdupz(sb.buf + 2, s - sb.buf - 2);

Since s and sb.buf are equal the second argument is -2 (except unsigned)
and xmemdupz allocates len + 1 bytes, ie. -1 meaning SIZE_MAX.

Avoid this by checking whether a flag character was found in the zeroth
position.

Reported-by: Ingy dot Net <ingy@ingy.net>
Signed-off-by: Øystein Walle <oystwa@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-09-08 14:55:07 -07:00
Jeff King 49ca2fba39 fetch: add branch.*.merge to default ref-prefix extension
When running "git pull" with no arguments, we'll do a default "git
fetch" and then try to merge the branch specified by the branch.*.merge
config. There's code in get_ref_map() to treat that "merge" branch as
something we want to fetch, even if it is not otherwise covered by the
default refspec.

This works fine with the v0 protocol, as the server tells us about all
of the refs, and get_ref_map() is the ultimate decider of what we fetch.

But in the v2 protocol, we send the ref-prefix extension to the server,
asking it to limit the ref advertisement. And we only tell it about the
default refspec for the remote; we don't mention the branch.*.merge
config at all.

This usually doesn't matter, because the default refspec matches
"refs/heads/*", which covers all branches. But if you explicitly use a
narrow refspec, then "git pull" on some branches may fail. The server
doesn't advertise the branch, so we don't fetch it, and "git pull"
thinks that it went away upstream.

We can fix this by including any branch.*.merge entries for the current
branch in the list of ref-prefixes we pass to the server. This only
needs to happen when using the default configured refspec (since
command-line refspecs are already added, and take precedence in deciding
what we fetch). We don't otherwise need to replicate any of the "what to
fetch" logic in get_ref_map(). These ref-prefixes are an optimization,
so it's OK if we tell the server to advertise the branch.*.merge ref,
even if we're not going to pull it. We'll just choose not to fetch it.

The test here is based on one constructed by Johannes. I modified the
branch names to trigger the ref-prefix issue (and be more descriptive),
and to confirm that "git pull" actually updated the local ref, which
should be more robust than just checking stderr.

Reported-by: Lana Deere <lana.deere@gmail.com>
Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-09-08 13:10:37 -07:00
Jeff King 080bc4990f fetch: stop checking for NULL transport->remote in do_fetch()
This field will never be NULL; if it were, we'd segfault earlier in the
function when we unconditionally check transport->remote->fetch_tags.
Likewise, many other functions dereference it unconditionally.

This is a small simplification, but it will make things easier as we
extend this conditional in the next patch.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-09-08 13:10:32 -07:00
SZEDER Gábor dd834d75ca notes, remote: show unknown subcommands between `'
Update the "unknown subcommand" error message in 'git notes' and 'git
remote' to wrap the offending argument between `', to make it
consistent with the "unknown switch/option/subcommand" error messages
in parse-options.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-09-07 12:06:12 -07:00
SZEDER Gábor 1c7c25aef1 notes: simplify default operation mode arguments check
'git notes' has a default operation mode, but when invoked without a
subcommand it doesn't accept any arguments (although the 'list'
subcommand implementing the default operation mode does accept
arguments).  The condition checking this ended up a bit awkward, so
let's make it clearer.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-09-07 12:06:12 -07:00
Junio C Hamano 56785a3fad Merge branch 'bc/gc-crontab-fix'
FreeBSD portability fix for "git maintenance" that spawns "crontab"
to schedule tasks.

* bc/gc-crontab-fix:
  gc: use temporary file for editing crontab
2022-09-05 18:33:41 -07:00
Junio C Hamano 3fe0121479 Merge branch 'ac/bitmap-lookup-table'
The pack bitmap file gained a bitmap-lookup table to speed up
locating the necessary bitmap for a given commit.

* ac/bitmap-lookup-table:
  pack-bitmap-write: drop unused pack_idx_entry parameters
  bitmap-lookup-table: add performance tests for lookup table
  pack-bitmap: prepare to read lookup table extension
  pack-bitmap-write: learn pack.writeBitmapLookupTable and add tests
  pack-bitmap-write.c: write lookup table extension
  bitmap: move `get commit positions` code to `bitmap_writer_finish`
  Documentation/technical: describe bitmap lookup table extension
2022-09-05 18:33:39 -07:00
Johannes Schindelin dd9603e228 git help: special-case scalar
With this commit, `git help scalar` will open the appropriate manual
or HTML page (instead of looking for `gitscalar`).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Victoria Dye <vdye@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-09-02 10:02:56 -07:00