t7800: improve test descriptions with empty arguments
Some of the tests in t7800 are executed repeatedly in a loop with
different arguments. To distinguish these tests, the value of that
variable is rendered into the test title. But given that one of the
values is the empty string, it results in a somewhat awkward test name:
difftool ignores exit code
Improve this by printing "without options" in case the value is empty.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Dragan Simic [Thu, 21 Mar 2024 06:06:08 +0000 (07:06 +0100)]
config.txt: describe handling of whitespace further
Make it more clear what the whitespace characters are in the context of git
configuration files, and significantly improve the description of the leading
and trailing whitespace handling, especially how it works out together with
the presence of inline comments.
Helped-by: Junio C Hamano <gitster@pobox.com> Helped-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Dragan Simic <dsimic@manjaro.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Dragan Simic [Thu, 21 Mar 2024 06:06:07 +0000 (07:06 +0100)]
t1300: add more tests for whitespace and inline comments
Add a handful of additional tests, to improve the coverage of the handling
of configuration file entries whose values contain internal whitespace,
leading and/or trailing whitespace, which may or may not be enclosed within
quotation marks, or which contain an additional inline comment.
At the same time, rework one already existing whitespace-related test a bit,
to ensure its consistency with the newly added tests. This change introduced
no functional changes to the already existing test.
Helped-by: Eric Sunshine <sunshine@sunshineco.com> Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Dragan Simic <dsimic@manjaro.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix a bug in function parse_value() that prevented whitespace characters
(i.e. spaces and horizontal tabs) found inside configuration option values
from being parsed and returned in their original form. The bug caused any
number of consecutive whitespace characters to be wrongly "squashed" into
the same number of space characters.
This bug was introduced back in July 2009, in commit ebdaae372b46 ("config:
Keep inner whitespace verbatim").
Further investigation showed that setting a configuration value, by invoking
git-config(1), converts value-internal horizontal tabs into "\t" escape
sequences, which the buggy value-parsing logic in function parse_value()
didn't "squash" into spaces. That's why the test included in the ebdaae37
commit passed, which presumably made the bug remain undetected for this long.
On the other hand, value-internal literal horizontal tab characters, found in
a configuration file edited by hand, do get "squashed" by the value-parsing
logic, so the right choice was to fix this bug by making the value-internal
whitespace characters preserved verbatim.
Signed-off-by: Dragan Simic <dsimic@manjaro.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Dragan Simic [Thu, 21 Mar 2024 06:06:05 +0000 (07:06 +0100)]
config: minor addition of whitespace
In general, binary operators should be enclosed in a pair of leading and
trailing space (SP) characters. Thus, clean up one spotted expression that
for some reason had a "bunched up" operator.
Signed-off-by: Dragan Simic <dsimic@manjaro.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* ps/reftable-reflog-iteration-perf:
refs/reftable: track last log record name via strbuf
reftable/record: use scratch buffer when decoding records
reftable/record: reuse message when decoding log records
reftable/record: reuse refnames when decoding log records
reftable/record: avoid copying author info
reftable/record: convert old and new object IDs to arrays
refs/reftable: reload correct stack when creating reflog iter
Junio C Hamano [Thu, 21 Mar 2024 21:55:13 +0000 (14:55 -0700)]
Merge branch 'jc/safe-implicit-bare'
Users with safe.bareRepository=explicit can still work from within
$GIT_DIR of a seconary worktree (which resides at .git/worktrees/$name/)
of the primary worktree without explicitly specifying the $GIT_DIR
environment variable or the --git-dir=<path> option.
* jc/safe-implicit-bare:
setup: notice more types of implicit bare repositories
Junio C Hamano [Thu, 21 Mar 2024 21:55:12 +0000 (14:55 -0700)]
Merge branch 'ps/reftable-block-search-fix'
The reftable code has its own custom binary search function whose
comparison callback has an unusual interface, which caused the
binary search to degenerate into a linear search, which has been
corrected.
* ps/reftable-block-search-fix:
reftable/block: fix binary search over restart counter
reftable/record: fix memory leak when decoding object records
Junio C Hamano [Thu, 21 Mar 2024 21:55:12 +0000 (14:55 -0700)]
Merge branch 'ps/reftable-stack-tempfile'
The code in reftable backend that creates new table files works
better with the tempfile framework to avoid leaving cruft after a
failure.
* ps/reftable-stack-tempfile:
reftable/stack: register compacted tables as tempfiles
reftable/stack: register lockfiles during compaction
reftable/stack: register new tables as tempfiles
lockfile: report when rollback fails
Note the weird ".nfs*" file? This file is maintained by NFS clients in
order to emulate delete-after-last-close semantics that we rely on in
the reftable code [1]. Instead of unlinking the file right away and
keeping it open in the client, the NFS client will rename it to ".nfs*"
and then delete that temporary file when the last reference to it gets
dropped. Quoting the NFS FAQ:
> D2. What is a "silly rename"? Why do these .nfsXXXXX files keep
> showing up?
>
> A. Unix applications often open a scratch file and then unlink it.
> They do this so that the file is not visible in the file system name
> space to any other applications, and so that the system will
> automatically clean up (delete) the file when the application exits.
> This is known as "delete on last close", and is a tradition among
> Unix applications.
>
> Because of the design of the NFS protocol, there is no way for a
> file to be deleted from the name space but still remain in use by an
> application. Thus NFS clients have to emulate this using what
> already exists in the protocol. If an open file is unlinked, an NFS
> client renames it to a special name that looks like ".nfsXXXXX".
> This "hides" the file while it remains in use. This is known as a
> "silly rename." Note that NFS servers have nothing to do with this
> behavior.
This of course throws off the assertion that we got exactly two files in
that directory.
The test in question triggers this behaviour by holding two open file
descriptors to the "tables.list" file. One of the references is because
we are about to append to the stack, whereas the other reference is
because we want to compact it. As the compaction has just finished we
already rewrote "tables.list" to point to the new contents, but the
other file descriptor pointing to the old version is still open. Thus we
trigger the delete-after-last-close emulation.
Furthermore, it was reported that this behaviour only triggers with 4f36b8597c (reftable/stack: fix race in up-to-date check, 2024-01-18).
This is expected as well because it is the first point in time where we
actually keep the "tables.list" file descriptor open for the stat cache.
Fix this bug by skipping over any files that start with a leading dot
when counting files. While we could explicitly check for a prefix of
".nfs", other network file systems like SMB for example do the same
trickery but with a ".smb" prefix. In any case though, this loosening of
the assertion should be fine given that the reftable library would never
write files with leading dots by itself.
[1]: https://nfs.sourceforge.net/#faq_d2
Reported-by: Chuck Lever <chuck.lever@oracle.com> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Wed, 20 Mar 2024 09:48:24 +0000 (05:48 -0400)]
contrib: drop hg-to-git script
The hg-to-git script is full of command injection vulnerabilities
against malicious branch and tag names. It's also old and largely
unmaintained; the last commit was over 4 years ago, and the last code
change before that was from 2013. Users are better off with a modern
remote-helper tool like cinnabar or remote-hg.
So rather than spending time to fix it, let's just get rid of it.
Reported-by: Matthew Rollings <admin@stealthcopter.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Wed, 20 Mar 2024 09:41:03 +0000 (05:41 -0400)]
transport-helper: send "true" value for object-format option
The documentation in gitremote-helpers.txt claims that after a helper
has advertised the "object-format" capability, Git may then send "option
object-format true" to indicate that it would like to hear which object
format the helper is using when it returns refs.
However, the code implementing this has always written just "option
object-format", without the extra "true" value. Nobody noticed in
practice or in the tests because the only two helpers we ship are:
- remote-curl, which quietly converts missing values into "true". This
goes all the way back to ef08ef9ea0 (remote-helpers: Support custom
transport options, 2009-10-30), despite the fact that I don't think
any other option has ever made use of it.
- remote-testgit in t5801 does insist on having a "true" value. But
since it sends the ":object-format" response regardless of whether
it thinks the caller asked for it (technically breaking protocol),
everything just works, albeit with an extra shell error:
printed to stderr, which you can see running t5801 with --verbose.
(The problem is that $val is the empty string, and since we don't
double-quote it in "test $val = true", we invoke "test = true"
instead).
When the documentation and code do not match, it is often good to fix
the documentation rather than break compatibility. And in this case, we
have had the mis-match since 8b85ee4f47 (transport-helper: implement
object-format extensions, 2020-05-25). However, the sha256 feature was
listed as experimental until 8e42eb0e9a (doc: sha256 is no longer
experimental, 2023-07-31).
It's possible there are some third party helpers that tried to follow
the documentation, and are broken. Changing the code will fix them. It's
also possible that there are ones that follow the code and will be
broken if we change it. I suspect neither is the case given that no
helper authors have brought this up as an issue (I only noticed it
because I was running t5801 in verbose mode for other reasons and
wondered about the weird shell error). That, coupled with the relative
new-ness of sha256, makes me think nobody has really worked on helpers
for it yet, which gives us an opportunity to correct the code before too
much time passes.
And doing so has some value: it brings "object-format" in line with the
syntax of other options, making the protocol more consistent. It also
lets us use set_helper_option(), which has better error reporting.
Note that we don't really need to allow any other values like "false"
here. The point is for Git to tell the helper that it understands
":object-format" lines coming back as part of the ref listing. There's
no point in future versions saying "no, I don't understand that".
To make sure everything works as expected, we can improve the
remote-testgit helper from t5801 to send the ":object-format" line only
if the other side correctly asked for it (which modern Git will always
do). With that test change and without the matching code fix here, t5801
will fail when run with GIT_TEST_DEFAULT_HASH=sha256.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Wed, 20 Mar 2024 09:37:40 +0000 (05:37 -0400)]
transport-helper: drop "object-format <algo>" option
The documentation in gitremote-helpers.txt claims that helpers should
accept an object-format option from Git whose value is either:
1. "true", in which case the helper is merely told that Git
understands the special ":object-format" response, and will send it
2. an algorithm name that the helper should use
However, Git has never sent the second form, and it's not clear if it
would ever be useful.
When interacting with a remote Git repository, we generally discover
what _their_ object format is, and then decide what to do with a
mismatch (where that is currently just "bail out", but could eventually
be on-the-fly conversion and interop). And that is true for native
protocols, but also for transport helpers like remote-curl that talk to
remote Git repositories. There we send back an ":object-format" line
telling Git what remote-curl detected on the other side.
And this is true even for pushes (since we get it via receive-pack's
advertisement). And it is even true for dumb-http, as we guess at the
algorithm based on the hash size, due to ac093d0790 (remote-curl: detect
algorithm for dumb HTTP by size, 2020-06-19).
The one case where it _isn't_ true is dumb-http talking to an empty
repository. There we have no clue what the remote hash is, so
remote-curl just sends back its default. If we kept the "object-format
<algo>" form then in theory Git could say "object-format sha256" to
change that default. But it doesn't really accomplish anything. We still
may or may not be mis-matched with the other side. For a fetch that's
OK, since it's by definition a noop. For a push into an empty
repository, it might matter (though the dumb http-push DAV code seems
happy to clobber a remote sha256 info/refs and corrupt the repository).
If we want to pursue making this work, I think we'd be better off
improving detection of the object format of empty repositories over
dumb-http (e.g., an "info/object-format" file).
But what about helpers that _aren't_ talking to another Git repo?
Consider something like git-cinnabar, which is converting on the fly
to/from hg. Most of the heavy lifting is done by fast-import/export, but
some oids may still pass between Git and the helper. Could
"object-format <algo>" be useful to tell the helper what oids we expect
to see?
Possibly, but in practice this isn't necessary. Git-cinnabar for example
already peeks at the local-repo .git/config to check its object-format
(and currently just bails if it is sha256).
So I think the "object-format" extension really is only useful for the
helper telling Git what object-format it found, and not the other way
around.
Note that this patch can't break any remote helpers; we're not changing
the code on the Git side at all, but just bringing the documentation in
line with what Git has always done. It does remove the receiving support
in remote-curl.c, but that code was never actually triggered.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Wed, 20 Mar 2024 09:34:17 +0000 (05:34 -0400)]
transport-helper: use write helpers more consistently
The transport-helper code provides some functions for writing to the
helper process, but there are a few spots that don't use them. We should
do so consistently because:
1. They detect errors on write (though in practice this means the
helper process went away, and we'd see the problem as soon as we
try to read the response).
2. They dump the written bytes to the GIT_TRANSPORT_HELPER_DEBUG
stream. It's doubly confusing to miss some writes but not others,
as you see a partial conversation.
The "list" ones go all the way back to the beginning of the transport
helper code; they were just missed when most writes were converted in bf3c523c3f (Add remote helper debug mode, 2009-12-09). The nearby
"object-format" write presumably just cargo-culted them, as it's only a
few lines away.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Wed, 20 Mar 2024 09:17:48 +0000 (05:17 -0400)]
doc/gitremote-helpers: fix more missing single-quotes
There are a few cases left in gitremote-helpers.txt that are missing a
closing quote, so you end up with:
'option deepen-since <timestamp>
with a stray opening quote instead of rendering correctly in italics.
These should have been part of 51d41dc243 (doc/gitremote-helpers: fix
missing single-quote, 2024-03-07), but apparently my eyesight is not
what it once was. Hopefully this is now all of them.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In log_write_email_headers(), we append our MIME headers to the set of
extra headers by creating a new strbuf, adding the existing headers, and
then adding our new ones. We had to do it this way when our output
buffer might point to the constant opt->extra_headers variable.
But since the previous commit, we always make a local copy of that
variable. Let's turn that into a strbuf, which lets the MIME code simply
append to it. That simplifies the function and avoids a pointless extra
copy of the headers.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Wed, 20 Mar 2024 00:35:33 +0000 (20:35 -0400)]
format-patch: return an allocated string from log_write_email_headers()
When pretty-printing a commit in the email format, we have to fill in
the "after subject" field of the pretty_print_context with any extra
headers the user provided (e.g., from "--to" or "--cc" options) plus any
special MIME headers.
We return an out-pointer that sometimes points to a newly heap-allocated
string and sometimes not. To avoid leaking, we store the allocated
version in a buffer with static lifetime, which is ugly. Worse, as we
extend the header feature, we'll end up having to repeat this ugly
pattern.
Instead, let's have our out-pointer pass ownership back to the caller,
and duplicate the string when necessary. This does mean one extra
allocation per commit when you use extra headers, but in the context of
format-patch which is showing diffs, I don't think that's even
measurable.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Wed, 20 Mar 2024 00:31:39 +0000 (20:31 -0400)]
log: do not set up extra_headers for non-email formats
The commit pretty-printer code has an "after_subject" parameter which it
uses to insert extra headers into the email format. In show_log() we set
this by calling log_write_email_headers() if we are using an email
format, but otherwise default the variable to the rev_info.extra_headers
variable.
Since the pretty-printer code will ignore after_subject unless we are
using an email format, this default is pointless. We can just set
after_subject directly, eliminating an extra variable.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
It's doubtful that this is a useful format at all (the "commit" lines
replace the "From" lines that would make it work as an actual mbox).
But I think that printing the subject as a header (like this patch does)
is the least surprising thing to do.
So let's drop this field, making the code a little simpler and easier to
reason about. Note that we do need to set the "rev" field of the
pretty_print_context in rev-list, since that is used to check for
subject_prefix, etc. It's not possible to set those fields via rev-list,
so we'll always just print "Subject: ". But unless we pass in our
rev_info, fmt_output_email_subject() would segfault trying to figure it
out.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Wed, 20 Mar 2024 00:28:51 +0000 (20:28 -0400)]
pretty: split oneline and email subject printing
The pp_title_line() function is used for two formats: the oneline format
and the subject line of the email format. But most of the logic in the
function does not make any sense for oneline; it is about special
formatting of email headers.
Lumping the two formats together made sense long ago in 4234a76167
(Extend --pretty=oneline to cover the first paragraph, 2007-06-11), when
there was a lot of manual logic to paste lines together. But later, 88c44735ab (pretty: factor out format_subject(), 2008-12-27) pulled that
logic into its own function.
We can implement the oneline format by just calling that one function.
This makes the intention of the code much more clear, as we know we only
need to worry about those extra email options when dealing with actual
email.
While the intent here is cleanup, it is possible to trigger these cases
in practice by running format-patch with an explicit --oneline option.
But if you did, the results are basically nonsense. For example, with
the preserve_subject flag:
$ printf "%s\n" one two three | git commit --allow-empty -F -
$ git format-patch -1 --stdout -k | grep ^Subject
Subject: =?UTF-8?q?one=0Atwo=0Athree?=
$ git format-patch -1 --stdout -k --oneline --no-signature 2af7fbe one
two
three
Or with extra headers:
$ git format-patch -1 --stdout --cc=me --oneline --no-signature 2af7fbe one two three Cc: me
So I'd actually consider this to be an improvement, though you are
probably crazy to use other formats with format-patch in the first place
(arguably it should forbid non-email formats entirely, but that's a
bigger change).
As a bonus, it eliminates some pointless extra allocations for the
oneline output. The email code, since it has to deal with wrapping,
formats into an extra auxiliary buffer. The speedup is tiny, though like
"rev-list --no-abbrev --format=oneline" seems to improve by a consistent
1-2% for me.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Wed, 20 Mar 2024 00:27:16 +0000 (20:27 -0400)]
shortlog: stop setting pp.print_email_subject
When shortlog processes a commit using its internal traversal, it may
pretty-print the subject line for the summary view. When we do so, we
set the "print_email_subject" flag in the pretty-print context. But this
flag does nothing! Since we are using CMIT_FMT_USERFORMAT, we skip most
of the usual formatting code entirely.
This flag is there due to commit 6d167fd7cc (pretty: use
fmt_output_email_subject(), 2017-03-01). But that just switched us away
from setting an empty "subject" header field, which was similarly
useless. That was added by dd2e794a21 (Refactor pretty_print_commit
arguments into a struct, 2009-10-19). Before using the struct, we had to
pass _something_ as the argument, so we passed the empty string (a NULL
would have worked equally well).
So this setting has never done anything, and we can drop the line. That
shortens the code, but more importantly, makes it easier to reason about
and refactor the other users of this flag.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Tue, 19 Mar 2024 22:56:44 +0000 (15:56 -0700)]
apply: parse names out of "diff --git" more carefully
"git apply" uses the pathname parsed out of the "diff --git" header
to decide which path is being patched, but this is used only when
there is no other names available in the patch. When there is any
content change (like we can see in this patch, that modifies the
contents of "apply.c") or rename (which comes with "rename from" and
"rename to" extended diff headers), the names are available without
having to parse this header.
When we do need to parse this header, a special care needs to be
taken, as the name of a directory or a file can have a SP in it so
it is not like "find a space, and take everything before the space
and that is the preimage filename, everything after the space is the
postimage filename". We have a loop that stops at every SP on the
"diff --git a/dir/file b/dir/foo" line and see if that SP is the
right place that separates such a pair of names.
Unfortunately, this loop can terminate prematurely when a crafted
directory name ended with a SP. The next pathname component after
that SP (i.e. the beginning of the possible postimage filename) will
be a slash, and instead of rejecting that position as the valid
separation point between pre- and post-image filenames and keep
looping, we stopped processing right there.
The fix is simple. Instead of stopping and giving up, keep going on
when we see such a condition.
Reported-by: Han Young <hanyang.tony@bytedance.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Mon, 18 Mar 2024 20:24:32 +0000 (13:24 -0700)]
Merge branch 'ps/reftable-stack-tempfile' into ps/pack-refs-auto
* ps/reftable-stack-tempfile:
reftable/stack: register compacted tables as tempfiles
reftable/stack: register lockfiles during compaction
reftable/stack: register new tables as tempfiles
lockfile: report when rollback fails
* jh/trace2-missing-def-param-fix:
trace2: emit 'def_param' set with 'cmd_name' event
trace2: avoid emitting 'def_param' set more than once
t0211: demonstrate missing 'def_param' events for certain commands
The description for `key_value_separator` incorrectly states that this
separator is inserted between trailer lines, which appears likely to
have been incorrectly copied from `separator` when this option was
added.
Update the description to correctly indicate that it is a separator that
appears between the key and the value of each trailer.
Signed-off-by: Brian Lyles <brianmlyles@gmail.com> Acked-by: Linus Arver <linusa@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Philippe Blain [Sat, 16 Mar 2024 21:16:30 +0000 (21:16 +0000)]
builtin/am: allow disabling conflict advice
When 'git am' or 'git rebase --apply' encounter a conflict, they show a
message instructing the user how to continue the operation. This message
can't be disabled.
Use ADVICE_MERGE_CONFLICT introduced in the previous commit to allow
disabling it. Update the tests accordingly, as the advice output is now
on stderr instead of stdout. In t4150, redirect stdout to 'out' and
stderr to 'err', since this is less confusing. In t4254, as we are
testing a specific failure mode of 'git am', simply disable the advice.
Note that we are not testing that this advice is shown in 'git rebase'
for the apply backend since 2ac0d6273f (rebase: change the default
backend from "am" to "merge", 2020-02-15).
Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk> Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Philippe Blain [Sat, 16 Mar 2024 21:16:29 +0000 (21:16 +0000)]
sequencer: allow disabling conflict advice
Allow disabling the advice shown when a squencer operation results in a
merge conflict through a new config 'advice.mergeConflict', which is
named generically such that it can be used by other commands eventually.
Remove that final '\n' in the first hunk in sequencer.c to avoid an
otherwise empty 'hint: ' line before the line 'hint: Disable this
message with "git config advice.mergeConflict false"' which is
automatically added by 'advise_if_enabled'.
Note that we use 'advise_if_enabled' for each message in the second hunk
in sequencer.c, instead of using 'if (show_hints &&
advice_enabled(...)', because the former instructs the user how to
disable the advice, which is more user-friendly.
Update the tests accordingly. Note that the body of the second test in
t3507-cherry-pick-conflict.sh is enclosed in double quotes, so we must
escape them in the added line. Note that t5520-pull.sh, which checks
that we display the advice for 'git rebase' (via 'git pull --rebase')
does not have to be updated because it only greps for a specific line in
the advice message.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
René Scharfe [Sat, 16 Mar 2024 21:09:47 +0000 (22:09 +0100)]
t-prio-queue: check result array bounds
Avoid reading past the end of the "result" array, which could otherwise
happen if the prio-queue were to yield more items than were put into it
due to an implementation bug, or if the array has not enough entries due
to a test bug.
Also check at the end whether all "result" entries were consumed, which
would not be the case if the prio-queue forgot some entries or the test
definition contained too many.
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
René Scharfe [Sat, 16 Mar 2024 20:45:51 +0000 (21:45 +0100)]
t-prio-queue: shorten array index message
If we get an unexpected result, the prio-queue unit test reports it like
this:
# check "result[j++] == show(get)" failed at t/unit-tests/t-prio-queue.c:43
# left: 5
# right: 1
# failed at result[] index 0
That last line repeats "failed" and "result" from the first line.
Shorten it to resemble a similar one in t-ctype and also remove the
incrementation from the first line to avoid possible distractions from
the message of which comparison went wrong where:
Junio C Hamano [Sat, 16 Mar 2024 05:57:22 +0000 (22:57 -0700)]
diff.*Prefix: use camelCase in the doc and test titles
We added documentation for diff.srcPrefix and diff.dstPrefix with
their names properly camelCased, but the diff.noPrefix is listed
there in all lowercase. Also these configuration variables, both
existing ones and the {src,dst}Prefix we recently added, were
spelled in all lowercase in the tests in t4013.
Now we are done with the main change, clean these up.
Signed-off-by: Junio C Hamano <gitster@pobox.com> Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Beat Bolli [Fri, 15 Mar 2024 19:45:58 +0000 (20:45 +0100)]
doc: avoid redundant use of cat
The update-hook-example.txt script uses this anti-pattern twice. Call grep
with the input file name directy. While at it, merge the two consecutive
grep calls.
Signed-off-by: Beat Bolli <dev+git@drbeat.li> Acked-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Brian Lyles [Sat, 16 Mar 2024 05:01:24 +0000 (00:01 -0500)]
docs: clarify file options in git-config `--edit`
The description for the `-e`/`--edit` option references scopes
inconsistently: system and global are referenced by their option name
(`--system`/`--global`), but repository (`--local` is not. Additionally,
neither `--worktree` nor `--file` are referenced at all, despite also
being a valid options.
Update the description to mention all four available scopes as well as
`--file`, referencing each consistently by their option name.
Signed-off-by: Brian Lyles <brianmlyles@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jiamu Sun [Thu, 14 Mar 2024 04:00:16 +0000 (04:00 +0000)]
bugreport.c: fix a crash in `git bugreport` with `--no-suffix` option
`git bugreport` does not complain when `--no-suffix` is given, but
it leads to a segmentation fault as the it is not prepared to see a
NULL assigned to the option_suffix variable.
Signed-off-by: Jiamu Sun <barroit@linux.com> Acked-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Fri, 15 Mar 2024 22:26:40 +0000 (15:26 -0700)]
config: allow tweaking whitespace between value and comment
Extending the previous step, this allows the whitespace placed after
the value before the "# comment message" to be tweaked by tweaking
the preprocessing rule to:
* If the given comment string begins with one or more whitespace
characters followed by '#', it is passed intact.
* If the given comment string begins with '#', a Space is
prepended.
* Otherwise, " # " (Space, '#', Space) is prefixed.
* A string with LF in it cannot be used as a comment string.
Unlike the previous step, which unconditionally added a space after
the value before writing the "# comment string", because the above
preprocessing already gives a whitespace before the '#', the
resulting string is written immediately after copying the value.
And the sanity checking rule becomes
* comment string after the above massaging that comes into
git_config_set_multivar_in_file_gently() must
- begin with zero or more whitespace characters followed by '#'.
- not have a LF in it.
I personally think this is over-engineered, but since I thought
things through anyway, here it is in the patch form. The logic to
tweak end-user supplied comment string is encapsulated in a new
helper function, git_config_prepare_comment_string(), so if new
front-end callers would want to use the same massaging rules, it is
easily reused.
Unfortunately I do not think of a way to tweak the preprocessing
rules further to optionally allow having no blank after the value,
i.e. to produce
[section]
variable = value#comment
(which is a valid way to say section.variable=value, by the way)
without sacrificing the ergonomics for the more usual case, so this
time I really stop here.
Junio C Hamano [Fri, 15 Mar 2024 19:43:58 +0000 (12:43 -0700)]
config: fix --comment formatting
When git adds comments itself (like "rebase -i" todo list and
"commit -e" log message editor), it always gives a comment
introducer "#" followed by a Space before the message, except for
the recently introduced "git config --comment", where the users are
forced to say " this is my comment" if they want to add their
comment in this usual format; otherwise their comment string will
end up without a space after the "#".
Make it more ergonomic, while keeping it possible to also use this
unusual style, by massaging the comment string at the UI layer with
a set of simple rules:
* If the given comment string begins with '#', it is passed intact.
* Otherwise, "# " is prefixed.
* A string with LF in it cannot be used as a comment string.
Right now there is only one "front-end" that accepts end-user
comment string and calls the underlying machinery to add or modify
configuration file with comments, but to make sure that the future
callers perform similar massaging as they see fit, add a sanity
check logic in git_config_set_multivar_in_file_gently(), which is
the single choke point in the codepaths that consumes the comment
string.
Junio C Hamano [Fri, 15 Mar 2024 23:05:59 +0000 (16:05 -0700)]
Merge branch 'kh/branch-ref-syntax-advice'
When git refuses to create a branch because the proposed branch
name is not a valid refname, an advice message is given to refer
the user to exact naming rules.
* kh/branch-ref-syntax-advice:
branch: advise about ref syntax rules
advice: use double quotes for regular quoting
advice: use backticks for verbatim
advice: make all entries stylistically consistent
t3200: improve test style
Ralph Seichter [Tue, 12 Mar 2024 21:47:00 +0000 (21:47 +0000)]
config: add --comment option to add a comment
Introduce the ability to append comments to modifications
made using git-config. Example usage:
git config --comment "changed via script" \
--add safe.directory /home/alice/repo.git
based on the proposed patch, the output produced is:
[safe]
directory = /home/alice/repo.git #changed via script
Users need to be able to distinguish between config entries made
using automation and entries made by a human. Automation can add
comments containing a URL pointing to explanations for the change
made, avoiding questions from users as to why their config file
was changed by a third party.
The implementation ensures that a # character is unconditionally
prepended to the provided comment string, and that the comment
text is appended as a suffix to the changed key-value-pair in the
same line of text. Multi-line comments (i.e. comments containing
linefeed) are rejected as errors, causing Git to exit without
making changes.
Comments are aimed at humans who inspect or change their Git
config using a pager or editor. Comments are not meant to be
read or displayed by git-config at a later time.
Signed-off-by: Ralph Seichter <github@seichter.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Brian C Tracy [Fri, 15 Mar 2024 05:47:31 +0000 (05:47 +0000)]
fuzz: add fuzzer for config parsing
Add a new fuzz target that exercises the parsing of git configs.
The existing git_config_from_mem function is a perfect entry point
for fuzzing as it exercises the same code paths as the rest of the
config parsing functions and offers an easily fuzzable interface.
Config parsing is a useful thing to fuzz because it operates on user
controlled data and is a central component of many git operations.
Signed-off-by: Brian C Tracy <brian.tracy33@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
John Cai [Fri, 15 Mar 2024 04:57:26 +0000 (04:57 +0000)]
t5300: fix test_with_bad_commit()
0f8edf7317 (index-pack: --fsck-objects to take an optional argument for
fsck msgs, 2024-02-01) added a test function test_with_bad_commit() that
contained two bugs. test_expect_fail was used instead of test_must_fail,
and a && was not included at the end of the line.
Fix these two issues in the test.
Signed-off-by: John Cai <johncai86@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Thu, 14 Mar 2024 21:05:25 +0000 (14:05 -0700)]
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.
Junio C Hamano [Thu, 14 Mar 2024 21:05:24 +0000 (14:05 -0700)]
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
Junio C Hamano [Thu, 14 Mar 2024 21:05:24 +0000 (14:05 -0700)]
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
Junio C Hamano [Thu, 14 Mar 2024 21:05:23 +0000 (14:05 -0700)]
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
Rubén Justo [Thu, 14 Mar 2024 18:08:58 +0000 (19:08 +0100)]
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.
Phillip Wood [Thu, 14 Mar 2024 17:05:07 +0000 (17:05 +0000)]
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>
Phillip Wood [Thu, 14 Mar 2024 17:05:06 +0000 (17:05 +0000)]
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>
Phillip Wood [Thu, 14 Mar 2024 17:05:05 +0000 (17:05 +0000)]
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>
Phillip Wood [Thu, 14 Mar 2024 17:05:04 +0000 (17:05 +0000)]
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>
projects / thirdparty / git.git / log
