Commit Graph

73324 Commits (v2.45.4)

Author SHA1 Message Date
Peter Hutterer 7fdc265633 diff: add diff.srcPrefix and diff.dstPrefix configuration variables
Allow the default prefixes "a/" and "b/" to be tweaked by the
diff.srcPrefix and diff.dstPrefix configuration variables.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-15 10:04:45 -07:00
Junio C Hamano 4f9b731bde The seventh batch
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-14 14:05:25 -07:00
Junio C Hamano c5a7ee124d Merge branch 'rj/complete-worktree-paths-fix'
The logic to complete the command line arguments to "git worktree"
subcommand (in contrib/) has been updated to correctly honor things
like "git -C dir" etc.

* rj/complete-worktree-paths-fix:
  completion: fix __git_complete_worktree_paths
2024-03-14 14:05:25 -07:00
Junio C Hamano 43100746e6 Merge branch 'rj/complete-reflog'
The command line completion script (in contrib/) learned to
complete "git reflog" better.

* rj/complete-reflog:
  completion: reflog subcommands and options
  completion: factor out __git_resolve_builtins
  completion: introduce __git_find_subcommand
  completion: reflog show <log-options>
  completion: reflog with implicit "show"
2024-03-14 14:05:24 -07:00
Junio C Hamano edae49e3c0 Merge branch 'jc/test-i18ngrep'
With release 2.44 we got rid of all uses of test_i18ngrep and there
is no in-flight topic that adds a new use of it.  Make a call to
test_i18ngrep a hard failure, so that we can remove it at the end
of this release cycle.

* jc/test-i18ngrep:
  test_i18ngrep: hard deprecate and forbid its use
2024-03-14 14:05:24 -07:00
Junio C Hamano 272fd9125a Merge branch 'gt/core-bare-in-templates'
Code simplification.

* gt/core-bare-in-templates:
  setup: remove unnecessary variable
2024-03-14 14:05:24 -07:00
Junio C Hamano 4fecb94887 Merge branch 'la/trailer-api'
Trailer API updates.

Acked-by: Christian Couder <christian.couder@gmail.com>
cf. <CAP8UFD1Zd+9q0z1JmfOf60S2vn5-sD3SafDvAJUzRFwHJKcb8A@mail.gmail.com>

* la/trailer-api:
  format_trailers_from_commit(): indirectly call trailer_info_get()
  format_trailer_info(): move "fast path" to caller
  format_trailers(): use strbuf instead of FILE
  trailer_info_get(): reorder parameters
  trailer: move interpret_trailers() to interpret-trailers.c
  trailer: reorder format_trailers_from_commit() parameters
  trailer: rename functions to use 'trailer'
  shortlog: add test for de-duplicating folded trailers
  trailer: free trailer_info _after_ all related usage
2024-03-14 14:05:24 -07:00
Junio C Hamano 26ab20ccb2 Merge branch 'kh/doc-commentchar-is-a-byte'
The "core.commentChar" configuration variable only allows an ASCII
character, which was not clearly documented, which has been
corrected.

* kh/doc-commentchar-is-a-byte:
  config: document `core.commentChar` as ASCII-only
2024-03-14 14:05:24 -07:00
Junio C Hamano 720c1129c4 Merge branch 'jh/fsmonitor-icase-corner-case-fix'
FSMonitor client code was confused when FSEvents were given in a
different case on a case-insensitive filesystem, which has been
corrected.

Acked-by: Patrick Steinhardt <ps@pks.im>
cf. <ZehofMaSZyUq8S1N@tanuki>

* jh/fsmonitor-icase-corner-case-fix:
  fsmonitor: support case-insensitive events
  fsmonitor: refactor bit invalidation in refresh callback
  fsmonitor: trace the new invalidated cache-entry count
  fsmonitor: return invalidated cache-entry count on non-directory event
  fsmonitor: remove custom loop from non-directory path handler
  fsmonitor: return invalidated cache-entry count on directory event
  fsmonitor: move untracked-cache invalidation into helper functions
  fsmonitor: refactor untracked-cache invalidation
  dir: create untracked_cache_invalidate_trimmed_path()
  fsmonitor: refactor refresh callback for non-directory events
  fsmonitor: clarify handling of directory events in callback helper
  fsmonitor: refactor refresh callback on directory events
  t7527: add case-insensitve test for FSMonitor
  name-hash: add index_dir_find()
2024-03-14 14:05:23 -07:00
Junio C Hamano 448a74e151 Merge branch 'ps/reftable-iteration-perf-part2'
The code to iterate over refs with the reftable backend has seen
some optimization.

* ps/reftable-iteration-perf-part2:
  refs/reftable: precompute prefix length
  reftable: allow inlining of a few functions
  reftable/record: decode keys in place
  reftable/record: reuse refname when copying
  reftable/record: reuse refname when decoding
  reftable/merged: avoid duplicate pqueue emptiness check
  reftable/merged: circumvent pqueue with single subiter
  reftable/merged: handle subiter cleanup on close only
  reftable/merged: remove unnecessary null check for subiters
  reftable/merged: make subiters own their records
  reftable/merged: advance subiter on subsequent iteration
  reftable/merged: make `merged_iter` structure private
  reftable/pq: use `size_t` to track iterator index
2024-03-14 14:05:23 -07:00
Junio C Hamano 066124da88 Merge branch 'so/clean-dry-run-without-force'
The implementation in "git clean" that makes "-n" and "-i" ignore
clean.requireForce has been simplified, together with the
documentation.

* so/clean-dry-run-without-force:
  clean: further clean-up of implementation around "--force"
  clean: improve -n and -f implementation and documentation
2024-03-14 14:05:23 -07:00
Rubén Justo 2f64da0790 checkout: plug some leaks in git-restore
In git-restore we need to free the pathspec and pathspec_from_file
values from the struct checkout_opts.

A simple fix could be to free them in cmd_restore, after the call to
checkout_main returns, like we are doing [1][2] in the sibling function
cmd_checkout.

However, we can do even better.

We have git-switch and git-restore, both of them spin-offs[3][4] of
git-checkout.  All three are implemented as thin wrappers around
checkout_main.  Considering this, it makes a lot of sense to do the
cleanup closer to checkout_main.

Move the cleanups, including the new_branch_info variable, to
checkout_main.

As a consequence, mark: t2070, t2071, t2072 and t6418 as leak-free.

 [1] 9081a421a6 (checkout: fix "branch info" memory leaks, 2021-11-16)

 [2] 7ce4088ab7 (parse-options: consistently allocate memory in
     fix_filename(), 2023-03-04)

 [3] d787d311db (checkout: split part of it to new command 'switch',
     2019-03-29)

 [4] 46e91b663b (checkout: split part of it to new command 'restore',
     2019-04-25)

Signed-off-by: Rubén Justo <rjusto@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-14 11:58:04 -07:00
Phillip Wood 5a99c1ac1a checkout: fix interaction between --conflict and --merge
When using "git checkout" to recreate merge conflicts or merge
uncommitted changes when switching branch "--conflict" sensibly implies
"--merge". Unfortunately the way this is implemented means that "git
checkout --conflict=diff3 --no-merge" implies "--merge" violating the
usual last-one-wins rule. Fix this by only overriding the value of
opts->merge if "--conflicts" comes after "--no-merge" or "-[-no]-merge"
is not given on the command line.

The behavior of "git checkout --merge --no-conflict" is unchanged and
will still merge on the basis that the "-[-no]-conflict" options are
primarily intended to affect the conflict style and so "--no-conflict"
should cancel a previous "--conflict" but not override "--merge".

Of the four new tests the second one tests the behavior change
introduced by this commit, the other three check that this commit does
not regress the existing behavior.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-14 10:08:53 -07:00
Phillip Wood dbeaf8e8c0 checkout: cleanup --conflict=<style> parsing
Passing an invalid conflict style name such as "--conflict=bad" gives
the error message

    error: unknown style 'bad' given for 'merge.conflictstyle'

which is unfortunate as it talks about a config setting rather than
the option given on the command line. This happens because the
implementation calls git_xmerge_config() to set the conflict style
using the value given on the command line. Use the newly added
parse_conflict_style_name() instead and pass the value down the call
chain to override the config setting. This also means we can avoid
setting up a struct config_context required for calling
git_xmerge_config().

The option is now parsed in a callback to avoid having to store the
option name. This is a change in behavior as now

    git checkout --conflict=bad --conflict=diff3

will error out when parsing "--conflict=bad" whereas before this change
it would succeed because it would only try to parse the value of the
last "--conflict" option given on the command line.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-14 10:08:53 -07:00
Phillip Wood 135cc712c3 merge options: add a conflict style member
Add a conflict_style member to `struct merge_options` and `struct
ll_merge_options` to allow callers to override the default conflict
style. This will be used in the next commit.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-14 10:08:52 -07:00
Phillip Wood 412aff7b33 merge-ll: introduce LL_MERGE_OPTIONS_INIT
Introduce a macro to initialize `struct ll_merge_options` in preparation
for the next commit that will add a new member that needs to be
initialized to a non-zero value.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-14 10:08:52 -07:00
Phillip Wood 7457014be5 xdiff-interface: refactor parsing of merge.conflictstyle
Factor out the code that parses of conflict style name so it can be
reused in a later commit that wants to parse the name given on the
command line.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-14 10:08:52 -07:00
Beat Bolli e4e9d5fa97 t0006: add more tests with a negative TZ offset
This test doesn't systematically check a negative timezone offset. Add a
test for each format that outputs the offset to improve our test
coverage.

Signed-off-by: Beat Bolli <dev+git@drbeat.li>
2024-03-14 09:54:31 -07:00
Beat Bolli 69e2bee1a3 date: make "iso-strict" conforming for the UTC timezone
ISO 8601-1:2020-12 specifies that a zero timezone offset must be denoted
with a "Z" suffix instead of the numeric "+00:00". Add the correponding
special case to show_date() and a new test.

Changing an established output format which might be depended on by
scripts is always problematic, but here we choose to adhere more closely
to the published standard.

Reported-by: Michael Osipov <michael.osipov@innomotics.com>
Signed-off-by: Beat Bolli <dev+git@drbeat.li>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-13 16:06:52 -07:00
Junio C Hamano f66e1a071b status: allow --untracked=false and friends
It is natural to expect that the "--untracked" option and the
status.showuntrackedFiles configuration variable to take a Boolean
value ("do you want me to show untracked files?"), but the current
code takes nothing but "no" as "no, please do not show any".

Allow the usual Boolean values to be given, and treat 'true' as
"normal", and 'false' as "no".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-13 10:43:32 -07:00
Junio C Hamano 63acdc4827 status: unify parsing of --untracked= and status.showUntrackedFiles
There are two code paths that take a string and parse it to enum
untracked_status_type.  Introduce a helper function and use it.

As these two places handle an error differently, add an additional
invalid value to the enum, and have the caller of the helper handle
the error condition, instead of dying or emitting error message from
the helper.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-13 10:43:32 -07:00
Jonas Wunderlich 71ccda7e6c doc: status.showUntrackedFiles does not take "false"
The `status.showUntrackedFiles` config option only accepts the
values "no", "normal" or "all", but not as this part of the man page
suggested "false".  While we are at it, camel-case the name of the
variable.

Signed-off-by: Jonas Wunderlich <git@03j.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-13 09:14:46 -07:00
Patrick Steinhardt 0eab85b90f t5601: exercise clones with "includeIf.*.onbranch"
It was reported that git-clone(1) started to fail in Git v2.44 when
cloning via HTTPS when the config contains an "includeIf.*.onbranch"
condition:

    $ git clone https://example.com/repo.git
    Cloning into 'repo'...
    BUG: refs.c:2083: reference backend is unknown
    error: git-remote-https died of signal 6

This regression was bisected to 0fcc285c5e (refs: refactor logic to look
up storage backends, 2023-12-29). This commit tightens the logic to look
up ref backends such that we now die when the backend has not yet been
detected by reading the gitconfig.

Now on its own, this commit wouldn't have caused the failure. But in
18c9cb7524 (builtin/clone: create the refdb with the correct object
format, 2023-12-12) we have also changed how git-clone(1) initializes
the refdb such that it happens after the remote helper is spawned, which
is required so that we can first learn about the object format used by
the remote repository before initializing the refdb. Starting with this
change, the remote helper will be unable to detect the repository right
from the start and thus have an unconfigured ref backend. Consequently,
when we try to resolve the "includeIf.*.onbranch" condition, we will now
fail to look up the refdb and die.

This regression has already been fixed via 199f44cb2e (builtin/clone:
allow remote helpers to detect repo, 2024-02-27), where we now
pre-initialize a partial refdb so that the remote helper can detect the
repository right from the start. But it's clear that we're lacking test
coverage of this functionality.

Add a test to avoid regressing in the future. Note that this test stops
short of defining the desired behaviour for the "onbranch" condition
during a clone. It's not quite clear how exactly it should behave, so
this is a leftover bit for the future.

Reported-by: Angelo Dureghello <angelo@kernel-space.org>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12 13:34:00 -07:00
Dirk Gouders 28636d797f Documentation/user-manual.txt: example for generating object hashes
Add a simple example on how object hashes can be generated manually.

Further, because the document suggests to have a look at the initial
commit, clarify that some details changed since that time.

Signed-off-by: Dirk Gouders <dirk@gouders.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12 13:32:11 -07:00
Jeff King 8b311478ad config: allow multi-byte core.commentChar
Now that all of the code handles multi-byte comment characters, it's
safe to allow users to set them.

There is one special case I kept: we still will not allow an empty
string for the commentChar. While it might make sense in some contexts
(e.g., output where you don't want any comment prefix), there are plenty
where it will behave badly (e.g., all of our starts_with() checks will
indicate that every line is a comment!). It might be reasonable to
assign some meaningful semantics, but it would probably involve checking
how each site behaves. In the interim let's forbid it and we can loosen
things later.

Likewise, the "commentChar cannot be a newline" rule is now extended to
"it cannot contain a newline" (for the same reason: it can confuse our
parsing loops).

Since comment_line_str is used in many parts of the code, it's hard to
cover all possibilities with tests. We can convert the existing
double-semicolon prefix test to show that "git status" works. And we'll
give it a more challenging case in t7507, where we confirm that
git-commit strips out the commit template along with any --verbose text
when reading the edited commit message back in. That covers the basics,
though it's possible there could be issues in more exotic spots (e.g.,
the sequencer todo list uses its own code).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12 13:28:11 -07:00
Jeff King 103d563f37 environment: drop comment_line_char compatibility macro
There is no longer any code which references the single-byte
comment_line_char. Let's drop it, clearing the way for true multi-byte
entries in comment_line_str.

It's possible there are topics in flight that have added new references
to comment_line_char. But we would prefer to fail compilation (and then
fix it) upon merging with this, rather than have them quietly ignore the
bytes after the first.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12 13:28:10 -07:00
Jeff King 78275b08e3 wt-status: drop custom comment-char stringification
In wt_longstatus_print_tracking() we may conditionally show a comment
prefix based on the wt_status->display_comment_prefix flag. We handle
that by creating a local "comment_line_string" that is either the empty
string or the comment character followed by a space.

For a single-byte comment, the maximum length of this string is 2 (plus
a NUL byte). But to handle multi-byte comment characters, it can be
arbitrarily large. One way to handle this is to just call
xstrfmt("%s ", comment_line_str), and then free it when we're done.

But we can simplify things further by just conditionally switching
between our prefix string and an empty string when formatting. We
couldn't just do that with the previous code, because the comment
character was a single byte. There's no way to have a "%c" format switch
between some character and "no character at all". Whereas with "%s" you
can switch between some string and the empty string. So now that we have
a comment string and not a comment char, we can just use it directly
when formatting. Do note that we have to also conditionally add the
trailing space at the same time.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12 13:28:10 -07:00
Jeff King 7eb35e07c6 sequencer: handle multi-byte comment characters when writing todo list
We already match multi-byte comment characters in parse_insn_line(),
thanks to the previous commit, yielding a TODO_COMMENT entry. But in
todo_list_to_strbuf(), we may call command_to_char() to convert that
back into something we can output.

We can't just return comment_line_char anymore, since it may require
multiple bytes. Instead, we'll return "0" for this case, which is the
same thing we'd return for a command which does not have a single-letter
abbreviation (e.g., "revert" or "noop"). There is only a single caller
of command_to_char(), and upon seeing "0" it falls back to outputting
the full name via command_to_string(). So we can handle TODO_COMMENT
there, returning the full string.

Note that there are many other callers of command_to_string(), which
will now behave differently if they pass TODO_COMMENT. But we would not
expect that to happen; prior to this commit, the function just calls
die() in this case. And looking at those callers, that makes sense;
e.g., do_pick_commit() will only be called when servicing a pick
command, and should never be called for a comment in the first place.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12 13:28:10 -07:00
Jeff King 2ec225d397 find multi-byte comment chars in unterminated buffers
As with the previous patch, we need to swap out single-byte matching for
something like starts_with() to match all bytes of a multi-byte comment
character. But for cases where the buffer is not NUL-terminated (and we
instead have an explicit size or end pointer), it's not safe to use
starts_with(), as it might walk off the end of the buffer.

Let's introduce a new starts_with_mem() that does the same thing but
also accepts the length of the "haystack" str and makes sure not to walk
past it.

Note that in most cases the existing code did not need a length check at
all, since it was written in a way that knew we had at least one byte
available (and that was all we checked). So I had to read each one to
find the appropriate bounds. The one exception is sequencer.c's
add_commented_lines(), where we can actually get rid of the length
check. Just like starts_with(), our starts_with_mem() handles an empty
haystack variable by not matching (assuming a non-empty prefix).

A few notes on the implementation of starts_with_mem():

  - it would be equally correct to take an "end" pointer (and indeed,
    many of the callers have this and have to subtract to come up with
    the length). I think taking a ptr/size combo is a more usual
    interface for our codebase, though, and has the added benefit that
    the function signature makes it harder to mix up the three
    parameters.

  - we could obviously build starts_with() on top of this by passing
    strlen(str) as the length. But it's possible that starts_with() is a
    relatively hot code path, and it should not pay that penalty (it can
    generally return an answer proportional to the size of the prefix,
    not the whole string).

  - it naively feels like xstrncmpz() should be able to do the same
    thing, but that's not quite true. If you pass the length of the
    haystack buffer, then strncmp() finds that a shorter prefix string
    is "less than" than the haystack, even if the haystack starts with
    the prefix. If you pass the length of the prefix, then you risk
    reading past the end of the haystack if it is shorter than the
    prefix. So I think we really do need a new function.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12 13:28:10 -07:00
Jeff King 600559b716 find multi-byte comment chars in NUL-terminated strings
Several parts of the code need to identify lines that begin with the
comment character, and do so with a simple byte equality check. As part
of the transition to handling multi-byte characters, we need to match
all of the bytes. For cases where we are looking in a NUL-terminated
string, we can just use starts_with(), which checks all of the
characters in comment_line_str.

Note that we can drop the "line.len" check in wt-status.c's
read_rebase_todolist(). The starts_with() function handles the case of
an empty haystack buffer (it will always return false for a non-empty
prefix).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12 13:28:10 -07:00
Jeff King f99e1d94f5 prefer comment_line_str to comment_line_char for printing
As part of our transition to multi-byte comment characters, we should
use the string variable rather than the historical character variable.
All of the sites adjusted here are just swapping out "%c" for "%s" in
format strings, or strbuf_addch() for strbuf_addstr(). The type system
and printf-attribute give the compiler enough information to make sure
our formats and variable changes all match (especially important for
cases where the format string is defined far away from its use, like
prepare_to_commit() in commit.c).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12 13:28:10 -07:00
Jeff King a1bb146aaf strbuf: accept a comment string for strbuf_add_commented_lines()
As part of our transition to multi-byte comment characters, let's take a
NUL-terminated string pointer for strbuf_add_commented_lines() rather
than a single character.

All of the callers have to be adjusted; most can just pass
comment_line_str rather than comment_line_char.

And now our "cheat" in strbuf_commented_addf() can go away, as we can
take the full string from it.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12 13:28:10 -07:00
Jeff King 3a35d96284 strbuf: accept a comment string for strbuf_commented_addf()
As part of our transition to multi-byte comment characters, let's take a
NUL-terminated string pointer for strbuf_commented_addf() rather than a
single character.

All of the callers have to be adjusted, but they can just pass
comment_line_str rather than comment_line_char.

Note that we rely on strbuf_add_commented_lines() under the hood, so
we'll cheat a bit to squeeze our string into a single character (for now
the two are equivalent, and we'll address this TODO in the next patch).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12 13:28:10 -07:00
Jeff King 2982b65690 strbuf: accept a comment string for strbuf_stripspace()
As part of our transition to multi-byte comment characters, let's take a
NUL-terminated string pointer for strbuf_stripspace(), rather than a
single character. We can continue to support its feature of ignoring
comments by accepting a NULL pointer (as opposed to the current behavior
of a NUL byte).

All of the callers have to be adjusted, but they can all just pass
comment_line_str (or NULL).

Inside the function we detect comments by comparing the first byte of a
line to the comment character. We'll adjust that to use starts_with(),
which will match multiple bytes (though for now, of course, we still
only allow a single byte, so it's academic).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12 13:28:10 -07:00
Jeff King 72a7d5d97f environment: store comment_line_char as a string
We'd like to eventually support multi-byte comment prefixes, but the
comment_line_char variable is referenced in many spots, making the
transition difficult.

Let's start by storing the character in a NUL-terminated string. That
will let us switch code over incrementally to the string format, and we
can easily support the existing code with a macro wrapper (since we'll
continue to allow only a single-byte prefix, this will behave
identically).

Once all references to the "char" variable have been converted, we can
drop it and enable longer strings.

We'll still have to touch all of the spots that create or set the
variable in this patch, but there are only a few (reading the config,
and the "auto" character selector).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12 13:28:10 -07:00
Jeff King 2786d058b6 strbuf: avoid shadowing global comment_line_char name
Several comment-related strbuf functions take a comment_line_char
parameter. There's also a global comment_line_char variable, which is
closely related (most callers pass it in as this parameter). Let's avoid
shadowing the global name. This makes it more obvious that we're not
using the global value, and it will be especially helpful as we refactor
the global in future patches (in particular, any macro trickery wouldn't
work because the preprocessor doesn't respect scope).

We'll use "comment_prefix". That should be descriptive enough, and as a
bonus is more neutral with respect to the "char" type (since we'll
eventually swap it out for a string).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12 13:28:10 -07:00
Jeff King 1751e581a3 commit: refactor base-case of adjust_comment_line_char()
When core.commentChar is set to "auto", we check a set of candidate
characters against the proposed buffer to see which if any can be used
without ambiguity. But before we do that, we optimize for the common
case that the default "#" is fine by just seeing if it is present in the
buffer at all.

The way we do this is a bit subtle, though: we assign the candidate
character to comment_line_char preemptively, then check if it works, and
return if it does. The subtle part is that sometimes setting
comment_line_char is important (after we return, the important outcome
is the fact that we have set the variable) and sometimes it is useless
(if our optimization fails, we go on to do the more careful checks and
eventually assign something else instead).

To make it more clear what is happening (and to make further refactoring
of comment_line_char easier), let's check our candidate character
directly, and then assign as part of returning if it worked out.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12 13:28:09 -07:00
Jeff King 3b45450db6 strbuf: avoid static variables in strbuf_add_commented_lines()
In strbuf_add_commented_lines(), we have to convert the single-byte
comment_line_char into a string to pass to add_lines(). We cache the
created string using a static-local variable. But this makes the
function non-reentrant, and it's doubtful that this provides any real
performance benefit given that we know the string always contains a
single character.

So let's just create it from scratch each time, and to give the compiler
the maximal opportunity to make it fast we'll ditch the over-complicated
xsnprintf() and just assign directly into the array.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12 13:28:09 -07:00
Jeff King db7f93093f strbuf: simplify comment-handling in add_lines() helper
In strbuf_add_commented_lines(), we prepare two strings with potential
prefixes: one with just the comment char, and one with an additional
space. In the add_lines() helper, we use the one without the extra space
for blank lines or lines starting with a tab.

While passing in two separate prefixes to the helper is very flexible,
it's more flexibility than we actually use (or are likely to use, since
the rules inside add_lines() only make sense if "prefix2" is a variant
of "prefix1" without the extra space). And setting up the two strings
makes refactoring in strbuf_add_commented_lines() awkward.

Instead, let's pass in a single string, and just let add_lines() add the
extra space to the result as appropriate.

We do still need to pass in a flag to trigger this behavior. The helper
is shared by strbuf_add_lines(), which passes in a NULL "prefix2" to
inhibit this extra handling.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12 13:28:09 -07:00
Jeff King 727565ef15 config: forbid newline as core.commentChar
Since we usually look for a comment char while parsing line-oriented
files, setting core.commentChar to a single newline can confuse our code
quite a bit. For example, using it with "git commit" causes us to fail
to recognize any of the template as comments, including it in the config
message. Which kind of makes sense, since the template content is on its
own line (so no line can "start" with a newline). In other spots I would
not be surprised if you can create more mischief (e.g., violating loop
assumptions) but I didn't dig into it.

Since comment characters are a local preference, to some degree this is
a case of "if it hurts, don't do it". But given that this would be a
silly and pointless thing to do, and that it makes it harder to reason
about code parsing comment lines, let's just forbid it.

There are other cases that are perhaps questionable (e.g., setting the
comment char to a single space), but they seem to behave reasonably (at
least a simple "git commit" will correctly identify and strip the
template lines). So I haven't worried about going on a hunt for every
stupid thing a user might do to themselves, and just focused on the most
confusing case.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12 13:28:09 -07:00
Junio C Hamano 945115026a The sixth batch
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-11 14:12:31 -07:00
Junio C Hamano 0aa44f0a3c Merge branch 'sj/t9117-path-is-file'
GSoC practice to replace "test -f" with "test_path_is_file".

* sj/t9117-path-is-file:
  t9117: prefer test_path_* helper functions
2024-03-11 14:12:31 -07:00
Junio C Hamano 5b6262b193 Merge branch 'kh/doc-dashed-commands-have-not-worked-for-a-long-time'
Doc update.

* kh/doc-dashed-commands-have-not-worked-for-a-long-time:
  gitcli: drop mention of “non-dashed form”
2024-03-11 14:12:31 -07:00
Junio C Hamano 572bf49341 Merge branch 'rs/t-ctype-simplify'
Code simplification to one unit-test program.

* rs/t-ctype-simplify:
  t-ctype: avoid duplicating class names
  t-ctype: align output of i
  t-ctype: simplify EOF check
  t-ctype: allow NUL anywhere in the specification string
2024-03-11 14:12:31 -07:00
Junio C Hamano ef7e896eca Merge branch 'es/config-doc-sort-sections'
Doc updates.

* es/config-doc-sort-sections:
  docs: sort configuration variable groupings alphabetically
2024-03-11 14:12:30 -07:00
Junio C Hamano 7745f92507 Merge branch 'js/merge-base-with-missing-commit'
Make sure failure return from merge_bases_many() is properly caught.

* js/merge-base-with-missing-commit:
  merge-ort/merge-recursive: do report errors in `merge_submodule()`
  merge-recursive: prepare for `merge_submodule()` to report errors
  commit-reach(repo_get_merge_bases_many_dirty): pass on errors
  commit-reach(repo_get_merge_bases_many): pass on "missing commits" errors
  commit-reach(get_octopus_merge_bases): pass on "missing commits" errors
  commit-reach(repo_get_merge_bases): pass on "missing commits" errors
  commit-reach(get_merge_bases_many_0): pass on "missing commits" errors
  commit-reach(merge_bases_many): pass on "missing commits" errors
  commit-reach(paint_down_to_common): start reporting errors
  commit-reach(paint_down_to_common): prepare for handling shallow commits
  commit-reach(repo_in_merge_bases_many): report missing commits
  commit-reach(repo_in_merge_bases_many): optionally expect missing commits
  commit-reach(paint_down_to_common): plug two memory leaks
2024-03-11 14:12:30 -07:00
Junio C Hamano 30b7c4bdca setup: notice more types of implicit bare repositories
Setting the safe.bareRepository configuration variable to explicit
stops git from using a bare repository, unless the repository is
explicitly specified, either by the "--git-dir=<path>" command line
option, or by exporting $GIT_DIR environment variable.  This may be
a reasonable measure to safeguard users from accidentally straying
into a bare repository in unexpected places, but often gets in the
way of users who need valid accesses to the repository.

Earlier, 45bb9162 (setup: allow cwd=.git w/ bareRepository=explicit,
2024-01-20) loosened the rule such that being inside the ".git"
directory of a non-bare repository does not really count as
accessing a "bare" repository.  The reason why such a loosening is
needed is because often hooks and third-party tools run from within
$GIT_DIR while working with a non-bare repository.

More importantly, the reason why this is safe is because a directory
whose contents look like that of a "bare" repository cannot be a
bare repository that came embedded within a checkout of a malicious
project, as long as its directory name is ".git", because ".git" is
not a name allowed for a directory in payload.

There are at least two other cases where tools have to work in a
bare-repository looking directory that is not an embedded bare
repository, and accesses to them are still not allowed by the recent
change.

 - A secondary worktree (whose name is $name) has its $GIT_DIR
   inside "worktrees/$name/" subdirectory of the $GIT_DIR of the
   primary worktree of the same repository.

 - A submodule worktree (whose name is $name) has its $GIT_DIR
   inside "modules/$name/" subdirectory of the $GIT_DIR of its
   superproject.

As long as the primary worktree or the superproject in these cases
are not bare, the pathname of these "looks like bare but not really"
directories will have "/.git/worktrees/" and "/.git/modules/" as a
substring in its leading part, and we can take advantage of the same
security guarantee allow git to work from these places.

Extend the earlier "in a directory called '.git' we are OK" logic
used for the primary worktree to also cover the secondary worktree's
and non-embedded submodule's $GIT_DIR, by moving the logic to a
helper function "is_implicit_bare_repo()".  We deliberately exclude
secondary worktrees and submodules of a bare repository, as these
are exactly what safe.bareRepository=explicit setting is designed to
forbid accesses to without an explicit GIT_DIR/--git-dir=<path>

Helped-by: Kyle Lippincott <spectral@google.com>
Helped-by: Kyle Meyer <kyle@kyleam.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-11 13:51:36 -07:00
Philippe Blain e1aaf309db ci(github): make Windows test artifacts name unique
If several jobs in the windows-test or vs-test matrices fail, the
upload-artifact action in each job tries to upload the test directories
of the failed tests as "failed-tests-windows.zip", which fails for all
jobs except the one which finishes first with the following error:

    Error: Failed to CreateArtifact: Received non-retryable error:
    Failed request: (409) Conflict: an artifact with this name
    already exists on the workflow run

Make the artifacts name unique by using the 'matrix.nr' token, and
disambiguate the vs-test artifacts from the windows-test ones.

Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-11 10:13:03 -07:00
Jean-Noël Avila 45d5ed3e50 doc: git-clone: format placeholders
With the new formatting rules, we use _<placeholders>_.

Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-11 09:58:11 -07:00
Jean-Noël Avila d3717e1e9c doc: git-clone: format verbatim words
We also apply the formatting to urls.txt which is included.

Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-11 09:58:11 -07:00