* [PATCH 2/3] format-patch: teach `--header-cmd`
From: Kristoffer Haugsbakk @ 2024-03-07 19:59 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk
In-Reply-To: <cover.1709841147.git.code@khaugsbakk.name>
Teach git-format-patch(1) `--header-cmd` (with negation) and the
accompanying config variable `format.headerCmd` which allows the user to
add extra headers per-patch.
format-patch knows `--add-header`. However, that seems most useful for
series-wide headers; you cannot really control what the header is like
per patch or specifically for the cover letter. To that end, teach
format-patch a new option which runs a command that has access to the
hash of the current commit (if it is a code patch) and the patch count
which is used for the patch files that this command outputs. Also
include an environment variable which tells the version of this API so
that the command can detect and error out in case the API changes.
This is inspired by `--header-cmd` of git-send-email(1).
§ Discussion
The command can use the provided commit hash to provide relevant
information in the header. For example, the command could output a
header for the current commit as well as the previously-published
commits:
X-Commit-Hash: 97b53c04894578b23d0c650f69885f734699afc7
X-Previous-Commits:
4ad5d4190649dcb5f26c73a6f15ab731891b9dfd
d275d1d179b90592ddd7b5da2ae4573b3f7a37b7
402b7937951073466bf4527caffd38175391c7da
Now interested parties can use this information to track where the
patches come from.
This information could of course be given between the
three-dash/three-hyphen line and the patch proper. However, the project
might prefer to use this part for extra patch information written by the
author and leave the above information for tooling; this way the extra
information does not need to disturb the reader.
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---
Notes (series):
Documentation/config/format.txt:
• I get the impression that `_` is the convention for placeholders now:
`_<cmd>_`
Documentation/config/format.txt | 5 ++++
Documentation/git-format-patch.txt | 26 ++++++++++++++++++
builtin/log.c | 43 ++++++++++++++++++++++++++++++
log-tree.c | 16 ++++++++++-
revision.h | 2 ++
t/t4014-format-patch.sh | 42 +++++++++++++++++++++++++++++
6 files changed, 133 insertions(+), 1 deletion(-)
diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt
index 7410e930e53..c184b865824 100644
--- a/Documentation/config/format.txt
+++ b/Documentation/config/format.txt
@@ -31,6 +31,11 @@ format.headers::
Additional email headers to include in a patch to be submitted
by mail. See linkgit:git-format-patch[1].
+format.headerCmd::
+ Command to run for each patch that should output RFC 2822 email
+ headers. Has access to some information per patch via
+ environment variables. See linkgit:git-format-patch[1].
+
format.to::
format.cc::
Additional recipients to include in a patch to be submitted
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 728bb3821c1..41c344902e9 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -303,6 +303,32 @@ feeding the result to `git send-email`.
`Cc:`, and custom) headers added so far from config or command
line.
+--[no-]header-cmd=<cmd>::
+ Run _<cmd>_ for each patch. _<cmd>_ should output valid RFC 2822
+ email headers. This can also be configured with
+ the configuration variable `format.headerCmd`. Can be turned off
+ with `--no-header-cmd`. This works independently of
+ `--[no-]add-header`.
++
+_<cmd>_ has access to these environment variables:
++
+ GIT_FP_HEADER_CMD_VERSION
++
+The version of this API. Currently `1`. _<cmd>_ may return exit code
+`2` in order to signal that it does not support the given version.
++
+ GIT_FP_HEADER_CMD_HASH
++
+The hash of the commit corresponding to the current patch. Not set if
+the current patch is the cover letter.
++
+ GIT_FP_HEADER_CMD_COUNT
++
+The current patch count. Increments for each patch.
++
+`git format-patch` will error out if _<cmd>_ returns a non-zero exit
+code.
+
--[no-]cover-letter::
In addition to the patches, generate a cover letter file
containing the branch description, shortlog and the overall diffstat. You can
diff --git a/builtin/log.c b/builtin/log.c
index db1808d7c13..eecbcdf1d6d 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -43,10 +43,13 @@
#include "tmp-objdir.h"
#include "tree.h"
#include "write-or-die.h"
+#include "run-command.h"
#define MAIL_DEFAULT_WRAP 72
#define COVER_FROM_AUTO_MAX_SUBJECT_LEN 100
#define FORMAT_PATCH_NAME_MAX_DEFAULT 64
+#define HC_VERSION "1"
+#define HC_NOT_SUPPORTED 2
/* Set a default date-time format for git log ("log.date" config variable) */
static const char *default_date_mode = NULL;
@@ -902,6 +905,7 @@ static int auto_number = 1;
static char *default_attach = NULL;
+static const char *header_cmd = NULL;
static struct string_list extra_hdr = STRING_LIST_INIT_NODUP;
static struct string_list extra_to = STRING_LIST_INIT_NODUP;
static struct string_list extra_cc = STRING_LIST_INIT_NODUP;
@@ -1100,6 +1104,8 @@ static int git_format_config(const char *var, const char *value,
format_no_prefix = 1;
return 0;
}
+ if (!strcmp(var, "format.headercmd"))
+ return git_config_string(&header_cmd, var, value);
/*
* ignore some porcelain config which would otherwise be parsed by
@@ -1419,6 +1425,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
show_range_diff(rev->rdiff1, rev->rdiff2, &range_diff_opts);
strvec_clear(&other_arg);
}
+ free((char *)pp.after_subject);
}
static char *clean_message_id(const char *msg_id)
@@ -1865,6 +1872,35 @@ static void infer_range_diff_ranges(struct strbuf *r1,
}
}
+/* Returns an owned pointer */
+static char *header_cmd_output(struct rev_info *rev, const struct commit *cmit)
+{
+ struct child_process header_cmd_proc = CHILD_PROCESS_INIT;
+ struct strbuf output = STRBUF_INIT;
+ int res;
+
+ strvec_pushl(&header_cmd_proc.args, header_cmd, NULL);
+ if (cmit)
+ strvec_pushf(&header_cmd_proc.env, "GIT_FP_HEADER_CMD_HASH=%s",
+ oid_to_hex(&cmit->object.oid));
+ strvec_pushl(&header_cmd_proc.env,
+ "GIT_FP_HEADER_CMD_VERSION=" HC_VERSION, NULL);
+ strvec_pushf(&header_cmd_proc.env, "GIT_FP_HEADER_CMD_COUNT=%" PRIuMAX,
+ (uintmax_t)rev->nr);
+ res = capture_command(&header_cmd_proc, &output, 0);
+ if (res) {
+ if (res == HC_NOT_SUPPORTED)
+ die(_("header-cmd %s: returned exit "
+ "code %d; the command does not support "
+ "version " HC_VERSION),
+ header_cmd, HC_NOT_SUPPORTED);
+ else
+ die(_("header-cmd %s: failed with exit code %d"),
+ header_cmd, res);
+ }
+ return strbuf_detach(&output, NULL);
+}
+
int cmd_format_patch(int argc, const char **argv, const char *prefix)
{
struct commit *commit;
@@ -1955,6 +1991,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
OPT_GROUP(N_("Messaging")),
OPT_CALLBACK(0, "add-header", NULL, N_("header"),
N_("add email header"), header_callback),
+ OPT_STRING(0, "header-cmd", &header_cmd, N_("email"), N_("command that will be run to generate headers")),
OPT_STRING_LIST(0, "to", &extra_to, N_("email"), N_("add To: header")),
OPT_STRING_LIST(0, "cc", &extra_cc, N_("email"), N_("add Cc: header")),
OPT_CALLBACK_F(0, "from", &from, N_("ident"),
@@ -2321,6 +2358,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
if (cover_letter) {
if (thread)
gen_message_id(&rev, "cover");
+ if (header_cmd)
+ rev.pe_headers = header_cmd_output(&rev, NULL);
make_cover_letter(&rev, !!output_directory,
origin, nr, list, description_file, branch_name, quiet);
print_bases(&bases, rev.diffopt.file);
@@ -2330,6 +2369,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
/* interdiff/range-diff in cover-letter; omit from patches */
rev.idiff_oid1 = NULL;
rev.rdiff1 = NULL;
+ free((char *)rev.pe_headers);
}
rev.add_signoff = do_signoff;
@@ -2376,6 +2416,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
gen_message_id(&rev, oid_to_hex(&commit->object.oid));
}
+ if (header_cmd)
+ rev.pe_headers = header_cmd_output(&rev, commit);
if (output_directory &&
open_next_file(rev.numbered_files ? NULL : commit, NULL, &rev, quiet))
die(_("failed to create output files"));
@@ -2402,6 +2444,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
}
if (output_directory)
fclose(rev.diffopt.file);
+ free((char *)rev.pe_headers);
}
stop_progress(&progress);
free(list);
diff --git a/log-tree.c b/log-tree.c
index 2eabd19962b..3ca383d099f 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -469,12 +469,24 @@ void fmt_output_email_subject(struct strbuf *sb, struct rev_info *opt)
}
}
+static char *extra_and_pe_headers(const char *extra_headers, const char *pe_headers) {
+ struct strbuf all_headers = STRBUF_INIT;
+
+ if (extra_headers)
+ strbuf_addstr(&all_headers, extra_headers);
+ if (pe_headers) {
+ strbuf_addstr(&all_headers, pe_headers);
+ }
+ return strbuf_detach(&all_headers, NULL);
+}
+
void log_write_email_headers(struct rev_info *opt, struct commit *commit,
const char **extra_headers_p,
int *need_8bit_cte_p,
int maybe_multipart)
{
- const char *extra_headers = opt->extra_headers;
+ const char *extra_headers =
+ extra_and_pe_headers(opt->extra_headers, opt->pe_headers);
const char *name = oid_to_hex(opt->zero_commit ?
null_oid() : &commit->object.oid);
@@ -519,6 +531,7 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,
extra_headers ? extra_headers : "",
mime_boundary_leader, opt->mime_boundary,
mime_boundary_leader, opt->mime_boundary);
+ free((char *)extra_headers);
extra_headers = strbuf_detach(&subject_buffer, NULL);
if (opt->numbered_files)
@@ -857,6 +870,7 @@ void show_log(struct rev_info *opt)
strbuf_release(&msgbuf);
free(ctx.notes_message);
+ free((char *)ctx.after_subject);
if (cmit_fmt_is_mail(ctx.fmt) && opt->idiff_oid1) {
struct diff_queue_struct dq;
diff --git a/revision.h b/revision.h
index 94c43138bc3..eb36bdea36e 100644
--- a/revision.h
+++ b/revision.h
@@ -291,6 +291,8 @@ struct rev_info {
struct string_list *ref_message_ids;
int add_signoff;
const char *extra_headers;
+ /* per-email headers */
+ const char *pe_headers;
const char *log_reencode;
const char *subject_prefix;
int patch_name_max;
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index e37a1411ee2..dfda21d4b2b 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -238,6 +238,48 @@ test_expect_failure 'configuration To: header (rfc2047)' '
grep "^To: =?UTF-8?q?R=20=C3=84=20Cipient?= <rcipient@example.com>\$" hdrs9
'
+test_expect_success '--header-cmd' '
+ write_script cmd <<-\EOF &&
+ printf "X-S: $GIT_FP_HEADER_CMD_HASH\n"
+ printf "X-V: $GIT_FP_HEADER_CMD_VERSION\n"
+ printf "X-C: $GIT_FP_HEADER_CMD_COUNT\n"
+ EOF
+ expect_sha1=$(git rev-parse side) &&
+ git format-patch --header-cmd=./cmd --stdout main..side >patch &&
+ grep "^X-S: $expect_sha1" patch &&
+ grep "^X-V: 1" patch &&
+ grep "^X-C: 3" patch
+'
+
+test_expect_success '--header-cmd with no output works' '
+ write_script cmd <<-\EOF &&
+ exit 0
+ EOF
+ git format-patch --header-cmd=./cmd --stdout main..side
+'
+
+test_expect_success '--header-cmd reports failed command' '
+ write_script cmd <<-\EOF &&
+ exit 1
+ EOF
+ cat > expect <<-\EOF &&
+ fatal: header-cmd ./cmd: failed with exit code 1
+ EOF
+ test_must_fail git format-patch --header-cmd=./cmd --stdout main..side >actual 2>&1 &&
+ test_cmp expect actual
+'
+
+test_expect_success '--header-cmd reports exit code 2' '
+ write_script cmd <<-\EOF &&
+ exit 2
+ EOF
+ cat > expect <<-\EOF &&
+ fatal: header-cmd ./cmd: returned exit code 2; the command does not support version 1
+ EOF
+ test_must_fail git format-patch --header-cmd=./cmd --stdout main..side >actual 2>&1 &&
+ test_cmp expect actual
+'
+
# check_patch <patch>: Verify that <patch> looks like a half-sane
# patch email to avoid a false positive with !grep
check_patch () {
--
2.44.0.169.gd259cac85a8
^ permalink raw reply related
* [PATCH 3/3] format-patch: check if header output looks valid
From: Kristoffer Haugsbakk @ 2024-03-07 19:59 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk
In-Reply-To: <cover.1709841147.git.code@khaugsbakk.name>
Implement a function based on `mailinfo.c:is_mail`.
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---
Notes (series):
Isolating this for review as its own commit so that I can point out the
provenance. May well be squashed into the main patch eventually.
builtin/log.c | 33 +++++++++++++++++++++++++++++++++
t/t4014-format-patch.sh | 13 +++++++++++++
2 files changed, 46 insertions(+)
diff --git a/builtin/log.c b/builtin/log.c
index eecbcdf1d6d..27e1a66dd03 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1872,6 +1872,35 @@ static void infer_range_diff_ranges(struct strbuf *r1,
}
}
+static int is_mail(struct strbuf *sb)
+{
+ const char *header_regex = "^[!-9;-~]+:";
+ regex_t regex;
+ int ret = 1, i;
+ struct string_list list = STRING_LIST_INIT_DUP;
+
+ if (regcomp(®ex, header_regex, REG_NOSUB | REG_EXTENDED))
+ die("invalid pattern: %s", header_regex);
+ string_list_split(&list, sb->buf, '\n', -1);
+ for (i = 0; i < list.nr; i++) {
+ /* End of header */
+ if (!*list.items[i].string && i == (list.nr - 1))
+ break;
+ /* Ignore indented folded lines */
+ if (*list.items[i].string == '\t' ||
+ *list.items[i].string == ' ')
+ continue;
+ /* It's a header if it matches header_regex */
+ if (regexec(®ex, list.items[i].string, 0, NULL, 0)) {
+ ret = 0;
+ break;
+ }
+ }
+ string_list_clear(&list, 1);
+ regfree(®ex);
+ return ret;
+}
+
/* Returns an owned pointer */
static char *header_cmd_output(struct rev_info *rev, const struct commit *cmit)
{
@@ -1898,6 +1927,10 @@ static char *header_cmd_output(struct rev_info *rev, const struct commit *cmit)
die(_("header-cmd %s: failed with exit code %d"),
header_cmd, res);
}
+ if (!is_mail(&output))
+ die(_("header-cmd %s: returned output which was "
+ "not recognized as valid RFC 2822 headers"),
+ header_cmd);
return strbuf_detach(&output, NULL);
}
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index dfda21d4b2b..98e0eb706e6 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -258,6 +258,19 @@ test_expect_success '--header-cmd with no output works' '
git format-patch --header-cmd=./cmd --stdout main..side
'
+test_expect_success '--header-cmd without headers-like output fails' '
+ write_script cmd <<-\EOF &&
+ printf "X-S: $GIT_FP_HEADER_CMD_HASH\n"
+ printf "\n"
+ printf "X-C: $GIT_FP_HEADER_CMD_COUNT\n"
+ EOF
+ cat > expect <<-\EOF &&
+ fatal: header-cmd ./cmd: returned output which was not recognized as valid RFC 2822 headers
+ EOF
+ test_must_fail git format-patch --header-cmd=./cmd --stdout main..side >actual 2>&1 &&
+ test_cmp expect actual
+'
+
test_expect_success '--header-cmd reports failed command' '
write_script cmd <<-\EOF &&
exit 1
--
2.44.0.169.gd259cac85a8
^ permalink raw reply related
* Re: Should --update-refs exclude refs pointing to the current HEAD?
From: Stefan Haller @ 2024-03-07 20:16 UTC (permalink / raw)
To: Elijah Newren
Cc: Junio C Hamano, git, Derrick Stolee, Phillip Wood,
Christian Couder
In-Reply-To: <CABPp-BE36Zhacdumd1JSc+7NXYpxZ=CQ1=ieebze=mDewpEUGA@mail.gmail.com>
Elijah, thanks for your patience with this. I appreciate the time and
energy you put into understanding what I want to achieve. The questions
you are asking help me understand my proposal better myself.
It seems that I didn't do a very good job at getting my point across so
far, so I'll try again in a more structured way.
Let's begin by describing two very different user scenarios:
1) Stacked branches. Git supports these reasonably well for simple cases
through the "rebase --update-refs" command (and the "rebase.updateRefs"
config), but since they are not a first-class concept, git needs to rely
on heuristics to determine which branches are part of a stack. For
simple cases this works very well, but more esoteric cases can have
problems (e.g. a non-linear topology of multiple stacks that may share
common base branches and then diverge, in which case rebasing one of
them destroys the others; or degenerate stacks involving "empty"
branches either in the middle or at the top, in which case there's no
way to tell what the order of the branches is supposed to be).
2) Copying a branch, and rebasing it away from the original one (for
non-stacked branches, see below). The use case is that you have a branch
called topic-1 (branched off main), which is pushed and in review
already, with CI running on it, and you want to test whether it works on
devel, so you make a new branch called topic-1-on-devel off of topic-1,
and rebase it onto devel. You want to make a draft PR of that new branch
to have CI run on it, too, and of course you want to keep the original
branch untouched. For me and most of my co-worker that I have observed
in pairing sessions, the natural way to achieve this is as described
above: checkout a new branch, and rebase it where you want it to go.
Next I'll describe my goals, and my non-goals. I know I can easily
achieve 2) by simply not using --update-refs, but I like to have
"rebase.updateRefs" set to true by default because it is so useful, and
having to remember to use --no-update-refs whenever I do 2) is annoying.
So my goal is to make 2) work well (in the simple, non-stacked case)
even when "rebase.updateRefs" is true, while not making 1) work any
worse in the "normal", non-degenerate case.
I'm _not_ trying to fix the problems that --update-refs has today (I
briefly mentioned some of them above, but there are more), and I'm not
trying to make 2) work well with stacked branches. It would certainly be
nice if that would work too, but I don't think it can without
introducing branch stacks as a first-class feature in git, so I'll have
to live with not supporting that case well. It would still be a big
improvement for me without that.
I'll now go on to respond to some of your questions inline below, but
I'll skip some of them in order to not make this too long. Do let me
know if there are still open questions that I didn't address.
On 07.03.24 06:36, Elijah Newren wrote:
> On Wed, Mar 6, 2024 at 1:00 PM Stefan Haller <lists@haller-berlin.de> wrote:
>>
>> On 06.03.24 03:57, Elijah Newren wrote:
>>
>>> 1) What if there is a branch that is "just a copy" of one of the
>>> branches earlier in the "stack"? Since it's "just a copy", shouldn't
>>> it be excluded for similar reasons to what you are arguing? And, if
>>> so, which branch is the copy?
>>
>> This is a good point, but in my experience it's a lot more rare. Maybe
>> I'm looking at all this just from my own experience, and there might be
>> other usecases that are very different from mine, but as far as I am
>> concerned, copies of branches are not long-lived.
>
>> There is no point in having two branches point at the same commit.
>
> But isn't that what you're doing?
Only briefly, not permanently. I only described this to illustrate why
it never happens to encounter branch copies in the middle of a stack.
>> When I create a copy of a
>> branch, I do that only to rebase the copy somewhere else _immediately_,
>> leaving the original branch where it was.
>
> If it is inherently tied like this, why not create the new branch
> immediately after the rebase (with active_branch@{1} as the start
> point), instead of creating it immediately before?
That would be the wrong way round. I want to leave the original branch
untouched, make a new branch and rebase that away from the original.
>> Which means that I encounter
>> copied branches only at the top of the stack, not in the middle. Which
>> means that I'm fine with keeping the current behavior of "rebase
>> --update-ref" to update both copies of that middle-of-the-stack branch,
>> because it never happens in practice for me.
>
> You've really lost me here; are you saying you're fine changing the
> design to add inherent edgecase bugs to the code because those edge
> cases "never happen in practice for me"?
Wait, now you are really turning things around. You make it sound like
my proposal is responsible for what you call a "bug" here. It's not, git
already behaves like this (and you may or may not consider that a
problem), and my proposal doesn't change anything about it. It doesn't
"fix" it, that's right (and this is what I referred to when I said "I'm
fine with it"), but it doesn't make it any worse either.
>> I don't see a contradiction here. I don't tend to do this in practice,
>> but I can totally imagine a tree of stacked branches that share some
>> common base branches in the beginning and then diverge into different
>> branches from there. It's true that "rebase --update-refs", when told to
>> rebase one of the leaf branches, will destroy this tree because it pulls
>> the base branches away from under the other leaf branches, but this is
>> unrelated to my proposal, it has this problem today already. And it's
>> awesome that git replay has a way to avoid this by rebasing the whole
>> tree at once, keeping everything intact. Still, I don't see what's bad
>> about excluding branches that point at the same commits as the leaf
>> branches it is told to rebase when using "replay --contains".
>
> By "leaf branches", do you mean (a) those commits explicitly mentioned
> on the command line for being replayed, (b) only the subset of the
> branches mentioned on the command line which aren't an ancestor of
> another commit being replayed, or (c) something else?
If I understand you right (and if I understand the user interface of
git-replay right), then what I mean is the combination of all single
commits that are mentioned on the command line, plus the right side of
all A..B ranges that are mentioned on the command line. In my mental
model those are "the things that are being rebased" (please let me know
if that mental model is wrong), and I am proposing to exclude all
branches from updating that point to any of those and are not mentioned
on the command line, because they can be considered copies.
> Let me re-ask my question another way. If someone runs
> git replay --onto A --contained ^B ^C D E F
> when branches G, H, & I are in the revision range of "^B ^C D E F",
> with G in particular pointing where D does and H pointing where E
> does, and E contains D in its history, and F contains commits that are
> in neither D nor E, how do I figure out which of D-I should be
> updated?
D, E, F, and I are updated, G and H are not; this seems very obvious to
me. D, E, and F because they are all mentioned explicitly; G and H are
not updated because they point to one of the "things-to-be-rebased", so
they are copies; I is updated because it is contained in E but does not
point at one of the "things-to-be-rebased", so it's part of a "stack"
(or whatever you want to call this topology).
It's a heuristic; we need a way to distinguish things that are part of a
stack from things that are copies. My heuristic for this relies on the
assumption that the stack is not degenerate in the sense that it doesn't
contain any "empty" branches in the middle or at the top of the stack,
otherwise it wouldn't be possible to distinguish the two.
>> No, I don't think I need HEAD to be special. "The thing that I'm
>> rebasing" is special, and it is always HEAD for git rebase, but it can
>> be something else for replay.
>
> But what exactly should that something else be? I still don't
> understand what that is from your explanation so far.
All the refs that are mentioned on the command line, either as a single
commit or as the second half of an A..B expression. It may well be that
I have some misconception of how exactly git replay works, this sounds
like the most likely explanation for why we don't understand each other.
> Something like `git log --format=%D --decorate-refs=refs/heads/
> ${base}..HEAD^1 | grep -v ^$`, plus adding in the current branch,
> right?
>
> Or is the concern with this suggestion the performance hit you'd take
> (which admittedly might be a problem with this solution, since you
> walk the commits an extra time)?
Yes, I can do that, and no, I'm not concerned about performance. We
already have all that data cached in memory anyway, so that's not a
problem. But this would only work for git replay, there's no way to do
the same thing for --update-ref. My goal is to offer the same features
both for the checked out branch (using rebase) and other branches (using
replay), and have them behave the same.
So my proposal is more about changing --update-ref, since I can solve it
manually for replay, as you described. However, _if_ we decide to change
"rebase --update-ref", then I think it would make sense to change
"replay --contains" in the same way, so that they behave more consistently.
>> One last remark: whenever I describe my use case involving copies of
>> branches, people tell me not to do that, use detached heads instead, or
>> other ways to achieve what I want. But then I don't understand why my
>> proposal would make a difference for them. If you don't use copied
>> branches, then why do you care whether "rebase --update-refs" or "replay
>> --contained" moves those copies or not? I still haven't heard a good
>> argument for why the current behavior is desirable, except for the one
>> example of a degenerate stack that Phillip Wood described in [1].
>
> The current behavior is easy to describe and explain to users, and
> generalizes nicely to cases of replaying multiple diverging and
> converging branches.
It sounds like you value the property of being easy to describe higher
than doing the expected thing in as many cases as possible.
-Stefan
^ permalink raw reply
* Re: [PATCH] doc/gitremote-helpers: fix missing single-quote
From: Junio C Hamano @ 2024-03-07 20:31 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20240307084313.GA2072022@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> The formatting around "option push-option" was missing its closing
> quote, leading to the output having a stray opening quote, rather than
> rendering the item in italics (as we do for all of the other options in
> the list).
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> Just happened to notice this while looking at the rendered manpage for a
> different option.
Thanks. This looks like an ancient typo. Applied.
>
> Documentation/gitremote-helpers.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt
> index ed8da428c9..07c8439a6f 100644
> --- a/Documentation/gitremote-helpers.txt
> +++ b/Documentation/gitremote-helpers.txt
> @@ -526,7 +526,7 @@ set by Git if the remote helper has the 'option' capability.
> 'option pushcert' {'true'|'false'}::
> GPG sign pushes.
>
> -'option push-option <string>::
> +'option push-option' <string>::
> Transmit <string> as a push option. As the push option
> must not contain LF or NUL characters, the string is not encoded.
^ permalink raw reply
* Re: [PATCH] git: extend --no-lazy-fetch to work across subprocesses
From: Junio C Hamano @ 2024-03-07 20:33 UTC (permalink / raw)
To: Jeff King; +Cc: git, Christian Couder
In-Reply-To: <20240307095638.GC2650063@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
>> So I do not have a strong opinion either way, if it is more
>> convenient to propagate the request out to other repositories when
>> we run processes in two or more repositories (e.g. "git clone
>> --local"), or if it is more convenient to make sure that the request
>> is limited to the target repository. Here is a version without the
>> local_repo_env[] change.
>
> Yeah, GIT_CEILING_DIRECTORIES is maybe a bad example. But I do think
> LITERAL_PATHSPECS is a better one, and the submodule-fetch example I
> gave would be genuinely surprising if it behaved differently than the
> superproject, I'd think.
>
> I do agree this is probably going to mostly be a debugging aid, so it
> might not matter much. But once in the wild these things tend to take on
> a life of their own. ;)
>
>> ----- >8 --------- >8 --------- >8 --------- >8 --------- >8 -----
>> Subject: [PATCH v3 3/3] git: extend --no-lazy-fetch to work across subprocesses
>
> So anyway, this version seems good to me.
Thanks.
^ permalink raw reply
* [PATCH v2 0/2] reftable/block: fix binary search over restart counter
From: Patrick Steinhardt @ 2024-03-07 20:35 UTC (permalink / raw)
To: git
In-Reply-To: <a4312698cceab5f2438c9dd34465da21d719e256.1709825186.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 542 bytes --]
Hi,
this is the second version of my patch series that fixes the binary
search over block restart counters. I've add another commit on top that
fixes a memory leak that was uncovered by the fix.
Patrick
Patrick Steinhardt (2):
reftable/record: fix memory leak when decoding object records
reftable/block: fix binary search over restart counter
reftable/block.c | 2 +-
reftable/record.c | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
base-commit: 43072b4ca132437f21975ac6acc6b72dc22fd398
--
2.43.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH v2 1/2] reftable/record: fix memory leak when decoding object records
From: Patrick Steinhardt @ 2024-03-07 20:35 UTC (permalink / raw)
To: git
In-Reply-To: <cover.1709843663.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 1106 bytes --]
When decoding records it is customary to reuse a `struct
reftable_ref_record` across calls. Thus, it may happen that the record
already holds some allocated memory. When decoding ref and log records
we handle this by releasing or reallocating held memory. But we fail to
do this for object records, which causes us to leak memory.
Fix this memory leak by releasing object records before we decode into
them. We may eventually want to reuse memory instead to avoid needless
reallocations. But for now, let's just plug the leak and be done.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
reftable/record.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/reftable/record.c b/reftable/record.c
index d6bb42e887..9c31feb35c 100644
--- a/reftable/record.c
+++ b/reftable/record.c
@@ -569,6 +569,8 @@ static int reftable_obj_record_decode(void *rec, struct strbuf key,
uint64_t last;
int j;
+ reftable_obj_record_release(r);
+
REFTABLE_ALLOC_ARRAY(r->hash_prefix, key.len);
memcpy(r->hash_prefix, key.buf, key.len);
r->hash_prefix_len = key.len;
--
2.43.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v2 2/2] reftable/block: fix binary search over restart counter
From: Patrick Steinhardt @ 2024-03-07 20:36 UTC (permalink / raw)
To: git
In-Reply-To: <cover.1709843663.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 3191 bytes --]
Records store their keys prefix-compressed. As many records will share a
common prefix (e.g. "refs/heads/"), this can end up saving quite a bit
of disk space. The downside of this is that it is not possible to just
seek into the middle of a block and consume the corresponding record
because it may depend on prefixes read from preceding records.
To help with this usecase, the reftable format writes every n'th record
without using prefix compression, which is called a "restart". The list
of restarts is stored at the end of each block so that a reader can
figure out entry points at which to read a full record without having to
read all preceding records.
This allows us to do a binary search over the records in a block when
searching for a particular key by iterating through the restarts until
we have found the section in which our record must be located. From
thereon we perform a linear search to locate the desired record.
This mechanism is broken though. In `block_reader_seek()` we call
`binsearch()` over the count of restarts in the current block. The
function we pass to compare records with each other computes the key at
the current index and then compares it to our search key by calling
`strbuf_cmp()`, returning its result directly. But `binsearch()` expects
us to return a truish value that indicates whether the current index is
smaller than the searched-for key. And unless our key exactly matches
the value at the restart counter we always end up returning a truish
value.
The consequence is that `binsearch()` essentially always returns 0,
indicacting to us that we must start searching right at the beginning of
the block. This works by chance because we now always do a linear scan
from the start of the block, and thus we would still end up finding the
desired record. But needless to say, this makes the optimization quite
useless.
Fix this bug by returning whether the current key is smaller than the
searched key. As the current behaviour was correct it is not possible to
write a test. Furthermore it is also not really possible to demonstrate
in a benchmark that this fix speeds up seeking records.
This may cause the reader to question whether this binary search makes
sense in the first place if it doesn't even help with performance. But
it would end up helping if we were to read a reftable with a much larger
block size. Blocks can be up to 16MB in size, in which case it will
become much more important to avoid the linear scan. We are not yet
ready to read or write such larger blocks though, so we have to live
without a benchmark demonstrating this.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
reftable/block.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/reftable/block.c b/reftable/block.c
index 72eb73b380..1663030386 100644
--- a/reftable/block.c
+++ b/reftable/block.c
@@ -302,7 +302,7 @@ static int restart_key_less(size_t idx, void *args)
result = strbuf_cmp(&a->key, &rkey);
strbuf_release(&rkey);
- return result;
+ return result < 0;
}
void block_iter_copy_from(struct block_iter *dest, struct block_iter *src)
--
2.43.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* Re: [PATCH] wt-status: Don't find scissors line beyond buf len
From: Junio C Hamano @ 2024-03-07 20:47 UTC (permalink / raw)
To: Florian Schmidt
Cc: git, Jonathan Davies, Phillip Wood, Denton Liu, Linus Arver
In-Reply-To: <xmqq34t1n91w.fsf@gitster.g>
From: Florian Schmidt <flosch@nutanix.com>
Date: Thu, 7 Mar 2024 18:37:38 +0000
Subject: [PATCH] wt-status: don't find scissors line beyond buf len
If
(a) There is a "---" divider in a commit message,
(b) At some point beyond that divider, there is a cut-line (that is,
"# ------------------------ >8 ------------------------") in the
commit message,
(c) the user does not explicitly set the "no-divider" option,
then "git interpret-trailers" will hang indefinitively.
This is because when (a) is true, find_end_of_log_message() will invoke
ignored_log_message_bytes() with a len that is intended to make it
ignore the part of the commit message beyond the divider. However,
ignored_log_message_bytes() calls wt_status_locate_end(), and that
function ignores the length restriction when it tries to locate the cut
line. If it manages to find one, the returned cutoff value is greater
than len. At this point, ignored_log_message_bytes() goes into an
infinite loop, because it won't advance the string parsing beyond len,
but the exit condition expects to reach cutoff.
Make wt_status_locate_end() honor the length parameter passed in, to
fix this issue.
In general, if wt_status_locate_end() is given a piece of the memory
that lacks NUL at all, strstr() may continue across page boundaries
and run into an unmapped page. For our current callers, this is not
a problem, as all of them except one uses a memory owned by a strbuf
(which guarantees an implicit NUL-termination after its payload),
and the one exeption in trailer.c:find_end_of_log_message() uses
strlen() to compute the length before calling this function.
Signed-off-by: Florian Schmidt <flosch@nutanix.com>
Reviewed-by: Jonathan Davies <jonathan.davies@nutanix.com>
[jc: tweaked the commit log message and the implementation a bit]
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* So here is the version I queued. I have a new paragraph at the
end of the log message to talk about use of strstr() and how it
is OK in the current codebase.
t/t7513-interpret-trailers.sh | 14 ++++++++++++++
wt-status.c | 7 +++++--
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index 6602790b5f..5efe70d675 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -1476,4 +1476,18 @@ test_expect_success 'suppress --- handling' '
test_cmp expected actual
'
+test_expect_success 'handling of --- lines in conjunction with cut-lines' '
+ echo "my-trailer: here" >expected &&
+
+ git interpret-trailers --parse >actual <<-\EOF &&
+ subject
+
+ my-trailer: here
+ ---
+ # ------------------------ >8 ------------------------
+ EOF
+
+ test_cmp expected actual
+'
+
test_done
diff --git a/wt-status.c b/wt-status.c
index 40b59be478..16c1b9b7ee 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1007,8 +1007,11 @@ size_t wt_status_locate_end(const char *s, size_t len)
strbuf_addf(&pattern, "\n%c %s", comment_line_char, cut_line);
if (starts_with(s, pattern.buf + 1))
len = 0;
- else if ((p = strstr(s, pattern.buf)))
- len = p - s + 1;
+ else if ((p = strstr(s, pattern.buf))) {
+ size_t newlen = p - s + 1;
+ if (newlen < len)
+ len = newlen;
+ }
strbuf_release(&pattern);
return len;
}
--
2.44.0-117-g43072b4ca1
^ permalink raw reply related
* [RFC PATCH 0/1] Add hostname condition to includeIf
From: Ignacio Encinas @ 2024-03-07 20:50 UTC (permalink / raw)
To: git; +Cc: Ignacio Encinas
Hello,
First of all hello everyone, and thanks for developing git :)
I recently came across [1], which proposes further extending
includeIf by supporting "hostname" as a condition.
I thought it would be a good feature to have in git so I gave
it a try. Let me know what you think.
If you like the idea, I would be happy to add similar conditions
like "username".
[1] https://github.com/gitgitgadget/git/issues/1665
Ignacio Encinas (1):
config: learn the "hostname:" includeIf condition
Documentation/config.txt | 9 +++++++++
config.c | 16 ++++++++++++++++
t/t1305-config-include.sh | 22 ++++++++++++++++++++++
3 files changed, 47 insertions(+)
base-commit: b387623c12f3f4a376e4d35a610fd3e55d7ea907
--
2.44.0
^ permalink raw reply
* [RFC PATCH 1/1] config: learn the "hostname:" includeIf condition
From: Ignacio Encinas @ 2024-03-07 20:50 UTC (permalink / raw)
To: git; +Cc: Ignacio Encinas
In-Reply-To: <20240307205006.467443-1-ignacio@iencinas.com>
Currently, customizing the configuration depending on the machine running
git has to be done manually.
Add support for a new includeIf keyword "hostname:" to conditionally
include configuration files depending on the hostname.
Signed-off-by: Ignacio Encinas <ignacio@iencinas.com>
---
Documentation/config.txt | 9 +++++++++
config.c | 16 ++++++++++++++++
t/t1305-config-include.sh | 22 ++++++++++++++++++++++
3 files changed, 47 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index e3a74dd1c1..9a22fd2609 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -186,6 +186,11 @@ As for the naming of this keyword, it is for forwards compatibility with
a naming scheme that supports more variable-based include conditions,
but currently Git only supports the exact keyword described above.
+`hostname`::
+ The data that follows the keyword `hostname:` is taken to be a
+ pattern with standard globbing wildcards. If the current
+ hostname matches the pattern, the include condition is met.
+
A few more notes on matching via `gitdir` and `gitdir/i`:
* Symlinks in `$GIT_DIR` are not resolved before matching.
@@ -261,6 +266,10 @@ Example
path = foo.inc
[remote "origin"]
url = https://example.com/git
+
+; include only if the hostname of the machine matches some-hostname
+[includeIf "hostname:some-hostname"]
+ path = foo.inc
----
Values
diff --git a/config.c b/config.c
index 3cfeb3d8bd..e0611fc342 100644
--- a/config.c
+++ b/config.c
@@ -317,6 +317,20 @@ static int include_by_branch(const char *cond, size_t cond_len)
return ret;
}
+static int include_by_hostname(const char *cond, size_t cond_len)
+{
+ int ret;
+ char my_host[HOST_NAME_MAX + 1];
+ struct strbuf pattern = STRBUF_INIT;
+ if (xgethostname(my_host, sizeof(my_host)))
+ return 0;
+
+ strbuf_add(&pattern, cond, cond_len);
+ ret = !wildmatch(pattern.buf, my_host, 0);
+ strbuf_release(&pattern);
+ return ret;
+}
+
static int add_remote_url(const char *var, const char *value,
const struct config_context *ctx UNUSED, void *data)
{
@@ -406,6 +420,8 @@ static int include_condition_is_true(const struct key_value_info *kvi,
else if (skip_prefix_mem(cond, cond_len, "hasconfig:remote.*.url:", &cond,
&cond_len))
return include_by_remote_url(inc, cond, cond_len);
+ else if (skip_prefix_mem(cond, cond_len, "hostname:", &cond, &cond_len))
+ return include_by_hostname(cond, cond_len);
/* unknown conditionals are always false */
return 0;
diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh
index 5cde79ef8c..ee78d9cade 100755
--- a/t/t1305-config-include.sh
+++ b/t/t1305-config-include.sh
@@ -357,4 +357,26 @@ test_expect_success 'include cycles are detected' '
grep "exceeded maximum include depth" stderr
'
+test_expect_success 'conditional include, hostname' '
+ echo "[includeIf \"hostname:$(hostname)a\"]path=bar12" >>.git/config &&
+ echo "[test]twelve=12" >.git/bar12 &&
+ test_must_fail git config test.twelve &&
+
+ echo "[includeIf \"hostname:$(hostname)\"]path=bar12" >>.git/config &&
+ echo 12 >expect &&
+ git config test.twelve >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'conditional include, hostname, wildcard' '
+ echo "[includeIf \"hostname:$(hostname)a*\"]path=bar13" >>.git/config &&
+ echo "[test]thirteen=13" >.git/bar13 &&
+ test_must_fail git config test.thirteen &&
+
+ echo "[includeIf \"hostname:$(hostname)*\"]path=bar13" >>.git/config &&
+ echo 13 >expect &&
+ git config test.thirteen >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.44.0
^ permalink raw reply related
* Re: [PATCH 2/4] reftable/stack: register new tables as tempfiles
From: Patrick Steinhardt @ 2024-03-07 20:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqo7bpncrt.fsf@gitster.g>
[-- Attachment #1: Type: text/plain, Size: 1381 bytes --]
On Thu, Mar 07, 2024 at 09:59:50AM -0800, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
[snip]
> I sense there might be some clean-up opportunities around here.
> After all, lockfile is (or at least pretends to be) built on top of
> tempfile, and it is for more permanent (as opposed to temporary)
> files, but it somehow wasn't a good fit to wrap new tables in this
> series?
Well, I didn't think lockfiles are a good fit here. I did convert
"tables.list" to use a lock because it's a natural fit given that we
want to use "table.list.lock". But newly written tables aren't written
to a file with ".lock" suffix, but instead to a file ending with
".temp.XXXXXX". This is intentionally so that two processes can write
new tables at the same point in time, even though two concurrent writes
will end up being mutually exclusive.
As lockfiles to me are rather about mutually exclusive locking I think
that using tempfiles directly is preferable. As far as I can see there
is also no real benefit with lockfiles in our context, except for the
mode handling. But given that we have "default_permissions" I'd say it
is preferable to consistently use these for now via chmod(3P).
I ain't got any strong opinions on this though, I'm rather being
pragmatic. So if there is a good reason to use lockfiles that I missed
then I wouldn't mind converting the code.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 2/4] reftable/stack: register new tables as tempfiles
From: Junio C Hamano @ 2024-03-07 21:06 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <Zeopa57o1fMxPoZg@framework>
Patrick Steinhardt <ps@pks.im> writes:
> On Thu, Mar 07, 2024 at 09:59:50AM -0800, Junio C Hamano wrote:
>> Patrick Steinhardt <ps@pks.im> writes:
> [snip]
>> I sense there might be some clean-up opportunities around here.
>> After all, lockfile is (or at least pretends to be) built on top of
>> tempfile, and it is for more permanent (as opposed to temporary)
>> files, but it somehow wasn't a good fit to wrap new tables in this
>> series?
> ...
> As lockfiles to me are rather about mutually exclusive locking I think
> that using tempfiles directly is preferable. As far as I can see there
> is also no real benefit with lockfiles in our context, except for the
> mode handling. But given that we have "default_permissions" I'd say it
> is preferable to consistently use these for now via chmod(3P).
I wasn't talking about the use of temp or lock API in _this_ series,
but was talking about the different permission bit handling between
the two API. The loose object codepath that uses the tempfile API
is closer to what you are doing, which suffers from the same "at
creation tempfile API does not bother with permission bits because
it may be removed at the end". The index codepath that uses the
hold_lock_file_for_update() does not have to care, as it gets the
permission right from the start.
Because of these differences, the loose object codepath has to do
the adjust_perm_bits() itself, and similarly, you have to fix the
permission the same.
These callers may become simpler if we give an option to the
git_mkstemps_mode() to return a file whose permission bits are
already correct from the start.
^ permalink raw reply
* Re: [PATCH] wt-status: Don't find scissors line beyond buf len
From: Eric Sunshine @ 2024-03-07 21:09 UTC (permalink / raw)
To: Junio C Hamano
Cc: Florian Schmidt, git, Jonathan Davies, Phillip Wood, Denton Liu,
Linus Arver
In-Reply-To: <xmqq7cidlqg5.fsf@gitster.g>
On Thu, Mar 7, 2024 at 3:47 PM Junio C Hamano <gitster@pobox.com> wrote:
> * So here is the version I queued. I have a new paragraph at the
> end of the log message to talk about use of strstr() and how it
> is OK in the current codebase.
> [jc: tweaked the commit log message and the implementation a bit]
>
> From: Florian Schmidt <flosch@nutanix.com>
>
> In general, if wt_status_locate_end() is given a piece of the memory
> that lacks NUL at all, strstr() may continue across page boundaries
> and run into an unmapped page. For our current callers, this is not
> a problem, as all of them except one uses a memory owned by a strbuf
> (which guarantees an implicit NUL-termination after its payload),
> and the one exeption in trailer.c:find_end_of_log_message() uses
> strlen() to compute the length before calling this function.
s/exeption/exception/
^ permalink raw reply
* Re: [PATCH v5 3/3] test-stdlib: show that git-std-lib is independent
From: Junio C Hamano @ 2024-03-07 21:13 UTC (permalink / raw)
To: Calvin Wan; +Cc: git, Jonathan Tan, phillip.wood123, Jeff King
In-Reply-To: <20240222175033.1489723-4-calvinwan@google.com>
Calvin Wan <calvinwan@google.com> writes:
> + strbuf_commented_addf(sb, '#', "%s", "foo");
Of course, this will need to be adjusted when it meets the "let's
allow more than one byte for a comment character" series by Peff.
It should now read
strbuf_commented_addf(sb, "#", "%s", "foo");
of course.
This is a usual "a function changes in one topic, while the other
topic adds more callers to it" problem a maintainer is expected to
handle fine, so there is nothing special that needs to be done by
contributors, but just giving you a head's up when you yourself test
your updated version to ensure it works well with other topics in
flight.
^ permalink raw reply
* Re: [PATCH] wt-status: Don't find scissors line beyond buf len
From: Kristoffer Haugsbakk @ 2024-03-07 21:15 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jonathan Davies, Phillip Wood, Denton Liu, Linus Arver,
Florian Schmidt
In-Reply-To: <xmqq7cidlqg5.fsf@gitster.g>
On Thu, Mar 7, 2024, at 21:47, Junio C Hamano wrote:
> Signed-off-by: Florian Schmidt <flosch@nutanix.com>
> Reviewed-by: Jonathan Davies <jonathan.davies@nutanix.com>
> [jc: tweaked the commit log message and the implementation a bit]
Just a question. Given the imperative mood principle/rule, why are these
bracket changelog lines always written in the past tense?
Cheers
--
Kristoffer Haugsbakk
^ permalink raw reply
* Re: [PATCH] wt-status: Don't find scissors line beyond buf len
From: Junio C Hamano @ 2024-03-07 21:24 UTC (permalink / raw)
To: Kristoffer Haugsbakk
Cc: git, Jonathan Davies, Phillip Wood, Denton Liu, Linus Arver,
Florian Schmidt
In-Reply-To: <f8de2b3a-9e12-49fe-a7d9-481317f10c4d@app.fastmail.com>
"Kristoffer Haugsbakk" <code@khaugsbakk.name> writes:
> On Thu, Mar 7, 2024, at 21:47, Junio C Hamano wrote:
>> Signed-off-by: Florian Schmidt <flosch@nutanix.com>
>> Reviewed-by: Jonathan Davies <jonathan.davies@nutanix.com>
>> [jc: tweaked the commit log message and the implementation a bit]
>
> Just a question. Given the imperative mood principle/rule, why are these
> bracket changelog lines always written in the past tense?
These are not giving orders to the code to become like so. The
trailer block records what happend to the patch in chronological
order---think of those written there at one level higher level,
"meta" comments.
^ permalink raw reply
* Re: [PATCH] wt-status: Don't find scissors line beyond buf len
From: Eric Sunshine @ 2024-03-07 21:26 UTC (permalink / raw)
To: Junio C Hamano
Cc: Kristoffer Haugsbakk, git, Jonathan Davies, Phillip Wood,
Denton Liu, Linus Arver, Florian Schmidt
In-Reply-To: <xmqqo7bpka6e.fsf@gitster.g>
On Thu, Mar 7, 2024 at 4:24 PM Junio C Hamano <gitster@pobox.com> wrote:
> "Kristoffer Haugsbakk" <code@khaugsbakk.name> writes:
> > On Thu, Mar 7, 2024, at 21:47, Junio C Hamano wrote:
> >> [jc: tweaked the commit log message and the implementation a bit]
> >
> > Just a question. Given the imperative mood principle/rule, why are these
> > bracket changelog lines always written in the past tense?
>
> These are not giving orders to the code to become like so. The
> trailer block records what happend to the patch in chronological
> order---think of those written there at one level higher level,
> "meta" comments.
Also, they are not always written in past tense[*].
[*]: https://lore.kernel.org/git/20240112171910.11131-1-ericsunshine@charter.net/
^ permalink raw reply
* Re: [PATCH] wt-status: Don't find scissors line beyond buf len
From: Kristoffer Haugsbakk @ 2024-03-07 21:30 UTC (permalink / raw)
To: Eric Sunshine
Cc: git, Jonathan Davies, Phillip Wood, Denton Liu, Linus Arver,
Florian Schmidt, Junio C Hamano
In-Reply-To: <CAPig+cRNa22A=fEmc__JvEjYiVF-QG8o7w0gukhbeL3e-PwVkA@mail.gmail.com>
On Thu, Mar 7, 2024 at 4:24 PM Junio C Hamano <gitster@pobox.com> wrote:
>> "Kristoffer Haugsbakk" <code@khaugsbakk.name> writes:
>> > On Thu, Mar 7, 2024, at 21:47, Junio C Hamano wrote:
>> >> [jc: tweaked the commit log message and the implementation a bit]
>> >
>> > Just a question. Given the imperative mood principle/rule, why are these
>> > bracket changelog lines always written in the past tense?
>>
>> These are not giving orders to the code to become like so. The
>> trailer block records what happend to the patch in chronological
>> order---think of those written there at one level higher level,
>> "meta" comments.
And when I think about it the trailers themselves are of course in the
past tense…
Thanks guys
On Thu, Mar 7, 2024, at 22:26, Eric Sunshine wrote:
>
> Also, they are not always written in past tense[*].
>
> [*]:
> https://lore.kernel.org/git/20240112171910.11131-1-ericsunshine@charter.net/
--
Kristoffer Haugsbakk
^ permalink raw reply
* Re: [RFC PATCH 1/1] config: learn the "hostname:" includeIf condition
From: Junio C Hamano @ 2024-03-07 21:40 UTC (permalink / raw)
To: Ignacio Encinas; +Cc: git
In-Reply-To: <20240307205006.467443-2-ignacio@iencinas.com>
Ignacio Encinas <ignacio@iencinas.com> writes:
> Currently, customizing the configuration depending on the machine running
> git has to be done manually.
>
> Add support for a new includeIf keyword "hostname:" to conditionally
> include configuration files depending on the hostname.
>
> Signed-off-by: Ignacio Encinas <ignacio@iencinas.com>
> ---
> Documentation/config.txt | 9 +++++++++
> config.c | 16 ++++++++++++++++
> t/t1305-config-include.sh | 22 ++++++++++++++++++++++
> 3 files changed, 47 insertions(+)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index e3a74dd1c1..9a22fd2609 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -186,6 +186,11 @@ As for the naming of this keyword, it is for forwards compatibility with
> a naming scheme that supports more variable-based include conditions,
> but currently Git only supports the exact keyword described above.
> +`hostname`::
> + The data that follows the keyword `hostname:` is taken to be a
> + pattern with standard globbing wildcards. If the current
> + hostname matches the pattern, the include condition is met.
> +
OK. This seems to copy its phrasing from the existing text for
"gitdir" and "onbranch", which greatly helps the description for
these features consistent.
> diff --git a/config.c b/config.c
> index 3cfeb3d8bd..e0611fc342 100644
> --- a/config.c
> +++ b/config.c
> @@ -317,6 +317,20 @@ static int include_by_branch(const char *cond, size_t cond_len)
> return ret;
> }
>
> +static int include_by_hostname(const char *cond, size_t cond_len)
> +{
> + int ret;
> + char my_host[HOST_NAME_MAX + 1];
> + struct strbuf pattern = STRBUF_INIT;
> + if (xgethostname(my_host, sizeof(my_host)))
> + return 0;
> +
> + strbuf_add(&pattern, cond, cond_len);
> + ret = !wildmatch(pattern.buf, my_host, 0);
> + strbuf_release(&pattern);
> + return ret;
> +}
Have a blank line between the end of the decl block (our codebase
frowns upon decl-after-statement) and the first statement,
i.e. before "if (xgethostname...".
Otherwise this looks reasonable to me.
> diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh
> index 5cde79ef8c..ee78d9cade 100755
> --- a/t/t1305-config-include.sh
> +++ b/t/t1305-config-include.sh
> @@ -357,4 +357,26 @@ test_expect_success 'include cycles are detected' '
> grep "exceeded maximum include depth" stderr
> '
>
> +test_expect_success 'conditional include, hostname' '
> + echo "[includeIf \"hostname:$(hostname)a\"]path=bar12" >>.git/config &&
> + echo "[test]twelve=12" >.git/bar12 &&
> + test_must_fail git config test.twelve &&
Emulating other tests in this file that uses here document may make
it a bit easier to read? E.g.,
cat >>.gitconfig <<-EOF &&
[includeIf "hostname:$(hostname)a"]
path = bar12
EOF
> + echo "[includeIf \"hostname:$(hostname)\"]path=bar12" >>.git/config &&
Ditto for the remainder of the patch.
Thanks.
^ permalink raw reply
* Re: [PATCH 2/2] doc/gitremote-helpers: match object-format option docs to code
From: brian m. carlson @ 2024-03-07 22:20 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20240307085632.GB2072294@coredump.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 1605 bytes --]
On 2024-03-07 at 08:56:32, Jeff King wrote:
> Git's transport-helper code has always sent "option object-format\n",
> and never provided the "true" or "algorithm" arguments. While the
> "algorithm" request is something we might need or want to eventually
> support, it probably makes sense for now to document the actual
> behavior, especially as it has been in place for several years, since
> 8b85ee4f47 (transport-helper: implement object-format extensions,
> 2020-05-25).
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> As I discussed in patch 1, remote-curl does handle the "true" thing
> correctly. And that's really the helper that matters in practice (it's
> possible some third party helper is looking for the explicit "true", but
> presumably they'd have reported their confusion to the list). So we
> could probably just start tacking on the "true" in transport-helper.c
> and leave that part of the documentation untouched.
>
> I'm less sure of the specific-algorithm thing, just because it seems
> like remote-curl would never make use of it anyway (preferring instead
> to match whatever algorithm is used by the http remote). But maybe there
> are pending interoperability plans that depend on this?
It was designed to allow indicating that we know how to support both
SHA-1 and SHA-256 and we want one or the other (so we don't need to do
an expensive conversion). However, if it's not implemented, I agree we
should document what's implemented, and then extend it when interop
comes.
--
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply
* Re: [PATCH v2 2/2] reftable/block: fix binary search over restart counter
From: Junio C Hamano @ 2024-03-07 23:29 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <370b608f9007abe9c0562d76894e2475d19867a1.1709843663.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> The consequence is that `binsearch()` essentially always returns 0,
> indicacting to us that we must start searching right at the beginning of
> the block. This works by chance because we now always do a linear scan
> from the start of the block, and thus we would still end up finding the
> desired record. But needless to say, this makes the optimization quite
> useless.
>
> Fix this bug by returning whether the current key is smaller than the
> searched key. As the current behaviour was correct it is not possible to
> write a test. Furthermore it is also not really possible to demonstrate
> in a benchmark that this fix speeds up seeking records.
This is an amusing bug.
I wonder if we inherited it from the original implementation---this
was imported from jgit, right?
Thanks for a detailed write-up.
The "it is a fix, but the breakage is well hidden and cannot be
observed only by checking for correctness" aspect of the bug
deserves the unusually large "number of paragraphs explaining the
change divided by number of changed lines" ratio ;-).
Applied.
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> reftable/block.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/reftable/block.c b/reftable/block.c
> index 72eb73b380..1663030386 100644
> --- a/reftable/block.c
> +++ b/reftable/block.c
> @@ -302,7 +302,7 @@ static int restart_key_less(size_t idx, void *args)
>
> result = strbuf_cmp(&a->key, &rkey);
> strbuf_release(&rkey);
> - return result;
> + return result < 0;
> }
>
> void block_iter_copy_from(struct block_iter *dest, struct block_iter *src)
^ permalink raw reply
* Re: [PATCH v2 2/2] reftable/block: fix binary search over restart counter
From: Junio C Hamano @ 2024-03-08 0:40 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <xmqq7cidk4e4.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> Patrick Steinhardt <ps@pks.im> writes:
>
>> The consequence is that `binsearch()` essentially always returns 0,
>> indicacting to us that we must start searching right at the beginning of
>> the block. This works by chance because we now always do a linear scan
>> from the start of the block, and thus we would still end up finding the
>> desired record. But needless to say, this makes the optimization quite
>> useless.
>> Fix this bug by returning whether the current key is smaller than the
>> searched key. As the current behaviour was correct it is not possible to
>> write a test. Furthermore it is also not really possible to demonstrate
>> in a benchmark that this fix speeds up seeking records.
>
> This is an amusing bug.
Having said all that.
I have to wonder if it is the custom implementation of binsearch()
the reftable/basic.c file has, not this particular comparison
callback. It makes an unusual expectation on the comparison
function, unlike bsearch(3) whose compar(a,b) is expected to return
an answer with the same sign as "a - b".
I just checked the binary search loops we have in the core part of
the system, like the one in hash-lookup.c (which takes advantage of
the random and uniform nature of hashed values to converge faster
than log2) and ones in builtin/pack-objects.c (both of which are
absolute bog-standard). Luckily, we do not use such an unusual
convention (well, we avoid overhead of compar callbacks to begin
with, so it is a bit of apples-to-oranges comparison).
^ permalink raw reply
* Re: [PATCH] trailer: fix comment/cut-line regression with opts->no_divider
From: Linus Arver @ 2024-03-08 2:21 UTC (permalink / raw)
To: Jeff King, Junio C Hamano; +Cc: Philippe Blain, Git mailing list
In-Reply-To: <20240220010936.GA1793660@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Mon, Feb 19, 2024 at 10:42:45AM -0800, Junio C Hamano wrote:
> [...]
>
> The fix itself is pretty simple: instead of returning early, no_divider
> just skips the "---" handling but still calls ignored_log_message_bytes().
I realize I am late to the discussion, but this fix (and patch)
looks right to me. FWIW I independently discovered the same problem and
figured out a fix locally in my larger refactor of this area (with the
same fix, to always call ignored_log_message_bytes() regardless of
no_divider). Thank you Peff!
Sorry for introducing the regression. My enthusiasm in changing things
up without unit tests is regrettable.
^ permalink raw reply
* What's cooking in git.git (Mar 2024, #02; Thu, 7)
From: Junio C Hamano @ 2024-03-08 2:26 UTC (permalink / raw)
To: git
Here are the topics that have been cooking in my tree. Commits
prefixed with '+' are in 'next' (being in 'next' is a sign that a
topic is stable enough to be used and are candidate to be in a
future release). Commits prefixed with '-' are only in 'seen', and
aren't considered "accepted" at all and may be annotated with an URL
to a message that raises issues but they are no means exhaustive. A
topic without enough support may be discarded after a long period of
no activity (of course they can be resubmit when new interests
arise).
The 'maint' branch now points at the 2.44 maintenance track.
Copies of the source code to Git live in many repositories, and the
following is a list of the ones I push into or their mirrors. Some
repositories have only a subset of branches.
With maint, master, next, seen, todo:
git://git.kernel.org/pub/scm/git/git.git/
git://repo.or.cz/alt-git.git/
https://kernel.googlesource.com/pub/scm/git/git/
https://github.com/git/git/
https://gitlab.com/git-vcs/git/
With all the integration branches and topics broken out:
https://github.com/gitster/git/
Even though the preformatted documentation in HTML and man format
are not sources, they are published in these repositories for
convenience (replace "htmldocs" with "manpages" for the manual
pages):
git://git.kernel.org/pub/scm/git/git-htmldocs.git/
https://github.com/gitster/git-htmldocs.git/
Release tarballs are available at:
https://www.kernel.org/pub/software/scm/git/
--------------------------------------------------
[Graduated to 'master']
* ak/rebase-autosquash (2024-02-27) 1 commit
(merged to 'next' on 2024-03-01 at 57a34830b7)
+ rebase: fix typo in autosquash documentation
Originally merged to 'next' on 2024-02-27
Typofix.
source: <pull.1676.git.1709015578890.gitgitgadget@gmail.com>
* cc/rev-list-allow-missing-tips (2024-02-28) 5 commits
(merged to 'next' on 2024-03-01 at fd7b109d04)
+ revision: fix --missing=[print|allow*] for annotated tags
(merged to 'next' on 2024-03-01 at ac0fc94378)
+ rev-list: allow missing tips with --missing=[print|allow*]
+ t6022: fix 'test' style and 'even though' typo
+ oidset: refactor oidset_insert_from_set()
+ revision: clarify a 'return NULL' in get_reference()
Originally merged to 'next' on 2024-02-28
"git rev-list --missing=print" has learned to optionally take
"--allow-missing-tips", which allows the objects at the starting
points to be missing.
source: <20240214142513.4002639-1-christian.couder@gmail.com>
* eg/add-uflags (2024-02-29) 1 commit
(merged to 'next' on 2024-03-01 at 5dbc997561)
+ add: use unsigned type for collection of bits
Originally merged to 'next' on 2024-02-29
Code clean-up practice.
source: <20240229194444.8499-2-giganteeugenio2@gmail.com>
* jc/doc-compat-util (2024-02-27) 1 commit
(merged to 'next' on 2024-03-01 at 89b76c65d7)
+ doc: clarify the wording on <git-compat-util.h> requirement
Originally merged to 'next' on 2024-02-27
Clarify wording in the CodingGuidelines that requires <git-compat-util.h>
to be the first header file.
source: <xmqqle76kdpr.fsf_-_@gitster.g>
* jc/no-include-of-compat-util-from-headers (2024-02-24) 1 commit
(merged to 'next' on 2024-03-01 at ebb921087e)
+ compat: drop inclusion of <git-compat-util.h>
Originally merged to 'next' on 2024-02-26
Header file clean-up.
source: <xmqqwmqtli18.fsf@gitster.g>
* jc/no-lazy-fetch (2024-02-27) 3 commits
(merged to 'next' on 2024-03-01 at 14303cdbfe)
+ git: extend --no-lazy-fetch to work across subprocesses
+ git: document GIT_NO_REPLACE_OBJECTS environment variable
+ git: --no-lazy-fetch option
Originally merged to 'next' on 2024-02-29
"git --no-lazy-fetch cmd" allows to run "cmd" while disabling lazy
fetching of objects from the promisor remote, which may be handy
for debugging.
source: <xmqq1q8xx38i.fsf@gitster.g>
source: <xmqq1q9cl3xv.fsf@gitster.g>
source: <xmqq1q9mmtpw.fsf@gitster.g>
* jk/reflog-special-cases-fix (2024-02-26) 3 commits
(merged to 'next' on 2024-03-01 at 2b67f6e668)
+ 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"
Originally merged to 'next' on 2024-02-27
The logic to access reflog entries by date and number had ugly
corner cases at the boundaries, which have been cleaned up.
source: <20240226100010.GA1214708@coredump.intra.peff.net>
* jk/textconv-cache-outside-repo-fix (2024-02-26) 1 commit
(merged to 'next' on 2024-03-01 at 8508b83758)
+ userdiff: skip textconv caching when not in a repository
Originally merged to 'next' on 2024-02-26
The code incorrectly attempted to use textconv cache when asked,
even when we are not running in a repository, which has been
corrected.
source: <20240226102729.GB2685773@coredump.intra.peff.net>
* jk/upload-pack-bounded-resources (2024-02-28) 9 commits
(merged to 'next' on 2024-03-01 at b70b6f0913)
+ upload-pack: free tree buffers after parsing
+ upload-pack: use PARSE_OBJECT_SKIP_HASH_CHECK in more places
+ upload-pack: always turn off save_commit_buffer
+ upload-pack: disallow object-info capability by default
+ upload-pack: accept only a single packfile-uri line
+ upload-pack: use a strmap for want-ref lines
+ upload-pack: use oidset for deepen_not list
+ upload-pack: switch deepen-not list to an oid_array
+ upload-pack: drop separate v2 "haves" array
Originally merged to 'next' on 2024-02-29
Various parts of upload-pack has been updated to bound the resource
consumption relative to the size of the repository to protect from
abusive clients.
source: <20240228223700.GA1157826@coredump.intra.peff.net>
* jk/upload-pack-v2-capability-cleanup (2024-02-29) 4 commits
(merged to 'next' on 2024-03-01 at 2750893db7)
+ upload-pack: only accept packfile-uris if we advertised it
+ upload-pack: use existing config mechanism for advertisement
+ upload-pack: centralize setup of sideband-all config
+ upload-pack: use repository struct to get config
Originally merged to 'next' on 2024-02-29
The upload-pack program, when talking over v2, accepted the
packfile-uris protocol extension from the client, even if it did
not advertise the capability, which has been corrected.
source: <20240228224625.GA1158651@coredump.intra.peff.net>
* js/merge-tree-3-trees (2024-02-23) 7 commits
(merged to 'next' on 2024-03-01 at a75dc95f04)
+ fill_tree_descriptor(): mark error message for translation
+ cache-tree: avoid an unnecessary check
+ Always check `parse_tree*()`'s return value
+ t4301: verify that merge-tree fails on missing blob objects
+ merge-ort: do check `parse_tree()`'s return value
+ merge-tree: fail with a non-zero exit code on missing tree objects
+ merge-tree: accept 3 trees as arguments
Originally merged to 'next' on 2024-02-28
"git merge-tree" has learned that the three trees involved in the
3-way merge only need to be trees, not necessarily commits.
source: <pull.1647.git.1706277694231.gitgitgadget@gmail.com>
source: <pull.1651.v4.git.1708677266.gitgitgadget@gmail.com>
* js/remove-cruft-files (2024-02-26) 1 commit
(merged to 'next' on 2024-03-01 at 63100a274b)
+ neue: remove a bogus empty file
Originally merged to 'next' on 2024-02-26
Remove an empty file that shouldn't have been added in the first
place.
source: <pull.1674.git.1708958183225.gitgitgadget@gmail.com>
* jt/commit-redundant-scissors-fix (2024-02-27) 2 commits
(merged to 'next' on 2024-03-01 at e5983498f1)
+ commit: unify logic to avoid multiple scissors lines when merging
+ commit: avoid redundant scissor line with --cleanup=scissors -v
Originally merged to 'next' on 2024-02-29
"git commit -v --cleanup=scissors" used to add the scissors line
twice in the log message buffer, which has been corrected.
source: <Zd2eLxPelxvP8FDk@localhost>
* kn/for-all-refs (2024-02-23) 6 commits
(merged to 'next' on 2024-03-01 at 76a1297ace)
+ 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()`
+ Merge branch 'ps/reftable-backend' into kn/for-all-refs
Originally merged to 'next' on 2024-02-27
"git for-each-ref" learned "--include-root-refs" option to show
even the stuff outside the 'refs/' hierarchy.
source: <20240223100112.44127-1-karthik.188@gmail.com>
* ml/log-merge-with-cherry-pick-and-other-pseudo-heads (2024-02-28) 2 commits
(merged to 'next' on 2024-03-01 at 339111ec08)
+ revision: implement `git log --merge` also for rebase/cherry-pick/revert
+ revision: ensure MERGE_HEAD is a ref in prepare_show_merge
Originally merged to 'next' on 2024-02-29
"git log --merge" learned to pay attention to CHERRY_PICK_HEAD and
other kinds of *_HEAD pseudorefs.
source: <20240228-ml-log-merge-with-cherry-pick-and-other-pseudo-heads-v6-0-8ec34c052b39@gmail.com>
* pb/ort-make-submodule-conflict-message-an-advice (2024-02-26) 1 commit
(merged to 'next' on 2024-03-01 at df880cde2e)
+ merge-ort: turn submodule conflict suggestions into an advice
Originally merged to 'next' on 2024-02-27
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.
source: <pull.1661.v2.git.git.1708954048301.gitgitgadget@gmail.com>
* ps/reftable-repo-init-fix (2024-02-27) 2 commits
(merged to 'next' on 2024-03-01 at abbf85051b)
+ refs/reftable: don't fail empty transactions in repo without HEAD
+ Merge branch 'ps/remote-helper-repo-initialization-fix' into ps/reftable-repo-init-fix
(this branch uses ps/remote-helper-repo-initialization-fix.)
Originally merged to 'next' on 2024-02-29
Clear the fallout from a fix for 2.44 regression.
source: <95be968e10bd02c64448786e690bbefe5c082577.1709041721.git.ps@pks.im>
* ps/remote-helper-repo-initialization-fix (2024-02-27) 1 commit
(merged to 'next' on 2024-03-01 at 7b79ffbd8f)
+ builtin/clone: allow remote helpers to detect repo
(this branch is used by ps/reftable-repo-init-fix.)
Originally merged to 'next' on 2024-02-29
A custom remote helper no longer cannot access the newly created
repository during "git clone", which is a regression in Git 2.44.
This has been corrected.
source: <9d888adf92e9a8af7c18847219f97d3e595e3e36.1709041721.git.ps@pks.im>
* rs/fetch-simplify-with-starts-with (2024-02-26) 1 commit
(merged to 'next' on 2024-03-01 at 000e015fff)
+ fetch: convert strncmp() with strlen() to starts_with()
Originally merged to 'next' on 2024-02-27
Code simplification.
source: <cb94b938-03f9-4dd3-84c1-f5244ca81be3@web.de>
* rs/name-rev-with-mempool (2024-02-26) 2 commits
(merged to 'next' on 2024-03-01 at d53eac1836)
+ name-rev: use mem_pool_strfmt()
+ mem-pool: add mem_pool_strfmt()
Originally merged to 'next' on 2024-02-27
Many small allocations "git name-rev" makes have been updated to
allocate from a mem-pool.
source: <20240225113947.89357-1-l.s.r@web.de>
* rs/submodule-prefix-simplify (2024-02-26) 1 commit
(merged to 'next' on 2024-03-01 at 05d4d90201)
+ submodule: use strvec_pushf() for --submodule-prefix
Originally merged to 'next' on 2024-02-27
Code simplification.
source: <8cd983fb-32b9-41c6-a9e7-a485b190488c@web.de>
* sg/upload-pack-error-message-fix (2024-02-26) 1 commit
(merged to 'next' on 2024-03-01 at b94664a7a0)
+ upload-pack: don't send null character in abort message to the client
Originally merged to 'next' on 2024-02-27
An error message from "git upload-pack", which responds to "git
fetch" requests, had a trialing NUL in it, which has been
corrected.
source: <20240225183452.1939334-1-szeder.dev@gmail.com>
--------------------------------------------------
[New Topics]
* ag/t0010-modernize (2024-03-05) 1 commit
(merged to 'next' on 2024-03-07 at 38339abc2d)
+ tests: modernize the test script t0010-racy-git.sh
GSoC practice to modernize a test script.
Will merge to 'master'.
source: <pull.1675.v3.git.1709676557639.gitgitgadget@gmail.com>
* fs/find-end-of-log-message-fix (2024-03-07) 1 commit
- wt-status: don't find scissors line beyond buf len
The code to find the effective end of log message can fall into an
endless loop, which has been corrected.
Waiting for review response.
source: <20240307183743.219951-1-flosch@nutanix.com>
* hd/config-mak-os390 (2024-03-06) 1 commit
(merged to 'next' on 2024-03-07 at 289d3ab691)
+ build: support z/OS (OS/390).
Platform specific tweaks for OS/390 has been added to
config.mak.uname.
Will merge to 'master'.
source: <pull.1663.v4.git.git.1709703857881.gitgitgadget@gmail.com>
* jk/core-comment-string (2024-03-07) 15 commits
- config: allow multi-byte core.commentChar
- environment: drop comment_line_char compatibility macro
- wt-status: drop custom comment-char stringification
- sequencer: handle multi-byte comment characters when writing todo list
- find multi-byte comment chars in unterminated buffers
- find multi-byte comment chars in NUL-terminated strings
- prefer comment_line_str to comment_line_char for printing
- strbuf: accept a comment string for strbuf_add_commented_lines()
- strbuf: accept a comment string for strbuf_commented_addf()
- strbuf: accept a comment string for strbuf_stripspace()
- environment: store comment_line_char as a string
- strbuf: avoid shadowing global comment_line_char name
- commit: refactor base-case of adjust_comment_line_char()
- strbuf: avoid static variables in strbuf_add_commented_lines()
- strbuf: simplify comment-handling in add_lines() helper
core.commentChar used to be limited to a single byte, but has been
updated to allow an arbitrary multi-byte sequence.
Will merge to 'next'?
source: <20240307091407.GA2072522@coredump.intra.peff.net>
* jk/doc-remote-helpers-markup-fix (2024-03-07) 1 commit
- doc/gitremote-helpers: fix missing single-quote
Doc mark-up fix.
Will merge to 'next'.
source: <20240307084313.GA2072022@coredump.intra.peff.net>
* js/build-fuzz-more-often (2024-03-05) 3 commits
- SQUASH???
- fuzz: link fuzz programs with `make all` on Linux
- ci: also define CXX environment variable
In addition to building the objects needed, try to link the objects
that are used in fuzzer tests, to make sure at least they build
without bitrot, in Linux CI runs.
Comments?
source: <cover.1709673020.git.steadmon@google.com>
* kh/branch-ref-syntax-advice (2024-03-05) 5 commits
(merged to 'next' on 2024-03-07 at 914f01967b)
+ 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
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.
Will merge to 'master'.
source: <cover.1709670287.git.code@khaugsbakk.name>
* kh/doc-commentchar-is-a-byte (2024-03-05) 1 commit
(merged to 'next' on 2024-03-06 at 5941655c04)
+ config: document `core.commentChar` as ASCII-only
The "core.commentChar" configuration variable only allows an ASCII
character, which was not clearly documented, which has been
corrected.
Will merge to 'master'.
source: <9633f9be5ddd9ab3df4b79ee934e1ed47e90bd1d.1709656683.git.code@khaugsbakk.name>
* ps/reftable-block-search-fix (2024-03-07) 2 commits
- reftable/block: fix binary search over restart counter
- reftable/record: fix memory leak when decoding object records
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.
Will merge to 'next'?
source: <cover.1709843663.git.ps@pks.im>
* ps/reftable-reflog-iteration-perf (2024-03-05) 8 commits
- 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
- Merge branch 'ps/reftable-iteration-perf-part2' into ps/reftable-reflog-iteration-perf
(this branch uses ps/reftable-iteration-perf-part2.)
The code to iterate over reflogs in the reftable has been optimized
to reduce memory allocation and deallocation.
Needs review.
source: <cover.1709640322.git.ps@pks.im>
* sj/userdiff-c-sharp (2024-03-06) 1 commit
- userdiff: better method/property matching for C#
The userdiff patterns for C# has been updated.
Needs review.
source: <pull.1682.v2.git.git.1709756493673.gitgitgadget@gmail.com>
--------------------------------------------------
[Cooking]
* es/config-doc-sort-sections (2024-02-29) 1 commit
(merged to 'next' on 2024-03-04 at 0752144ed7)
+ docs: sort configuration variable groupings alphabetically
Doc updates.
Will merge to 'master'.
source: <20240229190229.20222-1-ericsunshine@charter.net>
* kh/doc-dashed-commands-have-not-worked-for-a-long-time (2024-03-01) 1 commit
(merged to 'next' on 2024-03-04 at 7e070c67f9)
+ gitcli: drop mention of “non-dashed form”
Doc update.
Will merge to 'master'.
source: <5b34bc4e22816f7f19bd26c15a08fe4c749b72f8.1709316230.git.code@khaugsbakk.name>
* jc/xwrite-cleanup (2024-03-02) 3 commits
(merged to 'next' on 2024-03-07 at 43e66f7e4d)
+ repack: check error writing to pack-objects subprocess
+ sideband: avoid short write(2)
+ unpack: replace xwrite() loop with write_in_full()
Uses of xwrite() helper have been audited and updated for better
error checking and simpler code.
Will merge to 'master'.
source: <20240302190348.3946569-1-gitster@pobox.com>
* jc/test-i18ngrep (2024-03-02) 1 commit
(merged to 'next' on 2024-03-06 at 2c57ebc706)
+ test_i18ngrep: hard deprecate and forbid its use
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.
Will merge to 'master'.
source: <xmqq5xy4zhdc.fsf@gitster.g>
* gt/core-bare-in-templates (2024-03-04) 1 commit
(merged to 'next' on 2024-03-06 at e54ac5acf9)
+ setup: remove unnecessary variable
Code simplification.
Will merge to 'master'.
source: <20240304151811.511780-1-shyamthakkar001@gmail.com>
* so/clean-dry-run-without-force (2024-03-04) 2 commits
(merged to 'next' on 2024-03-06 at ccf2e123be)
+ clean: further clean-up of implementation around "--force"
+ clean: improve -n and -f implementation and documentation
The implementation in "git clean" that makes "-n" and "-i" ignore
clean.requireForce has been simplified, together with the
documentation.
Will merge to 'master'.
source: <87le6ziqzb.fsf_-_@osv.gnss.ru>
source: <20240303220600.2491792-1-gitster@pobox.com>
* jh/trace2-missing-def-param-fix (2024-03-07) 3 commits
- 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
Some trace2 events that lacked def_param have learned to show it,
enriching the output.
Reviewed-by: Josh Steadmon <steadmon@google.com>
cf. <ZejkVOVQBZhLVfHW@google.com>
Will merge to 'next'.
source: <pull.1679.v2.git.1709824949.gitgitgadget@gmail.com>
* ps/reftable-stack-tempfile (2024-03-07) 4 commits
- 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
The code in reftable backend that creates new table files works
better with the tempfile framework to avoid leaving cruft after a
failure.
Will merge to 'next'?
source: <cover.1709816483.git.ps@pks.im>
* rs/opt-parse-long-fixups (2024-03-03) 6 commits
- parse-options: rearrange long_name matching code
- parse-options: normalize arg and long_name before comparison
- parse-options: detect ambiguous self-negation
- parse-options: factor out register_abbrev() and struct parsed_option
- parse-options: set arg of abbreviated option lazily
- parse-options: recognize abbreviated negated option with arg
The parse-options code that deals with abbreviated long option
names have been cleaned up.
Needs review.
source: <20240303121944.20627-1-l.s.r@web.de>
* sj/t9117-path-is-file (2024-03-04) 1 commit
(merged to 'next' on 2024-03-04 at de5f6a74cb)
+ t9117: prefer test_path_* helper functions
GSoC practice to replace "test -f" with "test_path_is_file".
Will merge to 'master'.
source: <20240304095436.56399-2-shejialuo@gmail.com>
* vm/t7301-use-test-path-helpers (2024-03-06) 1 commit
(merged to 'next' on 2024-03-07 at e638654635)
+ t7301: use test_path_is_(missing|file)
GSoC practice to replace "test -f" with "test_path_is_file".
Will merge to 'master'.
source: <20240304171732.64457-2-vincenzo.mezzela@gmail.com>
* cw/git-std-lib (2024-02-28) 4 commits
. SQUASH??? get rid of apparent debugging crufts
. test-stdlib: show that git-std-lib is independent
. git-std-lib: introduce Git Standard Library
. pager: include stdint.h because uintmax_t is used
Split libgit.a out to a separate git-std-lib tor easier reuse.
Expecting a reroll.
source: <cover.1696021277.git.jonathantanmy@google.com>
* js/merge-base-with-missing-commit (2024-02-29) 11 commits
(merged to 'next' on 2024-03-01 at 3e3eabaee9)
+ 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
Originally merged to 'next' on 2024-02-29
Make sure failure return from merge_bases_many() is properly caught.
Needs an incremental fix-up.
cf.<20240301065805.GB2680308@coredump.intra.peff.net>
source: <pull.1657.v4.git.1709113457.gitgitgadget@gmail.com>
* rj/complete-worktree-paths-fix (2024-02-27) 1 commit
(merged to 'next' on 2024-03-06 at b6ba949383)
+ completion: fix __git_complete_worktree_paths
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.
Will merge to 'master'.
source: <b8f09e20-d0d3-4e0b-afe2-31affeb61052@gmail.com>
* rs/t-ctype-simplify (2024-03-03) 4 commits
(merged to 'next' on 2024-03-04 at 9bd84a8877)
+ 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
Code simplification to one unit-test program.
Will merge to 'master'.
source: <20240303101330.20187-1-l.s.r@web.de>
* pw/rebase-i-ignore-cherry-pick-help-environment (2024-02-27) 1 commit
- rebase -i: stop setting GIT_CHERRY_PICK_HELP
Code simplification by getting rid of code that sets an environment
variable that is no longer used.
Will merge to 'next'.
source: <pull.1678.git.1709042783847.gitgitgadget@gmail.com>
* as/option-names-in-messages (2024-03-05) 4 commits
(merged to 'next' on 2024-03-07 at 73ab51faba)
+ revision.c: trivial fix to message
+ builtin/clone.c: trivial fix of message
+ builtin/remote.c: trivial fix of error message
+ transport-helper.c: trivial fix of error message
Error message updates.
Will merge to 'master'.
source: <20240216101647.28837-1-ash@kambanaria.org>
* jh/fsmonitor-icase-corner-case-fix (2024-03-06) 14 commits
(merged to 'next' on 2024-03-06 at 356eafea7e)
+ 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()
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>
Will merge to 'master'.
source: <pull.1662.v3.git.1708983565.gitgitgadget@gmail.com>
* ps/reftable-iteration-perf-part2 (2024-03-04) 13 commits
(merged to 'next' on 2024-03-06 at e8ba314585)
+ 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
(this branch is used by ps/reftable-reflog-iteration-perf.)
The code to iterate over refs with the reftable backend has seen
some optimization.
Will merge to 'master'.
source: <cover.1709548907.git.ps@pks.im>
* js/cmake-with-test-tool (2024-02-23) 2 commits
- cmake: let `test-tool` run the unit tests, too
- Merge branch 'js/unit-test-suite-runner' into js/cmake-with-test-tool
(this branch uses js/unit-test-suite-runner.)
"test-tool" is now built in CMake build to also run the unit tests.
May want to roll it into the base topic.
source: <pull.1666.git.1708038924522.gitgitgadget@gmail.com>
* js/unit-test-suite-runner (2024-02-23) 8 commits
- ci: use test-tool as unit test runner on Windows
- t/Makefile: run unit tests alongside shell tests
- unit tests: add rule for running with test-tool
- test-tool run-command testsuite: support unit tests
- test-tool run-command testsuite: remove hardcoded filter
- test-tool run-command testsuite: get shell from env
- t0080: turn t-basic unit test into a helper
- Merge branch 'jk/unit-tests-buildfix' into js/unit-test-suite-runner
(this branch is used by js/cmake-with-test-tool.)
The "test-tool" has been taught to run testsuite tests in parallel,
bypassing the need to use the "prove" tool.
Needs review.
source: <cover.1708728717.git.steadmon@google.com>
* rj/complete-reflog (2024-03-03) 5 commits
(merged to 'next' on 2024-03-06 at 0f1a25debc)
+ 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"
The command line completion script (in contrib/) learned to
complete "git reflog" better.
Will merge to 'master'.
source: <ea6c8890-9ff3-46c9-b933-6a52083b1001@gmail.com>
* bk/complete-dirname-for-am-and-format-patch (2024-01-12) 1 commit
- completion: dir-type optargs for am, format-patch
Command line completion support (in contrib/) has been
updated for a few commands to complete directory names where a
directory name is expected.
Expecting a reroll.
cf. <40c3a824-a961-490b-94d4-4eb23c8f713d@gmail.com>
cf. <6683f24e-7e56-489d-be2d-8afe1fc38d2b@gmail.com>
source: <d37781c3-6af2-409b-95a8-660a9b92d20b@smtp-relay.sendinblue.com>
* bk/complete-send-email (2024-01-12) 1 commit
- completion: don't complete revs when --no-format-patch
Command line completion support (in contrib/) has been taught to
avoid offering revision names as candidates to "git send-email" when
the command is used to send pre-generated files.
Expecting a reroll.
cf. <CAC4O8c88Z3ZqxH2VVaNPpEGB3moL5dJcg3cOWuLWwQ_hLrJMtA@mail.gmail.com>
source: <a718b5ee-afb0-44bd-a299-3208fac43506@smtp-relay.sendinblue.com>
* la/trailer-api (2024-03-01) 9 commits
(merged to 'next' on 2024-03-06 at f119923ff6)
+ 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
Trailer API updates.
Acked-by: Christian Couder <christian.couder@gmail.com>
cf. <CAP8UFD1Zd+9q0z1JmfOf60S2vn5-sD3SafDvAJUzRFwHJKcb8A@mail.gmail.com>
Will merge to 'master'.
source: <pull.1632.v6.git.1709252086.gitgitgadget@gmail.com>
* tb/path-filter-fix (2024-01-31) 16 commits
- bloom: introduce `deinit_bloom_filters()`
- commit-graph: reuse existing Bloom filters where possible
- object.h: fix mis-aligned flag bits table
- commit-graph: new Bloom filter version that fixes murmur3
- commit-graph: unconditionally load Bloom filters
- bloom: prepare to discard incompatible Bloom filters
- bloom: annotate filters with hash version
- repo-settings: introduce commitgraph.changedPathsVersion
- t4216: test changed path filters with high bit paths
- t/helper/test-read-graph: implement `bloom-filters` mode
- bloom.h: make `load_bloom_filter_from_graph()` public
- t/helper/test-read-graph.c: extract `dump_graph_info()`
- gitformat-commit-graph: describe version 2 of BDAT
- commit-graph: ensure Bloom filters are read with consistent settings
- revision.c: consult Bloom filters for root commits
- t/t4216-log-bloom.sh: harden `test_bloom_filters_not_used()`
The Bloom filter used for path limited history traversal was broken
on systems whose "char" is unsigned; update the implementation and
bump the format version to 2.
Waiting for a final ack?
cf. <ZcFjkfbsBfk7JQIH@nand.local>
source: <cover.1706741516.git.me@ttaylorr.com>
* eb/hash-transition (2023-10-02) 30 commits
- t1016-compatObjectFormat: add tests to verify the conversion between objects
- t1006: test oid compatibility with cat-file
- t1006: rename sha1 to oid
- test-lib: compute the compatibility hash so tests may use it
- builtin/ls-tree: let the oid determine the output algorithm
- object-file: handle compat objects in check_object_signature
- tree-walk: init_tree_desc take an oid to get the hash algorithm
- builtin/cat-file: let the oid determine the output algorithm
- rev-parse: add an --output-object-format parameter
- repository: implement extensions.compatObjectFormat
- object-file: update object_info_extended to reencode objects
- object-file-convert: convert commits that embed signed tags
- object-file-convert: convert commit objects when writing
- object-file-convert: don't leak when converting tag objects
- object-file-convert: convert tag objects when writing
- object-file-convert: add a function to convert trees between algorithms
- object: factor out parse_mode out of fast-import and tree-walk into in object.h
- cache: add a function to read an OID of a specific algorithm
- tag: sign both hashes
- commit: export add_header_signature to support handling signatures on tags
- commit: convert mergetag before computing the signature of a commit
- commit: write commits for both hashes
- object-file: add a compat_oid_in parameter to write_object_file_flags
- object-file: update the loose object map when writing loose objects
- loose: compatibilty short name support
- loose: add a mapping between SHA-1 and SHA-256 for loose objects
- repository: add a compatibility hash algorithm
- object-names: support input of oids in any supported hash
- oid-array: teach oid-array to handle multiple kinds of oids
- object-file-convert: stubs for converting from one object format to another
Teach a repository to work with both SHA-1 and SHA-256 hash algorithms.
Will merge to and cook in 'next'.
cf. <xmqqv86z5359.fsf@gitster.g>
source: <878r8l929e.fsf@gmail.froward.int.ebiederm.org>
* jc/rerere-cleanup (2023-08-25) 4 commits
- rerere: modernize use of empty strbuf
- rerere: try_merge() should use LL_MERGE_ERROR when it means an error
- rerere: fix comment on handle_file() helper
- rerere: simplify check_one_conflict() helper function
Code clean-up.
Not ready to be reviewed yet.
source: <20230824205456.1231371-1-gitster@pobox.com>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox