* [PATCH v3 2/8] sha1-array: Implement oid_array_readonly_contains
2019-01-27 19:44 [PATCH v3 1/8] technical doc: add a design doc for the evolve command sxenos
@ 2019-01-27 19:44 ` sxenos
2019-01-28 2:05 ` Junio C Hamano
2019-01-27 19:44 ` [PATCH v3 3/8] ref-filter: Add the metas namespace to ref-filter sxenos
` (5 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: sxenos @ 2019-01-27 19:44 UTC (permalink / raw)
To: git; +Cc: Stefan Xenos, Stefan Xenos
From: Stefan Xenos <sxenos@gmail.com>
Implement a "readonly_contains" function for oid_array that won't
sort the array if it is unsorted. This can be used to test containment in
the rare situations where the array order matters.
The function has intentionally been given a name that is more cumbersome
than the "lookup" function, which is what most callers will will want
in most situations.
Signed-off-by: Stefan Xenos <sxenos@google.com>
---
sha1-array.c | 15 +++++++++++++++
sha1-array.h | 2 ++
t/helper/test-sha1-array.c | 6 ++++++
t/t0064-sha1-array.sh | 22 ++++++++++++++++++++++
4 files changed, 45 insertions(+)
diff --git a/sha1-array.c b/sha1-array.c
index b94e0ec0f5..071fce7e90 100644
--- a/sha1-array.c
+++ b/sha1-array.c
@@ -26,6 +26,21 @@ static const unsigned char *sha1_access(size_t index, void *table)
return array[index].hash;
}
+int oid_array_readonly_contains(const struct oid_array* array,
+ const struct object_id* oid)
+{
+ int i;
+ if (array->sorted) {
+ return sha1_pos(oid->hash, array->oid, array->nr, sha1_access) >= 0;
+ }
+ for (i = 0; i < array->nr; i++) {
+ if (hashcmp(array->oid[i].hash, oid->hash) == 0) {
+ return 1;
+ }
+ }
+ return 0;
+}
+
int oid_array_lookup(struct oid_array *array, const struct object_id *oid)
{
if (!array->sorted)
diff --git a/sha1-array.h b/sha1-array.h
index 232bf95017..7273bd5151 100644
--- a/sha1-array.h
+++ b/sha1-array.h
@@ -13,6 +13,8 @@ struct oid_array {
void oid_array_append(struct oid_array *array, const struct object_id *oid);
int oid_array_lookup(struct oid_array *array, const struct object_id *oid);
void oid_array_clear(struct oid_array *array);
+int oid_array_readonly_contains(const struct oid_array* array,
+ const struct object_id* oid);
typedef int (*for_each_oid_fn)(const struct object_id *oid,
void *data);
diff --git a/t/helper/test-sha1-array.c b/t/helper/test-sha1-array.c
index ad5e69f9d3..fefb1c984f 100644
--- a/t/helper/test-sha1-array.c
+++ b/t/helper/test-sha1-array.c
@@ -25,10 +25,16 @@ int cmd__sha1_array(int argc, const char **argv)
if (get_oid_hex(arg, &oid))
die("not a hexadecimal SHA1: %s", arg);
printf("%d\n", oid_array_lookup(&array, &oid));
+ } else if (skip_prefix(line.buf, "readonly_contains ", &arg)) {
+ if (get_oid_hex(arg, &oid))
+ die("not a hexadecimal SHA1: %s", arg);
+ printf("%d\n", oid_array_readonly_contains(&array, &oid));
} else if (!strcmp(line.buf, "clear"))
oid_array_clear(&array);
else if (!strcmp(line.buf, "for_each_unique"))
oid_array_for_each_unique(&array, print_oid, NULL);
+ else if (!strcmp(line.buf, "for_each"))
+ oid_array_for_each(&array, print_oid, NULL);
else
die("unknown command: %s", line.buf);
}
diff --git a/t/t0064-sha1-array.sh b/t/t0064-sha1-array.sh
index 5dda570b9a..c1bac6fcdd 100755
--- a/t/t0064-sha1-array.sh
+++ b/t/t0064-sha1-array.sh
@@ -32,6 +32,28 @@ test_expect_success 'ordered enumeration with duplicate suppression' '
test_cmp expect actual
'
+test_expect_success 'readonly_contains finds existing' '
+ echo 1 > expect &&
+ echoid "" 88 44 aa 55 >> expect &&
+ {
+ echoid append 88 44 aa 55 &&
+ echoid readonly_contains 55 &&
+ echo for_each
+ } | test-tool sha1-array >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'readonly_contains non-existing query' '
+ echo 0 > expect &&
+ echoid "" 88 44 aa 55 >> expect &&
+ {
+ echoid append 88 44 aa 55 &&
+ echoid readonly_contains 33 &&
+ echo for_each
+ } | test-tool sha1-array >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'lookup' '
{
echoid append 88 44 aa 55 &&
--
2.20.1.495.gaa96b0ce6b-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/8] sha1-array: Implement oid_array_readonly_contains
2019-01-27 19:44 ` [PATCH v3 2/8] sha1-array: Implement oid_array_readonly_contains sxenos
@ 2019-01-28 2:05 ` Junio C Hamano
0 siblings, 0 replies; 15+ messages in thread
From: Junio C Hamano @ 2019-01-28 2:05 UTC (permalink / raw)
To: sxenos; +Cc: git, Stefan Xenos
sxenos@google.com writes:
> Subject: Re: [PATCH v3 2/8] sha1-array: Implement oid_array_readonly_contains
Style: s/: Implement/: implement/
> From: Stefan Xenos <sxenos@gmail.com>
This line wants to say "Stefan Xenos <sxenos@google.com>" to match
S-o-b below (I am assuming that you are following your employer's
open source recommendation to contribute under your corp address).
Perhaps you would want user.email set to the corp address? I am
taking the above as an indication that the commits we are seeing
here have been made under your @gmail.com address and that is why
git-send-email is adding the in-body header.
> diff --git a/sha1-array.c b/sha1-array.c
> index b94e0ec0f5..071fce7e90 100644
> --- a/sha1-array.c
> +++ b/sha1-array.c
> @@ -26,6 +26,21 @@ static const unsigned char *sha1_access(size_t index, void *table)
> return array[index].hash;
> }
>
> +int oid_array_readonly_contains(const struct oid_array* array,
> + const struct object_id* oid)
> +{
> + int i;
Style: blank between decl and first stmt, perhaps?
> + if (array->sorted) {
> + return sha1_pos(oid->hash, array->oid, array->nr, sha1_access) >= 0;
No need for {} around a single statement.
> + }
> + for (i = 0; i < array->nr; i++) {
> + if (hashcmp(array->oid[i].hash, oid->hash) == 0) {
> + return 1;
Likewise.
> + }
> + }
> + return 0;
> +}
> ...
> diff --git a/t/t0064-sha1-array.sh b/t/t0064-sha1-array.sh
> index 5dda570b9a..c1bac6fcdd 100755
> --- a/t/t0064-sha1-array.sh
> +++ b/t/t0064-sha1-array.sh
> @@ -32,6 +32,28 @@ test_expect_success 'ordered enumeration with duplicate suppression' '
> test_cmp expect actual
> '
>
> +test_expect_success 'readonly_contains finds existing' '
> + echo 1 > expect &&
Style: no SP between redirection operator and its target, i.e.
echo 1 >expect &&
> + echoid "" 88 44 aa 55 >> expect &&
Likewise.
echoid "" 88 44 aa 55 >>expect &&
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 3/8] ref-filter: Add the metas namespace to ref-filter
2019-01-27 19:44 [PATCH v3 1/8] technical doc: add a design doc for the evolve command sxenos
2019-01-27 19:44 ` [PATCH v3 2/8] sha1-array: Implement oid_array_readonly_contains sxenos
@ 2019-01-27 19:44 ` sxenos
2019-01-27 19:44 ` [PATCH v3 4/8] evolve: Add support for parsing metacommits sxenos
` (4 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: sxenos @ 2019-01-27 19:44 UTC (permalink / raw)
To: git; +Cc: Stefan Xenos
From: Stefan Xenos <sxenos@google.com>
The metas namespace will contain refs for changes in progress. Add
support for searching this namespace.
Signed-off-by: Stefan Xenos <sxenos@google.com>
---
ref-filter.c | 8 ++++++--
ref-filter.h | 5 +++--
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 422a9c9ae3..4d7bd06880 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1925,7 +1925,8 @@ static int ref_kind_from_refname(const char *refname)
} ref_kind[] = {
{ "refs/heads/" , FILTER_REFS_BRANCHES },
{ "refs/remotes/" , FILTER_REFS_REMOTES },
- { "refs/tags/", FILTER_REFS_TAGS}
+ { "refs/tags/", FILTER_REFS_TAGS },
+ { "refs/metas/", FILTER_REFS_CHANGES }
};
if (!strcmp(refname, "HEAD"))
@@ -1943,7 +1944,8 @@ static int filter_ref_kind(struct ref_filter *filter, const char *refname)
{
if (filter->kind == FILTER_REFS_BRANCHES ||
filter->kind == FILTER_REFS_REMOTES ||
- filter->kind == FILTER_REFS_TAGS)
+ filter->kind == FILTER_REFS_TAGS ||
+ filter->kind == FILTER_REFS_CHANGES )
return filter->kind;
return ref_kind_from_refname(refname);
}
@@ -2128,6 +2130,8 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int
ret = for_each_fullref_in("refs/remotes/", ref_filter_handler, &ref_cbdata, broken);
else if (filter->kind == FILTER_REFS_TAGS)
ret = for_each_fullref_in("refs/tags/", ref_filter_handler, &ref_cbdata, broken);
+ else if (filter->kind == FILTER_REFS_CHANGES)
+ ret = for_each_fullref_in("refs/metas/", ref_filter_handler, &ref_cbdata, broken);
else if (filter->kind & FILTER_REFS_ALL)
ret = for_each_fullref_in_pattern(filter, ref_filter_handler, &ref_cbdata, broken);
if (!ret && (filter->kind & FILTER_REFS_DETACHED_HEAD))
diff --git a/ref-filter.h b/ref-filter.h
index 85c8ebc3b9..19a3e57845 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -18,9 +18,10 @@
#define FILTER_REFS_BRANCHES 0x0004
#define FILTER_REFS_REMOTES 0x0008
#define FILTER_REFS_OTHERS 0x0010
-#define FILTER_REFS_ALL (FILTER_REFS_TAGS | FILTER_REFS_BRANCHES | \
- FILTER_REFS_REMOTES | FILTER_REFS_OTHERS)
#define FILTER_REFS_DETACHED_HEAD 0x0020
+#define FILTER_REFS_CHANGES 0X0040
+#define FILTER_REFS_ALL (FILTER_REFS_TAGS | FILTER_REFS_BRANCHES | \
+ FILTER_REFS_REMOTES | FILTER_REFS_CHANGES | FILTER_REFS_OTHERS)
#define FILTER_REFS_KIND_MASK (FILTER_REFS_ALL | FILTER_REFS_DETACHED_HEAD)
struct atom_value;
--
2.20.1.495.gaa96b0ce6b-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v3 4/8] evolve: Add support for parsing metacommits
2019-01-27 19:44 [PATCH v3 1/8] technical doc: add a design doc for the evolve command sxenos
2019-01-27 19:44 ` [PATCH v3 2/8] sha1-array: Implement oid_array_readonly_contains sxenos
2019-01-27 19:44 ` [PATCH v3 3/8] ref-filter: Add the metas namespace to ref-filter sxenos
@ 2019-01-27 19:44 ` sxenos
2019-01-28 2:05 ` Junio C Hamano
2019-01-27 19:44 ` [PATCH v3 5/8] evolve: Add the change-table structure sxenos
` (3 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: sxenos @ 2019-01-27 19:44 UTC (permalink / raw)
To: git; +Cc: Stefan Xenos
From: Stefan Xenos <sxenos@google.com>
This patch adds the get_metacommit_content method, which can classify
commits as either metacommits or normal commits, determine whether they
are abandoned, and extract the content commit's object id from the
metacommit.
Signed-off-by: Stefan Xenos <sxenos@google.com>
---
Makefile | 1 +
metacommit-parser.c | 87 +++++++++++++++++++++++++++++++++++++++++++++
metacommit-parser.h | 16 +++++++++
3 files changed, 104 insertions(+)
create mode 100644 metacommit-parser.c
create mode 100644 metacommit-parser.h
diff --git a/Makefile b/Makefile
index 1a44c811aa..7ffc383f2b 100644
--- a/Makefile
+++ b/Makefile
@@ -919,6 +919,7 @@ LIB_OBJS += merge.o
LIB_OBJS += merge-blobs.o
LIB_OBJS += merge-recursive.o
LIB_OBJS += mergesort.o
+LIB_OBJS += metacommit-parser.o
LIB_OBJS += midx.o
LIB_OBJS += name-hash.o
LIB_OBJS += negotiator/default.o
diff --git a/metacommit-parser.c b/metacommit-parser.c
new file mode 100644
index 0000000000..5013a108a3
--- /dev/null
+++ b/metacommit-parser.c
@@ -0,0 +1,87 @@
+#include "cache.h"
+#include "metacommit-parser.h"
+#include "commit.h"
+
+/*
+ * Search the commit buffer for a line starting with the given key. Unlike
+ * find_commit_header, this also searches the commit message body.
+ */
+static const char *find_key(const char *msg, const char *key, size_t *out_len)
+{
+ int key_len = strlen(key);
+ const char *line = msg;
+
+ while (line) {
+ const char *eol = strchrnul(line, '\n');
+
+ if (eol - line > key_len &&
+ !strncmp(line, key, key_len) &&
+ line[key_len] == ' ') {
+ *out_len = eol - line - key_len - 1;
+ return line + key_len + 1;
+ }
+ line = *eol ? eol + 1 : NULL;
+ }
+ return NULL;
+}
+
+static struct commit *get_commit_by_index(struct commit_list *to_search, int index)
+{
+ while (to_search && index) {
+ to_search = to_search->next;
+ --index;
+ }
+
+ return to_search->item;
+}
+
+/*
+ * Writes the content parent's object id to "content".
+ * Returns the metacommit type. See the METACOMMIT_TYPE_* constants.
+ */
+int get_metacommit_content(
+ struct commit *commit, struct object_id *content)
+{
+ const char *buffer = get_commit_buffer(commit, NULL);
+ size_t parent_types_size;
+ const char *parent_types = find_key(buffer, "parent-type",
+ &parent_types_size);
+ const char *end;
+ int index = 0;
+ int ret;
+ struct commit *content_parent;
+
+ if (!parent_types) {
+ return METACOMMIT_TYPE_NONE;
+ }
+
+ end = &(parent_types[parent_types_size]);
+
+ while (1) {
+ char next = *parent_types;
+ if (next == ' ') {
+ index++;
+ }
+ if (next == 'c') {
+ ret = METACOMMIT_TYPE_NORMAL;
+ break;
+ }
+ if (next == 'a') {
+ ret = METACOMMIT_TYPE_ABANDONED;
+ break;
+ }
+ parent_types++;
+ if (parent_types >= end) {
+ return METACOMMIT_TYPE_NONE;
+ }
+ }
+
+ content_parent = get_commit_by_index(commit->parents, index);
+
+ if (!content_parent) {
+ return METACOMMIT_TYPE_NONE;
+ }
+
+ oidcpy(content, &(content_parent->object.oid));
+ return ret;
+}
diff --git a/metacommit-parser.h b/metacommit-parser.h
new file mode 100644
index 0000000000..e546f5a7e7
--- /dev/null
+++ b/metacommit-parser.h
@@ -0,0 +1,16 @@
+#ifndef METACOMMIT_PARSER_H
+#define METACOMMIT_PARSER_H
+
+// Indicates a normal commit (non-metacommit)
+#define METACOMMIT_TYPE_NONE 0
+// Indicates a metacommit with normal content (non-abandoned)
+#define METACOMMIT_TYPE_NORMAL 1
+// Indicates a metacommit with abandoned content
+#define METACOMMIT_TYPE_ABANDONED 2
+
+struct commit;
+
+extern int get_metacommit_content(
+ struct commit *commit, struct object_id *content);
+
+#endif
--
2.20.1.495.gaa96b0ce6b-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v3 4/8] evolve: Add support for parsing metacommits
2019-01-27 19:44 ` [PATCH v3 4/8] evolve: Add support for parsing metacommits sxenos
@ 2019-01-28 2:05 ` Junio C Hamano
0 siblings, 0 replies; 15+ messages in thread
From: Junio C Hamano @ 2019-01-28 2:05 UTC (permalink / raw)
To: sxenos; +Cc: git
sxenos@google.com writes:
> +/*
> + * Search the commit buffer for a line starting with the given key. Unlike
> + * find_commit_header, this also searches the commit message body.
> + */
> +static const char *find_key(const char *msg, const char *key, size_t *out_len)
> +{
> + int key_len = strlen(key);
> + const char *line = msg;
> +
> + while (line) {
> + const char *eol = strchrnul(line, '\n');
> +
> + if (eol - line > key_len &&
> + !strncmp(line, key, key_len) &&
Use of strncmp() here forces readers to wonder if and why we are
preparing the code to allow NUL in key[0..key_len] (as line..eol
would not, because we just used strchrnul()). But because key_len
was computed using strlen(key), there is no valid reason to do so.
If the code used memcmp(), it won't waste readers' time.
> + line[key_len] == ' ') {
> + *out_len = eol - line - key_len - 1;
> + return line + key_len + 1;
> + }
> + line = *eol ? eol + 1 : NULL;
> + }
> + return NULL;
> +}
> +
> +static struct commit *get_commit_by_index(struct commit_list *to_search, int index)
> +{
> + while (to_search && index) {
> + to_search = to_search->next;
> + --index;
Style: unlike some C++ shop, we tend to use post-increment/decrement
when we do not use the value.
> + }
> +
> + return to_search->item;
> +}
If a maliciously crafted parent-type field has excess ' ' to make
index larger than the number of "parent" field in the commit object,
the while() loop terminates upon noticing that to_search has become
NULL. And then this return statement dereferences that NULL
pointer.
> +/*
> + * Writes the content parent's object id to "content".
> + * Returns the metacommit type. See the METACOMMIT_TYPE_* constants.
> + */
> +int get_metacommit_content(
> + struct commit *commit, struct object_id *content)
> +{
> + const char *buffer = get_commit_buffer(commit, NULL);
> + size_t parent_types_size;
> + const char *parent_types = find_key(buffer, "parent-type",
> + &parent_types_size);
> + const char *end;
> + int index = 0;
> + int ret;
> + struct commit *content_parent;
> +
> + if (!parent_types) {
> + return METACOMMIT_TYPE_NONE;
Unnecessary brace?
> + }
> +
> + end = &(parent_types[parent_types_size]);
Unnecessary parenthesis?
> + while (1) {
> + char next = *parent_types;
> + if (next == ' ') {
> + index++;
> + }
> + if (next == 'c') {
> + ret = METACOMMIT_TYPE_NORMAL;
> + break;
> + }
> + if (next == 'a') {
> + ret = METACOMMIT_TYPE_ABANDONED;
> + break;
> + }
The parsing of this seems somewhat loose. If there is 'x' on the
line, this loop spins and consumes it without doing anything, which
means that the same commit with a parent-type field can be encoded
in different ways by adding arbitrary number of 'x' just after SP
after the "parent-type" keyword, no?
> + parent_types++;
> + if (parent_types >= end) {
> + return METACOMMIT_TYPE_NONE;
> + }
> + }
> +
> + content_parent = get_commit_by_index(commit->parents, index);
> +
> + if (!content_parent) {
> + return METACOMMIT_TYPE_NONE;
> + }
> +
> + oidcpy(content, &(content_parent->object.oid));
> + return ret;
> +}
> diff --git a/metacommit-parser.h b/metacommit-parser.h
> new file mode 100644
> index 0000000000..e546f5a7e7
> --- /dev/null
> +++ b/metacommit-parser.h
> @@ -0,0 +1,16 @@
> +#ifndef METACOMMIT_PARSER_H
> +#define METACOMMIT_PARSER_H
> +
> +// Indicates a normal commit (non-metacommit)
No C99 // comments please. Not in the header, and not in the code.
> +#define METACOMMIT_TYPE_NONE 0
> +// Indicates a metacommit with normal content (non-abandoned)
> +#define METACOMMIT_TYPE_NORMAL 1
> +// Indicates a metacommit with abandoned content
> +#define METACOMMIT_TYPE_ABANDONED 2
> +
> +struct commit;
> +
> +extern int get_metacommit_content(
> + struct commit *commit, struct object_id *content);
> +
> +#endif
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 5/8] evolve: Add the change-table structure
2019-01-27 19:44 [PATCH v3 1/8] technical doc: add a design doc for the evolve command sxenos
` (2 preceding siblings ...)
2019-01-27 19:44 ` [PATCH v3 4/8] evolve: Add support for parsing metacommits sxenos
@ 2019-01-27 19:44 ` sxenos
2019-01-28 8:01 ` Johannes Schindelin
2019-01-27 19:44 ` [PATCH v3 6/8] evolve: Add support for writing metacommits sxenos
` (2 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: sxenos @ 2019-01-27 19:44 UTC (permalink / raw)
To: git; +Cc: Stefan Xenos
From: Stefan Xenos <sxenos@google.com>
A change table stores a list of changes, and supports efficient lookup
from a commit hash to the list of changes that reference that commit
directly.
It can be used to look up content commits or metacommits at the head
of a change, but does not support lookup of commits referenced as part
of the commit history.
Signed-off-by: Stefan Xenos <sxenos@google.com>
---
Makefile | 1 +
change-table.c | 207 +++++++++++++++++++++++++++++++++++++++++++++++++
change-table.h | 138 +++++++++++++++++++++++++++++++++
3 files changed, 346 insertions(+)
create mode 100644 change-table.c
create mode 100644 change-table.h
diff --git a/Makefile b/Makefile
index 7ffc383f2b..09cfd3ef1b 100644
--- a/Makefile
+++ b/Makefile
@@ -844,6 +844,7 @@ LIB_OBJS += branch.o
LIB_OBJS += bulk-checkin.o
LIB_OBJS += bundle.o
LIB_OBJS += cache-tree.o
+LIB_OBJS += change-table.o
LIB_OBJS += chdir-notify.o
LIB_OBJS += checkout.o
LIB_OBJS += color.o
diff --git a/change-table.c b/change-table.c
new file mode 100644
index 0000000000..6daff5f58c
--- /dev/null
+++ b/change-table.c
@@ -0,0 +1,207 @@
+#include "cache.h"
+#include "change-table.h"
+#include "commit.h"
+#include "ref-filter.h"
+#include "metacommit-parser.h"
+
+void change_table_init(struct change_table *to_initialize)
+{
+ memset(to_initialize, 0, sizeof(*to_initialize));
+ mem_pool_init(&(to_initialize->memory_pool), 0);
+ to_initialize->memory_pool->block_alloc = 4*1024 - sizeof(struct mp_block);
+ oidmap_init(&(to_initialize->oid_to_metadata_index), 0);
+ string_list_init(&(to_initialize->refname_to_change_head), 1);
+}
+
+static void change_list_clear(struct change_list *to_clear) {
+ string_list_clear(&to_clear->additional_refnames, 0);
+}
+
+static void commit_change_list_entry_clear(
+ struct commit_change_list_entry *to_clear) {
+ change_list_clear(&(to_clear->changes));
+}
+
+static void change_head_array_clear(struct change_head_array *to_clear)
+{
+ FREE_AND_NULL(to_clear->array);
+}
+
+void change_table_clear(struct change_table *to_clear)
+{
+ struct oidmap_iter iter;
+ struct commit_change_list_entry *next;
+ for (next = oidmap_iter_first(&to_clear->oid_to_metadata_index, &iter);
+ next;
+ next = oidmap_iter_next(&iter)) {
+
+ commit_change_list_entry_clear(next);
+ }
+
+ oidmap_free(&to_clear->oid_to_metadata_index, 0);
+ string_list_clear(&(to_clear->refname_to_change_head), 0);
+ change_head_array_clear(&to_clear->heads);
+ mem_pool_discard(to_clear->memory_pool, 0);
+}
+
+/*
+ * Appends a new, empty, change_head struct to the end of the given array.
+ * Returns the index of the newly-added struct.
+ */
+static int change_head_array_append(struct change_head_array *to_add)
+{
+ int index = to_add->nr++;
+ struct change_head *new_head;
+ ALLOC_GROW(to_add->array, to_add->nr, to_add->alloc);
+ new_head = &(to_add->array[index]);
+ memset(new_head, 0, sizeof(*new_head));
+ return index;
+}
+
+static void add_head_to_commit(struct change_table *to_modify,
+ const struct object_id *to_add, const char *refname)
+{
+ struct commit_change_list_entry *entry;
+
+ // Note: the indices in the map are 1-based. 0 is used to indicate a missing
+ // element.
+ entry = oidmap_get(&(to_modify->oid_to_metadata_index), to_add);
+ if (!entry) {
+ entry = mem_pool_calloc(to_modify->memory_pool, 1,
+ sizeof(*entry));
+ oidcpy(&entry->entry.oid, to_add);
+ oidmap_put(&(to_modify->oid_to_metadata_index), entry);
+ string_list_init(&(entry->changes.additional_refnames), 0);
+ }
+
+ if (entry->changes.first_refname == NULL) {
+ entry->changes.first_refname = refname;
+ } else {
+ string_list_insert(&entry->changes.additional_refnames, refname);
+ }
+}
+
+void change_table_add(struct change_table *to_modify, const char *refname,
+ struct commit *to_add)
+{
+ struct change_head *new_head;
+ struct string_list_item *new_item;
+ long index;
+ int metacommit_type;
+
+ index = change_head_array_append(&to_modify->heads);
+ new_head = &(to_modify->heads.array[index]);
+
+ oidcpy(&new_head->head, &(to_add->object.oid));
+
+ metacommit_type = get_metacommit_content(to_add, &new_head->content);
+ if (metacommit_type == METACOMMIT_TYPE_NONE) {
+ oidcpy(&new_head->content, &(to_add->object.oid));
+ }
+ new_head->abandoned = (metacommit_type == METACOMMIT_TYPE_ABANDONED);
+ new_head->remote = starts_with(refname, "refs/remote/");
+ new_head->hidden = starts_with(refname, "refs/hiddenmetas/");
+
+ new_item = string_list_insert(&to_modify->refname_to_change_head, refname);
+ new_item->util = (void*)index;
+ // Use pointers to the copy of the string we're retaining locally
+ refname = new_item->string;
+
+ if (!oideq(&new_head->content, &new_head->head)) {
+ add_head_to_commit(to_modify, &(new_head->content), refname);
+ }
+ add_head_to_commit(to_modify, &(new_head->head), refname);
+}
+
+void change_table_add_all_visible(struct change_table *to_modify,
+ struct repository* repo)
+{
+ struct ref_filter filter;
+ const char *name_patterns[] = {NULL};
+ memset(&filter, 0, sizeof(filter));
+ filter.kind = FILTER_REFS_CHANGES;
+ filter.name_patterns = name_patterns;
+
+ change_table_add_matching_filter(to_modify, repo, &filter);
+}
+
+void change_table_add_matching_filter(struct change_table *to_modify,
+ struct repository* repo, struct ref_filter *filter)
+{
+ struct ref_array matching_refs;
+ int i;
+
+ memset(&matching_refs, 0, sizeof(matching_refs));
+ filter_refs(&matching_refs, filter, filter->kind);
+
+ // Determine the object id for the latest content commit for each change.
+ // Fetch the commit at the head of each change ref. If it's a normal commit,
+ // that's the commit we want. If it's a metacommit, locate its content parent
+ // and use that.
+
+ for (i = 0; i < matching_refs.nr; i++) {
+ struct ref_array_item *item = matching_refs.items[i];
+ struct commit *commit = item->commit;
+
+ commit = lookup_commit_reference_gently(repo, &(item->objectname), 1);
+
+ if (commit != NULL) {
+ change_table_add(to_modify, item->refname, commit);
+ }
+ }
+
+ ref_array_clear(&matching_refs);
+}
+
+static int return_true_callback(const char *refname, void *cb_data)
+{
+ return 1;
+}
+
+int change_table_has_change_referencing(struct change_table *changes,
+ const struct object_id *referenced_commit_id)
+{
+ return for_each_change_referencing(changes, referenced_commit_id,
+ return_true_callback, NULL);
+}
+
+int for_each_change_referencing(struct change_table *table,
+ const struct object_id *referenced_commit_id, each_change_fn fn, void *cb_data)
+{
+ const struct change_list *changes;
+ int i;
+ int retvalue;
+ struct commit_change_list_entry *entry;
+
+ entry = oidmap_get(&table->oid_to_metadata_index,
+ referenced_commit_id);
+ // If this commit isn't referenced by any changes, it won't be in the map
+ if (!entry) {
+ return 0;
+ }
+ changes = &(entry->changes);
+ if (changes->first_refname == NULL) {
+ return 0;
+ }
+ retvalue = fn(changes->first_refname, cb_data);
+ for (i = 0; retvalue == 0 && i < changes->additional_refnames.nr; i++) {
+ retvalue = fn(changes->additional_refnames.items[i].string, cb_data);
+ }
+ return retvalue;
+}
+
+struct change_head* get_change_head(struct change_table *heads,
+ const char* refname)
+{
+ struct string_list_item *item = string_list_lookup(
+ &heads->refname_to_change_head, refname);
+ long index;
+
+ if (!item) {
+ return NULL;
+ }
+
+ index = (long)item->util;
+ return &(heads->heads.array[index]);
+}
+
diff --git a/change-table.h b/change-table.h
new file mode 100644
index 0000000000..85bb19c3bf
--- /dev/null
+++ b/change-table.h
@@ -0,0 +1,138 @@
+#ifndef CHANGE_TABLE_H
+#define CHANGE_TABLE_H
+
+#include "oidmap.h"
+
+struct commit;
+struct ref_filter;
+
+/*
+ * This struct holds a list of change refs. The first element is stored inline,
+ * to optimize for small lists.
+ */
+struct change_list {
+ /* Ref name for the first change in the list, or null if none.
+ *
+ * This field is private. Use for_each_change_in to read.
+ */
+ const char* first_refname;
+ /* List of additional change refs. Note that this is empty if the list
+ * contains 0 or 1 elements.
+ *
+ * This field is private. Use for_each_change_in to read.
+ */
+ struct string_list additional_refnames;
+};
+
+/*
+ * Holds information about the head of a single change.
+ */
+struct change_head {
+ /*
+ * The location pointed to by the head of the change. May be a commit or a
+ * metacommit.
+ */
+ struct object_id head;
+ /*
+ * The content commit for the latest commit in the change. Always points to a
+ * real commit, never a metacommit.
+ */
+ struct object_id content;
+ /*
+ * Abandoned: indicates that the content commit should be removed from the
+ * history.
+ *
+ * Hidden: indicates that the change is an inactive change from the
+ * hiddenmetas namespace. Such changes will be hidden from the user by
+ * default.
+ *
+ * Deleted: indicates that the change has been removed from the repository.
+ * That is the ref was deleted since the time this struct was created. Such
+ * entries should be ignored.
+ */
+ int abandoned:1,
+ hidden:1,
+ remote:1,
+ deleted:1;
+};
+
+/*
+ * An array of change_head.
+ */
+struct change_head_array {
+ struct change_head* array;
+ int nr;
+ int alloc;
+};
+
+/*
+ * Holds the list of change refs whose content points to a particular content
+ * commit.
+ */
+struct commit_change_list_entry {
+ struct oidmap_entry entry;
+ struct change_list changes;
+};
+
+/*
+ * Holds information about the heads of each change, and permits effecient
+ * lookup from a commit to the changes that reference it directly.
+ *
+ * All fields should be considered private. Use the change_table functions
+ * to interact with this struct.
+ */
+struct change_table {
+ /**
+ * Memory pool for the objects allocated by the change table.
+ */
+ struct mem_pool *memory_pool;
+ /* Map object_id to commit_change_list_entry structs. */
+ struct oidmap oid_to_metadata_index;
+ /* List of ref names. The util value is an int index into change_metadata
+ * array.
+ */
+ struct string_list refname_to_change_head;
+ /* change_head structures for each head */
+ struct change_head_array heads;
+};
+
+extern void change_table_init(struct change_table *to_initialize);
+extern void change_table_clear(struct change_table *to_clear);
+
+/* Adds the given change head to the change_table struct */
+extern void change_table_add(struct change_table *to_modify,
+ const char *refname, struct commit *target);
+
+/* Adds the non-hidden local changes to the given change_table struct.
+ */
+extern void change_table_add_all_visible(struct change_table *to_modify,
+ struct repository *repo);
+
+/*
+ * Adds all changes matching the given ref filter to the given change_table
+ * struct.
+ */
+extern void change_table_add_matching_filter(struct change_table *to_modify,
+ struct repository* repo, struct ref_filter *filter);
+
+typedef int each_change_fn(const char *refname, void *cb_data);
+
+extern int change_table_has_change_referencing(struct change_table *changes,
+ const struct object_id *referenced_commit_id);
+
+/* Iterates over all changes that reference the given commit. For metacommits,
+ * this is the list of changes that point directly to that metacommit.
+ * For normal commits, this is the list of changes that have this commit as
+ * their latest content.
+ */
+extern int for_each_change_referencing(struct change_table *heads,
+ const struct object_id *referenced_commit_id, each_change_fn fn, void *cb_data);
+
+/**
+ * Returns the change head for the given refname. Returns NULL if no such change
+ * exists.
+ */
+extern struct change_head* get_change_head(struct change_table *heads,
+ const char* refname);
+
+#endif
--
2.20.1.495.gaa96b0ce6b-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v3 5/8] evolve: Add the change-table structure
2019-01-27 19:44 ` [PATCH v3 5/8] evolve: Add the change-table structure sxenos
@ 2019-01-28 8:01 ` Johannes Schindelin
2019-01-28 23:08 ` Johannes Schindelin
0 siblings, 1 reply; 15+ messages in thread
From: Johannes Schindelin @ 2019-01-28 8:01 UTC (permalink / raw)
To: Stefan Xenos; +Cc: git
Hi Stefan,
I did not yet have time to study your proposal in detail, but hope to do
so before the Contributor Summit so that I can have an informed opinion.
Just one thing:
On Sun, 27 Jan 2019, sxenos@google.com wrote:
> From: Stefan Xenos <sxenos@google.com>
>
> A change table stores a list of changes, and supports efficient lookup
> from a commit hash to the list of changes that reference that commit
> directly.
>
> It can be used to look up content commits or metacommits at the head
> of a change, but does not support lookup of commits referenced as part
> of the commit history.
>
> Signed-off-by: Stefan Xenos <sxenos@google.com>
> ---
> Makefile | 1 +
> change-table.c | 207 +++++++++++++++++++++++++++++++++++++++++++++++++
> change-table.h | 138 +++++++++++++++++++++++++++++++++
> 3 files changed, 346 insertions(+)
> create mode 100644 change-table.c
> create mode 100644 change-table.h
>
> diff --git a/Makefile b/Makefile
> index 7ffc383f2b..09cfd3ef1b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -844,6 +844,7 @@ LIB_OBJS += branch.o
> LIB_OBJS += bulk-checkin.o
> LIB_OBJS += bundle.o
> LIB_OBJS += cache-tree.o
> +LIB_OBJS += change-table.o
> LIB_OBJS += chdir-notify.o
> LIB_OBJS += checkout.o
> LIB_OBJS += color.o
> diff --git a/change-table.c b/change-table.c
> new file mode 100644
> index 0000000000..6daff5f58c
> --- /dev/null
> +++ b/change-table.c
> @@ -0,0 +1,207 @@
> +#include "cache.h"
> +#include "change-table.h"
> +#include "commit.h"
> +#include "ref-filter.h"
> +#include "metacommit-parser.h"
> +
> +void change_table_init(struct change_table *to_initialize)
> +{
> + memset(to_initialize, 0, sizeof(*to_initialize));
> + mem_pool_init(&(to_initialize->memory_pool), 0);
> + to_initialize->memory_pool->block_alloc = 4*1024 - sizeof(struct mp_block);
> + oidmap_init(&(to_initialize->oid_to_metadata_index), 0);
> + string_list_init(&(to_initialize->refname_to_change_head), 1);
> +}
> +
> +static void change_list_clear(struct change_list *to_clear) {
> + string_list_clear(&to_clear->additional_refnames, 0);
> +}
> +
> +static void commit_change_list_entry_clear(
> + struct commit_change_list_entry *to_clear) {
> + change_list_clear(&(to_clear->changes));
> +}
> +
> +static void change_head_array_clear(struct change_head_array *to_clear)
> +{
> + FREE_AND_NULL(to_clear->array);
> +}
> +
> +void change_table_clear(struct change_table *to_clear)
> +{
> + struct oidmap_iter iter;
> + struct commit_change_list_entry *next;
> + for (next = oidmap_iter_first(&to_clear->oid_to_metadata_index, &iter);
> + next;
> + next = oidmap_iter_next(&iter)) {
> +
> + commit_change_list_entry_clear(next);
> + }
> +
> + oidmap_free(&to_clear->oid_to_metadata_index, 0);
> + string_list_clear(&(to_clear->refname_to_change_head), 0);
> + change_head_array_clear(&to_clear->heads);
> + mem_pool_discard(to_clear->memory_pool, 0);
> +}
> +
> +/*
> + * Appends a new, empty, change_head struct to the end of the given array.
> + * Returns the index of the newly-added struct.
> + */
> +static int change_head_array_append(struct change_head_array *to_add)
> +{
> + int index = to_add->nr++;
> + struct change_head *new_head;
> + ALLOC_GROW(to_add->array, to_add->nr, to_add->alloc);
> + new_head = &(to_add->array[index]);
> + memset(new_head, 0, sizeof(*new_head));
> + return index;
> +}
> +
> +static void add_head_to_commit(struct change_table *to_modify,
> + const struct object_id *to_add, const char *refname)
> +{
> + struct commit_change_list_entry *entry;
> +
> + // Note: the indices in the map are 1-based. 0 is used to indicate a missing
> + // element.
> + entry = oidmap_get(&(to_modify->oid_to_metadata_index), to_add);
> + if (!entry) {
> + entry = mem_pool_calloc(to_modify->memory_pool, 1,
> + sizeof(*entry));
> + oidcpy(&entry->entry.oid, to_add);
> + oidmap_put(&(to_modify->oid_to_metadata_index), entry);
> + string_list_init(&(entry->changes.additional_refnames), 0);
> + }
> +
> + if (entry->changes.first_refname == NULL) {
> + entry->changes.first_refname = refname;
> + } else {
> + string_list_insert(&entry->changes.additional_refnames, refname);
> + }
> +}
> +
> +void change_table_add(struct change_table *to_modify, const char *refname,
> + struct commit *to_add)
> +{
> + struct change_head *new_head;
> + struct string_list_item *new_item;
> + long index;
> + int metacommit_type;
> +
> + index = change_head_array_append(&to_modify->heads);
> + new_head = &(to_modify->heads.array[index]);
> +
> + oidcpy(&new_head->head, &(to_add->object.oid));
> +
> + metacommit_type = get_metacommit_content(to_add, &new_head->content);
> + if (metacommit_type == METACOMMIT_TYPE_NONE) {
> + oidcpy(&new_head->content, &(to_add->object.oid));
> + }
> + new_head->abandoned = (metacommit_type == METACOMMIT_TYPE_ABANDONED);
> + new_head->remote = starts_with(refname, "refs/remote/");
> + new_head->hidden = starts_with(refname, "refs/hiddenmetas/");
> +
> + new_item = string_list_insert(&to_modify->refname_to_change_head, refname);
> + new_item->util = (void*)index;
This is not good. You are using a `long` here. The 80s called and want
their now-obsolete data types back.
If you want a data type that can take an integer but also a pointer, use
`intptr_t` instead.
But even that is not good practice. What you really want here is to use a
union of the data types that you want to store in that `util` field.
This is not merely academic, your code causes compile errors on Windows:
https://dev.azure.com/gitgitgadget/git/_build/results?buildId=400&view=logs&jobId=fd490c07-0b22-5182-fac9-6d67fe1e939b&taskId=ce91d5d6-0c55-50f5-8ab9-6695c03ab102&lineStart=430&lineEnd=440&colStart=1&colEnd=1
Ciao,
Johannes
> + // Use pointers to the copy of the string we're retaining locally
> + refname = new_item->string;
> +
> + if (!oideq(&new_head->content, &new_head->head)) {
> + add_head_to_commit(to_modify, &(new_head->content), refname);
> + }
> + add_head_to_commit(to_modify, &(new_head->head), refname);
> +}
> +
> +void change_table_add_all_visible(struct change_table *to_modify,
> + struct repository* repo)
> +{
> + struct ref_filter filter;
> + const char *name_patterns[] = {NULL};
> + memset(&filter, 0, sizeof(filter));
> + filter.kind = FILTER_REFS_CHANGES;
> + filter.name_patterns = name_patterns;
> +
> + change_table_add_matching_filter(to_modify, repo, &filter);
> +}
> +
> +void change_table_add_matching_filter(struct change_table *to_modify,
> + struct repository* repo, struct ref_filter *filter)
> +{
> + struct ref_array matching_refs;
> + int i;
> +
> + memset(&matching_refs, 0, sizeof(matching_refs));
> + filter_refs(&matching_refs, filter, filter->kind);
> +
> + // Determine the object id for the latest content commit for each change.
> + // Fetch the commit at the head of each change ref. If it's a normal commit,
> + // that's the commit we want. If it's a metacommit, locate its content parent
> + // and use that.
> +
> + for (i = 0; i < matching_refs.nr; i++) {
> + struct ref_array_item *item = matching_refs.items[i];
> + struct commit *commit = item->commit;
> +
> + commit = lookup_commit_reference_gently(repo, &(item->objectname), 1);
> +
> + if (commit != NULL) {
> + change_table_add(to_modify, item->refname, commit);
> + }
> + }
> +
> + ref_array_clear(&matching_refs);
> +}
> +
> +static int return_true_callback(const char *refname, void *cb_data)
> +{
> + return 1;
> +}
> +
> +int change_table_has_change_referencing(struct change_table *changes,
> + const struct object_id *referenced_commit_id)
> +{
> + return for_each_change_referencing(changes, referenced_commit_id,
> + return_true_callback, NULL);
> +}
> +
> +int for_each_change_referencing(struct change_table *table,
> + const struct object_id *referenced_commit_id, each_change_fn fn, void *cb_data)
> +{
> + const struct change_list *changes;
> + int i;
> + int retvalue;
> + struct commit_change_list_entry *entry;
> +
> + entry = oidmap_get(&table->oid_to_metadata_index,
> + referenced_commit_id);
> + // If this commit isn't referenced by any changes, it won't be in the map
> + if (!entry) {
> + return 0;
> + }
> + changes = &(entry->changes);
> + if (changes->first_refname == NULL) {
> + return 0;
> + }
> + retvalue = fn(changes->first_refname, cb_data);
> + for (i = 0; retvalue == 0 && i < changes->additional_refnames.nr; i++) {
> + retvalue = fn(changes->additional_refnames.items[i].string, cb_data);
> + }
> + return retvalue;
> +}
> +
> +struct change_head* get_change_head(struct change_table *heads,
> + const char* refname)
> +{
> + struct string_list_item *item = string_list_lookup(
> + &heads->refname_to_change_head, refname);
> + long index;
> +
> + if (!item) {
> + return NULL;
> + }
> +
> + index = (long)item->util;
> + return &(heads->heads.array[index]);
> +}
> +
> diff --git a/change-table.h b/change-table.h
> new file mode 100644
> index 0000000000..85bb19c3bf
> --- /dev/null
> +++ b/change-table.h
> @@ -0,0 +1,138 @@
> +#ifndef CHANGE_TABLE_H
> +#define CHANGE_TABLE_H
> +
> +#include "oidmap.h"
> +
> +struct commit;
> +struct ref_filter;
> +
> +/*
> + * This struct holds a list of change refs. The first element is stored inline,
> + * to optimize for small lists.
> + */
> +struct change_list {
> + /* Ref name for the first change in the list, or null if none.
> + *
> + * This field is private. Use for_each_change_in to read.
> + */
> + const char* first_refname;
> + /* List of additional change refs. Note that this is empty if the list
> + * contains 0 or 1 elements.
> + *
> + * This field is private. Use for_each_change_in to read.
> + */
> + struct string_list additional_refnames;
> +};
> +
> +/*
> + * Holds information about the head of a single change.
> + */
> +struct change_head {
> + /*
> + * The location pointed to by the head of the change. May be a commit or a
> + * metacommit.
> + */
> + struct object_id head;
> + /*
> + * The content commit for the latest commit in the change. Always points to a
> + * real commit, never a metacommit.
> + */
> + struct object_id content;
> + /*
> + * Abandoned: indicates that the content commit should be removed from the
> + * history.
> + *
> + * Hidden: indicates that the change is an inactive change from the
> + * hiddenmetas namespace. Such changes will be hidden from the user by
> + * default.
> + *
> + * Deleted: indicates that the change has been removed from the repository.
> + * That is the ref was deleted since the time this struct was created. Such
> + * entries should be ignored.
> + */
> + int abandoned:1,
> + hidden:1,
> + remote:1,
> + deleted:1;
> +};
> +
> +/*
> + * An array of change_head.
> + */
> +struct change_head_array {
> + struct change_head* array;
> + int nr;
> + int alloc;
> +};
> +
> +/*
> + * Holds the list of change refs whose content points to a particular content
> + * commit.
> + */
> +struct commit_change_list_entry {
> + struct oidmap_entry entry;
> + struct change_list changes;
> +};
> +
> +/*
> + * Holds information about the heads of each change, and permits effecient
> + * lookup from a commit to the changes that reference it directly.
> + *
> + * All fields should be considered private. Use the change_table functions
> + * to interact with this struct.
> + */
> +struct change_table {
> + /**
> + * Memory pool for the objects allocated by the change table.
> + */
> + struct mem_pool *memory_pool;
> + /* Map object_id to commit_change_list_entry structs. */
> + struct oidmap oid_to_metadata_index;
> + /* List of ref names. The util value is an int index into change_metadata
> + * array.
> + */
> + struct string_list refname_to_change_head;
> + /* change_head structures for each head */
> + struct change_head_array heads;
> +};
> +
> +extern void change_table_init(struct change_table *to_initialize);
> +extern void change_table_clear(struct change_table *to_clear);
> +
> +/* Adds the given change head to the change_table struct */
> +extern void change_table_add(struct change_table *to_modify,
> + const char *refname, struct commit *target);
> +
> +/* Adds the non-hidden local changes to the given change_table struct.
> + */
> +extern void change_table_add_all_visible(struct change_table *to_modify,
> + struct repository *repo);
> +
> +/*
> + * Adds all changes matching the given ref filter to the given change_table
> + * struct.
> + */
> +extern void change_table_add_matching_filter(struct change_table *to_modify,
> + struct repository* repo, struct ref_filter *filter);
> +
> +typedef int each_change_fn(const char *refname, void *cb_data);
> +
> +extern int change_table_has_change_referencing(struct change_table *changes,
> + const struct object_id *referenced_commit_id);
> +
> +/* Iterates over all changes that reference the given commit. For metacommits,
> + * this is the list of changes that point directly to that metacommit.
> + * For normal commits, this is the list of changes that have this commit as
> + * their latest content.
> + */
> +extern int for_each_change_referencing(struct change_table *heads,
> + const struct object_id *referenced_commit_id, each_change_fn fn, void *cb_data);
> +
> +/**
> + * Returns the change head for the given refname. Returns NULL if no such change
> + * exists.
> + */
> +extern struct change_head* get_change_head(struct change_table *heads,
> + const char* refname);
> +
> +#endif
> --
> 2.20.1.495.gaa96b0ce6b-goog
>
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 5/8] evolve: Add the change-table structure
2019-01-28 8:01 ` Johannes Schindelin
@ 2019-01-28 23:08 ` Johannes Schindelin
2019-01-28 23:24 ` Stefan Xenos
0 siblings, 1 reply; 15+ messages in thread
From: Johannes Schindelin @ 2019-01-28 23:08 UTC (permalink / raw)
To: Stefan Xenos; +Cc: git, gitster
Hi Junio,
On Mon, 28 Jan 2019, Johannes Schindelin wrote:
> On Sun, 27 Jan 2019, sxenos@google.com wrote:
>
> > + new_item->util = (void*)index;
>
> This is not good. You are using a `long` here. The 80s called and want
> their now-obsolete data types back.
>
> If you want a data type that can take an integer but also a pointer, use
> `intptr_t` instead.
>
> But even that is not good practice. What you really want here is to use a
> union of the data types that you want to store in that `util` field.
>
> This is not merely academic, your code causes compile errors on Windows:
>
> https://dev.azure.com/gitgitgadget/git/_build/results?buildId=400&view=logs&jobId=fd490c07-0b22-5182-fac9-6d67fe1e939b&taskId=ce91d5d6-0c55-50f5-8ab9-6695c03ab102&lineStart=430&lineEnd=440&colStart=1&colEnd=1
Since Stefan did not grace us with an answer, Junio, could I ask you to
squash this in (which is by no means a satisfactory fix, but it is a
stopgap to get `pu` building again)?
-- snipsnap --
diff --git a/change-table.c b/change-table.c
index 2e0d935de846..197ce2783532 100644
--- a/change-table.c
+++ b/change-table.c
@@ -103,7 +103,7 @@ void change_table_add(struct change_table *to_modify, const char *refname,
new_head->hidden = starts_with(refname, "refs/hiddenmetas/");
new_item = string_list_insert(&to_modify->refname_to_change_head, refname);
- new_item->util = (void*)index;
+ new_item->util = (void *)(intptr_t)index;
// Use pointers to the copy of the string we're retaining locally
refname = new_item->string;
@@ -201,6 +201,6 @@ struct change_head* get_change_head(struct change_table *heads,
return NULL;
}
- index = (long)item->util;
+ index = (long)(intptr_t)item->util;
return &(heads->heads.array[index]);
}
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v3 5/8] evolve: Add the change-table structure
2019-01-28 23:08 ` Johannes Schindelin
@ 2019-01-28 23:24 ` Stefan Xenos
2019-01-29 18:02 ` Junio C Hamano
0 siblings, 1 reply; 15+ messages in thread
From: Stefan Xenos @ 2019-01-28 23:24 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Junio C Hamano
Sorry, folks. I normally can't do any open source work on weekdays.
That also includes writing responses on the mailing list, so there
will normally be a week or two lag for me to iterate on this sort of
thing.
Feel free to either include this fix or revert my patch if there's a
problem with it - just let me know what you selected. I'll roll with
it and either resubmit with the requested changes or submit the
requested changes as follow-ups.
- Stefan
On Mon, Jan 28, 2019 at 3:08 PM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>
> Hi Junio,
>
> On Mon, 28 Jan 2019, Johannes Schindelin wrote:
>
> > On Sun, 27 Jan 2019, sxenos@google.com wrote:
> >
> > > + new_item->util = (void*)index;
> >
> > This is not good. You are using a `long` here. The 80s called and want
> > their now-obsolete data types back.
> >
> > If you want a data type that can take an integer but also a pointer, use
> > `intptr_t` instead.
> >
> > But even that is not good practice. What you really want here is to use a
> > union of the data types that you want to store in that `util` field.
> >
> > This is not merely academic, your code causes compile errors on Windows:
> >
> > https://dev.azure.com/gitgitgadget/git/_build/results?buildId=400&view=logs&jobId=fd490c07-0b22-5182-fac9-6d67fe1e939b&taskId=ce91d5d6-0c55-50f5-8ab9-6695c03ab102&lineStart=430&lineEnd=440&colStart=1&colEnd=1
>
> Since Stefan did not grace us with an answer, Junio, could I ask you to
> squash this in (which is by no means a satisfactory fix, but it is a
> stopgap to get `pu` building again)?
>
> -- snipsnap --
> diff --git a/change-table.c b/change-table.c
> index 2e0d935de846..197ce2783532 100644
> --- a/change-table.c
> +++ b/change-table.c
> @@ -103,7 +103,7 @@ void change_table_add(struct change_table *to_modify, const char *refname,
> new_head->hidden = starts_with(refname, "refs/hiddenmetas/");
>
> new_item = string_list_insert(&to_modify->refname_to_change_head, refname);
> - new_item->util = (void*)index;
> + new_item->util = (void *)(intptr_t)index;
> // Use pointers to the copy of the string we're retaining locally
> refname = new_item->string;
>
> @@ -201,6 +201,6 @@ struct change_head* get_change_head(struct change_table *heads,
> return NULL;
> }
>
> - index = (long)item->util;
> + index = (long)(intptr_t)item->util;
> return &(heads->heads.array[index]);
> }
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 5/8] evolve: Add the change-table structure
2019-01-28 23:24 ` Stefan Xenos
@ 2019-01-29 18:02 ` Junio C Hamano
2019-01-29 18:09 ` Stefan Xenos
0 siblings, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2019-01-29 18:02 UTC (permalink / raw)
To: Stefan Xenos; +Cc: Johannes Schindelin, git
Stefan Xenos <sxenos@google.com> writes:
> Sorry, folks. I normally can't do any open source work on weekdays.
> That also includes writing responses on the mailing list, so there
> will normally be a week or two lag for me to iterate on this sort of
> thing.
>
> Feel free to either include this fix or revert my patch if there's a
> problem with it - just let me know what you selected. I'll roll with
> it and either resubmit with the requested changes or submit the
> requested changes as follow-ups.
I think double casting Dscho did was not his ideal "fix" but he did
it as a mere workaround to force the 'pu' branch to compile. And I
also agree with him that the double casting workaround is too ugly
to live, compared to a union he suggests. So I'd rather kick the
branch out of 'pu' for now.
Thanks, both.
>
> - Stefan
>
> On Mon, Jan 28, 2019 at 3:08 PM Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>>
>> Hi Junio,
>>
>> On Mon, 28 Jan 2019, Johannes Schindelin wrote:
>>
>> > On Sun, 27 Jan 2019, sxenos@google.com wrote:
>> >
>> > > + new_item->util = (void*)index;
>> >
>> > This is not good. You are using a `long` here. The 80s called and want
>> > their now-obsolete data types back.
>> >
>> > If you want a data type that can take an integer but also a pointer, use
>> > `intptr_t` instead.
>> >
>> > But even that is not good practice. What you really want here is to use a
>> > union of the data types that you want to store in that `util` field.
>> >
>> > This is not merely academic, your code causes compile errors on Windows:
>> >
>> > https://dev.azure.com/gitgitgadget/git/_build/results?buildId=400&view=logs&jobId=fd490c07-0b22-5182-fac9-6d67fe1e939b&taskId=ce91d5d6-0c55-50f5-8ab9-6695c03ab102&lineStart=430&lineEnd=440&colStart=1&colEnd=1
>>
>> Since Stefan did not grace us with an answer, Junio, could I ask you to
>> squash this in (which is by no means a satisfactory fix, but it is a
>> stopgap to get `pu` building again)?
>>
>> -- snipsnap --
>> diff --git a/change-table.c b/change-table.c
>> index 2e0d935de846..197ce2783532 100644
>> --- a/change-table.c
>> +++ b/change-table.c
>> @@ -103,7 +103,7 @@ void change_table_add(struct change_table *to_modify, const char *refname,
>> new_head->hidden = starts_with(refname, "refs/hiddenmetas/");
>>
>> new_item = string_list_insert(&to_modify->refname_to_change_head, refname);
>> - new_item->util = (void*)index;
>> + new_item->util = (void *)(intptr_t)index;
>> // Use pointers to the copy of the string we're retaining locally
>> refname = new_item->string;
>>
>> @@ -201,6 +201,6 @@ struct change_head* get_change_head(struct change_table *heads,
>> return NULL;
>> }
>>
>> - index = (long)item->util;
>> + index = (long)(intptr_t)item->util;
>> return &(heads->heads.array[index]);
>> }
>>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 5/8] evolve: Add the change-table structure
2019-01-29 18:02 ` Junio C Hamano
@ 2019-01-29 18:09 ` Stefan Xenos
0 siblings, 0 replies; 15+ messages in thread
From: Stefan Xenos @ 2019-01-29 18:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git
Works with me. I'll resubmit without the double casting next chance I
have to work on it. My long-term plan for that struct was to use the
memory pool for all allocations anyway. I think that should let me
implement it without moving objects around, which will make their
addresses stable. That should let me use pointers for everything,
without the ints - so I probably won't need the union.
On Tue, Jan 29, 2019 at 10:02 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Stefan Xenos <sxenos@google.com> writes:
>
> > Sorry, folks. I normally can't do any open source work on weekdays.
> > That also includes writing responses on the mailing list, so there
> > will normally be a week or two lag for me to iterate on this sort of
> > thing.
> >
> > Feel free to either include this fix or revert my patch if there's a
> > problem with it - just let me know what you selected. I'll roll with
> > it and either resubmit with the requested changes or submit the
> > requested changes as follow-ups.
>
> I think double casting Dscho did was not his ideal "fix" but he did
> it as a mere workaround to force the 'pu' branch to compile. And I
> also agree with him that the double casting workaround is too ugly
> to live, compared to a union he suggests. So I'd rather kick the
> branch out of 'pu' for now.
>
> Thanks, both.
>
> >
> > - Stefan
> >
> > On Mon, Jan 28, 2019 at 3:08 PM Johannes Schindelin
> > <Johannes.Schindelin@gmx.de> wrote:
> >>
> >> Hi Junio,
> >>
> >> On Mon, 28 Jan 2019, Johannes Schindelin wrote:
> >>
> >> > On Sun, 27 Jan 2019, sxenos@google.com wrote:
> >> >
> >> > > + new_item->util = (void*)index;
> >> >
> >> > This is not good. You are using a `long` here. The 80s called and want
> >> > their now-obsolete data types back.
> >> >
> >> > If you want a data type that can take an integer but also a pointer, use
> >> > `intptr_t` instead.
> >> >
> >> > But even that is not good practice. What you really want here is to use a
> >> > union of the data types that you want to store in that `util` field.
> >> >
> >> > This is not merely academic, your code causes compile errors on Windows:
> >> >
> >> > https://dev.azure.com/gitgitgadget/git/_build/results?buildId=400&view=logs&jobId=fd490c07-0b22-5182-fac9-6d67fe1e939b&taskId=ce91d5d6-0c55-50f5-8ab9-6695c03ab102&lineStart=430&lineEnd=440&colStart=1&colEnd=1
> >>
> >> Since Stefan did not grace us with an answer, Junio, could I ask you to
> >> squash this in (which is by no means a satisfactory fix, but it is a
> >> stopgap to get `pu` building again)?
> >>
> >> -- snipsnap --
> >> diff --git a/change-table.c b/change-table.c
> >> index 2e0d935de846..197ce2783532 100644
> >> --- a/change-table.c
> >> +++ b/change-table.c
> >> @@ -103,7 +103,7 @@ void change_table_add(struct change_table *to_modify, const char *refname,
> >> new_head->hidden = starts_with(refname, "refs/hiddenmetas/");
> >>
> >> new_item = string_list_insert(&to_modify->refname_to_change_head, refname);
> >> - new_item->util = (void*)index;
> >> + new_item->util = (void *)(intptr_t)index;
> >> // Use pointers to the copy of the string we're retaining locally
> >> refname = new_item->string;
> >>
> >> @@ -201,6 +201,6 @@ struct change_head* get_change_head(struct change_table *heads,
> >> return NULL;
> >> }
> >>
> >> - index = (long)item->util;
> >> + index = (long)(intptr_t)item->util;
> >> return &(heads->heads.array[index]);
> >> }
> >>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 6/8] evolve: Add support for writing metacommits
2019-01-27 19:44 [PATCH v3 1/8] technical doc: add a design doc for the evolve command sxenos
` (3 preceding siblings ...)
2019-01-27 19:44 ` [PATCH v3 5/8] evolve: Add the change-table structure sxenos
@ 2019-01-27 19:44 ` sxenos
2019-01-27 19:44 ` [PATCH v3 7/8] evolve: Implement the git change command sxenos
2019-01-27 19:44 ` [PATCH v3 8/8] evolve: Add the git change list command sxenos
6 siblings, 0 replies; 15+ messages in thread
From: sxenos @ 2019-01-27 19:44 UTC (permalink / raw)
To: git; +Cc: Stefan Xenos
From: Stefan Xenos <sxenos@google.com>
metacommit.c supports the creation of metacommits and
adds the API needed to create and update changes.
Create the "modify_change" function that can be called from modification
commands like "rebase" and "git amend" to record obsolescences in the
change graph.
Create the "record_metacommit" function for recording more complicated
commit relationships in the commit graph.
Create the "write_metacommit" function for low-level creation of
metacommits.
Signed-off-by: Stefan Xenos <sxenos@google.com>
---
Makefile | 1 +
metacommit.c | 370 +++++++++++++++++++++++++++++++++++++++++++++++++++
metacommit.h | 39 ++++++
3 files changed, 410 insertions(+)
create mode 100644 metacommit.c
create mode 100644 metacommit.h
diff --git a/Makefile b/Makefile
index 09cfd3ef1b..a6be1780c5 100644
--- a/Makefile
+++ b/Makefile
@@ -920,6 +920,7 @@ LIB_OBJS += merge.o
LIB_OBJS += merge-blobs.o
LIB_OBJS += merge-recursive.o
LIB_OBJS += mergesort.o
+LIB_OBJS += metacommit.o
LIB_OBJS += metacommit-parser.o
LIB_OBJS += midx.o
LIB_OBJS += name-hash.o
diff --git a/metacommit.c b/metacommit.c
new file mode 100644
index 0000000000..409c2fa03f
--- /dev/null
+++ b/metacommit.c
@@ -0,0 +1,370 @@
+#include "cache.h"
+#include "metacommit.h"
+#include "commit.h"
+#include "change-table.h"
+#include "refs.h"
+
+void init_metacommit_data(struct metacommit_data *state)
+{
+ memset(state, 0, sizeof(*state));
+}
+
+void clear_metacommit_data(struct metacommit_data *state)
+{
+ oid_array_clear(&state->replace);
+ oid_array_clear(&state->origin);
+}
+
+static void compute_default_change_name(struct commit *initial_commit,
+ struct strbuf* result)
+{
+ const char *buffer = get_commit_buffer(initial_commit, NULL);
+ const char *subject;
+ const char *eol;
+ int len;
+ find_commit_subject(buffer, &subject);
+ eol = strchrnul(subject, '\n');
+ for (len = 0;subject < eol && len < 10; ++subject, ++len) {
+ char next = *subject;
+ if (isspace(next)) {
+ continue;
+ }
+
+ strbuf_addch(result, next);
+ }
+}
+
+/*
+ * Computes a change name for a change rooted at the given initial commit. Good
+ * change names should be memorable, unique, and easy to type. They are not
+ * required to match the commit comment.
+ */
+static void compute_change_name(struct commit *initial_commit, struct strbuf* result)
+{
+ struct strbuf default_name;
+ struct object_id unused;
+
+ strbuf_init(&default_name, 0);
+ if (initial_commit) {
+ compute_default_change_name(initial_commit, &default_name);
+ } else {
+ strbuf_addstr(&default_name, "change");
+ }
+ strbuf_addstr(result, "refs/metas/");
+ strbuf_addstr(result, default_name.buf);
+
+ // If there is already a change of this name, append a suffix
+ if (!read_ref(result->buf, &unused)) {
+ int suffix = 2;
+ int original_length = result->len;
+
+ while (1) {
+ strbuf_addf(result, "%d", suffix);
+ if (read_ref(result->buf, &unused)) {
+ break;
+ }
+ strbuf_remove(result, original_length, result->len - original_length);
+ ++suffix;
+ }
+ }
+
+ strbuf_release(&default_name);
+}
+
+struct resolve_metacommit_callback_data
+{
+ struct change_table* active_changes;
+ struct string_list *changes;
+ struct oid_array *heads;
+};
+
+static int resolve_metacommit_callback(const char *refname, void *cb_data)
+{
+ struct resolve_metacommit_callback_data *data = (struct resolve_metacommit_callback_data *)cb_data;
+ struct change_head *chhead;
+
+ chhead = get_change_head(data->active_changes, refname);
+
+ if (data->changes) {
+ string_list_append(data->changes, refname)->util = &(chhead->head);
+ }
+ if (data->heads) {
+ oid_array_append(data->heads, &(chhead->head));
+ }
+
+ return 0;
+}
+
+/*
+ * Produces the final form of a metacommit based on the current change refs.
+ */
+static void resolve_metacommit(
+ struct repository* repo,
+ struct change_table* active_changes,
+ const struct metacommit_data *to_resolve,
+ struct metacommit_data *resolved_output,
+ struct string_list *to_advance,
+ int allow_append)
+{
+ int i;
+ int len = to_resolve->replace.nr;
+ struct resolve_metacommit_callback_data cbdata;
+ int old_change_list_length = to_advance->nr;
+ struct commit* content;
+
+ oidcpy(&resolved_output->content, &to_resolve->content);
+
+ // First look for changes that point to any of the replacement edges in the
+ // metacommit. These will be the changes that get advanced by this metacommit.
+ resolved_output->abandoned = to_resolve->abandoned;
+ cbdata.active_changes = active_changes;
+ cbdata.changes = to_advance;
+ cbdata.heads = &(resolved_output->replace);
+
+ if (allow_append) {
+ for (i = 0; i < len; i++) {
+ int old_number = resolved_output->replace.nr;
+ for_each_change_referencing(active_changes, &(to_resolve->replace.oid[i]),
+ resolve_metacommit_callback, &cbdata);
+ // If no changes were found, use the unresolved value
+ if (old_number == resolved_output->replace.nr) {
+ oid_array_append(&(resolved_output->replace), &(to_resolve->replace.oid[i]));
+ }
+ }
+ }
+
+ cbdata.changes = NULL;
+ cbdata.heads = &(resolved_output->origin);
+
+ len = to_resolve->origin.nr;
+ for (i = 0; i < len; i++) {
+ int old_number = resolved_output->origin.nr;
+ for_each_change_referencing(active_changes, &(to_resolve->origin.oid[i]),
+ resolve_metacommit_callback, &cbdata);
+ if (old_number == resolved_output->origin.nr) {
+ oid_array_append(&(resolved_output->origin), &(to_resolve->origin.oid[i]));
+ }
+ }
+
+ // If no changes were advanced by this metacommit, we'll need to create a new
+ // one.
+ if (to_advance->nr == old_change_list_length) {
+ struct strbuf change_name;
+
+ strbuf_init(&change_name, 80);
+ content = lookup_commit_reference_gently(repo, &(to_resolve->content), 1);
+
+ compute_change_name(content, &change_name);
+ string_list_append(to_advance, change_name.buf);
+ strbuf_release(&change_name);
+ }
+}
+
+static void lookup_commits(
+ struct repository *repo,
+ struct oid_array *to_lookup,
+ struct commit_list **result)
+{
+ int i = to_lookup->nr;
+
+ while (--i >= 0) {
+ struct object_id *next = &(to_lookup->oid[i]);
+ struct commit *commit = lookup_commit_reference_gently(repo, next, 1);
+ commit_list_insert(commit, result);
+ }
+}
+
+#define PARENT_TYPE_PREFIX "parent-type "
+
+/*
+ * Creates a new metacommit object with the given content. Writes the object
+ * id of the newly-created commit to result.
+ */
+int write_metacommit(struct repository *repo, struct metacommit_data *state,
+ struct object_id *result)
+{
+ struct commit_list *parents = NULL;
+ struct strbuf comment;
+ int i;
+ struct commit *content;
+
+ strbuf_init(&comment, strlen(PARENT_TYPE_PREFIX)
+ + 1 + 2 * (state->origin.nr + state->replace.nr));
+ lookup_commits(repo, &state->origin, &parents);
+ lookup_commits(repo, &state->replace, &parents);
+ content = lookup_commit_reference_gently(repo, &state->content, 1);
+ if (!content) {
+ strbuf_release(&comment);
+ free_commit_list(parents);
+ return -1;
+ }
+ commit_list_insert(content, &parents);
+
+ strbuf_addstr(&comment, PARENT_TYPE_PREFIX);
+ strbuf_addstr(&comment, state->abandoned ? "a" : "c");
+ for (i = 0; i < state->replace.nr; i++) {
+ strbuf_addstr(&comment, " r");
+ }
+
+ for (i = 0; i < state->origin.nr; i++) {
+ strbuf_addstr(&comment, " o");
+ }
+
+ // The parents list will be freed by this call
+ commit_tree(comment.buf, comment.len, repo->hash_algo->empty_tree, parents,
+ result, NULL, NULL);
+
+ strbuf_release(&comment);
+ return 0;
+}
+
+/*
+ * Returns true iff the given metacommit is abandoned, has one or more origin
+ * parents, or has one or more replacement parents.
+ */
+static int is_nontrivial_metacommit(struct metacommit_data *state)
+{
+ return state->replace.nr || state->origin.nr || state->abandoned;
+}
+
+/*
+ * Records the relationships described by the given metacommit in the
+ * repository.
+ *
+ * If override_change is NULL (the default), an attempt will be made
+ * to append to existing changes wherever possible instead of creating new ones.
+ * If override_change is non-null, only the given change ref will be updated.
+ *
+ * options is a bitwise combination of the UPDATE_OPTION_* flags.
+ */
+int record_metacommit(struct repository *repo,
+ const struct metacommit_data *metacommit,
+ const char* override_change, int options, struct strbuf *err)
+{
+ static const char *msg = "updating change";
+ struct metacommit_data resolved_metacommit;
+ struct string_list changes;
+ struct object_id commit_target;
+ struct ref_transaction *transaction = NULL;
+ struct object_id old_head_working;
+ const struct object_id *old_head;
+ struct change_table chtable;
+ int i;
+ int ret = 0;
+ int force = (options & UPDATE_OPTION_FORCE);
+
+ init_metacommit_data(&resolved_metacommit);
+ string_list_init(&changes, 1);
+
+ change_table_init(&chtable);
+
+ change_table_add_all_visible(&chtable, repo);
+
+ resolve_metacommit(repo, &chtable, metacommit, &resolved_metacommit, &changes,
+ (options & UPDATE_OPTION_NOAPPEND) == 0);
+
+ if (override_change) {
+ old_head = &old_head_working;
+ string_list_clear(&changes, 0);
+ if (get_oid_committish(override_change, &old_head_working)) {
+ // ...then this is a newly-created change
+ old_head = &null_oid;
+ } else if (!force) {
+ if (!oid_array_readonly_contains(&(resolved_metacommit.replace),
+ &old_head_working)) {
+ // Attempted non-fast-forward change
+ strbuf_addf(err, _("non-fast-forward update to '%s'"),
+ override_change);
+ ret = -1;
+ goto cleanup;
+ }
+ }
+ // The expected "current" head of the change is stored in the util pointer
+ string_list_append(&changes, override_change)->util = (void*)old_head;
+ }
+
+ if (is_nontrivial_metacommit(&resolved_metacommit)) {
+ // If there are any origin or replacement parents, create a new metacommit
+ // object.
+ if (write_metacommit(repo, &resolved_metacommit, &commit_target) < 0) {
+ ret = -1;
+ goto cleanup;
+ }
+ } else {
+ // If the metacommit would only contain a content commit, point to the
+ // commit itself rather than creating a trivial metacommit.
+ oidcpy(&commit_target, &(resolved_metacommit.content));
+ }
+
+ // If a change already exists with this target and we're not forcing an
+ // update to some specific override_change && change, there's nothing to do.
+ if (!override_change
+ && change_table_has_change_referencing(&chtable, &commit_target)) {
+ // Not an error
+ goto cleanup;
+ }
+
+ transaction = ref_transaction_begin(err);
+
+ // Update the refs for each affected change
+ if (!transaction) {
+ ret = -1;
+ } else {
+ for (i = 0; i < changes.nr; i++) {
+ struct string_list_item *it = &(changes.items[i]);
+
+ // The expected current head of the change is stored in the util pointer.
+ // It is null if the change should be newly-created.
+ if (it->util) {
+ if (ref_transaction_update(transaction, it->string, &commit_target,
+ force ? NULL : it->util, 0, msg, err)) {
+
+ ret = -1;
+ }
+ } else {
+ if (ref_transaction_create(transaction, it->string,
+ &commit_target, 0, msg, err)) {
+
+ ret = -1;
+ }
+ }
+ }
+
+ if (!ret) {
+ if (ref_transaction_commit(transaction, err)) {
+ ret = -1;
+ }
+ }
+ }
+
+cleanup:
+ ref_transaction_free(transaction);
+ string_list_clear(&changes, 0);
+ clear_metacommit_data(&resolved_metacommit);
+ change_table_clear(&chtable);
+ return ret;
+}
+
+/*
+ * Should be invoked after a command that has "modify" semantics - commands that
+ * create a new commit based on an old commit and treat the new one as a
+ * replacement for the old one. This method records the replacement in the
+ * change graph, such that a future evolve operation will rebase children of
+ * the old commit onto the new commit.
+ */
+void modify_change(
+ struct repository *repo,
+ const struct object_id *old_commit,
+ const struct object_id *new_commit,
+ struct strbuf *err)
+{
+ struct metacommit_data metacommit;
+
+ init_metacommit_data(&metacommit);
+ oidcpy(&(metacommit.content), new_commit);
+ oid_array_append(&(metacommit.replace), old_commit);
+
+ record_metacommit(repo, &metacommit, NULL, 0, err);
+
+ clear_metacommit_data(&metacommit);
+}
diff --git a/metacommit.h b/metacommit.h
new file mode 100644
index 0000000000..1d4be9cdfb
--- /dev/null
+++ b/metacommit.h
@@ -0,0 +1,39 @@
+#ifndef METACOMMIT_H
+#define METACOMMIT_H
+
+// If specified, non-fast-forward changes are permitted.
+#define UPDATE_OPTION_FORCE 0x0001
+// If specified, no attempt will be made to append to existing changes.
+// Normally, if a metacommit points to a commit in its replace or origin
+// list and an existing change points to that same commit as its content, the
+// new metacommit will attempt to append to that same change. This may replace
+// the commit parent with one or more metacommits from the head of the appended
+// changes. This option disables this behavior, and will always create a new
+// change rather than reusing existing changes.
+#define UPDATE_OPTION_NOAPPEND 0x0002
+
+// Metacommit Data
+
+struct metacommit_data {
+ struct object_id content;
+ struct oid_array replace;
+ struct oid_array origin;
+ int abandoned;
+};
+
+extern void init_metacommit_data(struct metacommit_data *state);
+
+extern void clear_metacommit_data(struct metacommit_data *state);
+
+extern int record_metacommit(struct repository *repo,
+ const struct metacommit_data *metacommit,
+ const char* override_change, int options, struct strbuf *err);
+
+extern void modify_change(struct repository *repo,
+ const struct object_id *old_commit, const struct object_id *new_commit,
+ struct strbuf *err);
+
+extern int write_metacommit(struct repository *repo, struct metacommit_data *state,
+ struct object_id *result);
+
+#endif
--
2.20.1.495.gaa96b0ce6b-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v3 7/8] evolve: Implement the git change command
2019-01-27 19:44 [PATCH v3 1/8] technical doc: add a design doc for the evolve command sxenos
` (4 preceding siblings ...)
2019-01-27 19:44 ` [PATCH v3 6/8] evolve: Add support for writing metacommits sxenos
@ 2019-01-27 19:44 ` sxenos
2019-01-27 19:44 ` [PATCH v3 8/8] evolve: Add the git change list command sxenos
6 siblings, 0 replies; 15+ messages in thread
From: sxenos @ 2019-01-27 19:44 UTC (permalink / raw)
To: git; +Cc: Stefan Xenos
From: Stefan Xenos <sxenos@google.com>
Implement the git change update command, which
are sufficient for constructing change graphs.
For example, to create a new change (a stable name) that refers to HEAD:
git change update -c HEAD
To record a rebase or amend in the change graph:
git change update -c <new_commit> -r <old_commit>
To record a cherry-pick in the change graph:
git change update -c <new_commit> -o <original_commit>
Signed-off-by: Stefan Xenos <sxenos@google.com>
---
.gitignore | 1 +
Makefile | 1 +
builtin.h | 1 +
builtin/change.c | 175 +++++++++++++++++++++++++++++++++++++++++++++++
git.c | 1 +
5 files changed, 179 insertions(+)
create mode 100644 builtin/change.c
diff --git a/.gitignore b/.gitignore
index 0d77ea5894..8a084ac38b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -26,6 +26,7 @@
/git-branch
/git-bundle
/git-cat-file
+/git-change
/git-check-attr
/git-check-ignore
/git-check-mailmap
diff --git a/Makefile b/Makefile
index a6be1780c5..d6fab30eca 100644
--- a/Makefile
+++ b/Makefile
@@ -1035,6 +1035,7 @@ BUILTIN_OBJS += builtin/blame.o
BUILTIN_OBJS += builtin/branch.o
BUILTIN_OBJS += builtin/bundle.o
BUILTIN_OBJS += builtin/cat-file.o
+BUILTIN_OBJS += builtin/change.o
BUILTIN_OBJS += builtin/check-attr.o
BUILTIN_OBJS += builtin/check-ignore.o
BUILTIN_OBJS += builtin/check-mailmap.o
diff --git a/builtin.h b/builtin.h
index 6538932e99..d2d39d9da8 100644
--- a/builtin.h
+++ b/builtin.h
@@ -137,6 +137,7 @@ extern int cmd_blame(int argc, const char **argv, const char *prefix);
extern int cmd_branch(int argc, const char **argv, const char *prefix);
extern int cmd_bundle(int argc, const char **argv, const char *prefix);
extern int cmd_cat_file(int argc, const char **argv, const char *prefix);
+extern int cmd_change(int argc, const char **argv, const char *prefix);
extern int cmd_checkout(int argc, const char **argv, const char *prefix);
extern int cmd_checkout_index(int argc, const char **argv, const char *prefix);
extern int cmd_check_attr(int argc, const char **argv, const char *prefix);
diff --git a/builtin/change.c b/builtin/change.c
new file mode 100644
index 0000000000..ff7eb3b113
--- /dev/null
+++ b/builtin/change.c
@@ -0,0 +1,175 @@
+#include "builtin.h"
+#include "ref-filter.h"
+#include "parse-options.h"
+#include "metacommit.h"
+#include "config.h"
+
+static const char * const builtin_change_usage[] = {
+ N_("git change update [--force] [--replace <treeish>...] [--origin <treesih>...] [--content <newtreeish>]"),
+ NULL
+};
+
+static const char * const builtin_update_usage[] = {
+ N_("git change update [--force] [--replace <treeish>...] [--origin <treesih>...] [--content <newtreeish>]"),
+ NULL
+};
+
+struct update_state {
+ int options;
+ const char* change;
+ const char* content;
+ struct string_list replace;
+ struct string_list origin;
+};
+
+static void init_update_state(struct update_state *state)
+{
+ memset(state, 0, sizeof(*state));
+ state->content = "HEAD";
+ string_list_init(&state->replace, 0);
+ string_list_init(&state->origin, 0);
+}
+
+static void clear_update_state(struct update_state *state)
+{
+ string_list_clear(&state->replace, 0);
+ string_list_clear(&state->origin, 0);
+}
+
+static int update_option_parse_replace(const struct option *opt,
+ const char *arg, int unset)
+{
+ struct update_state *state = opt->value;
+ string_list_append(&state->replace, arg);
+ return 0;
+}
+
+static int update_option_parse_origin(const struct option *opt,
+ const char *arg, int unset)
+{
+ struct update_state *state = opt->value;
+ string_list_append(&state->origin, arg);
+ return 0;
+}
+
+static int resolve_commit(const char *committish, struct object_id *result)
+{
+ struct commit *commit;
+ if (get_oid_committish(committish, result))
+ die(_("Failed to resolve '%s' as a valid revision."), committish);
+ commit = lookup_commit_reference(the_repository, result);
+ if (!commit)
+ die(_("Could not parse object '%s'."), committish);
+ oidcpy(result, &commit->object.oid);
+ return 0;
+}
+
+static void resolve_commit_list(const struct string_list *commitsish_list,
+ struct oid_array* result)
+{
+ int i;
+ for (i = 0; i < commitsish_list->nr; i++) {
+ struct string_list_item *item = &commitsish_list->items[i];
+ struct object_id next;
+ resolve_commit(item->string, &next);
+ oid_array_append(result, &next);
+ }
+}
+
+/*
+ * Given the command-line options for the update command, fills in a
+ * metacommit_data with the corresponding changes.
+ */
+static void get_metacommit_from_command_line(
+ const struct update_state* commands, struct metacommit_data *result)
+{
+ resolve_commit(commands->content, &(result->content));
+ resolve_commit_list(&(commands->replace), &(result->replace));
+ resolve_commit_list(&(commands->origin), &(result->origin));
+}
+
+static int perform_update(
+ struct repository *repo,
+ const struct update_state *state,
+ struct strbuf *err)
+{
+ struct metacommit_data metacommit;
+ int ret;
+
+ init_metacommit_data(&metacommit);
+
+ get_metacommit_from_command_line(state, &metacommit);
+
+ ret = record_metacommit(repo, &metacommit, state->change, state->options, err);
+
+ clear_metacommit_data(&metacommit);
+
+ return ret;
+}
+
+static int change_update(int argc, const char **argv, const char* prefix)
+{
+ int result;
+ int force = 0;
+ int newchange = 0;
+ struct strbuf err = STRBUF_INIT;
+ struct update_state state;
+ struct option options[] = {
+ { OPTION_CALLBACK, 'r', "replace", &state, N_("commit"),
+ N_("marks the given commit as being obsolete"),
+ 0, update_option_parse_replace },
+ { OPTION_CALLBACK, 'o', "origin", &state, N_("commit"),
+ N_("marks the given commit as being the origin of this commit"),
+ 0, update_option_parse_origin },
+ OPT_BOOL('F', "force", &force,
+ N_("overwrite an existing change of the same name")),
+ OPT_STRING('c', "content", &state.content, N_("commit"),
+ N_("identifies the new content commit for the change")),
+ OPT_STRING('g', "change", &state.change, N_("commit"),
+ N_("name of the change to update")),
+ OPT_BOOL('n', "new", &newchange,
+ N_("create a new change - do not append to any existing change")),
+ OPT_END()
+ };
+
+ init_update_state(&state);
+
+ argc = parse_options(argc, argv, prefix, options, builtin_update_usage, 0);
+
+ if (force) state.options |= UPDATE_OPTION_FORCE;
+ if (newchange) state.options |= UPDATE_OPTION_NOAPPEND;
+
+ result = perform_update(the_repository, &state, &err);
+
+ if (result < 0) {
+ error("%s", err.buf);
+ strbuf_release(&err);
+ }
+
+ clear_update_state(&state);
+
+ return result;
+}
+
+int cmd_change(int argc, const char **argv, const char *prefix)
+{
+ // No options permitted before subcommand currently
+ struct option options[] = {
+ OPT_END()
+ };
+ int result = 1;
+
+ argc = parse_options(argc, argv, prefix, options, builtin_change_usage,
+ PARSE_OPT_STOP_AT_NON_OPTION);
+
+ if (argc < 1)
+ usage_with_options(builtin_change_usage, options);
+ else if (!strcmp(argv[0], "update"))
+ result = change_update(argc, argv, prefix);
+ else {
+ error(_("Unknown subcommand: %s"), argv[0]);
+ usage_with_options(builtin_change_usage, options);
+ }
+
+ return result ? 1 : 0;
+}
diff --git a/git.c b/git.c
index 0ce0e13f0f..f59f887238 100644
--- a/git.c
+++ b/git.c
@@ -453,6 +453,7 @@ static struct cmd_struct commands[] = {
{ "branch", cmd_branch, RUN_SETUP | DELAY_PAGER_CONFIG },
{ "bundle", cmd_bundle, RUN_SETUP_GENTLY | NO_PARSEOPT },
{ "cat-file", cmd_cat_file, RUN_SETUP },
+ { "change", cmd_change, RUN_SETUP },
{ "check-attr", cmd_check_attr, RUN_SETUP },
{ "check-ignore", cmd_check_ignore, RUN_SETUP | NEED_WORK_TREE },
{ "check-mailmap", cmd_check_mailmap, RUN_SETUP },
--
2.20.1.495.gaa96b0ce6b-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v3 8/8] evolve: Add the git change list command
2019-01-27 19:44 [PATCH v3 1/8] technical doc: add a design doc for the evolve command sxenos
` (5 preceding siblings ...)
2019-01-27 19:44 ` [PATCH v3 7/8] evolve: Implement the git change command sxenos
@ 2019-01-27 19:44 ` sxenos
6 siblings, 0 replies; 15+ messages in thread
From: sxenos @ 2019-01-27 19:44 UTC (permalink / raw)
To: git; +Cc: Stefan Xenos
From: Stefan Xenos <sxenos@google.com>
This command lists the ongoing changes from the refs/metas
namespace.
Signed-off-by: Stefan Xenos <sxenos@google.com>
---
builtin/change.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/builtin/change.c b/builtin/change.c
index ff7eb3b113..b63fe98665 100644
--- a/builtin/change.c
+++ b/builtin/change.c
@@ -5,15 +5,66 @@
#include "config.h"
static const char * const builtin_change_usage[] = {
+ N_("git change list [<pattern>...]"),
N_("git change update [--force] [--replace <treeish>...] [--origin <treesih>...] [--content <newtreeish>]"),
NULL
};
+static const char * const builtin_list_usage[] = {
+ N_("git change list [<pattern>...]"),
+ NULL
+};
+
static const char * const builtin_update_usage[] = {
N_("git change update [--force] [--replace <treeish>...] [--origin <treesih>...] [--content <newtreeish>]"),
NULL
};
+static int change_list(int argc, const char **argv, const char* prefix)
+{
+ struct option options[] = {
+ OPT_END()
+ };
+ struct ref_filter filter;
+ // TODO: Sorting temporarily disabled. See comments, below.
+ //struct ref_sorting *sorting = ref_default_sorting();
+ struct ref_format format = REF_FORMAT_INIT;
+ struct ref_array array;
+ int i;
+
+ argc = parse_options(argc, argv, prefix, options, builtin_list_usage, 0);
+
+ setup_ref_filter_porcelain_msg();
+
+ memset(&filter, 0, sizeof(filter));
+ memset(&array, 0, sizeof(array));
+
+ filter.kind = FILTER_REFS_CHANGES;
+ filter.name_patterns = argv;
+
+ filter_refs(&array, &filter, FILTER_REFS_CHANGES);
+
+ // TODO: This causes a crash. It sets one of the atom_value handlers to
+ // something invalid, which causes a crash later when we call
+ // show_ref_array_item. Figure out why this happens and put back the sorting.
+ //ref_array_sort(sorting, &array);
+
+ if (!format.format) {
+ format.format = "%(refname:lstrip=1)";
+ }
+
+ if (verify_ref_format(&format))
+ die(_("unable to parse format string"));
+
+ for (i = 0; i < array.nr; i++) {
+ show_ref_array_item(array.items[i], &format);
+ }
+
+ ref_array_clear(&array);
+
+ return 0;
+}
+
struct update_state {
int options;
const char* change;
@@ -164,6 +215,8 @@ int cmd_change(int argc, const char **argv, const char *prefix)
if (argc < 1)
usage_with_options(builtin_change_usage, options);
+ else if (!strcmp(argv[0], "list"))
+ result = change_list(argc, argv, prefix);
else if (!strcmp(argv[0], "update"))
result = change_update(argc, argv, prefix);
else {
--
2.20.1.495.gaa96b0ce6b-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread