Commit graph

11349 commits

Author SHA1 Message Date
Linus Arver a6c72e7046 trailer --only-input: prefer "configuration variables" over "rules"
Use the phrase "configuration variables" instead of "rules" because

(1) we already say "configuration variables" in multiple
    places in the docs (where the word "rules" is only used for describing
    "--only-input" behavior and for an unrelated case of mentioning how
    the trailers do not follow "rules for RFC 822 headers"), and

(2) this phrase is more specific than just "rules".

Signed-off-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-07 23:04:44 -07:00
Linus Arver 8c7d4acb07 trailer --parse help: expose aliased options
The existing description "set parsing options" is vague, because
arguably _all_ of the options for interpret-trailers have to do with
parsing to some degree.

Explain what this flag does to match what is in the docs, namely how
it is an alias for "--only-trailers --only-input --unfold".

Signed-off-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-07 23:04:44 -07:00
Linus Arver b674f25b81 trailer --no-divider help: describe usual "---" meaning
It's unclear what treating something "specially" means.

Signed-off-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-07 23:04:44 -07:00
Linus Arver 467bb1b97a trailer: trailer location is a place, not an action
Fix the help text to say "placement" instead of "action" because the
values are placements, not actions.

While we're at it, tweak the documentation to say "placements" instead
of "values", similar to how the existing language for "--if-exists" uses
the word "action" to describe both the syntax (with the phrase
"--if-exists <action>") and the possible values (with the phrase
"possible actions").

Signed-off-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-07 23:04:44 -07:00
Junio C Hamano f9712d75e6 Merge branch 'jc/parse-options-short-help'
Command line parser fix, and a small parse-options API update.

* jc/parse-options-short-help:
  short help: allow a gap smaller than USAGE_GAP
  remote: simplify "remote add --tags" help text
  short help: allow multi-line opthelp
2023-08-04 10:52:31 -07:00
Junio C Hamano 4d06001846 Merge branch 'ja/worktree-orphan-fix'
Fix tests with unportable regex patterns.

* ja/worktree-orphan-fix:
  t2400: rewrite regex to avoid unintentional PCRE
  builtin/worktree.c: convert tab in advice to space
  t2400: drop no-op `--sq` from rev-parse call
2023-08-04 10:52:30 -07:00
Junio C Hamano fea92e4cac Merge branch 'jc/tree-walk-drop-base-offset'
Code simplification.

* jc/tree-walk-drop-base-offset:
  tree-walk: drop unused base_offset from do_match()
  tree-walk: lose base_offset that is never used in tree_entry_interesting
2023-08-02 09:37:23 -07:00
Junio C Hamano ddcb8fd8b9 Merge branch 'rs/pack-objects-parseopt-fix'
Command line parser fix.

* rs/pack-objects-parseopt-fix:
  pack-objects: fix --no-quiet
  pack-objects: fix --no-keep-true-parents
2023-07-28 09:45:22 -07:00
Junio C Hamano 3085f949bf Merge branch 'rs/describe-parseopt-fix'
Command line parser fix.

* rs/describe-parseopt-fix:
  describe: fix --no-exact-match
2023-07-28 09:45:21 -07:00
Junio C Hamano e672bc4f76 Merge branch 'jc/parse-options-reset'
Command line parser fix.

* jc/parse-options-reset:
  reset: reject --no-(mixed|soft|hard|merge|keep) option
2023-07-27 15:26:37 -07:00
Junio C Hamano d6966f6fff Merge branch 'jc/parse-options-show-branch'
Command line parser fixes.

* jc/parse-options-show-branch:
  show-branch: reject --[no-](topo|date)-order
  show-branch: --no-sparse should give dense output
2023-07-27 15:26:37 -07:00
Junio C Hamano 9562f19026 Merge branch 'jc/transport-parseopt-fix'
Command line parser fixes.

* jc/transport-parseopt-fix:
  fetch: reject --no-ipv[46]
  parse-options: introduce OPT_IPVERSION()
2023-07-27 15:26:37 -07:00
Jacob Abel 7e42d4bf15 builtin/worktree.c: convert tab in advice to space
Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-26 14:49:02 -07:00
Junio C Hamano 9a5e3b5f47 Merge branch 'jc/branch-parseopt-fix'
Command line parser fixes.

* jc/branch-parseopt-fix:
  branch: reject "--no-all" and "--no-remotes" early
2023-07-26 14:13:15 -07:00
Junio C Hamano 914a353a12 Merge branch 'jc/am-parseopt-fix'
Code simplification.

* jc/am-parseopt-fix:
  am: simplify parsing of "--[no-]keep-cr"
2023-07-26 14:13:15 -07:00
Junio C Hamano 8ae477e2b4 Merge branch 'rs/ls-tree-no-full-name-fix'
Command line parser fix.

* rs/ls-tree-no-full-name-fix:
  ls-tree: fix --no-full-name
2023-07-26 14:13:15 -07:00
Junio C Hamano c5fcd34e1b Merge branch 'jk/unused-parameter'
Mark-up unused parameters in the code so that we can eventually
enable -Wunused-parameter by default.

* jk/unused-parameter:
  t/helper: mark unused callback void data parameters
  tag: mark unused parameters in each_tag_name_fn callbacks
  rev-parse: mark unused parameter in for_each_abbrev callback
  replace: mark unused parameter in each_mergetag_fn callback
  replace: mark unused parameter in ref callback
  merge-tree: mark unused parameter in traverse callback
  fsck: mark unused parameters in various fsck callbacks
  revisions: drop unused "opt" parameter in "tweak" callbacks
  count-objects: mark unused parameter in alternates callback
  am: mark unused keep_cr parameters
  http-push: mark unused parameter in xml callback
  http: mark unused parameters in curl callbacks
  do_for_each_ref_helper(): mark unused repository parameter
  test-ref-store: drop unimplemented reflog-expire command
2023-07-25 12:05:24 -07:00
Junio C Hamano 88d08c342a Merge branch 'ah/advise-force-pushing'
Help newbies by suggesting that there are cases where force-pushing
is a valid and sensible thing to update a branch at a remote
repository, rather than reconciling with merge/rebase.

* ah/advise-force-pushing:
  push: don't imply that integration is always required before pushing
  remote: don't imply that integration is always required before pushing
  wt-status: don't show divergence advice when committing
2023-07-25 12:05:23 -07:00
Junio C Hamano 39fe402d67 Merge branch 'tb/refs-exclusion-and-packed-refs'
Enumerating refs in the packed-refs file, while excluding refs that
match certain patterns, has been optimized.

* tb/refs-exclusion-and-packed-refs:
  ls-refs.c: avoid enumerating hidden refs where possible
  upload-pack.c: avoid enumerating hidden refs where possible
  builtin/receive-pack.c: avoid enumerating hidden references
  refs.h: implement `hidden_refs_to_excludes()`
  refs.h: let `for_each_namespaced_ref()` take excluded patterns
  revision.h: store hidden refs in a `strvec`
  refs/packed-backend.c: add trace2 counters for jump list
  refs/packed-backend.c: implement jump lists to avoid excluded pattern(s)
  refs/packed-backend.c: refactor `find_reference_location()`
  refs: plumb `exclude_patterns` argument throughout
  builtin/for-each-ref.c: add `--exclude` option
  ref-filter.c: parameterize match functions over patterns
  ref-filter: add `ref_filter_clear()`
  ref-filter: clear reachable list pointers after freeing
  ref-filter.h: provide `REF_FILTER_INIT`
  refs.c: rename `ref_filter`
2023-07-21 13:47:26 -07:00
René Scharfe 36f76d2a25 pack-objects: fix --no-quiet
Since 99fb6e04cb (pack-objects: convert to use parse_options(),
2012-02-01) git pack-objects has accepted the option --no-quiet, but it
does the same as --quiet.  That's because it's defined using OPT_SET_INT
with a value of 0, which sets 0 when negated, too.

Make --no-quiet equivalent to --progress and ignore it if --all-progress
was given.

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-21 10:04:04 -07:00
René Scharfe 3a5f308741 pack-objects: fix --no-keep-true-parents
Since 99fb6e04cb (pack-objects: convert to use parse_options(),
2012-02-01) git pack-objects has accepted --no-keep-true-parents, but
this option does the same as --keep-true-parents.  That's because it's
defined using OPT_SET_INT with a value of 0, which sets 0 when negated
as well.

Turn --no-keep-true-parents into the opposite of --keep-true-parents by
using OPT_BOOL and storing the option's status directly in a variable
named "grafts_keep_true_parents" instead of in negative form in
"grafts_replace_parents".

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-21 10:02:59 -07:00
René Scharfe c95ae3ff9c describe: fix --no-exact-match
Since 2c33f75754 (Teach git-describe --exact-match to avoid expensive
tag searches, 2008-02-24) git describe accepts --no-exact-match, but it
does the same as --exact-match, an alias for --candidates=0.  That's
because it's defined using OPT_SET_INT with a value of 0, which sets 0
when negated as well.

Let --no-exact-match set the number of candidates to the default value
instead.  Users that need a more specific lack of exactitude can specify
their preferred value using --candidates, as before.

The "--no-exact-match" option was not covered in the tests, so let's
add a few.  Also add a case where --exact-match option is used on a
commit that cannot be described without distance from tags and make
sure the command fails.

Signed-off-by: René Scharfe <l.s.r@web.de>
[jc: added trivial tests]
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-21 09:57:15 -07:00
Junio C Hamano 3821eb6c3d reset: reject --no-(mixed|soft|hard|merge|keep) option
"git reset --no-mixed" behaved exactly like "git reset --mixed",
which was nonsense.

If there were only two kinds, e.g. "mixed" vs "separate", it might
have made sense to make "git reset --no-mixed" behave identically to
"git reset --separate" and vice-versa, but because we have many
types of reset, let's just forbid "--no-mixed" and negated form of
other types.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-19 22:02:53 -07:00
Junio C Hamano 68cbb20e73 show-branch: reject --[no-](topo|date)-order
"git show-branch --no-topo-order" behaved exactly the same way as
"git show-branch --topo-order" did, which was nonsense.  This was
because we choose between topo- and date- by setting a variable to
either REV_SORT_IN_GRAPH_ORDER or REV_SORT_BY_COMMIT_DATE with
OPT_SET_INT() and REV_SORT_IN_GRAPH_ORDER happens to be 0.  The
OPT_SET_INT() macro assigns 0 to the target variable in respose to
the negated form of its option.

"--no-date-order" by luck behaves identically to "--topo-order"
exactly for the same reason, and it sort-of makes sense right now,
but the "sort-of makes sense" will quickly break down once we add a
third way to sort.  Not-A may be B when there are only two choices
between A and B, but once your choices become among A, B, and C,
not-A does not mean B.

Just mark these two ordering options to reject negation, and add a
test, which was missing.  "git show-branch --no-reflog" is also
unnegatable, so throw in a test for that while we are at it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-19 22:00:39 -07:00
Junio C Hamano d86a8f386d remote: simplify "remote add --tags" help text
The help text for the --tags option was split into two option[]
entries, which was a hacky way to give two lines of help text (the
second entry did not have either short or long help, and there was
no way to invoke its entry---it was there only for the help text).

As we now support multi-line text in the option help, let's make
the second line of the help a proper second line and remove the
hacky second entry.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-19 16:39:02 -07:00
Junio C Hamano 83bb8e5a06 show-branch: --no-sparse should give dense output
"git show-branch --no-sparse" behaved exactly the same way as "git
show-branch --sparse", which did not make any sense.  This was
because it used a variable "dense" initialized to 1 by default to
give "non sparse" behaviour, and OPT_SET_INT() to set the varilable
to 0 in response to the "--sparse" option.  Unfortunately,
OPT_SET_INT() sets 0 to the given variable when the option is
negated.

Flip the polarity of the variable "dense" by renaming it to "sparse"
and initializing it to 0, and have OPT_SET_INT() set the variable to
1 when "--sparse" is given.  This way, "--no-sparse" would set 0 to
the variable and would give us the "dense" behaviour.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-19 09:16:37 -07:00
Junio C Hamano ae2c912c04 parse-options: introduce OPT_IPVERSION()
The command line option parsing for "git clone", "git fetch", and
"git push" have duplicated implementations of parsing "--ipv4" and
"--ipv6" options, by having two OPT_SET_INT() for "ipv4" and "ipv6".

Introduce a new OPT_IPVERSION() macro and use it in these three
commands.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-18 14:35:54 -07:00
Junio C Hamano e12cb98e1e branch: reject "--no-all" and "--no-remotes" early
As the command line parser for "git branch --all" forgets to use
PARSE_OPT_NONEG, it accepted "git branch --no-all", and then passed
a nonsense value to the underlying machinery, leading to a fatal
error "filter_refs: invalid type".  The "--remotes" option had
exactly the same issue.

Catch the unsupported options early in the option parser.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-18 12:19:53 -07:00
Junio C Hamano 947ebd62a0 am: simplify parsing of "--[no-]keep-cr"
Command line options "--keep-cr" and its negation trigger
OPT_SET_INT_F(PARSE_OPT_NONEG) to set a variable to 1 and 0
respectively.  Using OPT_SET_INT() to implement the positive variant
that sets the variable to 1 without specifying PARSE_OPT_NONEG gives
us the negative variant to set it to 0 for free.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-18 12:19:31 -07:00
René Scharfe 991c552916 ls-tree: fix --no-full-name
Since 61fdbcf98b (ls-tree: migrate to parse-options, 2009-11-13) git
ls-tree has accepted the option --no-full-name, but it does the same
as --full-name, contrary to convention.  That's because it's defined
using OPT_SET_INT with a value of 0, where the negative variant sets
0 as well.

Turn --no-full-name into the opposite of --full-name by using OPT_BOOL
instead and storing the option's status directly in a variable named
"full_name" instead of in negated form in "chomp_prefix".

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-18 09:38:24 -07:00
Junio C Hamano c6a5e1a22e Merge branch 'tb/repack-cleanup'
The recent change to "git repack" made it react less nicely when a
leftover .idx file that no longer has the corresponding .pack file
in the repository, which has been corrected.

* tb/repack-cleanup:
  builtin/repack.c: avoid dir traversal in `collect_pack_filenames()`
  builtin/repack.c: only repack `.pack`s that exist
2023-07-18 07:28:53 -07:00
Junio C Hamano 6016ee0a71 Merge branch 'tb/fsck-no-progress'
"git fsck --no-progress" still spewed noise from the commit-graph
subsystem, which has been corrected.

* tb/fsck-no-progress:
  commit-graph.c: avoid duplicated progress output during `verify`
  commit-graph.c: pass progress to `verify_one_commit_graph()`
  commit-graph.c: iteratively verify commit-graph chains
  commit-graph.c: extract `verify_one_commit_graph()`
  fsck: suppress MIDX output with `--no-progress`
  fsck: suppress commit-graph output with `--no-progress`
2023-07-18 07:28:53 -07:00
Junio C Hamano ce481ac8b3 Merge branch 'cw/compat-util-header-cleanup'
Further shuffling of declarations across header files to streamline
file dependencies.

* cw/compat-util-header-cleanup:
  git-compat-util: move alloc macros to git-compat-util.h
  treewide: remove unnecessary includes for wrapper.h
  kwset: move translation table from ctype
  sane-ctype.h: create header for sane-ctype macros
  git-compat-util: move wrapper.c funcs to its header
  git-compat-util: move strbuf.c funcs to its header
2023-07-17 11:30:42 -07:00
Junio C Hamano 0e074fb4e5 Merge branch 'rs/ls-tree-prefix-simplify'
Code simplification.

* rs/ls-tree-prefix-simplify:
  ls-tree: simplify prefix handling
2023-07-17 11:30:42 -07:00
Jeff King cc2f810172 tag: mark unused parameters in each_tag_name_fn callbacks
We iterate over the set of input tag names using callbacks. But not all
operations need the same inputs, so some parameters go unused (but of
course not the same ones for each operation). Mark the unused ones to
avoid -Wunused-parameter warnings.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-13 17:24:00 -07:00
Jeff King 1e6459efca rev-parse: mark unused parameter in for_each_abbrev callback
We don't need to use the "data" parameter in this instance. Let's mark
it to avoid -Wunused-parameter warnings.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-13 17:24:00 -07:00
Jeff King 4c7b06f208 replace: mark unused parameter in each_mergetag_fn callback
We don't look at the "commit" parameter to our callback, as our
"mergetag_data" pointer contains the original name "ref", which we use
instead. But we can't get rid of it, since other for_each_mergetag
callbacks do use it. Let's mark the parameter to avoid
-Wunused-parameter warnings.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-13 17:24:00 -07:00
Jeff King 80d4e5f3a5 replace: mark unused parameter in ref callback
We don't look at the "flags" parameter, which is natural for something
that is just printing the contents of the replace refs. But let's mark
it to appease -Wunused-parameter.

This probably should have been part of 63e14ee2d6 (refs: mark unused
each_ref_fn parameters, 2022-08-19), but I missed it as this one is a
repo_each_ref_fn, which takes an extra repository argument.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-13 17:24:00 -07:00
Jeff King ee550abcce merge-tree: mark unused parameter in traverse callback
Our threeway_callback() does not bother to look at its "n" parameter. It
is static in this file and used only by trivial_merge_trees(), which
always passes 3 trees (hence the name "threeway"). It also does not look
at "dirmask". This is OK, as it handles directories specifically by
looking at the mode bits.

