* Re: What's cooking in git.git (Sep 2023, #05; Fri, 15)
From: Linus Arver @ 2023-09-20 19:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqjzskdh9t.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> Linus Arver <linusa@google.com> writes:
>
>>> Whatever improvements you have in mind, if they are
>>> minor, letting the above graduate (they have been in 'next' for a
>>> week without anybody complaining) and doing them as a follow-up
>>> series would be sensible, I would think.
>>>
>>> Thanks.
>>
>> Hmm, I don't think they are minor? See
>> https://github.com/listx/git/tree/trailer-libification-prep for the
>> current state of things.
>
> They do not look like "oops the patches I sent to the list was
> totally bogus and they need replacing"; aren't they rather "these
> are not yet complete and there are some of the follow-on work on top
> of them"?
Yes, precisely.
>> I need to still follow up to your last comment on "trailer: rename
>> *_DEFAULT enums to *_UNSPECIFIED" [2] (I was going to see if we needed
>> the "obvious solution" as you described).
>>
>> If it's too painful to move this out of 'next' now, I'm OK with it
>> graduating as is and doing a separate follow-up (I expect several more
>> of these to happen anyway). Up to you.
>
> Let's mark the topic as "waiting for follow-up updates" and keep it
> in 'next' until the follow-up patches become ready, then.
Sounds good.
>> Sorry for not noticing that this was in 'next' sooner and communicating
>> accordingly.
>
> Gotchas happen. Let's try to communicate better the next time.
>
> Thanks.
Thanks!
^ permalink raw reply
* [PATCH 2/2] ref-filter: add mailmap support
From: Kousik Sanagavarapu @ 2023-09-20 19:05 UTC (permalink / raw)
To: git; +Cc: Kousik Sanagavarapu, Christian Couder, Hariom Verma
In-Reply-To: <20230920191654.6133-1-five231003@gmail.com>
Add mailmap support to ref-filter formats which are similar in
pretty. This support is such that the following pretty placeholders are
equivalent to the new ref-filter atoms:
%aN = authorname:mailmap
%cN = committername:mailmap
%aE = authoremail:mailmap
%aL = authoremail:mailmap,localpart
%cE = committeremail:mailmap
%cL = committeremail:mailmap,localpart
Additionally, mailmap can also be used with ":trim" option for email by
doing something like "authoremail:mailmap,trim".
The above also applies for the "tagger" atom, that is,
"taggername:mailmap", "taggeremail:mailmap", "taggeremail:mailmap,trim"
and "taggername:mailmap,localpart".
The functionality of ":trim" and ":localpart" remains the same. That is,
":trim" gives the email, but without the angle brackets and ":localpart"
gives the part of the email before the '@' character (if such a
character is not found then we directly grab everything between the
angle brackets).
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---
Documentation/git-for-each-ref.txt | 6 +-
ref-filter.c | 152 ++++++++++++++++++++++-------
t/t6300-for-each-ref.sh | 83 ++++++++++++++++
3 files changed, 205 insertions(+), 36 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 11b2bc3121..e86d5700dd 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -303,7 +303,11 @@ Fields that have name-email-date tuple as its value (`author`,
and `date` to extract the named component. For email fields (`authoremail`,
`committeremail` and `taggeremail`), `:trim` can be appended to get the email
without angle brackets, and `:localpart` to get the part before the `@` symbol
-out of the trimmed email.
+out of the trimmed email. In addition to these, the `:mailmap` option and the
+corresponding `:mailmap,trim` and `:mailmap,localpart` can be used (order does
+not matter) to get values of the name and email according to the .mailmap file
+or according to the file set in the mailmap.file or mailmap.blob configuration
+variable (see linkgit:gitmailmap[5]).
The raw data in an object is `raw`.
diff --git a/ref-filter.c b/ref-filter.c
index fae9f4b8ed..e4d3510e28 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -13,6 +13,8 @@
#include "oid-array.h"
#include "repository.h"
#include "commit.h"
+#include "mailmap.h"
+#include "ident.h"
#include "remote.h"
#include "color.h"
#include "tag.h"
@@ -215,8 +217,16 @@ static struct used_atom {
struct {
enum { O_SIZE, O_SIZE_DISK } option;
} objectsize;
- struct email_option {
- enum { EO_RAW, EO_TRIM, EO_LOCALPART } option;
+ struct {
+ enum { N_RAW, N_MAILMAP } option;
+ } name_option;
+ struct {
+ enum {
+ EO_RAW = 0,
+ EO_TRIM = 1<<0,
+ EO_LOCALPART = 1<<1,
+ EO_MAILMAP = 1<<2,
+ } option;
} email_option;
struct {
enum { S_BARE, S_GRADE, S_SIGNER, S_KEY,
@@ -720,21 +730,55 @@ static int oid_atom_parser(struct ref_format *format UNUSED,
return 0;
}
-static int person_email_atom_parser(struct ref_format *format UNUSED,
- struct used_atom *atom,
- const char *arg, struct strbuf *err)
+static int person_name_atom_parser(struct ref_format *format UNUSED,
+ struct used_atom *atom,
+ const char *arg, struct strbuf *err)
{
if (!arg)
- atom->u.email_option.option = EO_RAW;
- else if (!strcmp(arg, "trim"))
- atom->u.email_option.option = EO_TRIM;
- else if (!strcmp(arg, "localpart"))
- atom->u.email_option.option = EO_LOCALPART;
+ atom->u.name_option.option = N_RAW;
+ else if (!strcmp(arg, "mailmap"))
+ atom->u.name_option.option = N_MAILMAP;
else
return err_bad_arg(err, atom->name, arg);
return 0;
}
+static int email_atom_option_parser(struct used_atom *atom,
+ const char **arg, struct strbuf *err)
+{
+ if (!*arg)
+ return EO_RAW;
+ if (skip_prefix(*arg, "trim", arg))
+ return EO_TRIM;
+ if (skip_prefix(*arg, "localpart", arg))
+ return EO_LOCALPART;
+ if (skip_prefix(*arg, "mailmap", arg))
+ return EO_MAILMAP;
+ return -1;
+}
+
+static int person_email_atom_parser(struct ref_format *format UNUSED,
+ struct used_atom *atom,
+ const char *arg, struct strbuf *err)
+{
+ for (;;) {
+ int opt = email_atom_option_parser(atom, &arg, err);
+ const char *bad_arg = arg;
+
+ if (opt < 0)
+ return err_bad_arg(err, atom->name, bad_arg);
+ atom->u.email_option.option |= opt;
+
+ if (!arg || !*arg)
+ break;
+ if (*arg == ',')
+ arg++;
+ else
+ return err_bad_arg(err, atom->name, bad_arg);
+ }
+ return 0;
+}
+
static int refname_atom_parser(struct ref_format *format UNUSED,
struct used_atom *atom,
const char *arg, struct strbuf *err)
@@ -877,15 +921,15 @@ static struct {
[ATOM_TYPE] = { "type", SOURCE_OBJ },
[ATOM_TAG] = { "tag", SOURCE_OBJ },
[ATOM_AUTHOR] = { "author", SOURCE_OBJ },
- [ATOM_AUTHORNAME] = { "authorname", SOURCE_OBJ },
+ [ATOM_AUTHORNAME] = { "authorname", SOURCE_OBJ, FIELD_STR, person_name_atom_parser },
[ATOM_AUTHOREMAIL] = { "authoremail", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },
[ATOM_AUTHORDATE] = { "authordate", SOURCE_OBJ, FIELD_TIME },
[ATOM_COMMITTER] = { "committer", SOURCE_OBJ },
- [ATOM_COMMITTERNAME] = { "committername", SOURCE_OBJ },
+ [ATOM_COMMITTERNAME] = { "committername", SOURCE_OBJ, FIELD_STR, person_name_atom_parser },
[ATOM_COMMITTEREMAIL] = { "committeremail", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },
[ATOM_COMMITTERDATE] = { "committerdate", SOURCE_OBJ, FIELD_TIME },
[ATOM_TAGGER] = { "tagger", SOURCE_OBJ },
- [ATOM_TAGGERNAME] = { "taggername", SOURCE_OBJ },
+ [ATOM_TAGGERNAME] = { "taggername", SOURCE_OBJ, FIELD_STR, person_name_atom_parser },
[ATOM_TAGGEREMAIL] = { "taggeremail", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },
[ATOM_TAGGERDATE] = { "taggerdate", SOURCE_OBJ, FIELD_TIME },
[ATOM_CREATOR] = { "creator", SOURCE_OBJ },
@@ -1486,32 +1530,49 @@ static const char *copy_name(const char *buf)
return xstrdup("");
}
+static const char *find_end_of_email(const char *email, int opt)
+{
+ const char *eoemail;
+
+ if (opt & EO_LOCALPART) {
+ eoemail = strchr(email, '@');
+ if (eoemail)
+ return eoemail;
+ return strchr(email, '>');
+ }
+
+ if (opt & EO_TRIM)
+ return strchr(email, '>');
+
+ /*
+ * The option here is either the raw email option or the raw
+ * mailmap option (that is EO_RAW or EO_MAILMAP). In such cases,
+ * we directly grab the whole email including the closing
+ * angle brackets.
+ *
+ * If EO_MAILMAP was set with any other option (that is either
+ * EO_TRIM or EO_LOCALPART), we already grab the end of email
+ * above.
+ */
+ eoemail = strchr(email, '>');
+ if (eoemail)
+ eoemail++;
+ return eoemail;
+}
+
static const char *copy_email(const char *buf, struct used_atom *atom)
{
const char *email = strchr(buf, '<');
const char *eoemail;
+ int opt = atom->u.email_option.option;
+
if (!email)
return xstrdup("");
- switch (atom->u.email_option.option) {
- case EO_RAW:
- eoemail = strchr(email, '>');
- if (eoemail)
- eoemail++;
- break;
- case EO_TRIM:
- email++;
- eoemail = strchr(email, '>');
- break;
- case EO_LOCALPART:
+
+ if (opt & (EO_LOCALPART | EO_TRIM))
email++;
- eoemail = strchr(email, '@');
- if (!eoemail)
- eoemail = strchr(email, '>');
- break;
- default:
- BUG("unknown email option");
- }
+ eoemail = find_end_of_email(email, opt);
if (!eoemail)
return xstrdup("");
return xmemdupz(email, eoemail - email);
@@ -1572,16 +1633,23 @@ static void grab_date(const char *buf, struct atom_value *v, const char *atomnam
v->value = 0;
}
+static struct string_list mailmap = STRING_LIST_INIT_NODUP;
+
/* See grab_values */
static void grab_person(const char *who, struct atom_value *val, int deref, void *buf)
{
int i;
int wholen = strlen(who);
const char *wholine = NULL;
+ const char *headers[] = { "author ", "committer ",
+ "tagger ", NULL };
for (i = 0; i < used_atom_cnt; i++) {
- const char *name = used_atom[i].name;
+ struct used_atom *atom = &used_atom[i];
+ const char *name = atom->name;
struct atom_value *v = &val[i];
+ struct strbuf mailmap_buf = STRBUF_INIT;
+
if (!!deref != (*name == '*'))
continue;
if (deref)
@@ -1589,22 +1657,36 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void
if (strncmp(who, name, wholen))
continue;
if (name[wholen] != 0 &&
- strcmp(name + wholen, "name") &&
+ !starts_with(name + wholen, "name") &&
!starts_with(name + wholen, "email") &&
!starts_with(name + wholen, "date"))
continue;
- if (!wholine)
+
+ if ((starts_with(name + wholen, "name") &&
+ (atom->u.name_option.option == N_MAILMAP)) ||
+ (starts_with(name + wholen, "email") &&
+ (atom->u.email_option.option & EO_MAILMAP))) {
+ if (!mailmap.items)
+ read_mailmap(&mailmap);
+ strbuf_addstr(&mailmap_buf, buf);
+ apply_mailmap_to_header(&mailmap_buf, headers, &mailmap);
+ wholine = find_wholine(who, wholen, mailmap_buf.buf);
+ } else {
wholine = find_wholine(who, wholen, buf);
+ }
+
if (!wholine)
return; /* no point looking for it */
if (name[wholen] == 0)
v->s = copy_line(wholine);
- else if (!strcmp(name + wholen, "name"))
+ else if (starts_with(name + wholen, "name"))
v->s = copy_name(wholine);
else if (starts_with(name + wholen, "email"))
v->s = copy_email(wholine, &used_atom[i]);
else if (starts_with(name + wholen, "date"))
grab_date(wholine, v, name);
+
+ strbuf_release(&mailmap_buf);
}
/*
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 15b4622f57..1fb464ca50 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -25,6 +25,13 @@ test_expect_success setup '
disklen sha1:138
disklen sha256:154
EOF
+
+ # setup .mailmap
+ cat >.mailmap <<-EOF &&
+ A Thor <athor@example.com> A U Thor <author@example.com>
+ C Mitter <cmitter@example.com> C O Mitter <committer@example.com>
+ EOF
+
setdate_and_increment &&
echo "Using $datestamp" > one &&
git add one &&
@@ -141,15 +148,31 @@ test_atom head '*objectname' ''
test_atom head '*objecttype' ''
test_atom head author 'A U Thor <author@example.com> 1151968724 +0200'
test_atom head authorname 'A U Thor'
+test_atom head authorname:mailmap 'A Thor'
test_atom head authoremail '<author@example.com>'
test_atom head authoremail:trim 'author@example.com'
test_atom head authoremail:localpart 'author'
+test_atom head authoremail:trim,localpart 'author'
+test_atom head authoremail:mailmap '<athor@example.com>'
+test_atom head authoremail:mailmap,trim 'athor@example.com'
+test_atom head authoremail:trim,mailmap 'athor@example.com'
+test_atom head authoremail:mailmap,localpart 'athor'
+test_atom head authoremail:localpart,mailmap 'athor'
+test_atom head authoremail:mailmap,trim,localpart,mailmap,trim 'athor'
test_atom head authordate 'Tue Jul 4 01:18:44 2006 +0200'
test_atom head committer 'C O Mitter <committer@example.com> 1151968723 +0200'
test_atom head committername 'C O Mitter'
+test_atom head committername:mailmap 'C Mitter'
test_atom head committeremail '<committer@example.com>'
test_atom head committeremail:trim 'committer@example.com'
test_atom head committeremail:localpart 'committer'
+test_atom head committeremail:localpart,trim 'committer'
+test_atom head committeremail:mailmap '<cmitter@example.com>'
+test_atom head committeremail:mailmap,trim 'cmitter@example.com'
+test_atom head committeremail:trim,mailmap 'cmitter@example.com'
+test_atom head committeremail:mailmap,localpart 'cmitter'
+test_atom head committeremail:localpart,mailmap 'cmitter'
+test_atom head committeremail:trim,mailmap,trim,trim,localpart 'cmitter'
test_atom head committerdate 'Tue Jul 4 01:18:43 2006 +0200'
test_atom head tag ''
test_atom head tagger ''
@@ -199,22 +222,46 @@ test_atom tag '*objectname' $(git rev-parse refs/tags/testtag^{})
test_atom tag '*objecttype' 'commit'
test_atom tag author ''
test_atom tag authorname ''
+test_atom tag authorname:mailmap ''
test_atom tag authoremail ''
test_atom tag authoremail:trim ''
test_atom tag authoremail:localpart ''
+test_atom tag authoremail:trim,localpart ''
+test_atom tag authoremail:mailmap ''
+test_atom tag authoremail:mailmap,trim ''
+test_atom tag authoremail:trim,mailmap ''
+test_atom tag authoremail:mailmap,localpart ''
+test_atom tag authoremail:localpart,mailmap ''
+test_atom tag authoremail:mailmap,trim,localpart,mailmap,trim ''
test_atom tag authordate ''
test_atom tag committer ''
test_atom tag committername ''
+test_atom tag committername:mailmap ''
test_atom tag committeremail ''
test_atom tag committeremail:trim ''
test_atom tag committeremail:localpart ''
+test_atom tag committeremail:localpart,trim ''
+test_atom tag committeremail:mailmap ''
+test_atom tag committeremail:mailmap,trim ''
+test_atom tag committeremail:trim,mailmap ''
+test_atom tag committeremail:mailmap,localpart ''
+test_atom tag committeremail:localpart,mailmap ''
+test_atom tag committeremail:trim,mailmap,trim,trim,localpart ''
test_atom tag committerdate ''
test_atom tag tag 'testtag'
test_atom tag tagger 'C O Mitter <committer@example.com> 1151968725 +0200'
test_atom tag taggername 'C O Mitter'
+test_atom tag taggername:mailmap 'C Mitter'
test_atom tag taggeremail '<committer@example.com>'
test_atom tag taggeremail:trim 'committer@example.com'
test_atom tag taggeremail:localpart 'committer'
+test_atom tag taggeremail:trim,localpart 'committer'
+test_atom tag taggeremail:mailmap '<cmitter@example.com>'
+test_atom tag taggeremail:mailmap,trim 'cmitter@example.com'
+test_atom tag taggeremail:trim,mailmap 'cmitter@example.com'
+test_atom tag taggeremail:mailmap,localpart 'cmitter'
+test_atom tag taggeremail:localpart,mailmap 'cmitter'
+test_atom tag taggeremail:trim,mailmap,trim,localpart,localpart 'cmitter'
test_atom tag taggerdate 'Tue Jul 4 01:18:45 2006 +0200'
test_atom tag creator 'C O Mitter <committer@example.com> 1151968725 +0200'
test_atom tag creatordate 'Tue Jul 4 01:18:45 2006 +0200'
@@ -284,9 +331,45 @@ test_bad_atom() {
test_bad_atom head 'authoremail:foo' \
'fatal: unrecognized %(authoremail) argument: foo'
+test_bad_atom head 'authoremail:mailmap,trim,bar' \
+ 'fatal: unrecognized %(authoremail) argument: bar'
+
+test_bad_atom head 'authoremail:trim,' \
+ 'fatal: unrecognized %(authoremail) argument: '
+
+test_bad_atom head 'authoremail:mailmaptrim' \
+ 'fatal: unrecognized %(authoremail) argument: trim'
+
+test_bad_atom head 'committeremail: ' \
+ 'fatal: unrecognized %(committeremail) argument: '
+
+test_bad_atom head 'committeremail: trim,foo' \
+ 'fatal: unrecognized %(committeremail) argument: trim,foo'
+
+test_bad_atom head 'committeremail:mailmap,localpart ' \
+ 'fatal: unrecognized %(committeremail) argument: '
+
+test_bad_atom head 'committeremail:trim_localpart' \
+ 'fatal: unrecognized %(committeremail) argument: _localpart'
+
+test_bad_atom head 'committeremail:localpart,,,trim' \
+ 'fatal: unrecognized %(committeremail) argument: ,,trim'
+
+test_bad_atom tag 'taggeremail:mailmap,trim, foo ' \
+ 'fatal: unrecognized %(taggeremail) argument: foo '
+
+test_bad_atom tag 'taggeremail:trim,localpart,' \
+ 'fatal: unrecognized %(taggeremail) argument: '
+
+test_bad_atom tag 'taggeremail:mailmap;localpart trim' \
+ 'fatal: unrecognized %(taggeremail) argument: ;localpart trim'
+
test_bad_atom tag 'taggeremail:localpart trim' \
'fatal: unrecognized %(taggeremail) argument: trim'
+test_bad_atom tag 'taggeremail:mailmap,mailmap,trim,qux,localpart,trim' \
+ 'fatal: unrecognized %(taggeremail) argument: qux,localpart,trim'
+
test_date () {
f=$1 &&
committer_date=$2 &&
--
2.42.0.160.g6905eb16ce.dirty
^ permalink raw reply related
* [PATCH 1/2] t/t6300: introduce test_bad_atom()
From: Kousik Sanagavarapu @ 2023-09-20 19:05 UTC (permalink / raw)
To: git; +Cc: Kousik Sanagavarapu, Christian Couder, Hariom Verma
In-Reply-To: <20230920191654.6133-1-five231003@gmail.com>
Introduce a new function "test_bad_atom()", which is similar to
"test_atom()" but should be used to check whether the correct error
message is shown on stderr.
Like "test_atom()", the new function takes three arguments. The three
arguments specify the ref, the format and the expected error message
respectively, with an optional fourth argument for tweaking
"test_expect_*" (which is by default "success").
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---
t/t6300-for-each-ref.sh | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 7b943fd34c..15b4622f57 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -267,6 +267,26 @@ test_expect_success 'arguments to %(objectname:short=) must be positive integers
test_must_fail git for-each-ref --format="%(objectname:short=foo)"
'
+test_bad_atom() {
+ case "$1" in
+ head) ref=refs/heads/main ;;
+ tag) ref=refs/tags/testtag ;;
+ sym) ref=refs/heads/sym ;;
+ *) ref=$1 ;;
+ esac
+ printf '%s\n' "$3">expect
+ test_expect_${4:-success} $PREREQ "err basic atom: $1 $2" "
+ test_must_fail git for-each-ref --format='%($2)' $ref 2>actual &&
+ test_cmp expect actual
+ "
+}
+
+test_bad_atom head 'authoremail:foo' \
+ 'fatal: unrecognized %(authoremail) argument: foo'
+
+test_bad_atom tag 'taggeremail:localpart trim' \
+ 'fatal: unrecognized %(taggeremail) argument: trim'
+
test_date () {
f=$1 &&
committer_date=$2 &&
--
2.42.0.160.g6905eb16ce.dirty
^ permalink raw reply related
* [PATCH 0/2] Add mailmap support to ref-filter
From: Kousik Sanagavarapu @ 2023-09-20 19:05 UTC (permalink / raw)
To: git; +Cc: Kousik Sanagavarapu
Add mailmap support to ref-filter, making ref-filter and pretty closer, which
is a part of the effort made to unify both ref-filter and pretty.
PATCH 1/2 - Introduces the "test_bad_atom()" function which checks for if the
given error message (either due to "err_bad_arg()" or any other err)
is correct.
PATCH 2/2 - The actual mailmap support.
Kousik Sanagavarapu (2):
t/t6300: introduce test_bad_atom()
ref-filter: add mailmap support
Documentation/git-for-each-ref.txt | 6 +-
ref-filter.c | 152 ++++++++++++++++++++++-------
t/t6300-for-each-ref.sh | 103 +++++++++++++++++++
3 files changed, 225 insertions(+), 36 deletions(-)
--
2.42.0.160.g6905eb16ce.dirty
^ permalink raw reply
* Re: [PATCH] completion: loosen and document the requirement around completing alias
From: Junio C Hamano @ 2023-09-20 18:46 UTC (permalink / raw)
To: Philippe Blain; +Cc: git
In-Reply-To: <xmqqy1h08zsp.fsf_-_@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> * We've discussed this when we reviewed the topic that just hit
> 'master'. Before we forget...
>
> contrib/completion/git-completion.bash | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
And here is an obligatory "test" update I will squash into the main
patch. The new ones copy a canonical ": git <cmd> ; ..." test and
remove 'git' and also the space before the semicolon.
diff --git c/t/t9902-completion.sh w/t/t9902-completion.sh
index 47e20fb8b1..a7c3b4eb63 100755
--- c/t/t9902-completion.sh
+++ w/t/t9902-completion.sh
@@ -2464,6 +2464,24 @@ test_expect_success 'completion used <cmd> completion for alias: !f() { : git <c
EOF
'
+test_expect_success 'completion used <cmd> completion for alias: !f() { : <cmd> ; ... }' '
+ test_config alias.co "!f() { : checkout ; if ... } f" &&
+ test_completion "git co m" <<-\EOF
+ main Z
+ mybranch Z
+ mytag Z
+ EOF
+'
+
+test_expect_success 'completion used <cmd> completion for alias: !f() { : <cmd>; ... }' '
+ test_config alias.co "!f() { : checkout; if ... } f" &&
+ test_completion "git co m" <<-\EOF
+ main Z
+ mybranch Z
+ mytag Z
+ EOF
+'
+
test_expect_success 'completion without explicit _git_xxx function' '
test_completion "git version --" <<-\EOF
--build-options Z
^ permalink raw reply related
* [PATCH] completion: loosen and document the requirement around completing alias
From: Junio C Hamano @ 2023-09-20 18:28 UTC (permalink / raw)
To: Philippe Blain; +Cc: git
In-Reply-To: <pull.1585.v2.git.1694538135853.gitgitgadget@gmail.com>
Recently we started to tell users to spell ": git foo ;" with
space(s) around 'foo' for an alias to be completed similarly
to the 'git foo' command. It however is easy to also allow users to
spell it in a more natural way with the semicolon attached to 'foo',
i.e. ": git foo;". Also, add a comment to note that 'git' is optional
and writing ": foo;" would complete the alias just fine.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* We've discussed this when we reviewed the topic that just hit
'master'. Before we forget...
contrib/completion/git-completion.bash | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git c/contrib/completion/git-completion.bash w/contrib/completion/git-completion.bash
index 47fd664ea5..477ef8157a 100644
--- c/contrib/completion/git-completion.bash
+++ w/contrib/completion/git-completion.bash
@@ -28,7 +28,8 @@
# completion style. For example '!f() { : git commit ; ... }; f' will
# tell the completion to use commit completion. This also works with aliases
# of form "!sh -c '...'". For example, "!sh -c ': git commit ; ... '".
-# Be sure to add a space between the command name and the ';'.
+# Note that "git" is optional --- '!f() { : commit; ...}; f' would complete
+# just like the 'git commit' command.
#
# If you have a command that is not part of git, but you would still
# like completion, you can use __git_complete:
@@ -1183,7 +1184,7 @@ __git_aliased_command ()
:) : skip null command ;;
\'*) : skip opening quote after sh -c ;;
*)
- cur="$word"
+ cur="${word%;}"
break
esac
done
^ permalink raw reply related
* Re: [PATCH] git-gui - use git-hook, honor core.hooksPath
From: Junio C Hamano @ 2023-09-20 16:58 UTC (permalink / raw)
To: Mark Levedahl; +Cc: Pratyush Yadav, johannes.schindelin, git
In-Reply-To: <573c6dc5-2102-cb65-8f71-dea37fff0c9b@gmail.com>
Mark Levedahl <mlevedahl@gmail.com> writes:
> Certainly, folks rolling their own can pull
> from upstream git-gui, but they take the risk of incompatibility with
> an outdated git. Other tools in Junio's tree have already made the
> switch to git-hook (send-email, git-p4) even though they are usually
> packaged separately from git-core, but also version locked to matching
> git-core.
The cross-version compatibility story is the same for "gitk" (which
I believe "git-gui" took the "do not too deeply depend on the
matching version of git" mantra from). I can understand the desire
and being able to aim for wider compatibility may be an advantage
for these tools that are not tightly bundled with the rest of the
system. It allowed them to evolve without waiting for Git to catch
up, for example.
But at this point in their history where these tools are very
mature, it may be fair to say that the cross-version compatibility
is becoming a lost cause.
^ permalink raw reply
* Re: [PATCH] attr: attr.allowInvalidSource config to allow invalid revision
From: Junio C Hamano @ 2023-09-20 16:06 UTC (permalink / raw)
To: John Cai via GitGitGadget; +Cc: git, John Cai
In-Reply-To: <pull.1577.git.git.1695218431033.gitgitgadget@gmail.com>
"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: John Cai <johncai86@gmail.com>
>
> 44451a2e5e (attr: teach "--attr-source=<tree>" global option to "git",
> 2023-05-06) provided the ability to pass in a treeish as the attr
> source. When a revision does not resolve to a valid tree is passed, Git
> will die. GitLab keeps bare repositories and always reads attributes
> from the default branch, so we pass in HEAD to --attr-source.
Makes sense.
> With empty repositories however, HEAD does not point to a valid treeish,
> causing Git to die. This means we would need to check for a valid
> treeish each time.
Naturally.
> To avoid this, let's add a configuration that allows
> Git to simply ignore --attr-source if it does not resolve to a valid
> tree.
Not convincing at all as to the reason why we want to do anything
"to avoid this". "git log" in a repository whose HEAD does not
point to a valid treeish. "git blame" dies with "no such ref:
HEAD". An empty repository (more precisely, an unborn history)
needs special casing if you want to present it if you do not want to
spew underlying error messages to the end users *anyway*. It is
unclear why seeing what commit the HEAD pointer points at (or which
branch it points at for that matter) is *an* *extra* and *otherwise*
*unnecessary* overhead that need to be avoided.
^ permalink raw reply
* Re: [PATCH v2] show doc: redirect user to git log manual instead of git diff-tree
From: Junio C Hamano @ 2023-09-20 15:52 UTC (permalink / raw)
To: Han Young; +Cc: git
In-Reply-To: <20230920132731.1259-1-hanyang.tony@bytedance.com>
Han Young <hanyang.tony@bytedance.com> writes:
> While git show accepts options that apply to the git diff-tree command,
> some options do not make sense in the context of git show.
> The options of git show are handled using the machinery of git log.
> The git log manual page is a better place to look into than git diff-tree
> for options that are not in the git show manual page.
>
> Signed-off-by: Han Young <hanyang.tony@bytedance.com>
> ---
> Changes since v1:
> * change wording to clarify not all options of `git log` are meant for `git show`
Nice. Thank you very much for not forgetting this topic.
> Documentation/git-show.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/git-show.txt b/Documentation/git-show.txt
> index 2b1bc7288d..fc46b3687b 100644
> --- a/Documentation/git-show.txt
> +++ b/Documentation/git-show.txt
> @@ -26,7 +26,7 @@ with --name-only).
>
> For plain blobs, it shows the plain contents.
>
> -The command takes options applicable to the 'git diff-tree' command to
> +Some options that 'git log' command understands can be used to
> control how the changes the commit introduces are shown.
>
> This manual page describes only the most frequently used options.
^ permalink raw reply
* Re: [PATCH] git-gui - use git-hook, honor core.hooksPath
From: Junio C Hamano @ 2023-09-20 15:49 UTC (permalink / raw)
To: Pratyush Yadav; +Cc: Mark Levedahl, johannes.schindelin, git
In-Reply-To: <mafs01qetq9kk.fsf@yadavpratyush.com>
Pratyush Yadav <me@yadavpratyush.com> writes:
> In the past, git-gui has tried to keep backward compatibility with all
> versions of Git, not just the latest ones. v2.36 is relatively new and
> this code would not work for anyone using an older version of Git.
>
> I have largely followed this practice for all the code I have written
> but I am not sure if it is a good idea to insist on it -- especially if
> it would end up adding some more complexity. I would be interested to
> hear what other people think about this.
Good point.
> Junio, I was under the impression that I would keep maintaining the tree
> until we found a replacement maintainer. If you are okay with being the
> interim maintainer, that sounds good to me. Let me know what works best.
I am actually not OK ;-).
I prefer to see somebody who does use git-gui, or at least somebody
who uses Git in a graphical environment in their daily work, to be
maintaining it. I am disqualified on both counts.
> I have applied another patch since my last pull request. So I can apply
> this one, send you a new one and sync our trees.
OK. I'll drop the copy I have on my end when it happens, then.
Thanks.
^ permalink raw reply
* Re: [REGRESSION] uninitialized value $address in git send-email
From: Junio C Hamano @ 2023-09-20 15:43 UTC (permalink / raw)
To: Bagas Sanjaya
Cc: Michael Strawbridge, Luben Tuikov,
Ævar Arnfjörð Bjarmason, Emily Shaffer,
Doug Anderson, Git Mailing List
In-Reply-To: <ZQrQsa5GJEVhBttT@debian.me>
Bagas Sanjaya <bagasdotme@gmail.com> writes:
> Originally, I was intended to report regression on handling multiple
> addresses passed in a single --to/--cc/--bcc option.
You refer to v2.40 and v2.41 in the message I am responding to, but
do you have a bisection? There seem to have been five topics around
send-email during that timeperiod.
$ git log --oneline --first-parent v2.40.0..v2.41.0 git-send-email.perl
b04671b638 Merge branch 'jc/send-email-pre-process-fix'
64477d20d7 Merge branch 'mc/send-email-header-cmd'
b6e9521956 Merge branch 'ms/send-email-feed-header-to-validate-hook'
c4c9d5586f Merge branch 'rj/send-email-validate-hook-count-messages'
647a2bb3ff Merge branch 'jc/spell-id-in-both-caps-in-message-id'
^ permalink raw reply
* Re: [REGRESSION] uninitialized value $address in git send-email
From: Junio C Hamano @ 2023-09-20 15:36 UTC (permalink / raw)
To: Michael Strawbridge
Cc: Bagas Sanjaya, Luben Tuikov,
Ævar Arnfjörð Bjarmason, Emily Shaffer,
Doug Anderson, Git Mailing List
In-Reply-To: <118975ef-c07f-c397-5288-7698e60516a7@amd.com>
Michael Strawbridge <michael.strawbridge@amd.com> writes:
> Whoops, somehow I missed the other responses on this thread until I
> looked on the web archive version of this mailing list. I see that a
> solution to "Use of uninitialized value $address" has already been proposed.
>
> I suppose I may have mistook what issue was being reported. I had
> originally understood the problem to be that hook related logic was
> failing with correct email addresses, but it seems rather that we are
> trying to fix an error that occurs when an email address that fails
> extract_valid_address_or_die() is given. Feel free to ignore my last
> email if that is all we are trying to solve.
I just had an impression that the original was complaining about the
command failing, and the patches addressed a side issue that the
error message that is given when the command fails uses an undefined
value. The report was not quite clear what Bagas considerd a
regression (e.g. did the command allow an invalid address like <pi@pi>
but now it complains?), though.
^ permalink raw reply
* Re: [PATCH v4] revision: add `--ignore-missing-links` user option
From: Junio C Hamano @ 2023-09-20 15:32 UTC (permalink / raw)
To: Karthik Nayak; +Cc: git, me
In-Reply-To: <20230920104507.21664-1-karthik.188@gmail.com>
> diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
> index a4a0cb93b2..8ee713db3d 100644
> --- a/Documentation/rev-list-options.txt
> +++ b/Documentation/rev-list-options.txt
> @@ -227,6 +227,15 @@ explicitly.
> Upon seeing an invalid object name in the input, pretend as if
> the bad input was not given.
>
> +--ignore-missing-links::
> + During traversal, if an object that is referenced does not
> + exist, instead of dying of a repository corruption, pretend as
> + if the reference itself does not exist. Running the command
> + with the `--boundary` option makes these missing commits,
> + together with the commits on the edge of revision ranges
> + (i.e. true boundary objects), appear on the output, prefixed
> + with '-'.
There needs an explanation of interaction with --missing=<action>
option here, no? "--missing=allow-any" and "--missing=print" are
sensible choices, I presume. The former allows the traversal to
proceed, as you described in one of your responses. Also with
"--missing=print", the user can more directly find out which are the
missing objects, even without using the "--boundary" that requires
them to sift between missing objects and the objects that are truly
on boundary.
Here is my attempt:
--ignore-missing-links::
During traversal, if an object that is referenced does not
exist, instead of dying of a repository corruption, allow
`--missing=<missing-action>` to decide what to do.
+
`--missing=print` will make the command print a list of missing
objects, prefixed with a "?" character.
+
`--missing=allow-any` will make the command proceed without doing
anything special. Used with `--boundary`, output these missing
objects mixed with the commits on the edge of revision ranges,
prefixed with a "-" character.
It might make sense to add
+
Use of this option with other 'missing-action' may probably not
give useful behaviour.
at the end, but it may not be useful to the readers to say "we allow
even more extra flexibility but haven't thought through what good
they would do".
> +# With `--ignore-missing-links`, we stop the traversal when we encounter a
> +# missing link. The boundary commit is not listed as we haven't used the
> +# `--boundary` options.
> +test_expect_success 'rev-list only prints main odb commits with --ignore-missing-links' '
> + hide_alternates &&
> +
> + git -C alt rev-list --objects --no-object-names \
> + --ignore-missing-links --missing=allow-any HEAD >actual.raw &&
> + git -C alt cat-file --batch-check="%(objectname)" \
> + --batch-all-objects >expect.raw &&
> +
> + sort actual.raw >actual &&
> + sort expect.raw >expect &&
> + test_cmp expect actual
> +'
This gives a good baseline. "--missing=print" without "--boundary"
may have more obvious use cases, but is there a practical use case
for the output from an invocation with "--missing=allow-any" without
"--boundary"? Just being curious if I am missing something obvious.
Perhaps add another test that uses "--missing=print" instead, and
check that the "? missing" output matches what we expect to be
missing? The same comment applies to the other test that uses
"--missing=allow-any" without "--boundary" we see later.
> +# With `--ignore-missing-links` and `--boundary`, we can even print those boundary
> +# commits.
> +test_expect_success 'rev-list prints boundary commit with --ignore-missing-links' '
> + git -C alt rev-list --ignore-missing-links --boundary HEAD >got &&
> + grep "^-$(git rev-parse HEAD)" got
> +'
This makes sure what we expect to appear in 'got' actually is in
'got', but we should also make sure 'got' does not have anything
unexpected.
> +test_expect_success "setup for rev-list --ignore-missing-links with missing objects" '
> + show_alternates &&
> + test_commit -C alt 11
> +'
> +
> +for obj in "HEAD^{tree}" "HEAD:11.t"
> +do
> + # The `--ignore-missing-links` option should ensure that git-rev-list(1)
> + # doesn't fail when used alongside `--objects` when a tree/blob is
> + # missing.
> + test_expect_success "rev-list --ignore-missing-links with missing $type" '
> + oid="$(git -C alt rev-parse $obj)" &&
> + path="alt/.git/objects/$(test_oid_to_path $oid)" &&
> +
> + mv "$path" "$path.hidden" &&
> + test_when_finished "mv $path.hidden $path" &&
In the first iteration, we check without the tree object and we only
ensure that removed tree does not appear in the output---but we know
the blob that is referenced by that removed tree will not appear in
the output, either, don't we? Don't we want to check that, too?
In the second iteration, we have resurrected the tree but removed
the blob that is referenced by the tree, so we would not see that
blob in the output, which makes sense.
> + git -C alt rev-list --ignore-missing-links --missing=allow-any --objects HEAD \
> + >actual &&
> + ! grep $oid actual
> + '
> +done
> +
> +test_done
Thanks.
^ permalink raw reply
* Re: [PATCH] git-gui - use git-hook, honor core.hooksPath
From: Mark Levedahl @ 2023-09-20 15:30 UTC (permalink / raw)
To: Pratyush Yadav; +Cc: gitster, johannes.schindelin, git
In-Reply-To: <mafs01qetq9kk.fsf@yadavpratyush.com>
On 9/20/23 09:05, Pratyush Yadav wrote:
> In the past, git-gui has tried to keep backward compatibility with all
> versions of Git, not just the latest ones. v2.36 is relatively new and
> this code would not work for anyone using an older version of Git.
>
> I have largely followed this practice for all the code I have written
> but I am not sure if it is a good idea to insist on it -- especially if
> it would end up adding some more complexity. I would be interested to
> hear what other people think about this.
>
I am not aware of any distribution (Linux, g4w, Mac) shipping anything
except the git-gui in Junio's tree, which is specific to the git-core
version, and the git-gui packages require (or are a part of) the same
version git-core package: no cross-version compatibility of git
components is assumed. Certainly, folks rolling their own can pull from
upstream git-gui, but they take the risk of incompatibility with an
outdated git. Other tools in Junio's tree have already made the switch
to git-hook (send-email, git-p4) even though they are usually packaged
separately from git-core, but also version locked to matching git-core.
Updating git-gui's hook execution to match git internals would be more
complex than what I implemented or what was there before. For instance,
I never looked at what git-hook's g4w compatibility code uses to test if
a hook is present and executable, it wouldn't surprise me to find
git-gui was missing something there, but who wants to bother? Also, the
commit language surrounding addition of git-hook is strongly suggestive
of other changes in configuration coming, meaning more changes to hook
execution code would be needed that are avoided by using git-hook. Note:
I have one more patch to send, removing yet another work-around for
early Cygwin tcl/tk, as more evidence of how many years it takes to
clean some of this stuff out and the difficulty of keeping git-gui up to
date.
I had considered the above when creating the patch, and I believe what I
did is the right approach.
Mark
^ permalink raw reply
* [PATCH v2 1/2] diff-merges: improve --diff-merges documentation
From: Sergey Organov @ 2023-09-20 15:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Sergey Organov
In-Reply-To: <20230920150244.171772-1-sorganov@gmail.com>
* Put descriptions of convenience shortcuts first, so they are the
first things reader observes rather than lengthy detailed stuff.
* Get rid of very long line containing all the --diff-merges formats
by replacing them with <format>, and putting each supported format
on its own line.
Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
Documentation/diff-options.txt | 98 ++++++++++++++++++----------------
Documentation/git-log.txt | 2 +-
2 files changed, 54 insertions(+), 46 deletions(-)
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 9f33f887711d..8035210c1418 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -43,66 +43,74 @@ endif::git-diff[]
endif::git-format-patch[]
ifdef::git-log[]
---diff-merges=(off|none|on|first-parent|1|separate|m|combined|c|dense-combined|cc|remerge|r)::
+-m::
+ Show diffs for merge commits in the default format. This is
+ similar to '--diff-merges=on' (which see) except `-m` will
+ produce no output unless `-p` is given as well.
+
+-c::
+ Produce combined diff output for merge commits.
+ Shortcut for '--diff-merges=combined -p'.
+
+--cc::
+ Produce dense combined diff output for merge commits.
+ Shortcut for '--diff-merges=dense-combined -p'.
+
+--remerge-diff::
+ Produce diff against re-merge.
+ Shortcut for '--diff-merges=remerge -p'.
+
--no-diff-merges::
+ Synonym for '--diff-merges=off'.
+
+--diff-merges=<format>::
Specify diff format to be used for merge commits. Default is
- {diff-merges-default} unless `--first-parent` is in use, in which case
- `first-parent` is the default.
+ {diff-merges-default} unless `--first-parent` is in use, in
+ which case `first-parent` is the default.
+
---diff-merges=(off|none):::
---no-diff-merges:::
+The following formats are supported:
++
+--
+off, none::
Disable output of diffs for merge commits. Useful to override
implied value.
+
---diff-merges=on:::
---diff-merges=m:::
--m:::
- This option makes diff output for merge commits to be shown in
- the default format. `-m` will produce the output only if `-p`
- is given as well. The default format could be changed using
+on, m::
+ Make diff output for merge commits to be shown in the default
+ format. The default format could be changed using
`log.diffMerges` configuration parameter, which default value
is `separate`.
+
---diff-merges=first-parent:::
---diff-merges=1:::
- This option makes merge commits show the full diff with
- respect to the first parent only.
+first-parent, 1::
+ Show full diff with respect to first parent. This is the same
+ format as `--patch` produces for non-merge commits.
+
---diff-merges=separate:::
- This makes merge commits show the full diff with respect to
- each of the parents. Separate log entry and diff is generated
- for each parent.
+separate::
+ Show full diff with respect to each of parents.
+ Separate log entry and diff is generated for each parent.
+
---diff-merges=remerge:::
---diff-merges=r:::
---remerge-diff:::
- With this option, two-parent merge commits are remerged to
- create a temporary tree object -- potentially containing files
- with conflict markers and such. A diff is then shown between
- that temporary tree and the actual merge commit.
+combined, c::
+ Show differences from each of the parents to the merge
+ result simultaneously instead of showing pairwise diff between
+ a parent and the result one at a time. Furthermore, it lists
+ only files which were modified from all parents.
++
+dense-combined, cc::
+ Further compress output produced by `--diff-merges=combined`
+ by omitting uninteresting hunks whose contents in the parents
+ have only two variants and the merge result picks one of them
+ without modification.
++
+remerge, r::
+ Remerge two-parent merge commits to create a temporary tree
+ object--potentially containing files with conflict markers
+ and such. A diff is then shown between that temporary tree
+ and the actual merge commit.
+
The output emitted when this option is used is subject to change, and
so is its interaction with other options (unless explicitly
documented).
-+
---diff-merges=combined:::
---diff-merges=c:::
--c:::
- With this option, diff output for a merge commit shows the
- differences from each of the parents to the merge result
- simultaneously instead of showing pairwise diff between a
- parent and the result one at a time. Furthermore, it lists
- only files which were modified from all parents. `-c` implies
- `-p`.
-+
---diff-merges=dense-combined:::
---diff-merges=cc:::
---cc:::
- With this option the output produced by
- `--diff-merges=combined` is further compressed by omitting
- uninteresting hunks whose contents in the parents have only
- two variants and the merge result picks one of them without
- modification. `--cc` implies `-p`.
+--
--combined-all-paths::
This flag causes combined diffs (used for merge commits) to
diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 2a66cf888074..9b7ec96e767a 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -124,7 +124,7 @@ Note that unless one of `--diff-merges` variants (including short
will not show a diff, even if a diff format like `--patch` is
selected, nor will they match search options like `-S`. The exception
is when `--first-parent` is in use, in which case `first-parent` is
-the default format.
+the default format for merge commits.
:git-log: 1
:diff-merges-default: `off`
--
2.25.1
^ permalink raw reply related
* [PATCH v2 2/2] diff-merges: introduce '-d' option
From: Sergey Organov @ 2023-09-20 15:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Sergey Organov
In-Reply-To: <20230920150244.171772-1-sorganov@gmail.com>
This option provides a shortcut to request diff with respect to first
parent for any kind of commit, universally. It's implemented as pure
synonym for "--diff-merges=first-parent --patch".
Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
Documentation/diff-options.txt | 4 ++++
Documentation/git-log.txt | 2 +-
diff-merges.c | 3 +++
t/t4013-diff-various.sh | 8 ++++++++
4 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 8035210c1418..19bb78ff6652 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -48,6 +48,10 @@ ifdef::git-log[]
similar to '--diff-merges=on' (which see) except `-m` will
produce no output unless `-p` is given as well.
+-d::
+ Produce diff with respect to first parent.
+ Shortcut for '--diff-merges=first-parent -p'.
+
-c::
Produce combined diff output for merge commits.
Shortcut for '--diff-merges=combined -p'.
diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 9b7ec96e767a..59bd74a1a596 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -120,7 +120,7 @@ By default, `git log` does not generate any diff output. The options
below can be used to show the changes made by each commit.
Note that unless one of `--diff-merges` variants (including short
-`-m`, `-c`, and `--cc` options) is explicitly given, merge commits
+`-d`, `-m`, `-c`, and `--cc` options) is explicitly given, merge commits
will not show a diff, even if a diff format like `--patch` is
selected, nor will they match search options like `-S`. The exception
is when `--first-parent` is in use, in which case `first-parent` is
diff --git a/diff-merges.c b/diff-merges.c
index ec97616db1df..6eb72e6fc28a 100644
--- a/diff-merges.c
+++ b/diff-merges.c
@@ -125,6 +125,9 @@ int diff_merges_parse_opts(struct rev_info *revs, const char **argv)
if (!suppress_m_parsing && !strcmp(arg, "-m")) {
set_to_default(revs);
revs->merges_need_diff = 0;
+ } else if (!strcmp(arg, "-d")) {
+ set_first_parent(revs);
+ revs->merges_imply_patch = 1;
} else if (!strcmp(arg, "-c")) {
set_combined(revs);
revs->merges_imply_patch = 1;
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 5de1d190759f..a07d6eb6dd97 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -473,6 +473,14 @@ test_expect_success 'log --diff-merges=on matches --diff-merges=separate' '
test_cmp expected actual
'
+test_expect_success 'log -d matches --diff-merges=1 -p' '
+ git log --diff-merges=1 -p master >result &&
+ process_diffs result >expected &&
+ git log -d master >result &&
+ process_diffs result >actual &&
+ test_cmp expected actual
+'
+
test_expect_success 'deny wrong log.diffMerges config' '
test_config log.diffMerges wrong-value &&
test_expect_code 128 git log
--
2.25.1
^ permalink raw reply related
* [PATCH v2 0/2] diff-merges: introduce '-d' option
From: Sergey Organov @ 2023-09-20 15:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Sergey Organov
In-Reply-To: <20230909125446.142715-1-sorganov@gmail.com>
This new convenience option requests full diff with respect to first
parent, so that
git log -d
will output diff with respect to first parent for every commit,
universally, no matter how many parents the commit turns out to have.
It's implemented as pure synonym for
--diff-merges=first-parent --patch
The first commit in the series tweaks diff-merges documentation a bit,
and is valuable by itself. It's put here as '-d' implementation commit
depends on it in its documentation part.
Note: the need for this new convenience option mostly emerged from
denial by the community of patches that modify '-m' behavior to imply
'-p' as the rest of similar options (such as --cc) do.
Updates in v2:
* Reordered documentation for diff-merges formats in accordance with
Junio recommendation.
* Removed clarification of surprising -m behavior due to controversy
with Junio on how exactly it should look like.
Sergey Organov (2):
diff-merges: improve --diff-merges documentation
diff-merges: introduce '-d' option
Documentation/diff-options.txt | 102 ++++++++++++++++++---------------
Documentation/git-log.txt | 4 +-
diff-merges.c | 3 +
t/t4013-diff-various.sh | 8 +++
4 files changed, 70 insertions(+), 47 deletions(-)
Interdiff against v1:
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index d773dafcb10a..19bb78ff6652 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -47,9 +47,6 @@ ifdef::git-log[]
Show diffs for merge commits in the default format. This is
similar to '--diff-merges=on' (which see) except `-m` will
produce no output unless `-p` is given as well.
-+
-Note: This option not implying `-p` is legacy feature that is
-preserved for the sake of backward compatibility.
-d::
Produce diff with respect to first parent.
@@ -96,16 +93,6 @@ separate::
Show full diff with respect to each of parents.
Separate log entry and diff is generated for each parent.
+
-remerge, r::
- Remerge two-parent merge commits to create a temporary tree
- object--potentially containing files with conflict markers
- and such. A diff is then shown between that temporary tree
- and the actual merge commit.
-+
-The output emitted when this option is used is subject to change, and
-so is its interaction with other options (unless explicitly
-documented).
-+
combined, c::
Show differences from each of the parents to the merge
result simultaneously instead of showing pairwise diff between
@@ -117,6 +104,16 @@ dense-combined, cc::
by omitting uninteresting hunks whose contents in the parents
have only two variants and the merge result picks one of them
without modification.
++
+remerge, r::
+ Remerge two-parent merge commits to create a temporary tree
+ object--potentially containing files with conflict markers
+ and such. A diff is then shown between that temporary tree
+ and the actual merge commit.
++
+The output emitted when this option is used is subject to change, and
+so is its interaction with other options (unless explicitly
+documented).
--
--combined-all-paths::
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v3 13/13] trailer doc: <token> is a <key> or <keyAlias>, not both
From: Junio C Hamano @ 2023-09-20 15:01 UTC (permalink / raw)
To: Jonathan Tan
Cc: Linus Arver via GitGitGadget, git, Christian Couder, Linus Arver
In-Reply-To: <20230919225926.2189091-1-jonathantanmy@google.com>
Jonathan Tan <jonathantanmy@google.com> writes:
> "Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes:
>> @@ -248,34 +258,40 @@ With `add`, a new trailer will be added.
>> +
>> With `doNothing`, nothing will be done.
>>
>> -trailer.<token>.key::
>> - This `key` will be used instead of <token> in the trailer. At
>> - the end of this key, a separator can appear and then some
>> - space characters. By default the only valid separator is ':',
>> - but this can be changed using the `trailer.separators` config
>> - variable.
>> +trailer.<keyAlias>.key::
>> + Defines a <keyAlias> for the <key>. The <keyAlias> must be a
>> + prefix (case does not matter) of the <key>. For example, in `git
>> + config trailer.ack.key "Acked-by"` the "Acked-by" is the <key> and
>> + the "ack" is the <keyAlias>. This configuration allows the shorter
>> + `--trailer "ack:..."` invocation on the command line using the "ack"
>> + <keyAlias> instead of the longer `--trailer "Acked-by:..."`.
>> ++
>> +At the end of the <key>, a separator can appear and then some
>> +space characters. By default the only valid separator is ':',
>> +but this can be changed using the `trailer.separators` config
>> +variable.
>
> I think all the other patches will be a great help to the user, but I'm
> on the fence about this one. Someone who knows these trailer components
> by their old names might be confused upon seeing tne new ones, so I'm
> inclined to minimize such changes. I do think that the new names make
> more sense, though.
As long as the new names describe the world order better than the
old description, I do not mind rephrasing the documentation, and you
seem to find the more descriptive <keyAlias> easier to understand
compared to the non-descriptive <token>. Adding a concrete example
(ack vs acked-by) is also a good change.
^ permalink raw reply
* Re: What's cooking in git.git (Sep 2023, #05; Fri, 15)
From: Junio C Hamano @ 2023-09-20 14:57 UTC (permalink / raw)
To: Linus Arver; +Cc: git
In-Reply-To: <owly1qetjqo1.fsf@fine.c.googlers.com>
Linus Arver <linusa@google.com> writes:
>> Whatever improvements you have in mind, if they are
>> minor, letting the above graduate (they have been in 'next' for a
>> week without anybody complaining) and doing them as a follow-up
>> series would be sensible, I would think.
>>
>> Thanks.
>
> Hmm, I don't think they are minor? See
> https://github.com/listx/git/tree/trailer-libification-prep for the
> current state of things.
They do not look like "oops the patches I sent to the list was
totally bogus and they need replacing"; aren't they rather "these
are not yet complete and there are some of the follow-on work on top
of them"?
> I need to still follow up to your last comment on "trailer: rename
> *_DEFAULT enums to *_UNSPECIFIED" [2] (I was going to see if we needed
> the "obvious solution" as you described).
>
> If it's too painful to move this out of 'next' now, I'm OK with it
> graduating as is and doing a separate follow-up (I expect several more
> of these to happen anyway). Up to you.
Let's mark the topic as "waiting for follow-up updates" and keep it
in 'next' until the follow-up patches become ready, then.
> Sorry for not noticing that this was in 'next' sooner and communicating
> accordingly.
Gotchas happen. Let's try to communicate better the next time.
Thanks.
^ permalink raw reply
* [PATCH] attr: attr.allowInvalidSource config to allow invalid revision
From: John Cai via GitGitGadget @ 2023-09-20 14:00 UTC (permalink / raw)
To: git; +Cc: John Cai, John Cai
From: John Cai <johncai86@gmail.com>
44451a2e5e (attr: teach "--attr-source=<tree>" global option to "git",
2023-05-06) provided the ability to pass in a treeish as the attr
source. When a revision does not resolve to a valid tree is passed, Git
will die. GitLab keeps bare repositories and always reads attributes
from the default branch, so we pass in HEAD to --attr-source.
With empty repositories however, HEAD does not point to a valid treeish,
causing Git to die. This means we would need to check for a valid
treeish each time. To avoid this, let's add a configuration that allows
Git to simply ignore --attr-source if it does not resolve to a valid
tree.
Signed-off-by: John Cai <johncai86@gmail.com>
---
attr: attr.allowInvalidSource config to allow empty revision
44451a2e5e (attr: teach "--attr-source=" global option to "git",
2023-05-06) provided the ability to pass in a treeish as the attr
source. When a revision does not resolve to a valid tree is passed, Git
will die. GitLab keeps bare repositories and always reads attributes
from the default branch, so we pass in HEAD to --attr-source.
With empty repositories however, HEAD does not point to a valid treeish,
causing Git to die. This means we would need to check for a valid
treeish each time. To avoid this, let's add a configuration that allows
Git to simply ignore --attr-source if it does not resolve to a valid
tree.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1577%2Fjohn-cai%2Fjc%2Fconfig-attr-invalid-source-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1577/john-cai/jc/config-attr-invalid-source-v1
Pull-Request: https://github.com/git/git/pull/1577
Documentation/config.txt | 2 ++
Documentation/config/attr.txt | 6 ++++++
attr.c | 11 +++++++++--
t/t0003-attributes.sh | 36 +++++++++++++++++++++++++++++++++++
4 files changed, 53 insertions(+), 2 deletions(-)
create mode 100644 Documentation/config/attr.txt
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 229b63a454c..b1891c2b5af 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -371,6 +371,8 @@ other popular tools, and describe them in your documentation.
include::config/advice.txt[]
+include::config/attr.txt[]
+
include::config/core.txt[]
include::config/add.txt[]
diff --git a/Documentation/config/attr.txt b/Documentation/config/attr.txt
new file mode 100644
index 00000000000..2218f0c982a
--- /dev/null
+++ b/Documentation/config/attr.txt
@@ -0,0 +1,6 @@
+attr.allowInvalidSource::
+ If `--attr-source` cannot resolve to a valid tree object, ignore
+ `--attr-source` instead of erroring out, and fall back to looking for
+ attributes in the default locations. Useful when passing `HEAD` into
+ `attr-source` since it allows `HEAD` to point to an unborn branch in
+ cases like an empty repository.
diff --git a/attr.c b/attr.c
index 71c84fbcf86..854a3720e3f 100644
--- a/attr.c
+++ b/attr.c
@@ -1208,8 +1208,15 @@ static void compute_default_attr_source(struct object_id *attr_source)
if (!default_attr_source_tree_object_name || !is_null_oid(attr_source))
return;
- if (repo_get_oid_treeish(the_repository, default_attr_source_tree_object_name, attr_source))
- die(_("bad --attr-source or GIT_ATTR_SOURCE"));
+
+ if (repo_get_oid_treeish(the_repository, default_attr_source_tree_object_name, attr_source)) {
+ int allow_invalid_attr_source = 0;
+
+ git_config_get_bool("attr.allowinvalidsource", &allow_invalid_attr_source);
+
+ if (!allow_invalid_attr_source)
+ die(_("bad --attr-source or GIT_ATTR_SOURCE"));
+ }
}
static struct object_id *default_attr_source(void)
diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index 26e082f05b4..3272237ee2b 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -342,6 +342,42 @@ test_expect_success 'bare repository: check that .gitattribute is ignored' '
)
'
+bad_attr_source_err="fatal: bad --attr-source or GIT_ATTR_SOURCE"
+
+test_expect_success 'attr.allowInvalidSource when HEAD is unborn' '
+ test_when_finished rm -rf empty &&
+ echo $bad_attr_source_err >expect_err &&
+ echo "f/path: test: unspecified" >expect &&
+ git init empty &&
+ test_must_fail git -C empty --attr-source=HEAD check-attr test -- f/path 2>err &&
+ test_cmp expect_err err &&
+ git -C empty -c attr.allowInvalidSource=true --attr-source=HEAD check-attr test -- f/path >actual 2>err &&
+ test_must_be_empty err &&
+ test_cmp expect actual
+'
+
+test_expect_success 'attr.allowInvalidSource when --attr-source points to non-existing ref' '
+ test_when_finished rm -rf empty &&
+ echo $bad_attr_source_err >expect_err &&
+ echo "f/path: test: unspecified" >expect &&
+ git init empty &&
+ test_must_fail git -C empty --attr-source=refs/does/not/exist check-attr test -- f/path 2>err &&
+ test_cmp expect_err err &&
+ git -C empty -c attr.allowInvalidSource=true --attr-source=refs/does/not/exist check-attr test -- f/path >actual 2>err &&
+ test_must_be_empty err &&
+ test_cmp expect actual
+'
+
+test_expect_success 'bad attr source defaults to reading .gitattributes file' '
+ test_when_finished rm -rf empty &&
+ git init empty &&
+ echo "f/path test=val" >empty/.gitattributes &&
+ echo "f/path: test: val" >expect &&
+ git -C empty -c attr.allowInvalidSource=true --attr-source=HEAD check-attr test -- f/path >actual 2>err &&
+ test_must_be_empty err &&
+ test_cmp expect actual
+'
+
test_expect_success 'bare repository: with --source' '
(
cd bare.git &&
base-commit: 1fc548b2d6a3596f3e1c1f8b1930d8dbd1e30bf3
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH v2] git-gui - re-enable use of hook scripts
From: Pratyush Yadav @ 2023-09-20 13:27 UTC (permalink / raw)
To: Mark Levedahl; +Cc: gitster, johannes.schindelin, me, git
In-Reply-To: <20230916210131.78593-1-mlevedahl@gmail.com>
Hi,
On Sat, Sep 16 2023, Mark Levedahl wrote:
> Earlier, commit aae9560a introduced search in $PATH to find executables
> before running them, avoiding an issue where on Windows a same named
> file in the current directory can be executed in preference to anything
> in a directory in $PATH. This search is intended to find an absolute
> path for a bare executable ( e.g, a function "foo") by finding the first
> instance of "foo" in a directory given in $PATH, and this search works
> correctly. The search is explicitly avoided for an executable named
> with an absolute path (e.g., /bin/sh), and that works as well.
>
> Unfortunately, the search is also applied to commands named with a
> relative path. A hook script (or executable) $HOOK is usually located
> relative to the project directory as .git/hooks/$HOOK. The search for
> this will generally fail as that relative path will (probably) not exist
> on any directory in $PATH. This means that git hooks in general now fail
> to run. Considerable mayhem could occur should a directory on $PATH be
> git controlled. If such a directory includes .git/hooks/$HOOK, that
> repository's $HOOK will be substituted for the one in the current
> project, with unknown consequences.
>
> This lookup failure also occurs in worktrees linked to a remote .git
> directory using git-new-workdir. However, a worktree using a .git file
> pointing to a separate git directory apparently avoids this: in that
> case the hook command is resolved to an absolute path before being
> passed down to the code introduced in aae9560a.
>
> Fix this by replacing the test for an "absolute" pathname to a check for
> a command name having more than one pathname component. This limits the
> search and absolute pathname resolution to bare commands. The new test
> uses tcl's "file split" command. Experiments on Linux and Windows, using
> tclsh, show that command names with relative and absolute paths always
> give at least two components, while a bare command gives only one.
>
> Linux: puts [file split {foo}] ==> foo
> Linux: puts [file split {/foo}] ==> / foo
> Linux: puts [file split {.git/foo}] ==> .git foo
> Windows: puts [file split {foo}] ==> foo
> Windows: puts [file split {c:\foo}] ==> c:/ foo
> Windows: puts [file split {.git\foo}] ==> .git foo
>
> The above results show the new test limits search and replacement
> to bare commands on both Linux and Windows.
>
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
Looks good. Thanks.
Reviewed-by: Pratyush Yadav <me@yadavpratyush.com>
> ---
> git-gui.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index 8bc8892..8603437 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -118,7 +118,7 @@ proc sanitize_command_line {command_line from_index} {
> set i $from_index
> while {$i < [llength $command_line]} {
> set cmd [lindex $command_line $i]
> - if {[file pathtype $cmd] ne "absolute"} {
> + if {[llength [file split $cmd]] < 2} {
> set fullpath [_which $cmd]
> if {$fullpath eq ""} {
> throw {NOT-FOUND} "$cmd not found in PATH"
--
Regards,
Pratyush Yadav
^ permalink raw reply
* [PATCH v2] show doc: redirect user to git log manual instead of git diff-tree
From: Han Young @ 2023-09-20 13:27 UTC (permalink / raw)
To: git; +Cc: Han Young
In-Reply-To: <20230905121219.69762-1-hanyang.tony@bytedance.com>
While git show accepts options that apply to the git diff-tree command,
some options do not make sense in the context of git show.
The options of git show are handled using the machinery of git log.
The git log manual page is a better place to look into than git diff-tree
for options that are not in the git show manual page.
Signed-off-by: Han Young <hanyang.tony@bytedance.com>
---
Changes since v1:
* change wording to clarify not all options of `git log` are meant for `git show`
Documentation/git-show.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/git-show.txt b/Documentation/git-show.txt
index 2b1bc7288d..fc46b3687b 100644
--- a/Documentation/git-show.txt
+++ b/Documentation/git-show.txt
@@ -26,7 +26,7 @@ with --name-only).
For plain blobs, it shows the plain contents.
-The command takes options applicable to the 'git diff-tree' command to
+Some options that 'git log' command understands can be used to
control how the changes the commit introduces are shown.
This manual page describes only the most frequently used options.
--
2.42.0
^ permalink raw reply related
* Re: [REGRESSION] uninitialized value $address in git send-email
From: Michael Strawbridge @ 2023-09-20 13:14 UTC (permalink / raw)
To: Bagas Sanjaya, Junio C Hamano, Luben Tuikov,
Ævar Arnfjörð Bjarmason, Emily Shaffer,
Doug Anderson
Cc: Git Mailing List
In-Reply-To: <ZQrQsa5GJEVhBttT@debian.me>
On 2023-09-20 07:00, Bagas Sanjaya wrote:
> On Tue, Sep 19, 2023 at 10:37:36AM -0400, Michael Strawbridge wrote:
>> I suppose I may have mistook what issue was being reported. I had
>> originally understood the problem to be that hook related logic was
>> failing with correct email addresses, but it seems rather that we are
>> trying to fix an error that occurs when an email address that fails
>> extract_valid_address_or_die() is given. Feel free to ignore my last
>> email if that is all we are trying to solve.
>>
> Originally, I was intended to report regression on handling multiple
> addresses passed in a single --to/--cc/--bcc option. Previously on Git v2.40,
> git-send-email(1) accepts `--to="foo <foo@foo.com>,bar <bar@bar.com>"
> as two separate --to addresses (with comma as separator). However, on
> v2.41 and up, instead I got perl error as I reported in this thread.
> Interestingly, that perl error can be reduced into one invalid addresses.
> The same thing also happens to --cc and --bcc. I used aforementioned
> trick when I was sending patches to LKML to save frin typing the same
> option multiple times, each with different address.
>
> If I need to send separate regression report for above use case,
> please let me know.
>
I'm probably not the best person to answer whether you should file
another report. Junio would know better the processes of this mailing list.
However, I believe that if you are just trying to have the
"uninitialized value $address" error disappear then the above patch by
Taylor Blau should work great. Feel free to try it by editing your
local copy of git-send-email usually found here:
/usr/lib/git-core/git-send-email
^ permalink raw reply
* Re: [PATCH] git-gui - use git-hook, honor core.hooksPath
From: Pratyush Yadav @ 2023-09-20 13:05 UTC (permalink / raw)
To: Mark Levedahl; +Cc: gitster, johannes.schindelin, me, git
In-Reply-To: <20230917192431.101775-1-mlevedahl@gmail.com>
Hi,
Thanks for the patch.
On Sun, Sep 17 2023, Mark Levedahl wrote:
> git-gui currently runs some hooks directly using its own code written
> before 2010, long predating git v2.9 that added the core.hooksPath
> configuration to override the assumed location at $GIT_DIR/hooks. Thus,
> git-gui looks for and runs hooks including prepare-commit-msg,
> commit-msg, pre-commit, post-commit, and post-checkout from
> $GIT_DIR/hooks, regardless of configuration. Commands (e.g., git-merge)
> that git-gui invokes directly do honor core.hooksPath, meaning the
> overall behaviour is inconsistent.
>
> Furthermore, since v2.36 git exposes its hook exection machinery via
> git-hook run, eliminating the need for others to maintain code
> duplicating that functionality. Using git-hook will both fix git-gui's
> current issues on hook configuration and (presumably) reduce the
> maintenance burden going forward. So, teach git-gui to use git-hook.
In the past, git-gui has tried to keep backward compatibility with all
versions of Git, not just the latest ones. v2.36 is relatively new and
this code would not work for anyone using an older version of Git.
I have largely followed this practice for all the code I have written
but I am not sure if it is a good idea to insist on it -- especially if
it would end up adding some more complexity. I would be interested to
hear what other people think about this.
Junio, I was under the impression that I would keep maintaining the tree
until we found a replacement maintainer. If you are okay with being the
interim maintainer, that sounds good to me. Let me know what works best.
I have applied another patch since my last pull request. So I can apply
this one, send you a new one and sync our trees.
>
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---
> git-gui.sh | 27 ++-------------------------
> 1 file changed, 2 insertions(+), 25 deletions(-)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index 8603437..3e5907a 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -661,31 +661,8 @@ proc git_write {args} {
> }
>
> proc githook_read {hook_name args} {
> - set pchook [gitdir hooks $hook_name]
> - lappend args 2>@1
> -
> - # On Windows [file executable] might lie so we need to ask
> - # the shell if the hook is executable. Yes that's annoying.
> - #
> - if {[is_Windows]} {
> - upvar #0 _sh interp
> - if {![info exists interp]} {
> - set interp [_which sh]
> - }
> - if {$interp eq {}} {
> - error "hook execution requires sh (not in PATH)"
> - }
> -
> - set scr {if test -x "$1";then exec "$@";fi}
> - set sh_c [list $interp -c $scr $interp $pchook]
> - return [_open_stdout_stderr [concat $sh_c $args]]
> - }
> -
> - if {[file executable $pchook]} {
> - return [_open_stdout_stderr [concat [list $pchook] $args]]
> - }
> -
> - return {}
> + set cmd [concat git hook run --ignore-missing $hook_name -- $args 2>@1]
> + return [_open_stdout_stderr $cmd]
LGTM, other than my concerns with backward compatibility.
> }
>
> proc kill_file_process {fd} {
--
Regards,
Pratyush Yadav
^ permalink raw reply
* [PATCH v2 5/6] refs: alternate reftable ref backend implementation
From: Han-Wen Nienhuys via GitGitGadget @ 2023-09-20 13:02 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
In-Reply-To: <pull.1574.v2.git.git.1695214968.gitgitgadget@gmail.com>
From: Han-Wen Nienhuys <hanwen@google.com>
This introduces the reftable backend as an alternative to the packed
backend.
This is an alternative to the approach which was explored in
https://github.com/git/git/pull/1215/. This alternative is simpler,
because we now longer have to worry about:
- pseudorefs and the HEAD ref
- worktrees
- commands that blur together files and references (cherry-pick, rebase)
This deviates from the spec that in
Documentation/technical/reftable.txt. It might be possible to update the
code such that all writes by default go to reftable directly. Then the
result would be compatible with an implementation that writes only
reftable (the reftable lock would still prevent races), but something
must be done about the HEAD ref (which JGit keeps in reftable too.)
Alternatively, JGit could be adapted to follow this implementation:
despite the code being there for 4 years now, I haven't heard of anyone
using it in production (exactly because CGit does not support it).
For this incremental path, the reftable format is arguably more
complex than necessary, as
- packed-refs doesn't support symrefs
- reflogs aren't moved into reftable
on the other hand, the code is already there, and it's well-structured
and well-tested.
refs/reftable-backend.c was created by cannibalizing the first version
of reftable support (github PR 1215 as mentioned above). It supports
reflogs and symrefs, even though those features are never exercised.
This implementation is a prototype, for the following reasons:
- no considerations of backward compatibility and configuring an
extension
- no support for converting between packed-refs and reftable
- no documentation
- test failures when setting GIT_TEST_REFTABLE=1.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
Makefile | 1 +
config.mak.uname | 2 +-
contrib/workdir/git-new-workdir | 2 +-
refs/files-backend.c | 18 +-
refs/refs-internal.h | 1 +
refs/reftable-backend.c | 1658 +++++++++++++++++++++++++++++++
refs/reftable-backend.h | 8 +
7 files changed, 1686 insertions(+), 4 deletions(-)
create mode 100644 refs/reftable-backend.c
create mode 100644 refs/reftable-backend.h
diff --git a/Makefile b/Makefile
index 57763093653..272d3f7f1e9 100644
--- a/Makefile
+++ b/Makefile
@@ -1118,6 +1118,7 @@ LIB_OBJS += reflog.o
LIB_OBJS += refs.o
LIB_OBJS += refs/debug.o
LIB_OBJS += refs/files-backend.o
+LIB_OBJS += refs/reftable-backend.o
LIB_OBJS += refs/iterator.o
LIB_OBJS += refs/packed-backend.o
LIB_OBJS += refs/ref-cache.o
diff --git a/config.mak.uname b/config.mak.uname
index 3bb03f423a0..843829c02fd 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -743,7 +743,7 @@ vcxproj:
# Make .vcxproj files and add them
perl contrib/buildsystems/generate -g Vcxproj
- git add -f git.sln {*,*/lib,t/helper/*}/*.vcxproj
+ git add -f git.sln {*,*/lib,*/libreftable,t/helper/*}/*.vcxproj
# Generate the LinkOrCopyBuiltins.targets and LinkOrCopyRemoteHttp.targets file
(echo '<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">' && \
diff --git a/contrib/workdir/git-new-workdir b/contrib/workdir/git-new-workdir
index 888c34a5215..989197aace0 100755
--- a/contrib/workdir/git-new-workdir
+++ b/contrib/workdir/git-new-workdir
@@ -79,7 +79,7 @@ trap cleanup $siglist
# create the links to the original repo. explicitly exclude index, HEAD and
# logs/HEAD from the list since they are purely related to the current working
# directory, and should not be shared.
-for x in config refs logs/refs objects info hooks packed-refs remotes rr-cache svn
+for x in config refs logs/refs objects info hooks packed-refs remotes rr-cache svn reftable
do
# create a containing directory if needed
case $x in
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 5d288bf38bb..2cd596bbeba 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1,4 +1,5 @@
#include "../git-compat-util.h"
+#include "../abspath.h"
#include "../config.h"
#include "../copy.h"
#include "../environment.h"
@@ -9,6 +10,7 @@
#include "refs-internal.h"
#include "ref-cache.h"
#include "packed-backend.h"
+#include "reftable-backend.h"
#include "../ident.h"
#include "../iterator.h"
#include "../dir-iterator.h"
@@ -98,13 +100,25 @@ static struct ref_store *files_ref_store_create(struct repository *repo,
struct files_ref_store *refs = xcalloc(1, sizeof(*refs));
struct ref_store *ref_store = (struct ref_store *)refs;
struct strbuf sb = STRBUF_INIT;
+ int has_reftable, has_packed;
base_ref_store_init(ref_store, repo, gitdir, &refs_be_files);
refs->store_flags = flags;
get_common_dir_noenv(&sb, gitdir);
refs->gitcommondir = strbuf_detach(&sb, NULL);
- refs->packed_ref_store =
- packed_ref_store_create(repo, refs->gitcommondir, flags);
+
+ strbuf_addf(&sb, "%s/reftable", refs->gitcommondir);
+ has_reftable = is_directory(sb.buf);
+ strbuf_reset(&sb);
+ strbuf_addf(&sb, "%s/packed-refs", refs->gitcommondir);
+ has_packed = file_exists(sb.buf);
+
+ if (!has_packed && !has_reftable)
+ has_reftable = git_env_bool("GIT_TEST_REFTABLE", 0);
+
+ refs->packed_ref_store = has_reftable
+ ? git_reftable_ref_store_create(repo, refs->gitcommondir, flags)
+ : packed_ref_store_create(repo, refs->gitcommondir, flags);
chdir_notify_reparent("files-backend $GIT_DIR", &refs->base.gitdir);
chdir_notify_reparent("files-backend $GIT_COMMONDIR",
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 0a15e8a2ac8..13bd9c79f3c 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -699,6 +699,7 @@ struct ref_storage_be {
};
extern struct ref_storage_be refs_be_files;
+extern struct ref_storage_be refs_be_reftable;
extern struct ref_storage_be refs_be_packed;
/*
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
new file mode 100644
index 00000000000..16064fff198
--- /dev/null
+++ b/refs/reftable-backend.c
@@ -0,0 +1,1658 @@
+#include "../git-compat-util.h"
+#include "../abspath.h"
+#include "../chdir-notify.h"
+#include "../config.h"
+#include "../environment.h"
+#include "../hash.h"
+#include "../hex.h"
+#include "../iterator.h"
+#include "../ident.h"
+#include "../lockfile.h"
+#include "../object.h"
+#include "../path.h"
+#include "../refs.h"
+#include "../reftable/reftable-stack.h"
+#include "../reftable/reftable-record.h"
+#include "../reftable/reftable-error.h"
+#include "../reftable/reftable-blocksource.h"
+#include "../reftable/reftable-reader.h"
+#include "../reftable/reftable-iterator.h"
+#include "../reftable/reftable-merged.h"
+#include "../reftable/reftable-generic.h"
+#include "../worktree.h"
+#include "refs-internal.h"
+#include "reftable-backend.h"
+#include "../repository.h"
+
+extern struct ref_storage_be refs_be_reftable;
+
+struct git_reftable_ref_store {
+ struct ref_store base;
+ unsigned int store_flags;
+
+ int err;
+ char *repo_dir;
+ char *reftable_dir;
+
+ struct reftable_stack *main_stack;
+
+ struct reftable_write_options write_options;
+};
+
+static struct reftable_stack *stack_for(struct git_reftable_ref_store *store,
+ const char *refname)
+{
+ return store->main_stack;
+}
+
+static int should_log(const char *refname)
+{
+ return log_all_ref_updates != LOG_REFS_NONE &&
+ (log_all_ref_updates == LOG_REFS_ALWAYS ||
+ log_all_ref_updates == LOG_REFS_UNSET ||
+ should_autocreate_reflog(refname));
+}
+
+static const char *bare_ref_name(const char *ref)
+{
+ const char *stripped;
+ parse_worktree_ref(ref, NULL, NULL, &stripped);
+ return stripped;
+}
+
+static int git_reftable_read_raw_ref(struct ref_store *ref_store,
+ const char *refname, struct object_id *oid,
+ struct strbuf *referent,
+ unsigned int *type, int *failure_errno);
+
+static void clear_reftable_log_record(struct reftable_log_record *log)
+{
+ switch (log->value_type) {
+ case REFTABLE_LOG_UPDATE:
+ /* when we write log records, the hashes are owned by a struct
+ * oid */
+ log->value.update.old_hash = NULL;
+ log->value.update.new_hash = NULL;
+ break;
+ case REFTABLE_LOG_DELETION:
+ break;
+ }
+ reftable_log_record_release(log);
+}
+
+static void fill_reftable_log_record(struct reftable_log_record *log)
+{
+ const char *info = git_committer_info(0);
+ struct ident_split split = { NULL };
+ int result = split_ident_line(&split, info, strlen(info));
+ int sign = 1;
+ assert(0 == result);
+
+ reftable_log_record_release(log);
+ log->value_type = REFTABLE_LOG_UPDATE;
+ log->value.update.name =
+ xstrndup(split.name_begin, split.name_end - split.name_begin);
+ log->value.update.email =
+ xstrndup(split.mail_begin, split.mail_end - split.mail_begin);
+ log->value.update.time = atol(split.date_begin);
+ if (*split.tz_begin == '-') {
+ sign = -1;
+ split.tz_begin++;
+ }
+ if (*split.tz_begin == '+') {
+ sign = 1;
+ split.tz_begin++;
+ }
+
+ log->value.update.tz_offset = sign * atoi(split.tz_begin);
+}
+
+struct ref_store *git_reftable_ref_store_create(struct repository *repo,
+ const char *gitdir,
+ unsigned int store_flags)
+{
+ struct git_reftable_ref_store *refs = xcalloc(1, sizeof(*refs));
+ struct ref_store *ref_store = (struct ref_store *)refs;
+ struct strbuf sb = STRBUF_INIT;
+ int shared = get_shared_repository();
+ if (shared < 0)
+ shared = -shared;
+
+ refs->write_options.block_size = 4096;
+ refs->write_options.hash_id = the_hash_algo->format_id;
+ if (shared && (shared & 0600))
+ refs->write_options.default_permissions = shared;
+
+ /* XXX should this use `path` or `gitdir.buf` ? */
+ base_ref_store_init(ref_store, repo, gitdir, &refs_be_reftable);
+ refs->store_flags = store_flags;
+ strbuf_addf(&sb, "%s/reftable", gitdir);
+ refs->reftable_dir = xstrdup(sb.buf);
+ safe_create_dir(refs->reftable_dir, 1);
+
+ refs->base.repo = repo;
+ strbuf_reset(&sb);
+
+ refs->err = reftable_new_stack(&refs->main_stack, refs->reftable_dir,
+ refs->write_options);
+ assert(refs->err != REFTABLE_API_ERROR);
+
+ strbuf_release(&sb);
+
+ /* TODO something with chdir_notify_reparent() ? */
+
+ return ref_store;
+}
+
+static int git_reftable_init_db(struct ref_store *ref_store, struct strbuf *err)
+{
+ struct git_reftable_ref_store *refs =
+ (struct git_reftable_ref_store *)ref_store;
+ safe_create_dir(refs->reftable_dir, 1);
+ return 0;
+}
+
+struct git_reftable_iterator {
+ struct ref_iterator base;
+ struct reftable_iterator iter;
+ struct reftable_ref_record ref;
+ struct object_id oid;
+ struct ref_store *ref_store;
+
+ unsigned int flags;
+ int err;
+ const char *prefix;
+};
+
+static int reftable_ref_iterator_advance(struct ref_iterator *ref_iterator)
+{
+ struct git_reftable_iterator *ri =
+ (struct git_reftable_iterator *)ref_iterator;
+ struct git_reftable_ref_store *refs =
+ (struct git_reftable_ref_store *)ri->ref_store;
+
+ while (ri->err == 0) {
+ int signed_flags = 0;
+ ri->err = reftable_iterator_next_ref(&ri->iter, &ri->ref);
+ if (ri->err) {
+ break;
+ }
+
+ ri->base.flags = 0;
+
+ if (!strcmp(ri->ref.refname, "HEAD")) {
+ /*
+ HEAD should not be produced by default.Other
+ pseudorefs (FETCH_HEAD etc.) shouldn't be
+ stored in reftables at all.
+ */
+ continue;
+ }
+ ri->base.refname = ri->ref.refname;
+ if (ri->prefix &&
+ strncmp(ri->prefix, ri->ref.refname, strlen(ri->prefix))) {
+ ri->err = 1;
+ break;
+ }
+ if (ri->flags & DO_FOR_EACH_PER_WORKTREE_ONLY &&
+ parse_worktree_ref(ri->base.refname, NULL, NULL, NULL) !=
+ REF_WORKTREE_CURRENT)
+ continue;
+
+ if (ri->flags & DO_FOR_EACH_INCLUDE_BROKEN &&
+ check_refname_format(ri->base.refname,
+ REFNAME_ALLOW_ONELEVEL)) {
+ /* This is odd, as REF_BAD_NAME and REF_ISBROKEN are
+ orthogonal, but it's what the spec says and the
+ files-backend does. */
+ ri->base.flags |= REF_BAD_NAME | REF_ISBROKEN;
+ break;
+ }
+
+ switch (ri->ref.value_type) {
+ case REFTABLE_REF_VAL1:
+ oidread(&ri->oid, ri->ref.value.val1);
+ break;
+ case REFTABLE_REF_VAL2:
+ oidread(&ri->oid, ri->ref.value.val2.value);
+ break;
+ case REFTABLE_REF_SYMREF:
+ ri->base.flags = REF_ISSYMREF;
+ break;
+ default:
+ abort();
+ }
+
+ ri->base.oid = &ri->oid;
+ if (!(ri->flags & DO_FOR_EACH_INCLUDE_BROKEN) &&
+ !ref_resolves_to_object(ri->base.refname, refs->base.repo,
+ ri->base.oid, ri->base.flags)) {
+ continue;
+ }
+
+ /* Arguably, resolving recursively following symlinks should be
+ * lifted to refs.c because it is shared between reftable and
+ * the files backend, but it's here now.
+ */
+ if (!refs_resolve_ref_unsafe(ri->ref_store, ri->ref.refname,
+ RESOLVE_REF_READING, &ri->oid,
+ &signed_flags)) {
+ ri->base.flags = signed_flags;
+ if (ri->ref.value_type == REFTABLE_REF_SYMREF &&
+ ri->flags & DO_FOR_EACH_OMIT_DANGLING_SYMREFS)
+ continue;
+
+ if (ri->ref.value_type == REFTABLE_REF_SYMREF &&
+ !(ri->flags & DO_FOR_EACH_INCLUDE_BROKEN) &&
+ (ri->base.flags & REF_ISBROKEN)) {
+ continue;
+ }
+
+ if (is_null_oid(&ri->oid)) {
+ oidclr(&ri->oid);
+ ri->base.flags |= REF_ISBROKEN;
+ }
+ }
+ break;
+ }
+
+ if (ri->err > 0) {
+ return ITER_DONE;
+ }
+ if (ri->err < 0) {
+ return ITER_ERROR;
+ }
+
+ return ITER_OK;
+}
+
+static int reftable_ref_iterator_peel(struct ref_iterator *ref_iterator,
+ struct object_id *peeled)
+{
+ struct git_reftable_iterator *ri =
+ (struct git_reftable_iterator *)ref_iterator;
+ if (ri->ref.value_type == REFTABLE_REF_VAL2) {
+ oidread(peeled, ri->ref.value.val2.target_value);
+ return 0;
+ }
+
+ return -1;
+}
+
+static int reftable_ref_iterator_abort(struct ref_iterator *ref_iterator)
+{
+ struct git_reftable_iterator *ri =
+ (struct git_reftable_iterator *)ref_iterator;
+ reftable_ref_record_release(&ri->ref);
+ reftable_iterator_destroy(&ri->iter);
+ return 0;
+}
+
+static struct ref_iterator_vtable reftable_ref_iterator_vtable = {
+ reftable_ref_iterator_advance, reftable_ref_iterator_peel,
+ reftable_ref_iterator_abort
+};
+
+static struct ref_iterator *
+git_reftable_ref_iterator_begin(struct ref_store *ref_store, const char *prefix,
+ const char **exclude_patterns_TODO,
+ unsigned int flags)
+{
+ struct git_reftable_ref_store *refs =
+ (struct git_reftable_ref_store *)ref_store;
+ struct git_reftable_iterator *ri = xcalloc(1, sizeof(*ri));
+
+ if (refs->err < 0) {
+ ri->err = refs->err;
+ } else {
+ struct reftable_merged_table *mt =
+ reftable_stack_merged_table(refs->main_stack);
+ ri->err = reftable_merged_table_seek_ref(mt, &ri->iter, prefix);
+ }
+
+ base_ref_iterator_init(&ri->base, &reftable_ref_iterator_vtable, 1);
+ ri->prefix = prefix;
+ ri->base.oid = &ri->oid;
+ ri->flags = flags;
+ ri->ref_store = ref_store;
+ return &ri->base;
+}
+
+static struct ref_transaction *git_reftable_transaction_begin(struct ref_store *ref_store,
+ struct strbuf *errbuf)
+{
+ struct git_reftable_ref_store *refs =
+ (struct git_reftable_ref_store *)ref_store;
+ struct reftable_addition *add = NULL;
+ struct ref_transaction *transaction = NULL;
+ struct reftable_stack *stack = refs->main_stack;
+
+ int err = refs->err;
+ if (err < 0) {
+ goto done;
+ }
+
+ err = reftable_stack_reload(stack);
+ if (err) {
+ goto done;
+ }
+
+ err = reftable_stack_new_addition(&add, stack);
+ if (err) {
+ strbuf_addf(errbuf, "reftable: transaction begin: %s",
+ reftable_error_str(err));
+ goto done;
+ }
+
+ CALLOC_ARRAY(transaction, 1);
+ transaction->backend_data = add;
+done:
+ return transaction;
+}
+
+static int git_reftable_transaction_prepare(struct ref_store *ref_store,
+ struct ref_transaction *transaction,
+ struct strbuf *errbuf)
+{
+ transaction->state = REF_TRANSACTION_PREPARED;
+ return 0;
+}
+
+static int git_reftable_transaction_abort(struct ref_store *ref_store,
+ struct ref_transaction *transaction,
+ struct strbuf *err)
+{
+ struct reftable_addition *add =
+ (struct reftable_addition *)transaction->backend_data;
+ reftable_addition_destroy(add);
+ transaction->backend_data = NULL;
+
+ /* XXX. Shouldn't this be handled generically in refs.c? */
+ transaction->state = REF_TRANSACTION_CLOSED;
+ return 0;
+}
+
+static int reftable_check_old_oid(struct ref_store *refs, const char *refname,
+ struct object_id *want_oid)
+{
+ struct object_id out_oid;
+ int out_flags = 0;
+ const char *resolved = refs_resolve_ref_unsafe(
+ refs, refname, RESOLVE_REF_READING, &out_oid, &out_flags);
+ if (is_null_oid(want_oid) != !resolved) {
+ return REFTABLE_LOCK_ERROR;
+ }
+
+ if (resolved && !oideq(&out_oid, want_oid)) {
+ return REFTABLE_LOCK_ERROR;
+ }
+
+ return 0;
+}
+
+static int ref_update_cmp(const void *a, const void *b)
+{
+ return strcmp((*(struct ref_update **)a)->refname,
+ (*(struct ref_update **)b)->refname);
+}
+
+static int write_transaction_table(struct reftable_writer *writer, void *arg)
+{
+ struct ref_transaction *transaction = (struct ref_transaction *)arg;
+ struct git_reftable_ref_store *refs =
+ (struct git_reftable_ref_store *)transaction->ref_store;
+ struct reftable_stack *stack =
+ stack_for(refs, transaction->updates[0]->refname);
+ uint64_t ts = reftable_stack_next_update_index(stack);
+ int err = 0;
+ int i = 0;
+ int log_count = 0;
+ struct reftable_log_record *logs =
+ calloc(transaction->nr, sizeof(*logs));
+ struct ref_update **sorted =
+ malloc(transaction->nr * sizeof(struct ref_update *));
+ struct reftable_merged_table *mt = reftable_stack_merged_table(stack);
+ struct reftable_table tab = { NULL };
+ struct reftable_ref_record ref = { NULL };
+ reftable_table_from_merged_table(&tab, mt);
+ COPY_ARRAY(sorted, transaction->updates, transaction->nr);
+ QSORT(sorted, transaction->nr, ref_update_cmp);
+ reftable_writer_set_limits(writer, ts, ts);
+
+ for (i = 0; i < transaction->nr; i++) {
+ struct ref_update *u = sorted[i];
+ struct reftable_log_record *log = &logs[log_count];
+ struct object_id old_id = *null_oid();
+
+ log->value.update.new_hash = NULL;
+ log->value.update.old_hash = NULL;
+ if ((u->flags & REF_FORCE_CREATE_REFLOG) ||
+ should_log(u->refname))
+ log_count++;
+ fill_reftable_log_record(log);
+
+ log->update_index = ts;
+ log->value_type = REFTABLE_LOG_UPDATE;
+ log->refname = xstrdup(u->refname);
+ log->value.update.new_hash = u->new_oid.hash;
+ log->value.update.message =
+ xstrndup(u->msg, refs->write_options.block_size / 2);
+
+ err = reftable_table_read_ref(&tab, u->refname, &ref);
+ if (err < 0)
+ goto done;
+ else if (err > 0) {
+ err = 0;
+ }
+
+ /* XXX if this is a symref (say, HEAD), should we deref the
+ * symref and check the update.old_hash against the referent? */
+ if (ref.value_type == REFTABLE_REF_VAL2 ||
+ ref.value_type == REFTABLE_REF_VAL1)
+ oidread(&old_id, ref.value.val1);
+
+ /* XXX fold together with the old_id check below? */
+ log->value.update.old_hash = old_id.hash;
+ if (u->flags & REF_LOG_ONLY) {
+ continue;
+ }
+
+ if (u->flags & REF_HAVE_NEW) {
+ struct reftable_ref_record ref = { NULL };
+ struct object_id peeled;
+
+ int peel_error = peel_object(&u->new_oid, &peeled);
+ ref.refname = (char *)u->refname;
+ ref.update_index = ts;
+
+ if (!peel_error) {
+ ref.value_type = REFTABLE_REF_VAL2;
+ ref.value.val2.target_value = peeled.hash;
+ ref.value.val2.value = u->new_oid.hash;
+ } else if (!is_null_oid(&u->new_oid)) {
+ ref.value_type = REFTABLE_REF_VAL1;
+ ref.value.val1 = u->new_oid.hash;
+ }
+
+ err = reftable_writer_add_ref(writer, &ref);
+ if (err < 0) {
+ goto done;
+ }
+ }
+ }
+
+ for (i = 0; i < log_count; i++) {
+ err = reftable_writer_add_log(writer, &logs[i]);
+ logs[i].value.update.new_hash = NULL;
+ logs[i].value.update.old_hash = NULL;
+ clear_reftable_log_record(&logs[i]);
+ if (err < 0) {
+ goto done;
+ }
+ }
+
+done:
+ assert(err != REFTABLE_API_ERROR);
+ reftable_ref_record_release(&ref);
+ free(logs);
+ free(sorted);
+ return err;
+}
+
+static int git_reftable_transaction_finish(struct ref_store *ref_store,
+ struct ref_transaction *transaction,
+ struct strbuf *errmsg)
+{
+ struct git_reftable_ref_store *refs =
+ (struct git_reftable_ref_store *)transaction->ref_store;
+ struct reftable_addition *add =
+ (struct reftable_addition *)transaction->backend_data;
+ int err = 0;
+ int i;
+
+ for (i = 0; i < transaction->nr; i++) {
+ struct ref_update *u = transaction->updates[i];
+ if (u->flags & REF_HAVE_OLD) {
+ err = reftable_check_old_oid(transaction->ref_store,
+ u->refname, &u->old_oid);
+ if (err < 0) {
+ goto done;
+ }
+ }
+ }
+ if (transaction->nr) {
+ err = reftable_addition_add(add, &write_transaction_table,
+ transaction);
+ if (err < 0) {
+ goto done;
+ }
+ }
+
+ err = reftable_addition_commit(add);
+
+ if (!err)
+ err = reftable_stack_auto_compact(refs->main_stack);
+done:
+ assert(err != REFTABLE_API_ERROR);
+ reftable_addition_destroy(add);
+ transaction->state = REF_TRANSACTION_CLOSED;
+ transaction->backend_data = NULL;
+ if (err) {
+ strbuf_addf(errmsg, "reftable: transaction failure: %s",
+ reftable_error_str(err));
+ return -1;
+ }
+ return err;
+}
+
+static int
+git_reftable_transaction_initial_commit(struct ref_store *ref_store,
+ struct ref_transaction *transaction,
+ struct strbuf *errmsg)
+{
+ int err = git_reftable_transaction_prepare(ref_store, transaction,
+ errmsg);
+ if (err)
+ return err;
+
+ return git_reftable_transaction_finish(ref_store, transaction, errmsg);
+}
+
+struct write_delete_refs_arg {
+ struct git_reftable_ref_store *refs;
+ struct reftable_stack *stack;
+ struct string_list *refnames;
+ const char *logmsg;
+ unsigned int flags;
+};
+
+static int write_delete_refs_table(struct reftable_writer *writer, void *argv)
+{
+ struct write_delete_refs_arg *arg =
+ (struct write_delete_refs_arg *)argv;
+ uint64_t ts = reftable_stack_next_update_index(arg->stack);
+ int err = 0;
+ int i = 0;
+
+ reftable_writer_set_limits(writer, ts, ts);
+ for (i = 0; i < arg->refnames->nr; i++) {
+ struct reftable_ref_record ref = {
+ .refname = (char *)arg->refnames->items[i].string,
+ .value_type = REFTABLE_REF_DELETION,
+ .update_index = ts,
+ };
+ err = reftable_writer_add_ref(writer, &ref);
+ if (err < 0) {
+ return err;
+ }
+ }
+
+ for (i = 0; i < arg->refnames->nr; i++) {
+ struct reftable_log_record log = {
+ .update_index = ts,
+ };
+ struct reftable_ref_record current = { NULL };
+ fill_reftable_log_record(&log);
+ log.update_index = ts;
+ log.refname = xstrdup(arg->refnames->items[i].string);
+ if (!should_log(log.refname)) {
+ continue;
+ }
+ log.value.update.message = xstrndup(
+ arg->logmsg, arg->refs->write_options.block_size / 2);
+ log.value.update.new_hash = NULL;
+ log.value.update.old_hash = NULL;
+ if (reftable_stack_read_ref(arg->stack, log.refname,
+ ¤t) == 0) {
+ log.value.update.old_hash =
+ reftable_ref_record_val1(¤t);
+ }
+ err = reftable_writer_add_log(writer, &log);
+ log.value.update.old_hash = NULL;
+ reftable_ref_record_release(¤t);
+
+ clear_reftable_log_record(&log);
+ if (err < 0) {
+ return err;
+ }
+ }
+ return 0;
+}
+
+static int git_reftable_delete_refs(struct ref_store *ref_store,
+ const char *msg,
+ struct string_list *refnames,
+ unsigned int flags)
+{
+ struct git_reftable_ref_store *refs =
+ (struct git_reftable_ref_store *)ref_store;
+ struct reftable_stack *stack = stack_for(
+ refs, refnames->nr ? refnames->items[0].string : NULL);
+ struct write_delete_refs_arg arg = {
+ .refs = refs,
+ .stack = stack,
+ .refnames = refnames,
+ .logmsg = msg,
+ .flags = flags,
+ };
+ int err = refs->err;
+ if (err < 0) {
+ goto done;
+ }
+
+ string_list_sort(refnames);
+ err = reftable_stack_reload(stack);
+ if (err) {
+ goto done;
+ }
+ err = reftable_stack_add(stack, &write_delete_refs_table, &arg);
+done:
+ assert(err != REFTABLE_API_ERROR);
+ return err;
+}
+
+static int git_reftable_pack_refs(struct ref_store *ref_store,
+ struct pack_refs_opts *opts)
+{
+ struct git_reftable_ref_store *refs =
+ (struct git_reftable_ref_store *)ref_store;
+
+ int err = refs->err;
+ if (err < 0) {
+ return err;
+ }
+ err = reftable_stack_compact_all(refs->main_stack, NULL);
+ if (err == 0)
+ err = reftable_stack_clean(refs->main_stack);
+
+ return err;
+}
+
+struct write_create_symref_arg {
+ struct git_reftable_ref_store *refs;
+ struct reftable_stack *stack;
+ const char *refname;
+ const char *target;
+ const char *logmsg;
+};
+
+static int write_create_symref_table(struct reftable_writer *writer, void *arg)
+{
+ struct write_create_symref_arg *create =
+ (struct write_create_symref_arg *)arg;
+ uint64_t ts = reftable_stack_next_update_index(create->stack);
+ int err = 0;
+
+ struct reftable_ref_record ref = {
+ .refname = (char *)create->refname,
+ .value_type = REFTABLE_REF_SYMREF,
+ .value.symref = (char *)create->target,
+ .update_index = ts,
+ };
+ reftable_writer_set_limits(writer, ts, ts);
+ err = reftable_writer_add_ref(writer, &ref);
+ if (err == 0) {
+ struct reftable_log_record log = { NULL };
+ struct object_id new_oid;
+ struct object_id old_oid;
+
+ fill_reftable_log_record(&log);
+ log.refname = xstrdup(create->refname);
+ if (!should_log(log.refname)) {
+ return err;
+ }
+ log.update_index = ts;
+ log.value.update.message =
+ xstrndup(create->logmsg,
+ create->refs->write_options.block_size / 2);
+ if (refs_resolve_ref_unsafe(
+ (struct ref_store *)create->refs, create->refname,
+ RESOLVE_REF_READING, &old_oid, NULL)) {
+ log.value.update.old_hash = old_oid.hash;
+ }
+
+ if (refs_resolve_ref_unsafe((struct ref_store *)create->refs,
+ create->target, RESOLVE_REF_READING,
+ &new_oid, NULL)) {
+ log.value.update.new_hash = new_oid.hash;
+ }
+
+ if (log.value.update.old_hash ||
+ log.value.update.new_hash) {
+ err = reftable_writer_add_log(writer, &log);
+ }
+ log.refname = NULL;
+ log.value.update.message = NULL;
+ log.value.update.old_hash = NULL;
+ log.value.update.new_hash = NULL;
+ clear_reftable_log_record(&log);
+ }
+ return err;
+}
+
+static int git_reftable_create_symref(struct ref_store *ref_store,
+ const char *refname, const char *target,
+ const char *logmsg)
+{
+ struct git_reftable_ref_store *refs =
+ (struct git_reftable_ref_store *)ref_store;
+ struct reftable_stack *stack = stack_for(refs, refname);
+ struct write_create_symref_arg arg = { .refs = refs,
+ .stack = stack,
+ .refname = refname,
+ .target = target,
+ .logmsg = logmsg };
+ int err = refs->err;
+ if (err < 0) {
+ goto done;
+ }
+ err = reftable_stack_reload(stack);
+ if (err) {
+ goto done;
+ }
+ err = reftable_stack_add(stack, &write_create_symref_table, &arg);
+done:
+ assert(err != REFTABLE_API_ERROR);
+ return err;
+}
+
+struct write_rename_arg {
+ struct git_reftable_ref_store *refs;
+ struct reftable_stack *stack;
+ const char *oldname;
+ const char *newname;
+ const char *logmsg;
+};
+
+static int write_rename_table(struct reftable_writer *writer, void *argv)
+{
+ struct write_rename_arg *arg = (struct write_rename_arg *)argv;
+ uint64_t ts = reftable_stack_next_update_index(arg->stack);
+ struct reftable_ref_record old_ref = { NULL };
+ struct reftable_ref_record new_ref = { NULL };
+ int err = reftable_stack_read_ref(arg->stack, arg->oldname, &old_ref);
+ struct reftable_ref_record todo[2] = {
+ {
+ .refname = (char *)arg->oldname,
+ .update_index = ts,
+ .value_type = REFTABLE_REF_DELETION,
+ },
+ old_ref,
+ };
+
+ if (err) {
+ goto done;
+ }
+
+ /* git-branch supports a --force, but the check is not atomic. */
+ if (!reftable_stack_read_ref(arg->stack, arg->newname, &new_ref)) {
+ goto done;
+ }
+
+ reftable_writer_set_limits(writer, ts, ts);
+
+ todo[1].update_index = ts;
+ todo[1].refname = (char *)arg->newname;
+
+ err = reftable_writer_add_refs(writer, todo, 2);
+ if (err < 0) {
+ goto done;
+ }
+
+ if (reftable_ref_record_val1(&old_ref)) {
+ uint8_t *val1 = reftable_ref_record_val1(&old_ref);
+ struct reftable_log_record todo[2] = { { NULL } };
+ int firstlog = 0;
+ int lastlog = 2;
+ char *msg = xstrndup(arg->logmsg,
+ arg->refs->write_options.block_size / 2);
+ fill_reftable_log_record(&todo[0]);
+ fill_reftable_log_record(&todo[1]);
+
+ todo[0].refname = xstrdup(arg->oldname);
+ todo[0].update_index = ts;
+ todo[0].value.update.message = msg;
+ todo[0].value.update.old_hash = val1;
+ todo[0].value.update.new_hash = NULL;
+
+ todo[1].refname = xstrdup(arg->newname);
+ todo[1].update_index = ts;
+ todo[1].value.update.old_hash = NULL;
+ todo[1].value.update.new_hash = val1;
+ todo[1].value.update.message = xstrdup(msg);
+
+ if (!should_log(todo[1].refname)) {
+ lastlog--;
+ }
+ if (!should_log(todo[0].refname)) {
+ firstlog++;
+ }
+ err = reftable_writer_add_logs(writer, &todo[firstlog],
+ lastlog - firstlog);
+
+ clear_reftable_log_record(&todo[0]);
+ clear_reftable_log_record(&todo[1]);
+ if (err < 0) {
+ goto done;
+ }
+
+ } else {
+ /* XXX what should we write into the reflog if we rename a
+ * symref? */
+ }
+
+done:
+ assert(err != REFTABLE_API_ERROR);
+ reftable_ref_record_release(&new_ref);
+ reftable_ref_record_release(&old_ref);
+ return err;
+}
+
+static int write_copy_table(struct reftable_writer *writer, void *argv)
+{
+ struct write_rename_arg *arg = (struct write_rename_arg *)argv;
+ uint64_t ts = reftable_stack_next_update_index(arg->stack);
+ struct reftable_ref_record old_ref = { NULL };
+ struct reftable_ref_record new_ref = { NULL };
+ struct reftable_log_record log = { NULL };
+ struct reftable_iterator it = { NULL };
+ int err = reftable_stack_read_ref(arg->stack, arg->oldname, &old_ref);
+ if (err) {
+ goto done;
+ }
+
+ /* git-branch supports a --force, but the check is not atomic. */
+ if (reftable_stack_read_ref(arg->stack, arg->newname, &new_ref) == 0) {
+ goto done;
+ }
+
+ reftable_writer_set_limits(writer, ts, ts);
+
+ FREE_AND_NULL(old_ref.refname);
+ old_ref.refname = xstrdup(arg->newname);
+ old_ref.update_index = ts;
+ err = reftable_writer_add_ref(writer, &old_ref);
+ if (err < 0) {
+ goto done;
+ }
+
+ /* XXX this copies the entire reflog history. Is this the right
+ * semantics? should clear out existing reflog entries for oldname? */
+ if (!should_log(arg->newname))
+ goto done;
+
+ err = reftable_merged_table_seek_log(
+ reftable_stack_merged_table(arg->stack), &it, arg->oldname);
+ if (err < 0) {
+ goto done;
+ }
+ while (1) {
+ int err = reftable_iterator_next_log(&it, &log);
+ if (err < 0) {
+ goto done;
+ }
+
+ if (err > 0 || strcmp(log.refname, arg->oldname)) {
+ break;
+ }
+ FREE_AND_NULL(log.refname);
+ log.refname = xstrdup(arg->newname);
+ reftable_writer_add_log(writer, &log);
+ reftable_log_record_release(&log);
+ }
+
+done:
+ assert(err != REFTABLE_API_ERROR);
+ reftable_ref_record_release(&new_ref);
+ reftable_ref_record_release(&old_ref);
+ reftable_log_record_release(&log);
+ reftable_iterator_destroy(&it);
+ return err;
+}
+
+static int git_reftable_rename_ref(struct ref_store *ref_store,
+ const char *oldrefname,
+ const char *newrefname, const char *logmsg)
+{
+ struct git_reftable_ref_store *refs =
+ (struct git_reftable_ref_store *)ref_store;
+ struct reftable_stack *stack = stack_for(refs, newrefname);
+ struct write_rename_arg arg = {
+ .refs = refs,
+ .stack = stack,
+ .oldname = oldrefname,
+ .newname = newrefname,
+ .logmsg = logmsg,
+ };
+ int err = refs->err;
+ if (err < 0) {
+ goto done;
+ }
+ err = reftable_stack_reload(stack);
+ if (err) {
+ goto done;
+ }
+
+ err = reftable_stack_add(stack, &write_rename_table, &arg);
+done:
+ assert(err != REFTABLE_API_ERROR);
+ return err;
+}
+
+static int git_reftable_copy_ref(struct ref_store *ref_store,
+ const char *oldrefname, const char *newrefname,
+ const char *logmsg)
+{
+ struct git_reftable_ref_store *refs =
+ (struct git_reftable_ref_store *)ref_store;
+ struct reftable_stack *stack = stack_for(refs, newrefname);
+ struct write_rename_arg arg = {
+ .refs = refs,
+ .stack = stack,
+ .oldname = oldrefname,
+ .newname = newrefname,
+ .logmsg = logmsg,
+ };
+ int err = refs->err;
+ if (err < 0) {
+ goto done;
+ }
+ err = reftable_stack_reload(stack);
+ if (err) {
+ goto done;
+ }
+
+ err = reftable_stack_add(stack, &write_copy_table, &arg);
+done:
+ assert(err != REFTABLE_API_ERROR);
+ return err;
+}
+
+struct git_reftable_reflog_ref_iterator {
+ struct ref_iterator base;
+ struct reftable_iterator iter;
+ struct reftable_log_record log;
+ struct object_id oid;
+ struct git_reftable_ref_store *refs;
+
+ /* Used when iterating over worktree & main */
+ struct reftable_merged_table *merged;
+ char *last_name;
+};
+
+static int
+git_reftable_reflog_ref_iterator_advance(struct ref_iterator *ref_iterator)
+{
+ struct git_reftable_reflog_ref_iterator *ri =
+ (struct git_reftable_reflog_ref_iterator *)ref_iterator;
+
+ while (1) {
+ int flags = 0;
+ int err = reftable_iterator_next_log(&ri->iter, &ri->log);
+
+ if (err > 0) {
+ return ITER_DONE;
+ }
+ if (err < 0) {
+ return ITER_ERROR;
+ }
+
+ ri->base.refname = ri->log.refname;
+ if (ri->last_name &&
+ !strcmp(ri->log.refname, ri->last_name)) {
+ /* we want the refnames that we have reflogs for, so we
+ * skip if we've already produced this name. This could
+ * be faster by seeking directly to
+ * reflog@update_index==0.
+ */
+ continue;
+ }
+
+ if (!refs_resolve_ref_unsafe(&ri->refs->base, ri->log.refname,
+ 0, &ri->oid, &flags)) {
+ error("bad ref for %s", ri->log.refname);
+ continue;
+ }
+
+ free(ri->last_name);
+ ri->last_name = xstrdup(ri->log.refname);
+ ri->base.oid = &ri->oid;
+ ri->base.flags = flags;
+ return ITER_OK;
+ }
+}
+
+static int
+git_reftable_reflog_ref_iterator_peel(struct ref_iterator *ref_iterator,
+ struct object_id *peeled)
+{
+ BUG("not supported.");
+ return -1;
+}
+
+static int
+git_reftable_reflog_ref_iterator_abort(struct ref_iterator *ref_iterator)
+{
+ struct git_reftable_reflog_ref_iterator *ri =
+ (struct git_reftable_reflog_ref_iterator *)ref_iterator;
+ reftable_log_record_release(&ri->log);
+ reftable_iterator_destroy(&ri->iter);
+ if (ri->merged)
+ reftable_merged_table_free(ri->merged);
+ return 0;
+}
+
+static struct ref_iterator_vtable git_reftable_reflog_ref_iterator_vtable = {
+ git_reftable_reflog_ref_iterator_advance,
+ git_reftable_reflog_ref_iterator_peel,
+ git_reftable_reflog_ref_iterator_abort
+};
+
+static struct ref_iterator *
+git_reftable_reflog_iterator_begin(struct ref_store *ref_store)
+{
+ struct git_reftable_ref_store *refs =
+ (struct git_reftable_ref_store *)ref_store;
+ struct git_reftable_reflog_ref_iterator *ri = xcalloc(1, sizeof(*ri));
+ struct reftable_stack *stack = refs->main_stack;
+ struct reftable_merged_table *mt =
+ reftable_stack_merged_table(stack);
+ int err = reftable_merged_table_seek_log(mt, &ri->iter, "");
+
+ ri->refs = refs;
+ if (err < 0) {
+ free(ri);
+ /* XXX how to handle errors in iterator_begin()? */
+ return NULL;
+ }
+ base_ref_iterator_init(&ri->base,
+ &git_reftable_reflog_ref_iterator_vtable, 1);
+ ri->base.oid = &ri->oid;
+
+ return (struct ref_iterator *)ri;
+}
+
+static int git_reftable_for_each_reflog_ent_newest_first(
+ struct ref_store *ref_store, const char *refname, each_reflog_ent_fn fn,
+ void *cb_data)
+{
+ struct reftable_iterator it = { NULL };
+ struct git_reftable_ref_store *refs =
+ (struct git_reftable_ref_store *)ref_store;
+ struct reftable_stack *stack = stack_for(refs, refname);
+ struct reftable_merged_table *mt = NULL;
+ int err = 0;
+ struct reftable_log_record log = { NULL };
+
+ if (refs->err < 0) {
+ return refs->err;
+ }
+ refname = bare_ref_name(refname);
+
+ mt = reftable_stack_merged_table(stack);
+ err = reftable_merged_table_seek_log(mt, &it, refname);
+ while (err == 0) {
+ struct object_id old_oid;
+ struct object_id new_oid;
+ const char *full_committer = "";
+
+ err = reftable_iterator_next_log(&it, &log);
+ if (err > 0) {
+ err = 0;
+ break;
+ }
+ if (err < 0) {
+ break;
+ }
+
+ if (strcmp(log.refname, refname)) {
+ break;
+ }
+
+ oidread(&old_oid, log.value.update.old_hash);
+ oidread(&new_oid, log.value.update.new_hash);
+
+ if (is_null_oid(&old_oid) && is_null_oid(&new_oid)) {
+ /* placeholder for existence. */
+ continue;
+ }
+
+ full_committer = fmt_ident(log.value.update.name,
+ log.value.update.email,
+ WANT_COMMITTER_IDENT,
+ /*date*/ NULL, IDENT_NO_DATE);
+ err = fn(&old_oid, &new_oid, full_committer,
+ log.value.update.time, log.value.update.tz_offset,
+ log.value.update.message, cb_data);
+ if (err)
+ break;
+ }
+
+ reftable_log_record_release(&log);
+ reftable_iterator_destroy(&it);
+ return err;
+}
+
+static int git_reftable_for_each_reflog_ent_oldest_first(
+ struct ref_store *ref_store, const char *refname, each_reflog_ent_fn fn,
+ void *cb_data)
+{
+ struct reftable_iterator it = { NULL };
+ struct git_reftable_ref_store *refs =
+ (struct git_reftable_ref_store *)ref_store;
+ struct reftable_stack *stack = stack_for(refs, refname);
+ struct reftable_merged_table *mt = NULL;
+ struct reftable_log_record *logs = NULL;
+ int cap = 0;
+ int len = 0;
+ int err = 0;
+ int i = 0;
+
+ if (refs->err < 0) {
+ return refs->err;
+ }
+ refname = bare_ref_name(refname);
+ mt = reftable_stack_merged_table(stack);
+ err = reftable_merged_table_seek_log(mt, &it, refname);
+
+ while (err == 0) {
+ struct reftable_log_record log = { NULL };
+ err = reftable_iterator_next_log(&it, &log);
+ if (err > 0) {
+ err = 0;
+ break;
+ }
+ if (err < 0) {
+ break;
+ }
+
+ if (strcmp(log.refname, refname)) {
+ break;
+ }
+
+ if (len == cap) {
+ cap = 2 * cap + 1;
+ logs = realloc(logs, cap * sizeof(*logs));
+ }
+
+ logs[len++] = log;
+ }
+
+ for (i = len; i--;) {
+ struct reftable_log_record *log = &logs[i];
+ struct object_id old_oid;
+ struct object_id new_oid;
+ const char *full_committer = "";
+
+ oidread(&old_oid, log->value.update.old_hash);
+ oidread(&new_oid, log->value.update.new_hash);
+
+ if (is_null_oid(&old_oid) && is_null_oid(&new_oid)) {
+ /* placeholder for existence. */
+ continue;
+ }
+
+ full_committer = fmt_ident(log->value.update.name,
+ log->value.update.email,
+ WANT_COMMITTER_IDENT, NULL,
+ IDENT_NO_DATE);
+ err = fn(&old_oid, &new_oid, full_committer,
+ log->value.update.time, log->value.update.tz_offset,
+ log->value.update.message, cb_data);
+ if (err) {
+ break;
+ }
+ }
+
+ for (i = 0; i < len; i++) {
+ reftable_log_record_release(&logs[i]);
+ }
+ free(logs);
+
+ reftable_iterator_destroy(&it);
+ return err;
+}
+
+static int git_reftable_reflog_exists(struct ref_store *ref_store,
+ const char *refname)
+{
+ struct reftable_iterator it = { NULL };
+ struct git_reftable_ref_store *refs =
+ (struct git_reftable_ref_store *)ref_store;
+ struct reftable_stack *stack = stack_for(refs, refname);
+ struct reftable_merged_table *mt = reftable_stack_merged_table(stack);
+ struct reftable_log_record log = { NULL };
+ int err = refs->err;
+
+ if (err < 0) {
+ goto done;
+ }
+
+ refname = bare_ref_name(refname);
+ err = reftable_merged_table_seek_log(mt, &it, refname);
+ if (err) {
+ goto done;
+ }
+ err = reftable_iterator_next_log(&it, &log);
+ if (err) {
+ goto done;
+ }
+
+ if (strcmp(log.refname, refname)) {
+ err = 1;
+ }
+
+done:
+ reftable_iterator_destroy(&it);
+ reftable_log_record_release(&log);
+ return !err;
+}
+
+struct write_reflog_existence_arg {
+ struct git_reftable_ref_store *refs;
+ const char *refname;
+ struct reftable_stack *stack;
+};
+
+static int write_reflog_existence_table(struct reftable_writer *writer,
+ void *argv)
+{
+ struct write_reflog_existence_arg *arg =
+ (struct write_reflog_existence_arg *)argv;
+ uint64_t ts = reftable_stack_next_update_index(arg->stack);
+ struct reftable_log_record log = { NULL };
+
+ int err = reftable_stack_read_log(arg->stack, arg->refname, &log);
+ if (err <= 0) {
+ goto done;
+ }
+
+ reftable_writer_set_limits(writer, ts, ts);
+
+ log.refname = (char *)arg->refname;
+ log.update_index = ts;
+ log.value_type = REFTABLE_LOG_UPDATE;
+ err = reftable_writer_add_log(writer, &log);
+
+ /* field is not malloced */
+ log.refname = NULL;
+
+done:
+ assert(err != REFTABLE_API_ERROR);
+ reftable_log_record_release(&log);
+ return err;
+}
+
+static int git_reftable_create_reflog(struct ref_store *ref_store,
+ const char *refname,
+ struct strbuf *errmsg)
+{
+ struct git_reftable_ref_store *refs =
+ (struct git_reftable_ref_store *)ref_store;
+ struct reftable_stack *stack = stack_for(refs, refname);
+ struct write_reflog_existence_arg arg = {
+ .refs = refs,
+ .stack = stack,
+ .refname = refname,
+ };
+ int err = refs->err;
+ if (err < 0) {
+ goto done;
+ }
+
+ err = reftable_stack_reload(stack);
+ if (err) {
+ goto done;
+ }
+
+ err = reftable_stack_add(stack, &write_reflog_existence_table, &arg);
+
+done:
+ return err;
+}
+
+struct write_reflog_delete_arg {
+ struct reftable_stack *stack;
+ const char *refname;
+};
+
+static int write_reflog_delete_table(struct reftable_writer *writer, void *argv)
+{
+ struct write_reflog_delete_arg *arg = argv;
+ struct reftable_merged_table *mt =
+ reftable_stack_merged_table(arg->stack);
+ struct reftable_log_record log = { NULL };
+ struct reftable_iterator it = { NULL };
+ uint64_t ts = reftable_stack_next_update_index(arg->stack);
+ int err = reftable_merged_table_seek_log(mt, &it, arg->refname);
+
+ reftable_writer_set_limits(writer, ts, ts);
+ while (err == 0) {
+ struct reftable_log_record tombstone = {
+ .refname = (char *)arg->refname,
+ .update_index = REFTABLE_LOG_DELETION,
+ };
+ err = reftable_iterator_next_log(&it, &log);
+ if (err > 0) {
+ err = 0;
+ break;
+ }
+
+ if (err < 0 || strcmp(log.refname, arg->refname)) {
+ break;
+ }
+ if (log.value_type == REFTABLE_LOG_DELETION)
+ continue;
+
+ tombstone.update_index = log.update_index;
+ err = reftable_writer_add_log(writer, &tombstone);
+ }
+
+ reftable_log_record_release(&log);
+ return err;
+}
+
+static int git_reftable_delete_reflog(struct ref_store *ref_store,
+ const char *refname)
+{
+ struct git_reftable_ref_store *refs =
+ (struct git_reftable_ref_store *)ref_store;
+ struct reftable_stack *stack = stack_for(refs, refname);
+ struct write_reflog_delete_arg arg = {
+ .stack = stack,
+ .refname = refname,
+ };
+ int err = reftable_stack_add(stack, &write_reflog_delete_table, &arg);
+ assert(err != REFTABLE_API_ERROR);
+ return err;
+}
+
+struct reflog_expiry_arg {
+ struct reftable_stack *stack;
+ struct reftable_log_record *records;
+ int len;
+ const char *refname;
+};
+
+static int write_reflog_expiry_table(struct reftable_writer *writer, void *argv)
+{
+ struct reflog_expiry_arg *arg = (struct reflog_expiry_arg *)argv;
+ uint64_t ts = reftable_stack_next_update_index(arg->stack);
+ int i = 0;
+ int live_records = 0;
+ uint64_t max_ts = 0;
+ for (i = 0; i < arg->len; i++) {
+ if (arg->records[i].value_type == REFTABLE_LOG_UPDATE)
+ live_records++;
+
+ if (max_ts < arg->records[i].update_index)
+ max_ts = arg->records[i].update_index;
+ }
+
+ reftable_writer_set_limits(writer, ts, ts);
+ if (live_records == 0) {
+ struct reftable_log_record log = {
+ .refname = (char *)arg->refname,
+ .update_index = max_ts + 1,
+ .value_type = REFTABLE_LOG_UPDATE,
+ /* existence dummy has null new/old oid */
+ };
+ int err;
+ if (log.update_index < ts)
+ log.update_index = ts;
+
+ err = reftable_writer_add_log(writer, &log);
+ if (err) {
+ return err;
+ }
+ }
+
+ for (i = 0; i < arg->len; i++) {
+ int err = reftable_writer_add_log(writer, &arg->records[i]);
+ if (err) {
+ return err;
+ }
+ }
+ return 0;
+}
+
+static int git_reftable_reflog_expire(
+ struct ref_store *ref_store, const char *refname, unsigned int flags,
+ reflog_expiry_prepare_fn prepare_fn,
+ reflog_expiry_should_prune_fn should_prune_fn,
+ reflog_expiry_cleanup_fn cleanup_fn, void *policy_cb_data)
+{
+ /*
+ For log expiry, we write tombstones in place of the expired entries,
+ This means that the entries are still retrievable by delving into the
+ stack, and expiring entries paradoxically takes extra memory.
+
+ This memory is only reclaimed when some operation issues a
+ git_reftable_pack_refs(), which will compact the entire stack and get
+ rid of deletion entries.
+
+ It would be better if the refs backend supported an API that sets a
+ criterion for all refs, passing the criterion to pack_refs().
+
+ On the plus side, because we do the expiration per ref, we can easily
+ insert the reflog existence dummies.
+ */
+ struct git_reftable_ref_store *refs =
+ (struct git_reftable_ref_store *)ref_store;
+ struct reftable_stack *stack = stack_for(refs, refname);
+ struct reftable_merged_table *mt = NULL;
+ struct reflog_expiry_arg arg = {
+ .stack = stack,
+ .refname = refname,
+ };
+ struct reftable_log_record *logs = NULL;
+ struct reftable_log_record *rewritten = NULL;
+ struct reftable_ref_record ref_record = { NULL };
+ int logs_len = 0;
+ int logs_cap = 0;
+ int i = 0;
+ uint8_t *last_hash = NULL;
+ struct reftable_iterator it = { NULL };
+ struct reftable_addition *add = NULL;
+ int err = 0;
+ struct object_id oid = { 0 };
+ if (refs->err < 0) {
+ return refs->err;
+ }
+ err = reftable_stack_reload(stack);
+ if (err) {
+ goto done;
+ }
+
+ mt = reftable_stack_merged_table(stack);
+ err = reftable_merged_table_seek_log(mt, &it, refname);
+ if (err < 0) {
+ goto done;
+ }
+
+ err = reftable_stack_new_addition(&add, stack);
+ if (err) {
+ goto done;
+ }
+ if (!reftable_stack_read_ref(stack, refname, &ref_record)) {
+ uint8_t *hash = reftable_ref_record_val1(&ref_record);
+ if (hash)
+ oidread(&oid, hash);
+ }
+
+ prepare_fn(refname, &oid, policy_cb_data);
+ while (1) {
+ struct reftable_log_record log = { NULL };
+ int err = reftable_iterator_next_log(&it, &log);
+ if (err < 0) {
+ goto done;
+ }
+
+ if (err > 0 || strcmp(log.refname, refname)) {
+ break;
+ }
+
+ if (logs_len >= logs_cap) {
+ int new_cap = logs_cap * 2 + 1;
+ logs = realloc(logs, new_cap * sizeof(*logs));
+ logs_cap = new_cap;
+ }
+ logs[logs_len++] = log;
+ }
+
+ rewritten = calloc(logs_len, sizeof(*rewritten));
+ for (i = logs_len - 1; i >= 0; i--) {
+ struct object_id ooid;
+ struct object_id noid;
+ struct reftable_log_record *dest = &rewritten[i];
+
+ *dest = logs[i];
+ oidread(&ooid, logs[i].value.update.old_hash);
+ oidread(&noid, logs[i].value.update.new_hash);
+
+ if (should_prune_fn(&ooid, &noid, logs[i].value.update.email,
+ (timestamp_t)logs[i].value.update.time,
+ logs[i].value.update.tz_offset,
+ logs[i].value.update.message,
+ policy_cb_data)) {
+ dest->value_type = REFTABLE_LOG_DELETION;
+ } else {
+ if ((flags & EXPIRE_REFLOGS_REWRITE) &&
+ last_hash) {
+ dest->value.update.old_hash = last_hash;
+ }
+ last_hash = logs[i].value.update.new_hash;
+ }
+ }
+
+ arg.records = rewritten;
+ arg.len = logs_len;
+ err = reftable_addition_add(add, &write_reflog_expiry_table, &arg);
+ if (err < 0) {
+ goto done;
+ }
+
+ if (!(flags & EXPIRE_REFLOGS_DRY_RUN)) {
+ /* future improvement: we could skip writing records that were
+ * not changed. */
+ err = reftable_addition_commit(add);
+ }
+
+done:
+ if (add) {
+ cleanup_fn(policy_cb_data);
+ }
+ assert(err != REFTABLE_API_ERROR);
+ reftable_addition_destroy(add);
+ for (i = 0; i < logs_len; i++)
+ reftable_log_record_release(&logs[i]);
+ free(logs);
+ free(rewritten);
+ reftable_iterator_destroy(&it);
+ return err;
+}
+
+static int git_reftable_read_raw_ref(struct ref_store *ref_store,
+ const char *refname, struct object_id *oid,
+ struct strbuf *referent,
+ unsigned int *type, int *failure_errno)
+{
+ struct git_reftable_ref_store *refs =
+ (struct git_reftable_ref_store *)ref_store;
+ struct reftable_stack *stack = stack_for(refs, refname);
+ struct reftable_ref_record ref = { NULL };
+ int err = 0;
+
+ refname = bare_ref_name(refname); /* XXX - in which other cases should
+ we do this? */
+ if (refs->err < 0) {
+ return refs->err;
+ }
+
+ /* This is usually not needed, but Git doesn't signal to ref backend if
+ a subprocess updated the ref DB. So we always check.
+ */
+ err = reftable_stack_reload(stack);
+ if (err) {
+ goto done;
+ }
+
+ err = reftable_stack_read_ref(stack, refname, &ref);
+ if (err > 0) {
+ *failure_errno = ENOENT;
+ err = -1;
+ goto done;
+ }
+ if (err < 0) {
+ goto done;
+ }
+
+ if (ref.value_type == REFTABLE_REF_SYMREF) {
+ strbuf_reset(referent);
+ strbuf_addstr(referent, ref.value.symref);
+ *type |= REF_ISSYMREF;
+ } else if (reftable_ref_record_val1(&ref)) {
+ oidread(oid, reftable_ref_record_val1(&ref));
+ } else {
+ /* We got a tombstone, which should not happen. */
+ BUG("Got reftable_ref_record with value type %d",
+ ref.value_type);
+ }
+
+done:
+ assert(err != REFTABLE_API_ERROR);
+ reftable_ref_record_release(&ref);
+ return err;
+}
+
+static int git_reftable_read_symbolic_ref(struct ref_store *ref_store,
+ const char *refname,
+ struct strbuf *referent)
+{
+ struct git_reftable_ref_store *refs =
+ (struct git_reftable_ref_store *)ref_store;
+ struct reftable_stack *stack = stack_for(refs, refname);
+ struct reftable_ref_record ref = { NULL };
+ int err = 0;
+
+ err = reftable_stack_read_ref(stack, refname, &ref);
+ if (err == 0 && ref.value_type == REFTABLE_REF_SYMREF) {
+ strbuf_addstr(referent, ref.value.symref);
+ } else {
+ err = -1;
+ }
+
+ reftable_ref_record_release(&ref);
+ return err;
+}
+
+struct ref_storage_be refs_be_reftable = {
+ .next = &refs_be_files,
+ .name = "reftable",
+ .init = git_reftable_ref_store_create,
+ .init_db = git_reftable_init_db,
+ .transaction_begin = git_reftable_transaction_begin,
+ .transaction_prepare = git_reftable_transaction_prepare,
+ .transaction_finish = git_reftable_transaction_finish,
+ .transaction_abort = git_reftable_transaction_abort,
+ .initial_transaction_commit = git_reftable_transaction_initial_commit,
+
+ .pack_refs = git_reftable_pack_refs,
+ .create_symref = git_reftable_create_symref,
+ .delete_refs = git_reftable_delete_refs,
+ .rename_ref = git_reftable_rename_ref,
+ .copy_ref = git_reftable_copy_ref,
+
+ .iterator_begin = git_reftable_ref_iterator_begin,
+ .read_raw_ref = git_reftable_read_raw_ref,
+ .read_symbolic_ref = git_reftable_read_symbolic_ref,
+
+ .reflog_iterator_begin = git_reftable_reflog_iterator_begin,
+ .for_each_reflog_ent = git_reftable_for_each_reflog_ent_oldest_first,
+ .for_each_reflog_ent_reverse =
+ git_reftable_for_each_reflog_ent_newest_first,
+ .reflog_exists = git_reftable_reflog_exists,
+ .create_reflog = git_reftable_create_reflog,
+ .delete_reflog = git_reftable_delete_reflog,
+ .reflog_expire = git_reftable_reflog_expire,
+};
diff --git a/refs/reftable-backend.h b/refs/reftable-backend.h
new file mode 100644
index 00000000000..bf5fd42229a
--- /dev/null
+++ b/refs/reftable-backend.h
@@ -0,0 +1,8 @@
+#ifndef REFS_REFTABLE_BACKEND_H
+#define REFS_REFTABLE_BACKEND_H
+
+struct ref_store *git_reftable_ref_store_create(struct repository *repo,
+ const char *gitdir,
+ unsigned int store_flags);
+
+#endif /* REFS_REFTABLE_BACKEND_H */
--
gitgitgadget
^ permalink raw reply related
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