Commit Graph

72589 Commits (30ff05094c145397d88ead89c3937d1a058ed98a)

Author SHA1 Message Date
Kristoffer Haugsbakk 8fbd903e58 branch: advise about ref syntax rules
git-branch(1) will error out if you give it a bad ref name. But the user
might not understand why or what part of the name is illegal.

The user might know that there are some limitations based on the *loose
ref* format (filenames), but there are also further rules for
easier integration with shell-based tools, pathname expansion, and
playing well with reference name expressions.

The man page for git-check-ref-format(1) contains these rules. Let’s
advise about it since that is not a command that you just happen
upon. Also make this advise configurable since you might not want to be
reminded every time you make a little typo.

Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-05 13:04:26 -08:00
Kristoffer Haugsbakk 15cb03728f advice: use double quotes for regular quoting
Use double quotes like we use for “die” in this document.

Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-05 13:04:26 -08:00
Kristoffer Haugsbakk 3ccc4782ce advice: use backticks for verbatim
Use backticks for inline-verbatim rather than single quotes. Also quote
the unquoted ref globs.

Also replace “the add command” with “`git add`”.

Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-05 13:04:26 -08:00
Kristoffer Haugsbakk 95c987e6fa advice: make all entries stylistically consistent
In general, rewrite entries to the following form:

1. Clause or sentence describing when the advice is shown
2. Optional “to <verb>” clause which says what the advice is
   about (e.g. for resetNoRefresh: tell the user that they can use
   `--no-refresh`)

Concretely:

1. Use “shown” instead of “advice shown”
   • “advice” is implied and a bit repetitive
2. Use “when” instead of “if”
3. Lead with “Shown when” and end the entry with the effect it has,
   where applicable
4. Use “the user” instead of “a user” or “you”
5. implicitIdentity: rewrite description in order to lead with *when*
   the advice is shown (see point (3))
6. Prefer the present tense (with the exception of pushNonFFMatching)
7. waitingForEditor: give example of relevance in this new context
8. pushUpdateRejected: exception to the above principles

Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-05 13:04:25 -08:00
Kristoffer Haugsbakk 8c5001c68e t3200: improve test style
Some tests use a preliminary heredoc for `expect` or have setup and
teardown commands before and after, respectively. It is however
preferred to keep all the logic in the test itself. Let’s move these
into the tests.

Also:

• Remove a now-irrelevant comment about test placement and switch back
  to `main` post-test
• Prefer indented literal heredocs (`-\EOF`) except for a block which
  says that this is intentional

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-05 13:04:25 -08:00
Kristoffer Haugsbakk fb7c556f58 config: document `core.commentChar` as ASCII-only
d3b3419f8f (config: tell the user that we expect an ASCII character,
2023-03-27) updated an error message to make clear that this option
specifically wants an ASCII character but neglected to consider the
config documentation.

Reported-by: Manlio Perillo <manlio.perillo@gmail.com>
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-05 09:51:30 -08:00
Junio C Hamano 43072b4ca1 The fourth batch
Also update the DEF_VER in GIT-VERSION-GEN, which I forgot to do
earlier (it should have been done when we started the new cycle).

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-05 09:44:44 -08:00
Junio C Hamano 53ac1f106f Merge branch 'ak/rebase-autosquash'
Typofix.

* ak/rebase-autosquash:
  rebase: fix typo in autosquash documentation
2024-03-05 09:44:44 -08:00
Junio C Hamano d037212d97 Merge branch 'kn/for-all-refs'
"git for-each-ref" learned "--include-root-refs" option to show
even the stuff outside the 'refs/' hierarchy.

* kn/for-all-refs:
  for-each-ref: add new option to include root refs
  ref-filter: rename 'FILTER_REFS_ALL' to 'FILTER_REFS_REGULAR'
  refs: introduce `refs_for_each_include_root_refs()`
  refs: extract out `loose_fill_ref_dir_regular_file()`
  refs: introduce `is_pseudoref()` and `is_headref()`
2024-03-05 09:44:44 -08:00
Junio C Hamano 661f379791 Merge branch 'pb/ort-make-submodule-conflict-message-an-advice'
When a merge conflicted at a submodule, merge-ort backend used to
unconditionally give a lengthy message to suggest how to resolve
it.  Now the message can be squelched as an advice message.

* pb/ort-make-submodule-conflict-message-an-advice:
  merge-ort: turn submodule conflict suggestions into an advice
2024-03-05 09:44:43 -08:00
Junio C Hamano 53929db7c4 Merge branch 'jc/doc-compat-util'
Clarify wording in the CodingGuidelines that requires <git-compat-util.h>
to be the first header file.

* jc/doc-compat-util:
  doc: clarify the wording on <git-compat-util.h> requirement
2024-03-05 09:44:43 -08:00
Junio C Hamano e58a4de3bb Merge branch 'sg/upload-pack-error-message-fix'
An error message from "git upload-pack", which responds to "git
fetch" requests, had a trialing NUL in it, which has been
corrected.

* sg/upload-pack-error-message-fix:
  upload-pack: don't send null character in abort message to the client
2024-03-05 09:44:43 -08:00
Junio C Hamano d31a515e9c Merge branch 'rs/submodule-prefix-simplify'
Code simplification.

* rs/submodule-prefix-simplify:
  submodule: use strvec_pushf() for --submodule-prefix
2024-03-05 09:44:43 -08:00
Junio C Hamano b5111647cb Merge branch 'rs/name-rev-with-mempool'
Many small allocations "git name-rev" makes have been updated to
allocate from a mem-pool.

* rs/name-rev-with-mempool:
  name-rev: use mem_pool_strfmt()
  mem-pool: add mem_pool_strfmt()
2024-03-05 09:44:43 -08:00
Junio C Hamano 6f74483667 Merge branch 'rs/fetch-simplify-with-starts-with'
Code simplification.

* rs/fetch-simplify-with-starts-with:
  fetch: convert strncmp() with strlen() to starts_with()
2024-03-05 09:44:42 -08:00
Junio C Hamano 74522bbd98 Merge branch 'jk/reflog-special-cases-fix'
The logic to access reflog entries by date and number had ugly
corner cases at the boundaries, which have been cleaned up.

* jk/reflog-special-cases-fix:
  read_ref_at(): special-case ref@{0} for an empty reflog
  get_oid_basic(): special-case ref@{n} for oldest reflog entry
  Revert "refs: allow @{n} to work with n-sized reflog"
2024-03-05 09:44:42 -08:00
Junio C Hamano 542d093b1d Merge branch 'jc/no-include-of-compat-util-from-headers'
Header file clean-up.

* jc/no-include-of-compat-util-from-headers:
  compat: drop inclusion of <git-compat-util.h>
2024-03-05 09:44:42 -08:00
Junio C Hamano d619abf7fa Merge branch 'js/remove-cruft-files'
Remove an empty file that shouldn't have been added in the first
place.

* js/remove-cruft-files:
  neue: remove a bogus empty file
2024-03-05 09:44:42 -08:00
Junio C Hamano 6249de53a3 Merge branch 'jk/textconv-cache-outside-repo-fix'
The code incorrectly attempted to use textconv cache when asked,
even when we are not running in a repository, which has been
corrected.

* jk/textconv-cache-outside-repo-fix:
  userdiff: skip textconv caching when not in a repository
2024-03-05 09:44:42 -08:00
Junio C Hamano 105ec9ae8d clean: further clean-up of implementation around "--force"
We clarified how "clean.requireForce" interacts with the "--dry-run"
option in the previous commit, both in the implementation and in the
documentation.  Even when "git clean" (without other options) is
required to be used with "--force" (i.e. either clean.requireForce
is unset, or explicitly set to true) to protect end-users from
casual invocation of the command by mistake, "--dry-run" does not
require "--force" to be used, because it is already its own
protection mechanism by being a no-op to the working tree files.

The previous commit, however, missed another clean-up opportunity
around the same area.  Just like in the "--dry-run" mode, the
command in the "--interactive" mode does not require "--force",
either.  This is because by going interactive and giving the end
user one more chance to confirm, the mode itself is serving as its
own protection mechanism.

Let's take things one step further, and unify the code that defines
interaction between "--force" and these two other options.  Just
like we added explanation for the reason why "--dry-run" does not
honor "clean.requireForce", give an explanation for the reason why
"--interactive" makes "clean.requireForce" to be ignored.

Finally, add some tests to show the interaction between "--force"
and "--interactive".  We already have tests that show interaction
between "--force" and "--dry-run", but didn't test "--interactive".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-04 14:05:13 -08:00
Patrick Steinhardt 43f70eaea0 refs/reftable: precompute prefix length
We're recomputing the prefix length on every iteration of the ref
iterator. Precompute it for another speedup when iterating over 1
million refs:

    Benchmark 1: show-ref: single matching ref (revision = HEAD~)
      Time (mean ± σ):     100.3 ms ±   3.7 ms    [User: 97.3 ms, System: 2.8 ms]
      Range (min … max):    97.5 ms … 139.7 ms    1000 runs

    Benchmark 2: show-ref: single matching ref (revision = HEAD)
      Time (mean ± σ):      95.8 ms ±   3.4 ms    [User: 92.9 ms, System: 2.8 ms]
      Range (min … max):    93.0 ms … 121.9 ms    1000 runs

    Summary
      show-ref: single matching ref (revision = HEAD) ran
        1.05 ± 0.05 times faster than show-ref: single matching ref (revision = HEAD~)

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-04 10:19:58 -08:00
Patrick Steinhardt f1bf54aee3 reftable: allow inlining of a few functions
We have a few functions which are basically just accessors to
structures. As those functions are executed inside the hot loop when
iterating through many refs, the fact that they cannot be inlined is
costing us some performance.

Move the function definitions into their respective headers so that they
can be inlined. This results in a performance improvement when iterating
over 1 million refs:

    Benchmark 1: show-ref: single matching ref (revision = HEAD~)
      Time (mean ± σ):     105.9 ms ±   3.6 ms    [User: 103.0 ms, System: 2.8 ms]
      Range (min … max):   103.1 ms … 133.4 ms    1000 runs

    Benchmark 2: show-ref: single matching ref (revision = HEAD)
      Time (mean ± σ):     100.7 ms ±   3.4 ms    [User: 97.8 ms, System: 2.8 ms]
      Range (min … max):    97.8 ms … 124.0 ms    1000 runs

    Summary
      show-ref: single matching ref (revision = HEAD) ran
        1.05 ± 0.05 times faster than show-ref: single matching ref (revision = HEAD~)

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-04 10:19:49 -08:00
Patrick Steinhardt daf4f43d0d reftable/record: decode keys in place
When reading a record from a block, we need to decode the record's key.
As reftable keys are prefix-compressed, meaning they reuse a prefix from
the preceding record's key, this is a bit more involved than just having
to copy the relevant bytes: we need to figure out the prefix and suffix
lengths, copy the prefix from the preceding record and finally copy the
suffix from the current record.

This is done by passing three buffers to `reftable_decode_key()`: one
buffer that holds the result, one buffer that holds the last key, and
one buffer that points to the current record. The final key is then
assembled by calling `strbuf_add()` twice to copy over the prefix and
suffix.

Performing two memory copies is inefficient though. And we can indeed do
better by decoding keys in place. Instead of providing two buffers, the
caller may only call a single buffer that is already pre-populated with
the last key. Like this, we only have to call `strbuf_setlen()` to trim
the record to its prefix and then `strbuf_add()` to add the suffix.

This refactoring leads to a noticeable performance bump when iterating
over 1 million refs:

  Benchmark 1: show-ref: single matching ref (revision = HEAD~)
    Time (mean ± σ):     112.2 ms ±   3.9 ms    [User: 109.3 ms, System: 2.8 ms]
    Range (min … max):   109.2 ms … 149.6 ms    1000 runs

  Benchmark 2: show-ref: single matching ref (revision = HEAD)
    Time (mean ± σ):     106.0 ms ±   3.5 ms    [User: 103.2 ms, System: 2.7 ms]
    Range (min … max):   103.2 ms … 133.7 ms    1000 runs

  Summary
    show-ref: single matching ref (revision = HEAD) ran
      1.06 ± 0.05 times faster than show-ref: single matching ref (revision = HEAD~)

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-04 10:19:49 -08:00
Patrick Steinhardt 6620f9134c reftable/record: reuse refname when copying
Do the same optimization as in the preceding commit, but this time for
`reftable_record_copy()`. While not as noticeable, it still results in a
small speedup when iterating over 1 million refs:

  Benchmark 1: show-ref: single matching ref (revision = HEAD~)
    Time (mean ± σ):     114.0 ms ±   3.8 ms    [User: 111.1 ms, System: 2.7 ms]
    Range (min … max):   110.9 ms … 144.3 ms    1000 runs

  Benchmark 2: show-ref: single matching ref (revision = HEAD)
    Time (mean ± σ):     112.5 ms ±   3.7 ms    [User: 109.5 ms, System: 2.8 ms]
    Range (min … max):   109.2 ms … 140.7 ms    1000 runs

  Summary
    show-ref: single matching ref (revision = HEAD) ran
      1.01 ± 0.05 times faster than show-ref: single matching ref (revision = HEAD~)

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-04 10:19:49 -08:00
Patrick Steinhardt 71d9a2e991 reftable/record: reuse refname when decoding
When decoding a reftable record we will first release the user-provided
record and then decode the new record into it. This is quite inefficient
as we basically need to reallocate at least the refname every time.

Refactor the function to start tracking the refname capacity. Like this,
we can stow away the refname, release, restore and then grow the refname
to the required number of bytes via `REFTABLE_ALLOC_GROW()`.

This refactoring is safe to do because all functions that assigning to
the refname will first call `reftable_ref_record_release()`, which will
zero out the complete record after releasing memory.

This change results in a nice speedup when iterating over 1 million
refs:

  Benchmark 1: show-ref: single matching ref (revision = HEAD~)

    Time (mean ± σ):     124.0 ms ±   3.9 ms    [User: 121.1 ms, System: 2.7 ms]
    Range (min … max):   120.4 ms … 152.7 ms    1000 runs

  Benchmark 2: show-ref: single matching ref (revision = HEAD)
    Time (mean ± σ):     114.4 ms ±   3.7 ms    [User: 111.5 ms, System: 2.7 ms]
    Range (min … max):   111.0 ms … 152.1 ms    1000 runs

  Summary
    show-ref: single matching ref (revision = HEAD) ran
      1.08 ± 0.05 times faster than show-ref: single matching ref (revision = HEAD~)

Furthermore, with this change we now perform a mostly constant number of
allocations when iterating. Before this change:

  HEAP SUMMARY:
      in use at exit: 13,603 bytes in 125 blocks
    total heap usage: 1,006,620 allocs, 1,006,495 frees, 25,398,363 bytes allocated

After this change:

  HEAP SUMMARY:
      in use at exit: 13,603 bytes in 125 blocks
    total heap usage: 6,623 allocs, 6,498 frees, 509,592 bytes allocated

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-04 10:19:40 -08:00
Patrick Steinhardt 080f8c4565 reftable/merged: avoid duplicate pqueue emptiness check
When calling `merged_iter_next_void()` we first check whether the iter
has been exhausted already. We already perform this check two levels
down the stack in `merged_iter_next_entry()` though, which makes this
check redundant.

Now if this check was there to accelerate the common case it might have
made sense to keep it. But the iterator being exhausted is rather the
uncommon case because you can expect most reftable stacks to contain
more than two refs.

Simplify the code by removing the check. As `merged_iter_next_void()` is
basically empty except for calling `merged_iter_next()` now, merge these
two functions. This also results in a tiny speedup when iterating over
many refs:

    Benchmark 1: show-ref: single matching ref (revision = HEAD~)
      Time (mean ± σ):     125.6 ms ±   3.8 ms    [User: 122.7 ms, System: 2.8 ms]
      Range (min … max):   122.4 ms … 153.4 ms    1000 runs

    Benchmark 2: show-ref: single matching ref (revision = HEAD)
      Time (mean ± σ):     124.0 ms ±   3.9 ms    [User: 121.1 ms, System: 2.8 ms]
      Range (min … max):   120.1 ms … 156.4 ms    1000 runs

    Summary
      show-ref: single matching ref (revision = HEAD) ran
        1.01 ± 0.04 times faster than show-ref: single matching ref (revision = HEAD~)

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-04 10:19:40 -08:00
Patrick Steinhardt f8c1a8e2e1 reftable/merged: circumvent pqueue with single subiter
The merged iterator uses a priority queue to order records so that we
can yielid them in the expected order. This priority queue of course
comes with some overhead as we need to add, compare and remove entries
in that priority queue.

In the general case, that overhead cannot really be avoided. But when we
have a single subiter left then there is no need to use the priority
queue anymore because the order is exactly the same as what that subiter
would return.

While having a single subiter may sound like an edge case, it happens
more frequently than one might think. In the most common scenario, you
can expect a repository to have a single large table that contains most
of the records and then a set of smaller tables which contain later
additions to the reftable stack. In this case it is quite likely that we
exhaust subiters of those smaller stacks before exhausting the large
table.

Special-case this and return records directly from the remaining
subiter. This results in a sizeable speedup when iterating over 1m refs
in a repository with a single table:

  Benchmark 1: show-ref: single matching ref (revision = HEAD~)
    Time (mean ± σ):     135.4 ms ±   4.4 ms    [User: 132.5 ms, System: 2.8 ms]
    Range (min … max):   131.0 ms … 166.3 ms    1000 runs

  Benchmark 2: show-ref: single matching ref (revision = HEAD)
    Time (mean ± σ):     126.3 ms ±   3.9 ms    [User: 123.3 ms, System: 2.8 ms]
    Range (min … max):   122.7 ms … 157.0 ms    1000 runs

  Summary
    show-ref: single matching ref (revision = HEAD) ran
      1.07 ± 0.05 times faster than show-ref: single matching ref (revision = HEAD~)

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-04 10:19:40 -08:00
Patrick Steinhardt 3b6dd6ad1d reftable/merged: handle subiter cleanup on close only
When advancing one of the subiters fails we immediately release
resources associated with that subiter. This is not necessary though as
we will release these resources when closing the merged iterator anyway.

Drop the logic and only release resources when the merged iterator is
done. This is a mere cleanup that should help reduce the cognitive load
when reading through the code.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-04 10:19:39 -08:00
Patrick Steinhardt 2d71a1d4a2 reftable/merged: remove unnecessary null check for subiters
Whenever we advance a subiter we first call `iterator_is_null()`. This
is not needed though because we only ever advance subiters which have
entries in the priority queue, and we do not end entries to the priority
queue when the subiter has been exhausted.

Drop the check as well as the now-unused function. This results in a
surprisingly big speedup:

    Benchmark 1: show-ref: single matching ref (revision = HEAD~)
      Time (mean ± σ):     138.1 ms ±   4.4 ms    [User: 135.1 ms, System: 2.8 ms]
      Range (min … max):   133.4 ms … 167.3 ms    1000 runs

    Benchmark 2: show-ref: single matching ref (revision = HEAD)
      Time (mean ± σ):     134.4 ms ±   4.2 ms    [User: 131.5 ms, System: 2.8 ms]
      Range (min … max):   130.0 ms … 164.0 ms    1000 runs

    Summary
      show-ref: single matching ref (revision = HEAD) ran
        1.03 ± 0.05 times faster than show-ref: single matching ref (revision = HEAD~)

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-04 10:19:39 -08:00
Patrick Steinhardt bb2d6be4c1 reftable/merged: make subiters own their records
For each subiterator, the merged table needs to track their current
record. This record is owned by the priority queue though instead of by
the merged iterator. This is not optimal performance-wise.

For one, we need to move around records whenever we add or remove a
record from the priority queue. Thus, the bigger the entries the more
bytes we need to copy around. And compared to pointers, a reftable
record is rather on the bigger side. The other issue is that this makes
it harder to reuse the records.

Refactor the code so that the merged iterator tracks ownership of the
records per-subiter. Instead of having records in the priority queue, we
can now use mere pointers to the per-subiter records. This also allows
us to swap records between the caller and the per-subiter record instead
of doing an actual copy via `reftable_record_copy_from()`, which removes
the need to release the caller-provided record.