Other traverse_info callbacks need these, so we can't get drop them from
the interface. But let's annotate these ones to avoid complaints from
-Wunused-parameter.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-13 17:24:00 -07:00
Jeff King 0b4e9013f1 fsck: mark unused parameters in various fsck callbacks
There are a few callback functions which are used with the fsck code,
but it's natural that not all callbacks need all parameters. For
reporting, even something as obvious as "the oid of the object which had
a problem" is not always used, as some callers are only checking a
single object in the first place. And for both reporting and walking,
things like void data pointers and the fsck_options aren't always
necessary.

But since each such parameter is used by _some_ callback, we have to
keep them in the interface. Mark the unused ones in specific callbacks
to avoid triggering -Wunused-parameter.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-13 17:24:00 -07:00
Jeff King cc88afad62 revisions: drop unused "opt" parameter in "tweak" callbacks
The setup_revision_opt struct has a "tweak" function pointer, which can
be used to adjust parameters after setup_revisions() parses arguments,
but before it finalizes setup. In addition to the rev_info struct, the
callback receives a pointer to the setup_revision_opt, as well.

But none of the existing callbacks looks at the extra "opt" parameter,
leading to -Wunused-parameter warnings.

We could mark it as UNUSED, but instead let's remove it entirely. It's
conceivable that it could be useful for a callback to have access to the
"opt" struct. But in the 13 years that this mechanism has existed,
nobody has used it. So let's just drop it in the name of simplifying.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-13 17:24:00 -07:00
Jeff King 506d35f13d count-objects: mark unused parameter in alternates callback
Callbacks to for_each_altodb() get a void data pointer, but we don't
need it here. Mark it as unused to silence -Wunused-parameter.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-13 17:24:00 -07:00
Jeff King a8a8e75e9e am: mark unused keep_cr parameters
When parsing the input, we have a "keep_cr" parameter to tell us how to
handle line endings. But this doesn't apply to stgit or hg patches
(which are not mailbox formats where we have to worry about that), so we
ignore the parameter entirely in those functions.

Let's mark these as unused so that -Wunused-parameter does not complain
about them.

Note that we could just drop these parameters entirely. They are
necessary to conform to the mail_conv_fn interface used by
split_mail_conv(), but these two callbacks are the only ones used with
that function. The other formats (which _do_ care about keep_cr) use
split_mail_mbox(). But it's conceivable that we'd eventually add another
format that does care about this option, so let's leave it as part of
the generic interface.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-13 17:23:59 -07:00
Alex Henrie c577d65158 push: don't imply that integration is always required before pushing
In a narrow but common case, the user is the only author of a branch and
doesn't mind overwriting the corresponding branch on the remote. This
workflow is especially common on GitHub, GitLab, and Gerrit, which keep
a permanent record of every version of a branch that is pushed while a
pull request is open for that branch. On those platforms, force-pushing
is encouraged and is analogous to emailing a new version of a patchset.

When giving advice about divergent branches, tell the user about
`git pull`, but don't unconditionally instruct the user to do it. A less
prescriptive message will help prevent users from thinking that they are
required to create an integrated history instead of simply replacing the
previous history. Also, don't put `git pull` in an awkward
parenthetical, because `git pull` can always be used to reconcile
branches and is the normal way to do so.

Due to the difficulty of knowing which command for force-pushing is best
suited to the user's situation, no specific advice is given about
force-pushing. Instead, the user is directed to the Git documentation to
read about possible ways forward that do not involve integration.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-13 09:14:58 -07:00
Alex Henrie b6f3da5132 wt-status: don't show divergence advice when committing
When the user is in the middle of making a commit, they are not yet at
the point where they are ready to think about integrating their local
branch with the corresponding remote branch or force-pushing over the
remote branch. Don't include advice on how to deal with divergent
branches in the commit template, to avoid giving the impression that the
divergence needs to be dealt with immediately. Similar advice will be
printed when it is most relevant, that is, if the user does try to push
without first reconciling the two branches.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-13 09:14:58 -07:00
Taylor Blau def390d593 builtin/repack.c: avoid dir traversal in collect_pack_filenames()
When repacking, the function `collect_pack_filenames()` is responsible
for collecting the set of existing packs in the repository, and
partitioning them into "kept" (if the pack has a ".keep" file or was
given via `--keep-pack`) and "nonkept" (otherwise) lists.

This function comes from the original C port of git-repack.sh from back
in a1bbc6c017 (repack: rewrite the shell script in C, 2013-09-15),
where it first appears as `get_non_kept_pack_filenames()`. At the time,
the implementation was a fairly direct translation from the relevant
portion of git-repack.sh, which looped over the results of

    find "$PACKDIR" -type f -name '*.pack'

either ignoring the pack as kept, or adding it to the list of existing
packs.

So the choice to directly translate this function in terms of
`readdir()` in a1bbc6c017 made sense. At the time, it was possible to
refine the C version in terms of packed_git structs, but was never done.

However, manually enumerating a repository's packs via `readdir()` is
confusing and error-prone. It leads to frustrating inconsistencies
between which packs Git considers to be part of a repository (i.e.,
could be found in the list of packs from `get_all_packs()`), and which
packs `collect_pack_filenames()` considers to meet the same criteria.

This bit us in 73320e49ad (builtin/repack.c: only collect fully-formed
packs, 2023-06-07), and again in the previous commit.

Prevent these issues from biting us in the future by implementing the
`collect_pack_filenames()` function by looping over an array of pointers
to `packed_git` structs, ensuring that we use the same criteria to
determine the set of available packs.

One gotcha here is that we have to ignore non-local packs, since the
original version of `collect_pack_filenames()` only looks at the local
pack directory to collect existing packs.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-11 13:07:51 -07:00
Derrick Stolee 0af067276e builtin/repack.c: only repack .packs that exist
In 73320e49ad (builtin/repack.c: only collect fully-formed packs,
2023-06-07), we switched the check for which packs to collect by
starting at the .idx files and looking for matching .pack files. This
avoids trying to repack pack-files that have not had their pack-indexes
installed yet.

However, it does cause maintenance to halt if we find the (problematic,
but not insurmountable) case of a .idx file without a corresponding
.pack file. In an environment where packfile maintenance is a critical
function, such a hard stop is costly and requires human intervention to
resolve (by deleting the .idx file).

This was not the case before. We successfully repacked through this
scenario until the recent change to scan for .idx files.

Further, if we are actually in a case where objects are missing, we
detect this at a different point during the reachability walk.

In other cases, Git prepares its list of packfiles by scanning .idx
files and then only adds it to the packfile list if the corresponding
.pack file exists. It even does so without a warning! (See
add_packed_git() in packfile.c for details.)

This case is much less likely to occur than the failures seen before
73320e49ad. Packfiles are "installed" by writing the .pack file before
the .idx and that process can be interrupted. Packfiles _should_ be
deleted by deleting the .idx first, followed by the .pack file, but
unlink_pack_path() does not do this: it deletes the .pack _first_,
allowing a window where this process could be interrupted. We leave the
consideration of changing this order as a separate concern. Knowing that
this condition is possible from interrupted Git processes and not other
tools lends some weight that Git should be more flexible around this
scenario.

Add a check to see if the .pack file exists before adding it to the list
for repacking. This will stop a number of maintenance failures seen in
production but fixed by deleting the .idx files.

This brings us closer to the case before 73320e49ad in that 'git
repack' will not fail when there is an orphaned .idx file, at least, not
due to the way we scan for packfiles. In the case that the .pack file
was erroneously deleted without copies of its objects in other installed
packfiles, then 'git repack' will fail due to the reachable object walk.

This does resolve the case where automated repacks will no longer be
halted on this case. The tests in t7700 show both these successful
scenarios and the case of failing if the .pack was truly required.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-11 13:07:50 -07:00
Taylor Blau cc2a1f98ac builtin/receive-pack.c: avoid enumerating hidden references
Now that `refs_for_each_fullref_in()` has the ability to avoid
enumerating references matching certain pattern(s), use that to avoid
visiting hidden refs when constructing the ref advertisement via
receive-pack.

Note that since this exclusion is best-effort, we still need
`show_ref_cb()` to check whether or not each reference is hidden or not
before including it in the advertisement.

As was the case when applying this same optimization to `upload-pack`,
`receive-pack`'s reference advertisement phase can proceed much quicker
by avoiding enumerating references that will not be part of the
advertisement.