This results in a noticeable speedup when iterating through many refs.
The following benchmark iterates through 1 million refs:

  Benchmark 1: show-ref: single matching ref (revision = HEAD~)
    Time (mean ± σ):     145.5 ms ±   4.5 ms    [User: 142.5 ms, System: 2.8 ms]
    Range (min … max):   141.3 ms … 177.0 ms    1000 runs

  Benchmark 2: show-ref: single matching ref (revision = HEAD)
    Time (mean ± σ):     139.0 ms ±   4.7 ms    [User: 136.1 ms, System: 2.8 ms]
    Range (min … max):   134.2 ms … 182.2 ms    1000 runs

  Summary
    show-ref: single matching ref (revision = HEAD) ran
      1.05 ± 0.05 times faster than show-ref: single matching ref (revision = HEAD~)

This refactoring also allows a subsequent refactoring where we start
reusing memory allocated by the reftable records because we do not need
to release the caller-provided record anymore.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-04 10:19:39 -08:00
Patrick Steinhardt aad8ad6fe1 reftable/merged: advance subiter on subsequent iteration
When advancing the merged iterator, we pop the topmost entry from its
priority queue and then advance the sub-iterator that the entry belongs
to, adding the result as a new entry. This is quite sensible in the case
where the merged iterator is used to actually iterate through records.
But the merged iterator is also used when we look up a single record,
only, so advancing the sub-iterator is wasted effort because we would
never even look at the result.

Instead of immediately advancing the sub-iterator, we can also defer
this to the next iteration of the merged iterator by storing the
intent-to-advance. This results in a small speedup when reading many
records. The following benchmark creates 10000 refs, which will also end
up with many ref lookups:

    Benchmark 1: update-ref: create many refs (revision = HEAD~)
      Time (mean ± σ):     337.2 ms ±   7.3 ms    [User: 200.1 ms, System: 136.9 ms]
      Range (min … max):   329.3 ms … 373.2 ms    100 runs

    Benchmark 2: update-ref: create many refs (revision = HEAD)
      Time (mean ± σ):     332.5 ms ±   5.9 ms    [User: 197.2 ms, System: 135.1 ms]
      Range (min … max):   327.6 ms … 359.8 ms    100 runs

    Summary
      update-ref: create many refs (revision = HEAD) ran
        1.01 ± 0.03 times faster than update-ref: create many refs (revision = HEAD~)

While this speedup alone isn't really worth it, this refactoring will
also allow two additional optimizations in subsequent patches. First, it
will allow us to special-case when there is only a single sub-iter left
to circumvent the priority queue altogether. And second, it makes it
easier to avoid copying records to the caller.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-04 10:19:30 -08:00
Patrick Steinhardt 48929d2e47 reftable/merged: make `merged_iter` structure private
The `merged_iter` structure is not used anywhere outside of "merged.c",
but is declared in its header. Move it into the code file so that it is
clear that its implementation details are never exposed to anything.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-04 10:19:30 -08:00
Patrick Steinhardt 5c11529c66 reftable/pq: use `size_t` to track iterator index
The reftable priority queue is used by the merged iterator to yield
records from its sub-iterators in the expected order. Each entry has a
record corresponding to such a sub-iterator as well as an index that
indicates which sub-iterator the record belongs to. But while the
sub-iterators are tracked with a `size_t`, we store the index as an
`int` in the entry.

Fix this and use `size_t` consistently.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-04 10:19:30 -08:00
Ghanshyam Thakkar 8145a8fd02 setup: remove unnecessary variable
The TODO comment suggested to heed core.bare from template config file
if no command line override given. And the prev_bare_repository
variable seems to have been placed for this sole purpose as it is not
used anywhere else.

However, it was clarified by Junio [1] that such values (including
core.bare) are ignored intentionally and does not make sense to
propagate them from template config to repository config. Also, the
directories for the worktree and repository are already created, and
therefore the bare/non-bare decision has already been made, by the
point we reach the codepath where the TODO comment is placed.
Therefore, prev_bare_repository does not have a usecase with/without
supporting core.bare from template. And the removal of
prev_bare_repository is safe as proved by the later part of the
comment:

    "Unfortunately, the line above is equivalent to
        is_bare_repository_cfg = !work_tree;
    which ignores the config entirely even if no `--[no-]bare`
    command line option was present.

    To see why, note that before this function, there was this call:
        prev_bare_repository = is_bare_repository()
    expanding the right hand side:
        = is_bare_repository_cfg && !get_git_work_tree()
        = is_bare_repository_cfg && !work_tree
    note that the last simplification above is valid because nothing
    calls repo_init() or set_git_work_tree() between any of the
    relevant calls in the code, and thus the !get_git_work_tree()
    calls will return the same result each time.  So, what we are
    interested in computing is the right hand side of the line of
    code just above this comment:
        prev_bare_repository || !work_tree
        = is_bare_repository_cfg && !work_tree || !work_tree
        = !work_tree
    because "A && !B || !B == !B" for all boolean values of A & B."

Therefore, remove the TODO comment and remove prev_bare_repository
variable. Also, update relevant testcases and remove one redundant
testcase.

[1]: https://lore.kernel.org/git/xmqqjzonpy9l.fsf@gitster.g/

Helped-by: Elijah Newren <newren@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-04 10:18:31 -08:00
shejialuo 0332e813d6 t9117: prefer test_path_* helper functions
test -(e|d) does not provide a nice error message when we hit test
failures, so use test_path_exists, test_path_is_dir instead.

Signed-off-by: shejialuo <shejialuo@gmail.com>
Acked-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-04 09:50:21 -08:00
Rubén Justo 1284f9cc11 completion: reflog subcommands and options
Make generic the completion for reflog subcommands and its options.

Note that we still need to special case the options for "show".

Signed-off-by: Rubén Justo <rjusto@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-03 14:21:39 -08:00
Rubén Justo 476a236e72 completion: factor out __git_resolve_builtins
We're going to use the result of "git xxx --git-completion-helper" not
only for feeding COMPREPLY.

Therefore, factor out the execution and the caching of its results in
__gitcomp_builtin, to a new function __git_resolve_builtins.

While we're here, move an important comment we have in the function to
its header, so it gains visibility.

Signed-off-by: Rubén Justo <rjusto@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-03 14:21:39 -08:00
Rubén Justo 3fec482b5f completion: introduce __git_find_subcommand
Let's have a function to get the current subcommand when completing
commands that follow the syntax:

    git <command> <subcommand>

As a convenience, let's allow an optional "default subcommand" to be
returned if none is found.

Signed-off-by: Rubén Justo <rjusto@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-03 14:21:38 -08:00
Rubén Justo c689c38bc2 completion: reflog show <log-options>
Let's add completion for <log-options> in "reflog show" so that the user
can easily discover uses like:

   $ git reflog --since=1.day.ago

Signed-off-by: Rubén Justo <rjusto@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-03 14:21:38 -08:00
Rubén Justo 85452a1d4b completion: reflog with implicit "show"
When no subcommand is specified to "reflog", we assume "show" [1]:

    $ git reflog -h
    usage: git reflog [show] [<log-options>] [<ref>]
    ...

This implicit "show" is not being completed correctly:

    $ git checkout -b default
    $ git reflog def<TAB><TAB>
    ... no completion options ...

The expected result is:

    $ git reflog default

This happens because we're completing references after seeing a valid
subcommand in the command line.  This prevents the implicit "show" from
working properly, but also introduces a new problem: it keeps offering
subcommand options when the subcommand is implicit:

    $ git checkout -b explore
    $ git reflog default ex<TAB>
    ...
    $ git reflog default expire

The expected result is:

    $ git reflog default explore

To fix this, complete references even if no subcommand is present, or in
other words when the subcommand is implicit "show".

Also, only include completion options for subcommands when completing
the right position in the command line.

  1. cf39f54efc (git reflog show, 2007-02-08)

Signed-off-by: Rubén Justo <rjusto@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-03 14:21:38 -08:00
Sergey Organov 12a4883feb clean: improve -n and -f implementation and documentation
What -n actually does in addition to its documented behavior is
ignoring of configuration variable clean.requireForce, that makes
sense provided -n prevents files removal anyway.

So, first, document this in the manual, and then modify implementation
to make this more explicit in the code.

Improved implementation also stops to share single internal variable
'force' between command-line -f option and configuration variable
clean.requireForce, resulting in more clear logic.

Two error messages with slightly different text depending on if
clean.requireForce was explicitly set or not, are merged into a single
one.

The resulting error message now does not mention -n as well, as it
neither matches intended clean.requireForce usage nor reflects
clarified implementation.

Documentation of clean.requireForce is changed accordingly.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-03 09:50:04 -08:00
René Scharfe 6cf06e9c6e t-ctype: avoid duplicating class names
TEST_CTYPE_FUNC defines a function for testing a character classifier,
TEST_CHAR_CLASS calls it, causing the class name to be mentioned twice.

Avoid the need to define a class-specific function by letting
TEST_CHAR_CLASS do all the work.  This is done by using the internal
functions test__run_begin() and test__run_end(), but they do exist to be
used in test macros after all.

Alternatively we could unroll the loop to provide a very long expression
that tests all 256 characters and EOF and hand that to TEST, but that
seems awkward and hard to read.

No change of behavior or output intended.

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-03 09:47:33 -08:00
René Scharfe 7a8d6c0a10 t-ctype: align output of i
The unit test reports misclassified characters like this:

   # check "isdigit(i) == !!memchr("123456789", i, len)" failed at t/unit-tests/t-ctype.c:36
   #    left: 1
   #   right: 0
   #        i: 0x30

Reduce the indent of i to put its colon directly below the ones in the
preceding lines for consistency.

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-03 09:47:33 -08:00
René Scharfe 752cb6ef81 t-ctype: simplify EOF check
EOF is not a member of any character class.  If a classifier function
returns a non-zero result for it, presumably by mistake, then the unit
test check reports:

   # check "!iseof(EOF)" failed at t/unit-tests/t-ctype.c:53
   #       i: 0xffffffff (EOF)

The numeric value of EOF is not particularly interesting in this
context.  Stop printing the second line.

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-03 09:47:33 -08:00
René Scharfe 980013e90d t-ctype: allow NUL anywhere in the specification string
Replace the custom function is_in() for looking up a character in the
specification string with memchr(3) and sizeof.  This is shorter,
simpler and allows NUL anywhere in the string, which may come in handy
if we ever want to support more character classes that contain it.

Getting the string size using sizeof only works in a macro and with a
string constant.  Use ARRAY_SIZE and compile-time checks to make sure we
are not passed a string pointer.

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-03 09:47:33 -08:00
Junio C Hamano 4c9355ff48 repack: check error writing to pack-objects subprocess
When "git repack" repacks promisor objects, it starts a pack-objects
subprocess and uses xwrite() to send object names over the pipe to
it, but without any error checking.  An I/O error or short write
(even though a short write is unlikely for such a small amount of
data) can result in a packfile that lacks certain objects we wanted
to put in there, leading to a silent repository corruption.

Use write_in_full(), instead of xwrite(), to mitigate short write
risks, check errors from it, and abort if we see a failure.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-02 11:12:16 -08:00
Junio C Hamano 36ffba1c7b sideband: avoid short write(2)
The sideband demultiplexor writes the data it receives on sideband
with xwrite().  We can lose data if the underlying write(2) results
in a short write.

If they are limited to unimportant bytes like eye-candy progress
meter, it may be OK to lose them, but lets be careful and ensure
that we use write_in_full() instead.  Note that the original does
not check for errors, and this rewrite does not check for one.  At
least not yet.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-02 11:12:16 -08:00
Junio C Hamano fa6c383309 unpack: replace xwrite() loop with write_in_full()
We have two packfile stream consumers, index-pack and
unpack-objects, that allow excess payload after the packfile stream
data. Their code to relay excess data hasn't changed significantly
since their original implementation that appeared in 67e5a5ec
(git-unpack-objects: re-write to read from stdin, 2005-06-28) and
9bee2478 (mimic unpack-objects when --stdin is used with index-pack,
2006-10-25).

These code blocks contain hand-rolled loops using xwrite(), written
before our write_in_full() helper existed. This helper now provides
the same functionality.

Replace these loops with write_in_full() for shorter, clearer
code. Update related variables accordingly.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-02 11:12:16 -08:00
Junio C Hamano 381a83dfa3 test_i18ngrep: hard deprecate and forbid its use
Since v2.44.0-rc0~109 (Merge branch 'sp/test-i18ngrep', 2023-12-27)
none of the tests we have, either in 'master' or in flight and
collected in 'seen', use test_i18ngrep.

Perhaps it is good time to update test_i18ngrep to BUG to avoid
people adding new calls to it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-02 10:21:10 -08:00
Junio C Hamano b387623c12 The third batch
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-01 14:38:56 -08:00