(Below, we're still using linux.git with one hidden refs/pull/N ref per
commit):

    $ hyperfine -L v ,.compile 'git{v} -c transfer.hideRefs=refs/pull receive-pack --advertise-refs .git'
    Benchmark 1: git -c transfer.hideRefs=refs/pull receive-pack --advertise-refs .git
      Time (mean ± σ):      89.1 ms ±   1.7 ms    [User: 82.0 ms, System: 7.0 ms]
      Range (min … max):    87.7 ms …  95.5 ms    31 runs

    Benchmark 2: git.compile -c transfer.hideRefs=refs/pull receive-pack --advertise-refs .git
      Time (mean ± σ):       4.5 ms ±   0.2 ms    [User: 0.5 ms, System: 3.9 ms]
      Range (min … max):     4.1 ms …   5.6 ms    508 runs

    Summary
      'git.compile -c transfer.hideRefs=refs/pull receive-pack --advertise-refs .git' ran
       20.00 ± 1.05 times faster than 'git -c transfer.hideRefs=refs/pull receive-pack --advertise-refs .git'

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-10 14:48:56 -07:00
Taylor Blau c45841fff8 revision.h: store hidden refs in a strvec
In subsequent commits, it will be convenient to have a 'const char **'
of hidden refs (matching `transfer.hiderefs`, `uploadpack.hideRefs`,
etc.), instead of a `string_list`.

Convert spots throughout the tree that store the list of hidden refs
from a `string_list` to a `strvec`.

Note that in `parse_hide_refs_config()` there is an ugly const-cast used
to avoid an extra copy of each value before trimming any trailing slash
characters. This could instead be written as:

    ref = xstrdup(value);
    len = strlen(ref);
    while (len && ref[len - 1] == '/')
            ref[--len] = '\0';
    strvec_push(hide_refs, ref);
    free(ref);

but the double-copy (once when calling `xstrdup()`, and another via
`strvec_push()`) is wasteful.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-10 14:48:56 -07:00
Taylor Blau 8255dd8a3d builtin/for-each-ref.c: add --exclude option
When using `for-each-ref`, it is sometimes convenient for the caller to
be able to exclude certain parts of the references.

For example, if there are many `refs/__hidden__/*` references, the
caller may want to emit all references *except* the hidden ones.
Currently, the only way to do this is to post-process the output, like:

    $ git for-each-ref --format='%(refname)' | grep -v '^refs/hidden/'

Which is do-able, but requires processing a potentially large quantity
of references.

Teach `git for-each-ref` a new `--exclude=<pattern>` option, which
excludes references from the results if they match one or more excluded
patterns.

This patch provides a naive implementation where the `ref_filter` still
sees all references (including ones that it will discard) and is left to
check whether each reference matches any excluded pattern(s) before
emitting them.

By culling out references we know the caller doesn't care about, we can
avoid allocating memory for their storage, as well as spending time
sorting the output (among other things). Even the naive implementation
provides a significant speed-up on a modified copy of linux.git (that
has a hidden ref pointing at each commit):

    $ hyperfine \
      'git.compile for-each-ref --format="%(objectname) %(refname)" | grep -vE "[0-9a-f]{40} refs/pull/"' \
      'git.compile for-each-ref --format="%(objectname) %(refname)" --exclude refs/pull/'
    Benchmark 1: git.compile for-each-ref --format="%(objectname) %(refname)" | grep -vE "[0-9a-f]{40} refs/pull/"
      Time (mean ± σ):     820.1 ms ±   2.0 ms    [User: 703.7 ms, System: 152.0 ms]
      Range (min … max):   817.7 ms … 823.3 ms    10 runs

    Benchmark 2: git.compile for-each-ref --format="%(objectname) %(refname)" --exclude refs/pull/
      Time (mean ± σ):     106.6 ms ±   1.1 ms    [User: 99.4 ms, System: 7.1 ms]
      Range (min … max):   104.7 ms … 109.1 ms    27 runs

    Summary
      'git.compile for-each-ref --format="%(objectname) %(refname)" --exclude refs/pull/' ran
        7.69 ± 0.08 times faster than 'git.compile for-each-ref --format="%(objectname) %(refname)" | grep -vE "[0-9a-f]{40} refs/pull/"'

Subsequent patches will improve on this by avoiding visiting excluded
sections of the `packed-refs` file in certain cases.

Co-authored-by: Jeff King <peff@peff.net>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-10 14:48:55 -07:00