* [PATCH v3 04/11] Prepare `paint_down_to_common()` for handling shallow commits
From: Johannes Schindelin via GitGitGadget @ 2024-02-27 13:28 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.1657.v3.git.1709040497.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
When `git fetch --update-shallow` needs to test for commit ancestry, it
can naturally run into a missing object (e.g. if it is a parent of a
shallow commit). To accommodate, the merge base logic will need to be
able to handle this situation gracefully.
Currently, that logic pretends that a missing parent commit is
equivalent to a missing parent commit, and for the purpose of
`--update-shallow` that is exactly what we need it to do.
Therefore, let's introduce a flag to indicate when that is precisely the
logic we want.
We need a flag, and cannot rely on `is_repository_shallow()` to indicate
that situation, because that function would return 0: There may not
actually be a `shallow` file, as demonstrated e.g. by t5537.10 ("add new
shallow root with receive.updateshallow on") and t5538.4 ("add new
shallow root with receive.updateshallow on").
Note: shallow commits' parents are set to `NULL` internally already,
therefore there is no need to special-case shallow repositories here, as
the merge-base logic will not try to access parent commits of shallow
commits.
Likewise, partial clones aren't an issue either: If a commit is missing
during the revision walk in the merge-base logic, it is fetched via
`promisor_remote_get_direct()`. And not only the single missing commit
object: Due to the way the "promised" objects are fetched (in
`fetch_objects()` in `promisor-remote.c`, using `fetch
--filter=blob:none`), there is no actual way to fetch a single commit
object, as the remote side will pass that commit OID to `pack-objects
--revs [...]` which in turn passes it to `rev-list` which interprets
this as a commit _range_ instead of a single object. Therefore, in
partial clones (unless they are shallow in addition), all commits
reachable from a commit that is in the local object database are also
present in that local database.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
commit-reach.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/commit-reach.c b/commit-reach.c
index 5ff71d72d51..7112b10eeea 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -53,7 +53,8 @@ static int queue_has_nonstale(struct prio_queue *queue)
static struct commit_list *paint_down_to_common(struct repository *r,
struct commit *one, int n,
struct commit **twos,
- timestamp_t min_generation)
+ timestamp_t min_generation,
+ int ignore_missing_commits)
{
struct prio_queue queue = { compare_commits_by_gen_then_commit_date };
struct commit_list *result = NULL;
@@ -108,6 +109,13 @@ static struct commit_list *paint_down_to_common(struct repository *r,
if (repo_parse_commit(r, p)) {
clear_prio_queue(&queue);
free_commit_list(result);
+ /*
+ * At this stage, we know that the commit is
+ * missing: `repo_parse_commit()` uses
+ * `OBJECT_INFO_DIE_IF_CORRUPT` and therefore
+ * corrupt commits would already have been
+ * dispatched with a `die()`.
+ */
return NULL;
}
p->object.flags |= flags;
@@ -143,7 +151,7 @@ static struct commit_list *merge_bases_many(struct repository *r,
return NULL;
}
- list = paint_down_to_common(r, one, n, twos, 0);
+ list = paint_down_to_common(r, one, n, twos, 0, 0);
while (list) {
struct commit *commit = pop_commit(&list);
@@ -214,7 +222,7 @@ static int remove_redundant_no_gen(struct repository *r,
min_generation = curr_generation;
}
common = paint_down_to_common(r, array[i], filled,
- work, min_generation);
+ work, min_generation, 0);
if (array[i]->object.flags & PARENT2)
redundant[i] = 1;
for (j = 0; j < filled; j++)
@@ -504,7 +512,7 @@ int repo_in_merge_bases_many(struct repository *r, struct commit *commit,
bases = paint_down_to_common(r, commit,
nr_reference, reference,
- generation);
+ generation, ignore_missing_commits);
if (commit->object.flags & PARENT2)
ret = 1;
clear_commit_marks(commit, all_flags);
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 03/11] Start reporting missing commits in `repo_in_merge_bases_many()`
From: Johannes Schindelin via GitGitGadget @ 2024-02-27 13:28 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.1657.v3.git.1709040497.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Some functions in Git's source code follow the convention that returning
a negative value indicates a fatal error, e.g. repository corruption.
Let's use this convention in `repo_in_merge_bases()` to report when one
of the specified commits is missing (i.e. when `repo_parse_commit()`
reports an error).
Also adjust the callers of `repo_in_merge_bases()` to handle such
negative return values.
Note: As of this patch, errors are returned only if any of the specified
merge heads is missing. Over the course of the next patches, missing
commits will also be reported by the `paint_down_to_common()` function,
which is called by `repo_in_merge_bases_many()`, and those errors will
be properly propagated back to the caller at that stage.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/branch.c | 12 +++++--
builtin/fast-import.c | 6 +++-
builtin/fetch.c | 2 ++
builtin/log.c | 7 ++--
builtin/merge-base.c | 6 +++-
builtin/pull.c | 4 +++
builtin/receive-pack.c | 6 +++-
commit-reach.c | 8 +++--
http-push.c | 5 ++-
merge-ort.c | 81 ++++++++++++++++++++++++++++++++++++------
merge-recursive.c | 54 +++++++++++++++++++++++-----
shallow.c | 18 ++++++----
12 files changed, 173 insertions(+), 36 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index e7ee9bd0f15..7f9e79237f3 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -161,6 +161,8 @@ static int branch_merged(int kind, const char *name,
merged = reference_rev ? repo_in_merge_bases(the_repository, rev,
reference_rev) : 0;
+ if (merged < 0)
+ exit(128);
/*
* After the safety valve is fully redefined to "check with
@@ -169,9 +171,13 @@ static int branch_merged(int kind, const char *name,
* any of the following code, but during the transition period,
* a gentle reminder is in order.
*/
- if ((head_rev != reference_rev) &&
- (head_rev ? repo_in_merge_bases(the_repository, rev, head_rev) : 0) != merged) {
- if (merged)
+ if (head_rev != reference_rev) {
+ int expect = head_rev ? repo_in_merge_bases(the_repository, rev, head_rev) : 0;
+ if (expect < 0)
+ exit(128);
+ if (expect == merged)
+ ; /* okay */
+ else if (merged)
warning(_("deleting branch '%s' that has been merged to\n"
" '%s', but not yet merged to HEAD"),
name, reference_name);
diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index 444f41cf8ca..14c2efa88fc 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -1625,6 +1625,7 @@ static int update_branch(struct branch *b)
oidclr(&old_oid);
if (!force_update && !is_null_oid(&old_oid)) {
struct commit *old_cmit, *new_cmit;
+ int ret;
old_cmit = lookup_commit_reference_gently(the_repository,
&old_oid, 0);
@@ -1633,7 +1634,10 @@ static int update_branch(struct branch *b)
if (!old_cmit || !new_cmit)
return error("Branch %s is missing commits.", b->name);
- if (!repo_in_merge_bases(the_repository, old_cmit, new_cmit)) {
+ ret = repo_in_merge_bases(the_repository, old_cmit, new_cmit);
+ if (ret < 0)
+ exit(128);
+ if (!ret) {
warning("Not updating %s"
" (new tip %s does not contain %s)",
b->name, oid_to_hex(&b->oid),
diff --git a/builtin/fetch.c b/builtin/fetch.c
index fd134ba74d9..0584a1f8b64 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -978,6 +978,8 @@ static int update_local_ref(struct ref *ref,
uint64_t t_before = getnanotime();
fast_forward = repo_in_merge_bases(the_repository, current,
updated);
+ if (fast_forward < 0)
+ exit(128);
forced_updates_ms += (getnanotime() - t_before) / 1000000;
} else {
fast_forward = 1;
diff --git a/builtin/log.c b/builtin/log.c
index ba775d7b5cf..1705da71aca 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1623,7 +1623,7 @@ static struct commit *get_base_commit(const char *base_commit,
{
struct commit *base = NULL;
struct commit **rev;
- int i = 0, rev_nr = 0, auto_select, die_on_failure;
+ int i = 0, rev_nr = 0, auto_select, die_on_failure, ret;
switch (auto_base) {
case AUTO_BASE_NEVER:
@@ -1723,7 +1723,10 @@ static struct commit *get_base_commit(const char *base_commit,
rev_nr = DIV_ROUND_UP(rev_nr, 2);
}
- if (!repo_in_merge_bases(the_repository, base, rev[0])) {
+ ret = repo_in_merge_bases(the_repository, base, rev[0]);
+ if (ret < 0)
+ exit(128);
+ if (!ret) {
if (die_on_failure) {
die(_("base commit should be the ancestor of revision list"));
} else {
diff --git a/builtin/merge-base.c b/builtin/merge-base.c
index e68b7fe45d7..0308fd73289 100644
--- a/builtin/merge-base.c
+++ b/builtin/merge-base.c
@@ -103,12 +103,16 @@ static int handle_octopus(int count, const char **args, int show_all)
static int handle_is_ancestor(int argc, const char **argv)
{
struct commit *one, *two;
+ int ret;
if (argc != 2)
die("--is-ancestor takes exactly two commits");
one = get_commit_reference(argv[0]);
two = get_commit_reference(argv[1]);
- if (repo_in_merge_bases(the_repository, one, two))
+ ret = repo_in_merge_bases(the_repository, one, two);
+ if (ret < 0)
+ exit(128);
+ if (ret)
return 0;
else
return 1;
diff --git a/builtin/pull.c b/builtin/pull.c
index be2b2c9ebc9..e6f2942c0c5 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -931,6 +931,8 @@ static int get_can_ff(struct object_id *orig_head,
merge_head = lookup_commit_reference(the_repository, orig_merge_head);
ret = repo_is_descendant_of(the_repository, merge_head, list);
free_commit_list(list);
+ if (ret < 0)
+ exit(128);
return ret;
}
@@ -955,6 +957,8 @@ static int already_up_to_date(struct object_id *orig_head,
commit_list_insert(theirs, &list);
ok = repo_is_descendant_of(the_repository, ours, list);
free_commit_list(list);
+ if (ok < 0)
+ exit(128);
if (!ok)
return 0;
}
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 8c4f0cb90a9..956fea6293e 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1546,6 +1546,7 @@ static const char *update(struct command *cmd, struct shallow_info *si)
starts_with(name, "refs/heads/")) {
struct object *old_object, *new_object;
struct commit *old_commit, *new_commit;
+ int ret2;
old_object = parse_object(the_repository, old_oid);
new_object = parse_object(the_repository, new_oid);
@@ -1559,7 +1560,10 @@ static const char *update(struct command *cmd, struct shallow_info *si)
}
old_commit = (struct commit *)old_object;
new_commit = (struct commit *)new_object;
- if (!repo_in_merge_bases(the_repository, old_commit, new_commit)) {
+ ret2 = repo_in_merge_bases(the_repository, old_commit, new_commit);
+ if (ret2 < 0)
+ exit(128);
+ if (!ret2) {
rp_error("denying non-fast-forward %s"
" (you should pull first)", name);
ret = "non-fast-forward";
diff --git a/commit-reach.c b/commit-reach.c
index 5c1b5256598..5ff71d72d51 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -464,11 +464,13 @@ int repo_is_descendant_of(struct repository *r,
} else {
while (with_commit) {
struct commit *other;
+ int ret;
other = with_commit->item;
with_commit = with_commit->next;
- if (repo_in_merge_bases_many(r, other, 1, &commit, 0))
- return 1;
+ ret = repo_in_merge_bases_many(r, other, 1, &commit, 0);
+ if (ret)
+ return ret;
}
return 0;
}
@@ -598,6 +600,8 @@ int ref_newer(const struct object_id *new_oid, const struct object_id *old_oid)
commit_list_insert(old_commit, &old_commit_list);
ret = repo_is_descendant_of(the_repository,
new_commit, old_commit_list);
+ if (ret < 0)
+ exit(128);
free_commit_list(old_commit_list);
return ret;
}
diff --git a/http-push.c b/http-push.c
index a704f490fdb..24c16a4f5ff 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1576,8 +1576,11 @@ static int verify_merge_base(struct object_id *head_oid, struct ref *remote)
struct commit *head = lookup_commit_or_die(head_oid, "HEAD");
struct commit *branch = lookup_commit_or_die(&remote->old_oid,
remote->name);
+ int ret = repo_in_merge_bases(the_repository, branch, head);
- return repo_in_merge_bases(the_repository, branch, head);
+ if (ret < 0)
+ exit(128);
+ return ret;
}
static int delete_remote_branch(const char *pattern, int force)
diff --git a/merge-ort.c b/merge-ort.c
index 6491070d965..9f3af46333a 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -544,6 +544,7 @@ enum conflict_and_info_types {
CONFLICT_SUBMODULE_HISTORY_NOT_AVAILABLE,
CONFLICT_SUBMODULE_MAY_HAVE_REWINDS,
CONFLICT_SUBMODULE_NULL_MERGE_BASE,
+ CONFLICT_SUBMODULE_CORRUPT,
/* Keep this entry _last_ in the list */
NB_CONFLICT_TYPES,
@@ -596,7 +597,9 @@ static const char *type_short_descriptions[] = {
[CONFLICT_SUBMODULE_MAY_HAVE_REWINDS] =
"CONFLICT (submodule may have rewinds)",
[CONFLICT_SUBMODULE_NULL_MERGE_BASE] =
- "CONFLICT (submodule lacks merge base)"
+ "CONFLICT (submodule lacks merge base)",
+ [CONFLICT_SUBMODULE_CORRUPT] =
+ "CONFLICT (submodule corrupt)"
};
struct logical_conflict_info {
@@ -1710,7 +1713,14 @@ static int find_first_merges(struct repository *repo,
die("revision walk setup failed");
while ((commit = get_revision(&revs)) != NULL) {
struct object *o = &(commit->object);
- if (repo_in_merge_bases(repo, b, commit))
+ int ret = repo_in_merge_bases(repo, b, commit);
+
+ if (ret < 0) {
+ object_array_clear(&merges);
+ release_revisions(&revs);
+ return ret;
+ }
+ if (ret > 0)
add_object_array(o, NULL, &merges);
}
reset_revision_walk();
@@ -1725,9 +1735,17 @@ static int find_first_merges(struct repository *repo,
contains_another = 0;
for (j = 0; j < merges.nr; j++) {
struct commit *m2 = (struct commit *) merges.objects[j].item;
- if (i != j && repo_in_merge_bases(repo, m2, m1)) {
- contains_another = 1;
- break;
+ if (i != j) {
+ int ret = repo_in_merge_bases(repo, m2, m1);
+ if (ret < 0) {
+ object_array_clear(&merges);
+ release_revisions(&revs);
+ return ret;
+ }
+ if (ret > 0) {
+ contains_another = 1;
+ break;
+ }
}
}
@@ -1749,7 +1767,7 @@ static int merge_submodule(struct merge_options *opt,
{
struct repository subrepo;
struct strbuf sb = STRBUF_INIT;
- int ret = 0;
+ int ret = 0, ret2;
struct commit *commit_o, *commit_a, *commit_b;
int parent_count;
struct object_array merges;
@@ -1796,8 +1814,26 @@ static int merge_submodule(struct merge_options *opt,
}
/* check whether both changes are forward */
- if (!repo_in_merge_bases(&subrepo, commit_o, commit_a) ||
- !repo_in_merge_bases(&subrepo, commit_o, commit_b)) {
+ ret2 = repo_in_merge_bases(&subrepo, commit_o, commit_a);
+ if (ret2 < 0) {
+ path_msg(opt, CONFLICT_SUBMODULE_CORRUPT, 0,
+ path, NULL, NULL, NULL,
+ _("Failed to merge submodule %s "
+ "(repository corrupt)"),
+ path);
+ goto cleanup;
+ }
+ if (ret2 > 0)
+ ret2 = repo_in_merge_bases(&subrepo, commit_o, commit_b);
+ if (ret2 < 0) {
+ path_msg(opt, CONFLICT_SUBMODULE_CORRUPT, 0,
+ path, NULL, NULL, NULL,
+ _("Failed to merge submodule %s "
+ "(repository corrupt)"),
+ path);
+ goto cleanup;
+ }
+ if (!ret2) {
path_msg(opt, CONFLICT_SUBMODULE_MAY_HAVE_REWINDS, 0,
path, NULL, NULL, NULL,
_("Failed to merge submodule %s "
@@ -1807,7 +1843,16 @@ static int merge_submodule(struct merge_options *opt,
}
/* Case #1: a is contained in b or vice versa */
- if (repo_in_merge_bases(&subrepo, commit_a, commit_b)) {
+ ret2 = repo_in_merge_bases(&subrepo, commit_a, commit_b);
+ if (ret2 < 0) {
+ path_msg(opt, CONFLICT_SUBMODULE_CORRUPT, 0,
+ path, NULL, NULL, NULL,
+ _("Failed to merge submodule %s "
+ "(repository corrupt)"),
+ path);
+ goto cleanup;
+ }
+ if (ret2 > 0) {
oidcpy(result, b);
path_msg(opt, INFO_SUBMODULE_FAST_FORWARDING, 1,
path, NULL, NULL, NULL,
@@ -1816,7 +1861,16 @@ static int merge_submodule(struct merge_options *opt,
ret = 1;
goto cleanup;
}
- if (repo_in_merge_bases(&subrepo, commit_b, commit_a)) {
+ ret2 = repo_in_merge_bases(&subrepo, commit_b, commit_a);
+ if (ret2 < 0) {
+ path_msg(opt, CONFLICT_SUBMODULE_CORRUPT, 0,
+ path, NULL, NULL, NULL,
+ _("Failed to merge submodule %s "
+ "(repository corrupt)"),
+ path);
+ goto cleanup;
+ }
+ if (ret2 > 0) {
oidcpy(result, a);
path_msg(opt, INFO_SUBMODULE_FAST_FORWARDING, 1,
path, NULL, NULL, NULL,
@@ -1841,6 +1895,13 @@ static int merge_submodule(struct merge_options *opt,
parent_count = find_first_merges(&subrepo, path, commit_a, commit_b,
&merges);
switch (parent_count) {
+ case -1:
+ path_msg(opt, CONFLICT_SUBMODULE_CORRUPT, 0,
+ path, NULL, NULL, NULL,
+ _("Failed to merge submodule %s "
+ "(repository corrupt)"),
+ path);
+ break;
case 0:
path_msg(opt, CONFLICT_SUBMODULE_FAILED_TO_MERGE, 0,
path, NULL, NULL, NULL,
diff --git a/merge-recursive.c b/merge-recursive.c
index e3beb0801b1..0d931cc14ad 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1144,7 +1144,13 @@ static int find_first_merges(struct repository *repo,
die("revision walk setup failed");
while ((commit = get_revision(&revs)) != NULL) {
struct object *o = &(commit->object);
- if (repo_in_merge_bases(repo, b, commit))
+ int ret = repo_in_merge_bases(repo, b, commit);
+ if (ret < 0) {
+ object_array_clear(&merges);
+ release_revisions(&revs);
+ return ret;
+ }
+ if (ret)
add_object_array(o, NULL, &merges);
}
reset_revision_walk();
@@ -1159,9 +1165,17 @@ static int find_first_merges(struct repository *repo,
contains_another = 0;
for (j = 0; j < merges.nr; j++) {
struct commit *m2 = (struct commit *) merges.objects[j].item;
- if (i != j && repo_in_merge_bases(repo, m2, m1)) {
- contains_another = 1;
- break;
+ if (i != j) {
+ int ret = repo_in_merge_bases(repo, m2, m1);
+ if (ret < 0) {
+ object_array_clear(&merges);
+ release_revisions(&revs);
+ return ret;
+ }
+ if (ret > 0) {
+ contains_another = 1;
+ break;
+ }
}
}
@@ -1197,7 +1211,7 @@ static int merge_submodule(struct merge_options *opt,
const struct object_id *b)
{
struct repository subrepo;
- int ret = 0;
+ int ret = 0, ret2;
struct commit *commit_base, *commit_a, *commit_b;
int parent_count;
struct object_array merges;
@@ -1234,14 +1248,29 @@ static int merge_submodule(struct merge_options *opt,
}
/* check whether both changes are forward */
- if (!repo_in_merge_bases(&subrepo, commit_base, commit_a) ||
- !repo_in_merge_bases(&subrepo, commit_base, commit_b)) {
+ ret2 = repo_in_merge_bases(&subrepo, commit_base, commit_a);
+ if (ret2 < 0) {
+ output(opt, 1, _("Failed to merge submodule %s (repository corrupt)"), path);
+ goto cleanup;
+ }
+ if (ret2 > 0)
+ ret2 = repo_in_merge_bases(&subrepo, commit_base, commit_b);
+ if (ret2 < 0) {
+ output(opt, 1, _("Failed to merge submodule %s (repository corrupt)"), path);
+ goto cleanup;
+ }
+ if (!ret2) {
output(opt, 1, _("Failed to merge submodule %s (commits don't follow merge-base)"), path);
goto cleanup;
}
/* Case #1: a is contained in b or vice versa */
- if (repo_in_merge_bases(&subrepo, commit_a, commit_b)) {
+ ret2 = repo_in_merge_bases(&subrepo, commit_a, commit_b);
+ if (ret2 < 0) {
+ output(opt, 1, _("Failed to merge submodule %s (repository corrupt)"), path);
+ goto cleanup;
+ }
+ if (ret2) {
oidcpy(result, b);
if (show(opt, 3)) {
output(opt, 3, _("Fast-forwarding submodule %s to the following commit:"), path);
@@ -1254,7 +1283,12 @@ static int merge_submodule(struct merge_options *opt,
ret = 1;
goto cleanup;
}
- if (repo_in_merge_bases(&subrepo, commit_b, commit_a)) {
+ ret2 = repo_in_merge_bases(&subrepo, commit_b, commit_a);
+ if (ret2 < 0) {
+ output(opt, 1, _("Failed to merge submodule %s (repository corrupt)"), path);
+ goto cleanup;
+ }
+ if (ret2) {
oidcpy(result, a);
if (show(opt, 3)) {
output(opt, 3, _("Fast-forwarding submodule %s to the following commit:"), path);
@@ -1402,6 +1436,8 @@ static int merge_mode_and_contents(struct merge_options *opt,
&o->oid,
&a->oid,
&b->oid);
+ if (result->clean < 0)
+ return -1;
} else if (S_ISLNK(a->mode)) {
switch (opt->recursive_variant) {
case MERGE_VARIANT_NORMAL:
diff --git a/shallow.c b/shallow.c
index dfcc1f86a7f..f71496f35c3 100644
--- a/shallow.c
+++ b/shallow.c
@@ -795,12 +795,16 @@ static void post_assign_shallow(struct shallow_info *info,
if (!*bitmap)
continue;
for (j = 0; j < bitmap_nr; j++)
- if (bitmap[0][j] &&
- /* Step 7, reachability test at commit level */
- !repo_in_merge_bases_many(the_repository, c, ca.nr, ca.commits, 1)) {
- update_refstatus(ref_status, info->ref->nr, *bitmap);
- dst++;
- break;
+ if (bitmap[0][j]) {
+ /* Step 7, reachability test at commit level */
+ int ret = repo_in_merge_bases_many(the_repository, c, ca.nr, ca.commits, 1);
+ if (ret < 0)
+ exit(128);
+ if (!ret) {
+ update_refstatus(ref_status, info->ref->nr, *bitmap);
+ dst++;
+ break;
+ }
}
}
info->nr_ours = dst;
@@ -830,6 +834,8 @@ int delayed_reachability_test(struct shallow_info *si, int c)
si->nr_commits,
si->commits,
1);
+ if (si->reachable[c] < 0)
+ exit(128);
si->need_reachability_test[c] = 0;
}
return si->reachable[c];
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 05/11] commit-reach: start reporting errors in `paint_down_to_common()`
From: Johannes Schindelin via GitGitGadget @ 2024-02-27 13:28 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.1657.v3.git.1709040497.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
If a commit cannot be parsed, it is currently ignored when looking for
merge bases. That's undesirable as the operation can pretend success in
a corrupt repository, even though the command should fail with an error
message.
Let's start at the bottom of the stack by teaching the
`paint_down_to_common()` function to return an `int`: if negative, it
indicates fatal error, if 0 success.
This requires a couple of callers to be adjusted accordingly.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
commit-reach.c | 66 ++++++++++++++++++++++++++++++++++----------------
1 file changed, 45 insertions(+), 21 deletions(-)
diff --git a/commit-reach.c b/commit-reach.c
index 7112b10eeea..9148a7dcbc0 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -50,14 +50,14 @@ static int queue_has_nonstale(struct prio_queue *queue)
}
/* all input commits in one and twos[] must have been parsed! */
-static struct commit_list *paint_down_to_common(struct repository *r,
- struct commit *one, int n,
- struct commit **twos,
- timestamp_t min_generation,
- int ignore_missing_commits)
+static int paint_down_to_common(struct repository *r,
+ struct commit *one, int n,
+ struct commit **twos,
+ timestamp_t min_generation,
+ int ignore_missing_commits,
+ struct commit_list **result)
{
struct prio_queue queue = { compare_commits_by_gen_then_commit_date };
- struct commit_list *result = NULL;
int i;
timestamp_t last_gen = GENERATION_NUMBER_INFINITY;
@@ -66,8 +66,8 @@ static struct commit_list *paint_down_to_common(struct repository *r,
one->object.flags |= PARENT1;
if (!n) {
- commit_list_append(one, &result);
- return result;
+ commit_list_append(one, result);
+ return 0;
}
prio_queue_put(&queue, one);
@@ -95,7 +95,7 @@ static struct commit_list *paint_down_to_common(struct repository *r,
if (flags == (PARENT1 | PARENT2)) {
if (!(commit->object.flags & RESULT)) {
commit->object.flags |= RESULT;
- commit_list_insert_by_date(commit, &result);
+ commit_list_insert_by_date(commit, result);
}
/* Mark parents of a found merge stale */
flags |= STALE;
@@ -108,7 +108,8 @@ static struct commit_list *paint_down_to_common(struct repository *r,
continue;
if (repo_parse_commit(r, p)) {
clear_prio_queue(&queue);
- free_commit_list(result);
+ free_commit_list(*result);
+ *result = NULL;
/*
* At this stage, we know that the commit is
* missing: `repo_parse_commit()` uses
@@ -116,7 +117,10 @@ static struct commit_list *paint_down_to_common(struct repository *r,
* corrupt commits would already have been
* dispatched with a `die()`.
*/
- return NULL;
+ if (ignore_missing_commits)
+ return 0;
+ return error(_("could not parse commit %s"),
+ oid_to_hex(&p->object.oid));
}
p->object.flags |= flags;
prio_queue_put(&queue, p);
@@ -124,7 +128,7 @@ static struct commit_list *paint_down_to_common(struct repository *r,
}
clear_prio_queue(&queue);
- return result;
+ return 0;
}
static struct commit_list *merge_bases_many(struct repository *r,
@@ -151,7 +155,10 @@ static struct commit_list *merge_bases_many(struct repository *r,
return NULL;
}
- list = paint_down_to_common(r, one, n, twos, 0, 0);
+ if (paint_down_to_common(r, one, n, twos, 0, 0, &list) < 0) {
+ free_commit_list(list);
+ return NULL;
+ }
while (list) {
struct commit *commit = pop_commit(&list);
@@ -205,7 +212,7 @@ static int remove_redundant_no_gen(struct repository *r,
for (i = 0; i < cnt; i++)
repo_parse_commit(r, array[i]);
for (i = 0; i < cnt; i++) {
- struct commit_list *common;
+ struct commit_list *common = NULL;
timestamp_t min_generation = commit_graph_generation(array[i]);
if (redundant[i])
@@ -221,8 +228,16 @@ static int remove_redundant_no_gen(struct repository *r,
if (curr_generation < min_generation)
min_generation = curr_generation;
}
- common = paint_down_to_common(r, array[i], filled,
- work, min_generation, 0);
+ if (paint_down_to_common(r, array[i], filled,
+ work, min_generation, 0, &common)) {
+ clear_commit_marks(array[i], all_flags);
+ clear_commit_marks_many(filled, work, all_flags);
+ free_commit_list(common);
+ free(work);
+ free(redundant);
+ free(filled_index);
+ return -1;
+ }
if (array[i]->object.flags & PARENT2)
redundant[i] = 1;
for (j = 0; j < filled; j++)
@@ -422,6 +437,10 @@ static struct commit_list *get_merge_bases_many_0(struct repository *r,
clear_commit_marks_many(n, twos, all_flags);
cnt = remove_redundant(r, rslt, cnt);
+ if (cnt < 0) {
+ free(rslt);
+ return NULL;
+ }
result = NULL;
for (i = 0; i < cnt; i++)
commit_list_insert_by_date(rslt[i], &result);
@@ -491,7 +510,7 @@ int repo_in_merge_bases_many(struct repository *r, struct commit *commit,
int nr_reference, struct commit **reference,
int ignore_missing_commits)
{
- struct commit_list *bases;
+ struct commit_list *bases = NULL;
int ret = 0, i;
timestamp_t generation, max_generation = GENERATION_NUMBER_ZERO;
@@ -510,10 +529,11 @@ int repo_in_merge_bases_many(struct repository *r, struct commit *commit,
if (generation > max_generation)
return ret;
- bases = paint_down_to_common(r, commit,
- nr_reference, reference,
- generation, ignore_missing_commits);
- if (commit->object.flags & PARENT2)
+ if (paint_down_to_common(r, commit,
+ nr_reference, reference,
+ generation, ignore_missing_commits, &bases))
+ ret = -1;
+ else if (commit->object.flags & PARENT2)
ret = 1;
clear_commit_marks(commit, all_flags);
clear_commit_marks_many(nr_reference, reference, all_flags);
@@ -566,6 +586,10 @@ struct commit_list *reduce_heads(struct commit_list *heads)
}
}
num_head = remove_redundant(the_repository, array, num_head);
+ if (num_head < 0) {
+ free(array);
+ return NULL;
+ }
for (i = 0; i < num_head; i++)
tail = &commit_list_insert(array[i], tail)->next;
free(array);
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 06/11] merge_bases_many(): pass on errors from `paint_down_to_common()`
From: Johannes Schindelin via GitGitGadget @ 2024-02-27 13:28 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.1657.v3.git.1709040497.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
The `paint_down_to_common()` function was just taught to indicate
parsing errors, and now the `merge_bases_many()` function is aware of
that, too.
One tricky aspect is that `merge_bases_many()` parses commits of its
own, but wants to gracefully handle the scenario where NULL is passed as
a merge head, returning the empty list of merge bases. The way this was
handled involved calling `repo_parse_commit(NULL)` and relying on it to
return an error. This has to be done differently now so that we can
handle missing commits correctly by producing a fatal error.
Next step: adjust the caller of `merge_bases_many()`:
`get_merge_bases_many_0()`.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
commit-reach.c | 35 ++++++++++++++++++++++-------------
1 file changed, 22 insertions(+), 13 deletions(-)
diff --git a/commit-reach.c b/commit-reach.c
index 9148a7dcbc0..2c74583c8e0 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -131,41 +131,49 @@ static int paint_down_to_common(struct repository *r,
return 0;
}
-static struct commit_list *merge_bases_many(struct repository *r,
- struct commit *one, int n,
- struct commit **twos)
+static int merge_bases_many(struct repository *r,
+ struct commit *one, int n,
+ struct commit **twos,
+ struct commit_list **result)
{
struct commit_list *list = NULL;
- struct commit_list *result = NULL;
int i;
for (i = 0; i < n; i++) {
- if (one == twos[i])
+ if (one == twos[i]) {
/*
* We do not mark this even with RESULT so we do not
* have to clean it up.
*/
- return commit_list_insert(one, &result);
+ *result = commit_list_insert(one, result);
+ return 0;
+ }
}
+ if (!one)
+ return 0;
if (repo_parse_commit(r, one))
- return NULL;
+ return error(_("could not parse commit %s"),
+ oid_to_hex(&one->object.oid));
for (i = 0; i < n; i++) {
+ if (!twos[i])
+ return 0;
if (repo_parse_commit(r, twos[i]))
- return NULL;
+ return error(_("could not parse commit %s"),
+ oid_to_hex(&twos[i]->object.oid));
}
if (paint_down_to_common(r, one, n, twos, 0, 0, &list) < 0) {
free_commit_list(list);
- return NULL;
+ return -1;
}
while (list) {
struct commit *commit = pop_commit(&list);
if (!(commit->object.flags & STALE))
- commit_list_insert_by_date(commit, &result);
+ commit_list_insert_by_date(commit, result);
}
- return result;
+ return 0;
}
struct commit_list *get_octopus_merge_bases(struct commit_list *in)
@@ -410,10 +418,11 @@ static struct commit_list *get_merge_bases_many_0(struct repository *r,
{
struct commit_list *list;
struct commit **rslt;
- struct commit_list *result;
+ struct commit_list *result = NULL;
int cnt, i;
- result = merge_bases_many(r, one, n, twos);
+ if (merge_bases_many(r, one, n, twos, &result) < 0)
+ return NULL;
for (i = 0; i < n; i++) {
if (one == twos[i])
return result;
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 07/11] get_merge_bases_many_0(): pass on errors from `merge_bases_many()`
From: Johannes Schindelin via GitGitGadget @ 2024-02-27 13:28 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.1657.v3.git.1709040497.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
The `merge_bases_many()` function was just taught to indicate
parsing errors, and now the `get_merge_bases_many_0()` function is aware
of that, too.
Next step: adjust the callers of `get_merge_bases_many_0()`.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
commit-reach.c | 57 +++++++++++++++++++++++++++++++-------------------
1 file changed, 36 insertions(+), 21 deletions(-)
diff --git a/commit-reach.c b/commit-reach.c
index 2c74583c8e0..5fa0abc7d1e 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -410,37 +410,38 @@ static int remove_redundant(struct repository *r, struct commit **array, int cnt
return remove_redundant_no_gen(r, array, cnt);
}
-static struct commit_list *get_merge_bases_many_0(struct repository *r,
- struct commit *one,
- int n,
- struct commit **twos,
- int cleanup)
+static int get_merge_bases_many_0(struct repository *r,
+ struct commit *one,
+ int n,
+ struct commit **twos,
+ int cleanup,
+ struct commit_list **result)
{
struct commit_list *list;
struct commit **rslt;
- struct commit_list *result = NULL;
int cnt, i;
- if (merge_bases_many(r, one, n, twos, &result) < 0)
- return NULL;
+ if (merge_bases_many(r, one, n, twos, result) < 0)
+ return -1;
for (i = 0; i < n; i++) {
if (one == twos[i])
- return result;
+ return 0;
}
- if (!result || !result->next) {
+ if (!*result || !(*result)->next) {
if (cleanup) {
clear_commit_marks(one, all_flags);
clear_commit_marks_many(n, twos, all_flags);
}
- return result;
+ return 0;
}
/* There are more than one */
- cnt = commit_list_count(result);
+ cnt = commit_list_count(*result);
CALLOC_ARRAY(rslt, cnt);
- for (list = result, i = 0; list; list = list->next)
+ for (list = *result, i = 0; list; list = list->next)
rslt[i++] = list->item;
- free_commit_list(result);
+ free_commit_list(*result);
+ *result = NULL;
clear_commit_marks(one, all_flags);
clear_commit_marks_many(n, twos, all_flags);
@@ -448,13 +449,12 @@ static struct commit_list *get_merge_bases_many_0(struct repository *r,
cnt = remove_redundant(r, rslt, cnt);
if (cnt < 0) {
free(rslt);
- return NULL;
+ return -1;
}
- result = NULL;
for (i = 0; i < cnt; i++)
- commit_list_insert_by_date(rslt[i], &result);
+ commit_list_insert_by_date(rslt[i], result);
free(rslt);
- return result;
+ return 0;
}
struct commit_list *repo_get_merge_bases_many(struct repository *r,
@@ -462,7 +462,12 @@ struct commit_list *repo_get_merge_bases_many(struct repository *r,
int n,
struct commit **twos)
{
- return get_merge_bases_many_0(r, one, n, twos, 1);
+ struct commit_list *result = NULL;
+ if (get_merge_bases_many_0(r, one, n, twos, 1, &result) < 0) {
+ free_commit_list(result);
+ return NULL;
+ }
+ return result;
}
struct commit_list *repo_get_merge_bases_many_dirty(struct repository *r,
@@ -470,14 +475,24 @@ struct commit_list *repo_get_merge_bases_many_dirty(struct repository *r,
int n,
struct commit **twos)
{
- return get_merge_bases_many_0(r, one, n, twos, 0);
+ struct commit_list *result = NULL;
+ if (get_merge_bases_many_0(r, one, n, twos, 0, &result) < 0) {
+ free_commit_list(result);
+ return NULL;
+ }
+ return result;
}
struct commit_list *repo_get_merge_bases(struct repository *r,
struct commit *one,
struct commit *two)
{
- return get_merge_bases_many_0(r, one, 1, &two, 1);
+ struct commit_list *result = NULL;
+ if (get_merge_bases_many_0(r, one, 1, &two, 1, &result) < 0) {
+ free_commit_list(result);
+ return NULL;
+ }
+ return result;
}
/*
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 08/11] repo_get_merge_bases(): pass on errors from `merge_bases_many()`
From: Johannes Schindelin via GitGitGadget @ 2024-02-27 13:28 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.1657.v3.git.1709040497.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
The `merge_bases_many()` function was just taught to indicate parsing
errors, and now the `repo_get_merge_bases()` function (which is also
surfaced via the `repo_get_merge_bases()` macro) is aware of that, too.
Naturally, there are a lot of callers that need to be adjusted now, too.
Next step: adjust the callers of `get_octopus_merge_bases()`.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/log.c | 10 +++++-----
builtin/merge-tree.c | 5 +++--
builtin/merge.c | 20 ++++++++++++--------
builtin/rebase.c | 8 +++++---
builtin/rev-parse.c | 5 +++--
commit-reach.c | 23 +++++++++++------------
commit-reach.h | 7 ++++---
diff-lib.c | 5 +++--
log-tree.c | 5 +++--
merge-ort.c | 6 +++++-
merge-recursive.c | 4 +++-
notes-merge.c | 3 ++-
object-name.c | 7 +++++--
revision.c | 12 ++++++++----
sequencer.c | 8 ++++++--
submodule.c | 7 ++++++-
t/t4301-merge-tree-write-tree.sh | 12 ++++++++++++
17 files changed, 96 insertions(+), 51 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index 1705da71aca..befafd6ae04 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1702,11 +1702,11 @@ static struct commit *get_base_commit(const char *base_commit,
*/
while (rev_nr > 1) {
for (i = 0; i < rev_nr / 2; i++) {
- struct commit_list *merge_base;
- merge_base = repo_get_merge_bases(the_repository,
- rev[2 * i],
- rev[2 * i + 1]);
- if (!merge_base || merge_base->next) {
+ struct commit_list *merge_base = NULL;
+ if (repo_get_merge_bases(the_repository,
+ rev[2 * i],
+ rev[2 * i + 1], &merge_base) < 0 ||
+ !merge_base || merge_base->next) {
if (die_on_failure) {
die(_("failed to find exact merge base"));
} else {
diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index a35e0452d66..76200250629 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -463,8 +463,9 @@ static int real_merge(struct merge_tree_options *o,
* Get the merge bases, in reverse order; see comment above
* merge_incore_recursive in merge-ort.h
*/
- merge_bases = repo_get_merge_bases(the_repository, parent1,
- parent2);
+ if (repo_get_merge_bases(the_repository, parent1,
+ parent2, &merge_bases) < 0)
+ exit(128);
if (!merge_bases && !o->allow_unrelated_histories)
die(_("refusing to merge unrelated histories"));
merge_bases = reverse_commit_list(merge_bases);
diff --git a/builtin/merge.c b/builtin/merge.c
index d748d46e135..ac9d58adc29 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1517,10 +1517,13 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
if (!remoteheads)
; /* already up-to-date */
- else if (!remoteheads->next)
- common = repo_get_merge_bases(the_repository, head_commit,
- remoteheads->item);
- else {
+ else if (!remoteheads->next) {
+ if (repo_get_merge_bases(the_repository, head_commit,
+ remoteheads->item, &common) < 0) {
+ ret = 2;
+ goto done;
+ }
+ } else {
struct commit_list *list = remoteheads;
commit_list_insert(head_commit, &list);
common = get_octopus_merge_bases(list);
@@ -1631,7 +1634,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
struct commit_list *j;
for (j = remoteheads; j; j = j->next) {
- struct commit_list *common_one;
+ struct commit_list *common_one = NULL;
struct commit *common_item;
/*
@@ -1639,9 +1642,10 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
* merge_bases again, otherwise "git merge HEAD^
* HEAD^^" would be missed.
*/
- common_one = repo_get_merge_bases(the_repository,
- head_commit,
- j->item);
+ if (repo_get_merge_bases(the_repository, head_commit,
+ j->item, &common_one) < 0)
+ exit(128);
+
common_item = common_one->item;
free_commit_list(common_one);
if (!oideq(&common_item->object.oid, &j->item->object.oid)) {
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 043c65dccd9..06a55fc7325 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -879,7 +879,8 @@ static int can_fast_forward(struct commit *onto, struct commit *upstream,
if (!upstream)
goto done;
- merge_bases = repo_get_merge_bases(the_repository, upstream, head);
+ if (repo_get_merge_bases(the_repository, upstream, head, &merge_bases) < 0)
+ exit(128);
if (!merge_bases || merge_bases->next)
goto done;
@@ -898,8 +899,9 @@ static void fill_branch_base(struct rebase_options *options,
{
struct commit_list *merge_bases = NULL;
- merge_bases = repo_get_merge_bases(the_repository, options->onto,
- options->orig_head);
+ if (repo_get_merge_bases(the_repository, options->onto,
+ options->orig_head, &merge_bases) < 0)
+ exit(128);
if (!merge_bases || merge_bases->next)
oidcpy(branch_base, null_oid());
else
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index fde8861ca4e..c97d0f6144c 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -297,7 +297,7 @@ static int try_difference(const char *arg)
show_rev(NORMAL, &end_oid, end);
show_rev(symmetric ? NORMAL : REVERSED, &start_oid, start);
if (symmetric) {
- struct commit_list *exclude;
+ struct commit_list *exclude = NULL;
struct commit *a, *b;
a = lookup_commit_reference(the_repository, &start_oid);
b = lookup_commit_reference(the_repository, &end_oid);
@@ -305,7 +305,8 @@ static int try_difference(const char *arg)
*dotdot = '.';
return 0;
}
- exclude = repo_get_merge_bases(the_repository, a, b);
+ if (repo_get_merge_bases(the_repository, a, b, &exclude) < 0)
+ exit(128);
while (exclude) {
struct commit *commit = pop_commit(&exclude);
show_rev(REVERSED, &commit->object.oid, NULL);
diff --git a/commit-reach.c b/commit-reach.c
index 5fa0abc7d1e..10e625ff51b 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -189,9 +189,12 @@ struct commit_list *get_octopus_merge_bases(struct commit_list *in)
struct commit_list *new_commits = NULL, *end = NULL;
for (j = ret; j; j = j->next) {
- struct commit_list *bases;
- bases = repo_get_merge_bases(the_repository, i->item,
- j->item);
+ struct commit_list *bases = NULL;
+ if (repo_get_merge_bases(the_repository, i->item,
+ j->item, &bases) < 0) {
+ free_commit_list(bases);
+ return NULL;
+ }
if (!new_commits)
new_commits = bases;
else
@@ -483,16 +486,12 @@ struct commit_list *repo_get_merge_bases_many_dirty(struct repository *r,
return result;
}
-struct commit_list *repo_get_merge_bases(struct repository *r,
- struct commit *one,
- struct commit *two)
+int repo_get_merge_bases(struct repository *r,
+ struct commit *one,
+ struct commit *two,
+ struct commit_list **result)
{
- struct commit_list *result = NULL;
- if (get_merge_bases_many_0(r, one, 1, &two, 1, &result) < 0) {
- free_commit_list(result);
- return NULL;
- }
- return result;
+ return get_merge_bases_many_0(r, one, 1, &two, 1, result);
}
/*
diff --git a/commit-reach.h b/commit-reach.h
index 68f81549a44..2c6fcdd34f6 100644
--- a/commit-reach.h
+++ b/commit-reach.h
@@ -9,9 +9,10 @@ struct ref_filter;
struct object_id;
struct object_array;
-struct commit_list *repo_get_merge_bases(struct repository *r,
- struct commit *rev1,
- struct commit *rev2);
+int repo_get_merge_bases(struct repository *r,
+ struct commit *rev1,
+ struct commit *rev2,
+ struct commit_list **result);
struct commit_list *repo_get_merge_bases_many(struct repository *r,
struct commit *one, int n,
struct commit **twos);
diff --git a/diff-lib.c b/diff-lib.c
index 0e9ec4f68af..498224ccce2 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -565,7 +565,7 @@ void diff_get_merge_base(const struct rev_info *revs, struct object_id *mb)
{
int i;
struct commit *mb_child[2] = {0};
- struct commit_list *merge_bases;
+ struct commit_list *merge_bases = NULL;
for (i = 0; i < revs->pending.nr; i++) {
struct object *obj = revs->pending.objects[i].item;
@@ -592,7 +592,8 @@ void diff_get_merge_base(const struct rev_info *revs, struct object_id *mb)
mb_child[1] = lookup_commit_reference(the_repository, &oid);
}
- merge_bases = repo_get_merge_bases(the_repository, mb_child[0], mb_child[1]);
+ if (repo_get_merge_bases(the_repository, mb_child[0], mb_child[1], &merge_bases) < 0)
+ exit(128);
if (!merge_bases)
die(_("no merge base found"));
if (merge_bases->next)
diff --git a/log-tree.c b/log-tree.c
index 504da6b519e..4f337766a39 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -1010,7 +1010,7 @@ static int do_remerge_diff(struct rev_info *opt,
struct object_id *oid)
{
struct merge_options o;
- struct commit_list *bases;
+ struct commit_list *bases = NULL;
struct merge_result res = {0};
struct pretty_print_context ctx = {0};
struct commit *parent1 = parents->item;
@@ -1035,7 +1035,8 @@ static int do_remerge_diff(struct rev_info *opt,
/* Parse the relevant commits and get the merge bases */
parse_commit_or_die(parent1);
parse_commit_or_die(parent2);
- bases = repo_get_merge_bases(the_repository, parent1, parent2);
+ if (repo_get_merge_bases(the_repository, parent1, parent2, &bases) < 0)
+ exit(128);
/* Re-merge the parents */
merge_incore_recursive(&o, bases, parent1, parent2, &res);
diff --git a/merge-ort.c b/merge-ort.c
index 9f3af46333a..90d8495ca1f 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -5066,7 +5066,11 @@ static void merge_ort_internal(struct merge_options *opt,
struct strbuf merge_base_abbrev = STRBUF_INIT;
if (!merge_bases) {
- merge_bases = repo_get_merge_bases(the_repository, h1, h2);
+ if (repo_get_merge_bases(the_repository, h1, h2,
+ &merge_bases) < 0) {
+ result->clean = -1;
+ return;
+ }
/* See merge-ort.h:merge_incore_recursive() declaration NOTE */
merge_bases = reverse_commit_list(merge_bases);
}
diff --git a/merge-recursive.c b/merge-recursive.c
index 0d931cc14ad..d609373d960 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -3638,7 +3638,9 @@ static int merge_recursive_internal(struct merge_options *opt,
}
if (!merge_bases) {
- merge_bases = repo_get_merge_bases(the_repository, h1, h2);
+ if (repo_get_merge_bases(the_repository, h1, h2,
+ &merge_bases) < 0)
+ return -1;
merge_bases = reverse_commit_list(merge_bases);
}
diff --git a/notes-merge.c b/notes-merge.c
index 8799b522a55..51282934ae6 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -607,7 +607,8 @@ int notes_merge(struct notes_merge_options *o,
assert(local && remote);
/* Find merge bases */
- bases = repo_get_merge_bases(the_repository, local, remote);
+ if (repo_get_merge_bases(the_repository, local, remote, &bases) < 0)
+ exit(128);
if (!bases) {
base_oid = null_oid();
base_tree_oid = the_hash_algo->empty_tree;
diff --git a/object-name.c b/object-name.c
index 0bfa29dbbfe..63bec6d9a2b 100644
--- a/object-name.c
+++ b/object-name.c
@@ -1481,7 +1481,7 @@ int repo_get_oid_mb(struct repository *r,
struct object_id *oid)
{
struct commit *one, *two;
- struct commit_list *mbs;
+ struct commit_list *mbs = NULL;
struct object_id oid_tmp;
const char *dots;
int st;
@@ -1509,7 +1509,10 @@ int repo_get_oid_mb(struct repository *r,
two = lookup_commit_reference_gently(r, &oid_tmp, 0);
if (!two)
return -1;
- mbs = repo_get_merge_bases(r, one, two);
+ if (repo_get_merge_bases(r, one, two, &mbs) < 0) {
+ free_commit_list(mbs);
+ return -1;
+ }
if (!mbs || mbs->next)
st = -1;
else {
diff --git a/revision.c b/revision.c
index 00d5c29bfce..eb0d550842f 100644
--- a/revision.c
+++ b/revision.c
@@ -1965,7 +1965,7 @@ static void add_pending_commit_list(struct rev_info *revs,
static void prepare_show_merge(struct rev_info *revs)
{
- struct commit_list *bases;
+ struct commit_list *bases = NULL;
struct commit *head, *other;
struct object_id oid;
const char **prune = NULL;
@@ -1980,7 +1980,8 @@ static void prepare_show_merge(struct rev_info *revs)
other = lookup_commit_or_die(&oid, "MERGE_HEAD");
add_pending_object(revs, &head->object, "HEAD");
add_pending_object(revs, &other->object, "MERGE_HEAD");
- bases = repo_get_merge_bases(the_repository, head, other);
+ if (repo_get_merge_bases(the_repository, head, other, &bases) < 0)
+ exit(128);
add_rev_cmdline_list(revs, bases, REV_CMD_MERGE_BASE, UNINTERESTING | BOTTOM);
add_pending_commit_list(revs, bases, UNINTERESTING | BOTTOM);
free_commit_list(bases);
@@ -2068,14 +2069,17 @@ static int handle_dotdot_1(const char *arg, char *dotdot,
} else {
/* A...B -- find merge bases between the two */
struct commit *a, *b;
- struct commit_list *exclude;
+ struct commit_list *exclude = NULL;
a = lookup_commit_reference(revs->repo, &a_obj->oid);
b = lookup_commit_reference(revs->repo, &b_obj->oid);
if (!a || !b)
return dotdot_missing(arg, dotdot, revs, symmetric);
- exclude = repo_get_merge_bases(the_repository, a, b);
+ if (repo_get_merge_bases(the_repository, a, b, &exclude) < 0) {
+ free_commit_list(exclude);
+ return -1;
+ }
add_rev_cmdline_list(revs, exclude, REV_CMD_MERGE_BASE,
flags_exclude);
add_pending_commit_list(revs, exclude, flags_exclude);
diff --git a/sequencer.c b/sequencer.c
index d584cac8ed9..4417f2f1956 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3913,7 +3913,7 @@ static int do_merge(struct repository *r,
int run_commit_flags = 0;
struct strbuf ref_name = STRBUF_INIT;
struct commit *head_commit, *merge_commit, *i;
- struct commit_list *bases, *j;
+ struct commit_list *bases = NULL, *j;
struct commit_list *to_merge = NULL, **tail = &to_merge;
const char *strategy = !opts->xopts.nr &&
(!opts->strategy ||
@@ -4139,7 +4139,11 @@ static int do_merge(struct repository *r,
}
merge_commit = to_merge->item;
- bases = repo_get_merge_bases(r, head_commit, merge_commit);
+ if (repo_get_merge_bases(r, head_commit, merge_commit, &bases) < 0) {
+ ret = -1;
+ goto leave_merge;
+ }
+
if (bases && oideq(&merge_commit->object.oid,
&bases->item->object.oid)) {
ret = 0;
diff --git a/submodule.c b/submodule.c
index e603a19a876..04931a5474b 100644
--- a/submodule.c
+++ b/submodule.c
@@ -595,7 +595,12 @@ static void show_submodule_header(struct diff_options *o,
(!is_null_oid(two) && !*right))
message = "(commits not present)";
- *merge_bases = repo_get_merge_bases(sub, *left, *right);
+ *merge_bases = NULL;
+ if (repo_get_merge_bases(sub, *left, *right, merge_bases) < 0) {
+ message = "(corrupt repository)";
+ goto output_header;
+ }
+
if (*merge_bases) {
if ((*merge_bases)->item == *left)
fast_forward = 1;
diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh
index b2c8a43fce3..5d1e7aca4c8 100755
--- a/t/t4301-merge-tree-write-tree.sh
+++ b/t/t4301-merge-tree-write-tree.sh
@@ -945,4 +945,16 @@ test_expect_success 'check the input format when --stdin is passed' '
test_cmp expect actual
'
+test_expect_success 'error out on missing commits as well' '
+ git init --bare missing-commit.git &&
+ git rev-list --objects side1 side3 >list-including-initial &&
+ grep -v ^$(git rev-parse side1^) <list-including-initial >list &&
+ git pack-objects missing-commit.git/objects/pack/missing-initial <list &&
+ side1=$(git rev-parse side1) &&
+ side3=$(git rev-parse side3) &&
+ test_must_fail git --git-dir=missing-commit.git \
+ merge-tree --allow-unrelated-histories $side1 $side3 >actual &&
+ test_must_be_empty actual
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 09/11] get_octopus_merge_bases(): pass on errors from `merge_bases_many()`
From: Johannes Schindelin via GitGitGadget @ 2024-02-27 13:28 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.1657.v3.git.1709040497.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
The `merge_bases_many()` function was just taught to indicate parsing
errors, and now the `repo_get_merge_bases()` function (which is also
surfaced via the `get_merge_bases()` macro) is aware of that, too.
Naturally, the callers need to be adjusted now, too.
Next step: adjust `repo_get_merge_bases_many()`.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/merge-base.c | 8 ++++++--
builtin/merge.c | 6 +++++-
builtin/pull.c | 5 +++--
commit-reach.c | 20 +++++++++++---------
commit-reach.h | 2 +-
5 files changed, 26 insertions(+), 15 deletions(-)
diff --git a/builtin/merge-base.c b/builtin/merge-base.c
index 0308fd73289..2edffc5487e 100644
--- a/builtin/merge-base.c
+++ b/builtin/merge-base.c
@@ -77,13 +77,17 @@ static int handle_independent(int count, const char **args)
static int handle_octopus(int count, const char **args, int show_all)
{
struct commit_list *revs = NULL;
- struct commit_list *result, *rev;
+ struct commit_list *result = NULL, *rev;
int i;
for (i = count - 1; i >= 0; i--)
commit_list_insert(get_commit_reference(args[i]), &revs);
- result = get_octopus_merge_bases(revs);
+ if (get_octopus_merge_bases(revs, &result) < 0) {
+ free_commit_list(revs);
+ free_commit_list(result);
+ return 128;
+ }
free_commit_list(revs);
reduce_heads_replace(&result);
diff --git a/builtin/merge.c b/builtin/merge.c
index ac9d58adc29..94c5b693972 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1526,7 +1526,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
} else {
struct commit_list *list = remoteheads;
commit_list_insert(head_commit, &list);
- common = get_octopus_merge_bases(list);
+ if (get_octopus_merge_bases(list, &common) < 0) {
+ free(list);
+ ret = 2;
+ goto done;
+ }
free(list);
}
diff --git a/builtin/pull.c b/builtin/pull.c
index e6f2942c0c5..0c5a55f2f4d 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -820,7 +820,7 @@ static int get_octopus_merge_base(struct object_id *merge_base,
const struct object_id *merge_head,
const struct object_id *fork_point)
{
- struct commit_list *revs = NULL, *result;
+ struct commit_list *revs = NULL, *result = NULL;
commit_list_insert(lookup_commit_reference(the_repository, curr_head),
&revs);
@@ -830,7 +830,8 @@ static int get_octopus_merge_base(struct object_id *merge_base,
commit_list_insert(lookup_commit_reference(the_repository, fork_point),
&revs);
- result = get_octopus_merge_bases(revs);
+ if (get_octopus_merge_bases(revs, &result) < 0)
+ exit(128);
free_commit_list(revs);
reduce_heads_replace(&result);
diff --git a/commit-reach.c b/commit-reach.c
index 10e625ff51b..fa21a8f2f6b 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -176,24 +176,26 @@ static int merge_bases_many(struct repository *r,
return 0;
}
-struct commit_list *get_octopus_merge_bases(struct commit_list *in)
+int get_octopus_merge_bases(struct commit_list *in, struct commit_list **result)
{
- struct commit_list *i, *j, *k, *ret = NULL;
+ struct commit_list *i, *j, *k;
if (!in)
- return ret;
+ return 0;
- commit_list_insert(in->item, &ret);
+ commit_list_insert(in->item, result);
for (i = in->next; i; i = i->next) {
struct commit_list *new_commits = NULL, *end = NULL;
- for (j = ret; j; j = j->next) {
+ for (j = *result; j; j = j->next) {
struct commit_list *bases = NULL;
if (repo_get_merge_bases(the_repository, i->item,
j->item, &bases) < 0) {
free_commit_list(bases);
- return NULL;
+ free_commit_list(*result);
+ *result = NULL;
+ return -1;
}
if (!new_commits)
new_commits = bases;
@@ -202,10 +204,10 @@ struct commit_list *get_octopus_merge_bases(struct commit_list *in)
for (k = bases; k; k = k->next)
end = k;
}
- free_commit_list(ret);
- ret = new_commits;
+ free_commit_list(*result);
+ *result = new_commits;
}
- return ret;
+ return 0;
}
static int remove_redundant_no_gen(struct repository *r,
diff --git a/commit-reach.h b/commit-reach.h
index 2c6fcdd34f6..4690b6ecd0c 100644
--- a/commit-reach.h
+++ b/commit-reach.h
@@ -21,7 +21,7 @@ struct commit_list *repo_get_merge_bases_many_dirty(struct repository *r,
struct commit *one, int n,
struct commit **twos);
-struct commit_list *get_octopus_merge_bases(struct commit_list *in);
+int get_octopus_merge_bases(struct commit_list *in, struct commit_list **result);
int repo_is_descendant_of(struct repository *r,
struct commit *commit,
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 10/11] repo_get_merge_bases_many(): pass on errors from `merge_bases_many()`
From: Johannes Schindelin via GitGitGadget @ 2024-02-27 13:28 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.1657.v3.git.1709040497.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
The `merge_bases_many()` function was just taught to indicate parsing
errors, and now the `repo_get_merge_bases_many()` function is aware of
that, too.
Naturally, there are a lot of callers that need to be adjusted now, too.
Next stop: `repo_get_merge_bases_dirty()`.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
bisect.c | 7 ++++---
builtin/log.c | 13 +++++++------
commit-reach.c | 16 ++++++----------
commit-reach.h | 7 ++++---
commit.c | 7 ++++---
t/helper/test-reach.c | 9 ++++++---
6 files changed, 31 insertions(+), 28 deletions(-)
diff --git a/bisect.c b/bisect.c
index 1be8e0a2711..2018466d69f 100644
--- a/bisect.c
+++ b/bisect.c
@@ -851,10 +851,11 @@ static void handle_skipped_merge_base(const struct object_id *mb)
static enum bisect_error check_merge_bases(int rev_nr, struct commit **rev, int no_checkout)
{
enum bisect_error res = BISECT_OK;
- struct commit_list *result;
+ struct commit_list *result = NULL;
- result = repo_get_merge_bases_many(the_repository, rev[0], rev_nr - 1,
- rev + 1);
+ if (repo_get_merge_bases_many(the_repository, rev[0], rev_nr - 1,
+ rev + 1, &result) < 0)
+ exit(128);
for (; result; result = result->next) {
const struct object_id *mb = &result->item->object.oid;
diff --git a/builtin/log.c b/builtin/log.c
index befafd6ae04..c75790a7cec 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1656,7 +1656,7 @@ static struct commit *get_base_commit(const char *base_commit,
struct branch *curr_branch = branch_get(NULL);
const char *upstream = branch_get_upstream(curr_branch, NULL);
if (upstream) {
- struct commit_list *base_list;
+ struct commit_list *base_list = NULL;
struct commit *commit;
struct object_id oid;
@@ -1667,11 +1667,12 @@ static struct commit *get_base_commit(const char *base_commit,
return NULL;
}
commit = lookup_commit_or_die(&oid, "upstream base");
- base_list = repo_get_merge_bases_many(the_repository,
- commit, total,
- list);
- /* There should be one and only one merge base. */
- if (!base_list || base_list->next) {
+ if (repo_get_merge_bases_many(the_repository,
+ commit, total,
+ list,
+ &base_list) < 0 ||
+ /* There should be one and only one merge base. */
+ !base_list || base_list->next) {
if (die_on_failure) {
die(_("could not find exact merge base"));
} else {
diff --git a/commit-reach.c b/commit-reach.c
index fa21a8f2f6b..954a05399f1 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -462,17 +462,13 @@ static int get_merge_bases_many_0(struct repository *r,
return 0;
}
-struct commit_list *repo_get_merge_bases_many(struct repository *r,
- struct commit *one,
- int n,
- struct commit **twos)
+int repo_get_merge_bases_many(struct repository *r,
+ struct commit *one,
+ int n,
+ struct commit **twos,
+ struct commit_list **result)
{
- struct commit_list *result = NULL;
- if (get_merge_bases_many_0(r, one, n, twos, 1, &result) < 0) {
- free_commit_list(result);
- return NULL;
- }
- return result;
+ return get_merge_bases_many_0(r, one, n, twos, 1, result);
}
struct commit_list *repo_get_merge_bases_many_dirty(struct repository *r,
diff --git a/commit-reach.h b/commit-reach.h
index 4690b6ecd0c..458043f4d58 100644
--- a/commit-reach.h
+++ b/commit-reach.h
@@ -13,9 +13,10 @@ int repo_get_merge_bases(struct repository *r,
struct commit *rev1,
struct commit *rev2,
struct commit_list **result);
-struct commit_list *repo_get_merge_bases_many(struct repository *r,
- struct commit *one, int n,
- struct commit **twos);
+int repo_get_merge_bases_many(struct repository *r,
+ struct commit *one, int n,
+ struct commit **twos,
+ struct commit_list **result);
/* To be used only when object flags after this call no longer matter */
struct commit_list *repo_get_merge_bases_many_dirty(struct repository *r,
struct commit *one, int n,
diff --git a/commit.c b/commit.c
index 8405d7c3fce..00add5d81c6 100644
--- a/commit.c
+++ b/commit.c
@@ -1054,7 +1054,7 @@ struct commit *get_fork_point(const char *refname, struct commit *commit)
{
struct object_id oid;
struct rev_collect revs;
- struct commit_list *bases;
+ struct commit_list *bases = NULL;
int i;
struct commit *ret = NULL;
char *full_refname;
@@ -1079,8 +1079,9 @@ struct commit *get_fork_point(const char *refname, struct commit *commit)
for (i = 0; i < revs.nr; i++)
revs.commit[i]->object.flags &= ~TMP_MARK;
- bases = repo_get_merge_bases_many(the_repository, commit, revs.nr,
- revs.commit);
+ if (repo_get_merge_bases_many(the_repository, commit, revs.nr,
+ revs.commit, &bases) < 0)
+ exit(128);
/*
* There should be one and only one merge base, when we found
diff --git a/t/helper/test-reach.c b/t/helper/test-reach.c
index aa816e168ea..84ee9da8681 100644
--- a/t/helper/test-reach.c
+++ b/t/helper/test-reach.c
@@ -117,9 +117,12 @@ int cmd__reach(int ac, const char **av)
else if (!strcmp(av[1], "is_descendant_of"))
printf("%s(A,X):%d\n", av[1], repo_is_descendant_of(r, A, X));
else if (!strcmp(av[1], "get_merge_bases_many")) {
- struct commit_list *list = repo_get_merge_bases_many(the_repository,
- A, X_nr,
- X_array);
+ struct commit_list *list = NULL;
+ if (repo_get_merge_bases_many(the_repository,
+ A, X_nr,
+ X_array,
+ &list) < 0)
+ exit(128);
printf("%s(A,X):\n", av[1]);
print_sorted_commit_ids(list);
} else if (!strcmp(av[1], "reduce_heads")) {
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 11/11] repo_get_merge_bases_many_dirty(): pass on errors from `merge_bases_many()`
From: Johannes Schindelin via GitGitGadget @ 2024-02-27 13:28 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.1657.v3.git.1709040497.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
The `merge_bases_many()` function was just taught to indicate parsing
errors, and now the `repo_get_merge_bases_many_dirty()` function is
aware of that, too.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/merge-base.c | 9 ++++++---
commit-reach.c | 16 ++++++----------
commit-reach.h | 7 ++++---
3 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/builtin/merge-base.c b/builtin/merge-base.c
index 2edffc5487e..a8a1ca53968 100644
--- a/builtin/merge-base.c
+++ b/builtin/merge-base.c
@@ -13,10 +13,13 @@
static int show_merge_base(struct commit **rev, int rev_nr, int show_all)
{
- struct commit_list *result, *r;
+ struct commit_list *result = NULL, *r;
- result = repo_get_merge_bases_many_dirty(the_repository, rev[0],
- rev_nr - 1, rev + 1);
+ if (repo_get_merge_bases_many_dirty(the_repository, rev[0],
+ rev_nr - 1, rev + 1, &result) < 0) {
+ free_commit_list(result);
+ return -1;
+ }
if (!result)
return 1;
diff --git a/commit-reach.c b/commit-reach.c
index 954a05399f1..2c69cb83d6f 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -471,17 +471,13 @@ int repo_get_merge_bases_many(struct repository *r,
return get_merge_bases_many_0(r, one, n, twos, 1, result);
}
-struct commit_list *repo_get_merge_bases_many_dirty(struct repository *r,
- struct commit *one,
- int n,
- struct commit **twos)
+int repo_get_merge_bases_many_dirty(struct repository *r,
+ struct commit *one,
+ int n,
+ struct commit **twos,
+ struct commit_list **result)
{
- struct commit_list *result = NULL;
- if (get_merge_bases_many_0(r, one, n, twos, 0, &result) < 0) {
- free_commit_list(result);
- return NULL;
- }
- return result;
+ return get_merge_bases_many_0(r, one, n, twos, 0, result);
}
int repo_get_merge_bases(struct repository *r,
diff --git a/commit-reach.h b/commit-reach.h
index 458043f4d58..bf63cc468fd 100644
--- a/commit-reach.h
+++ b/commit-reach.h
@@ -18,9 +18,10 @@ int repo_get_merge_bases_many(struct repository *r,
struct commit **twos,
struct commit_list **result);
/* To be used only when object flags after this call no longer matter */
-struct commit_list *repo_get_merge_bases_many_dirty(struct repository *r,
- struct commit *one, int n,
- struct commit **twos);
+int repo_get_merge_bases_many_dirty(struct repository *r,
+ struct commit *one, int n,
+ struct commit **twos,
+ struct commit_list **result);
int get_octopus_merge_bases(struct commit_list *in, struct commit_list **result);
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH v5 0/2] Implement `git log --merge` also for rebase/cherry-pick/revert
From: Phillip Wood @ 2024-02-27 14:00 UTC (permalink / raw)
To: Philippe Blain, git
Cc: Johannes Sixt, Elijah Newren, Michael Lohmann, Phillip Wood,
Patrick Steinhardt, Junio C Hamano, Michael Lohmann
In-Reply-To: <20240225-ml-log-merge-with-cherry-pick-and-other-pseudo-heads-v5-0-af1ef2d9e44d@gmail.com>
On 25/02/2024 21:56, Philippe Blain wrote:
> Changes in v5:
> - Marked error messages for translation and tweaked them as suggested by Phillip
> - Reworded the message of 2/2 as suggested by Phillip
> - Removed the change to gitk's doc in 2/2 as pointed out by Johannes
> - Fixed the trailers in 2/2
> - Improved the doc in 2/2 as suggested by Phillip and Jean-Noël
These changes look good, thanks for making them. I agree with the other
reviewers that it would be nice to improve the wording of the error
message when we find a symbolic ref. Everything else looks good to me.
Thanks
Phillip
^ permalink raw reply
* [PATCH] rebase -i: stop setting GIT_CHERRY_PICK_HELP
From: Phillip Wood via GitGitGadget @ 2024-02-27 14:06 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood, Phillip Wood
From: Phillip Wood <phillip.wood@dunelm.org.uk>
Setting this environment variable causes the sequencer to display a
custom message when it stops for the user to resolve conflicts and
remove CHERRY_PICK_HEAD. Setting it in "git rebase" is a vestige of
the scripted implementation, now that it is a builtin command we do
not need to communicate with the sequencer machinery via environment
variables.
Move the conflicts advice to use when rebasing into
sequencer.c so we do not need to pass it via the environment.
Note that we retain the changes in e4301f73fff (sequencer: unset
GIT_CHERRY_PICK_HELP for 'exec' commands, 2024-02-02) just in case
GIT_CHERRY_PICK_HELP is set in the environment when "git rebase" is
run.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
rebase -i: stop setting GIT_CHERRY_PICK_HELP
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1678%2Fphillipwood%2Frebase-stop-setting-GIT_CHERRY_PICK_HELP-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1678/phillipwood/rebase-stop-setting-GIT_CHERRY_PICK_HELP-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1678
builtin/rebase.c | 14 +++-----------
sequencer.c | 14 +++++++++++++-
sequencer.h | 2 ++
3 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 5b086f651a6..d0cc518c931 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -567,13 +567,6 @@ static int move_to_original_branch(struct rebase_options *opts)
return ret;
}
-static const char *resolvemsg =
-N_("Resolve all conflicts manually, mark them as resolved with\n"
-"\"git add/rm <conflicted_files>\", then run \"git rebase --continue\".\n"
-"You can instead skip this commit: run \"git rebase --skip\".\n"
-"To abort and get back to the state before \"git rebase\", run "
-"\"git rebase --abort\".");
-
static int run_am(struct rebase_options *opts)
{
struct child_process am = CHILD_PROCESS_INIT;
@@ -587,7 +580,7 @@ static int run_am(struct rebase_options *opts)
opts->reflog_action);
if (opts->action == ACTION_CONTINUE) {
strvec_push(&am.args, "--resolved");
- strvec_pushf(&am.args, "--resolvemsg=%s", resolvemsg);
+ strvec_pushf(&am.args, "--resolvemsg=%s", rebase_resolvemsg);
if (opts->gpg_sign_opt)
strvec_push(&am.args, opts->gpg_sign_opt);
status = run_command(&am);
@@ -598,7 +591,7 @@ static int run_am(struct rebase_options *opts)
}
if (opts->action == ACTION_SKIP) {
strvec_push(&am.args, "--skip");
- strvec_pushf(&am.args, "--resolvemsg=%s", resolvemsg);
+ strvec_pushf(&am.args, "--resolvemsg=%s", rebase_resolvemsg);
status = run_command(&am);
if (status)
return status;
@@ -672,7 +665,7 @@ static int run_am(struct rebase_options *opts)
strvec_pushv(&am.args, opts->git_am_opts.v);
strvec_push(&am.args, "--rebasing");
- strvec_pushf(&am.args, "--resolvemsg=%s", resolvemsg);
+ strvec_pushf(&am.args, "--resolvemsg=%s", rebase_resolvemsg);
strvec_push(&am.args, "--patch-format=mboxrd");
if (opts->allow_rerere_autoupdate == RERERE_AUTOUPDATE)
strvec_push(&am.args, "--rerere-autoupdate");
@@ -700,7 +693,6 @@ static int run_specific_rebase(struct rebase_options *opts)
if (opts->type == REBASE_MERGE) {
/* Run sequencer-based rebase */
- setenv("GIT_CHERRY_PICK_HELP", resolvemsg, 1);
if (!(opts->flags & REBASE_INTERACTIVE_EXPLICIT))
setenv("GIT_SEQUENCE_EDITOR", ":", 1);
if (opts->gpg_sign_opt) {
diff --git a/sequencer.c b/sequencer.c
index f49a871ac06..76027ad5f5c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -461,10 +461,22 @@ static void free_message(struct commit *commit, struct commit_message *msg)
repo_unuse_commit_buffer(the_repository, commit, msg->message);
}
+const char *rebase_resolvemsg =
+N_("Resolve all conflicts manually, mark them as resolved with\n"
+"\"git add/rm <conflicted_files>\", then run \"git rebase --continue\".\n"
+"You can instead skip this commit: run \"git rebase --skip\".\n"
+"To abort and get back to the state before \"git rebase\", run "
+"\"git rebase --abort\".");
+
static void print_advice(struct repository *r, int show_hint,
struct replay_opts *opts)
{
- char *msg = getenv("GIT_CHERRY_PICK_HELP");
+ const char *msg;
+
+ if (is_rebase_i(opts))
+ msg = rebase_resolvemsg;
+ else
+ msg = getenv("GIT_CHERRY_PICK_HELP");
if (msg) {
advise("%s\n", msg);
diff --git a/sequencer.h b/sequencer.h
index dcef7bb99c0..437eabd38af 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -14,6 +14,8 @@ const char *rebase_path_todo(void);
const char *rebase_path_todo_backup(void);
const char *rebase_path_dropped(void);
+extern const char *rebase_resolvemsg;
+
#define APPEND_SIGNOFF_DEDUP (1u << 0)
enum replay_action {
base-commit: 3c2a3fdc388747b9eaf4a4a4f2035c1c9ddb26d0
--
gitgitgadget
^ permalink raw reply related
* RE: [BUG] 2.44.0 t7704.9 Fails on NonStop ia64
From: rsbecker @ 2024-02-27 14:10 UTC (permalink / raw)
To: 'Patrick Steinhardt', phillip.wood
Cc: 'Torsten Bögershausen', git
In-Reply-To: <Zd2hMmIzHKQ7JE45@tanuki>
On Tuesday, February 27, 2024 3:46 AM, Patrick Steinhardt wrote:
>On Mon, Feb 26, 2024 at 03:32:14PM +0000, Phillip Wood wrote:
>> Hi Randal
>>
>> [cc'ing Patrick for the reftable writer]
>>
>> > The question is which call is bad? The cruft stuff is relatively new
>> > and I don't know the code.
>> >
>> > > > reftable/writer.c: int n = w->write(w->write_arg,
zeroed,
>> > > > w->pending_padding);
>> > > > reftable/writer.c: n = w->write(w->write_arg, data, len);
>>
>> Neither of these appear to check for short writes and
>> reftable_fd_write() is a thin wrapper around write(). Maybe
>> reftable_fd_write() should be using write_in_full()?
>
>It already does starting with 85a8c899ce (reftable: handle interrupted
writes, 2023-12-11):
>
>```
>static ssize_t reftable_fd_write(void *arg, const void *data, size_t sz) {
> int *fdp = (int *)arg;
> return write_in_full(*fdp, data, sz);
>}
Unfortunately, this fix is included in what I am testing but does not impact
the issue I am seeing one way or another, but thank you.
^ permalink raw reply
* [GSOC][PATCH 0/2] Add builtin patterns for userdiff in JavaScript, as Microproject.
From: Sergius Nyah @ 2024-02-27 14:19 UTC (permalink / raw)
To: christian.couder, pk; +Cc: git, Sergius Nyah
In-Reply-To: <CANAnif-OganZLi0Cu_uq=nveC+u5n14c=o_DQHT-wFOqQ9Vs0Q@mail.gmail.com>
Firstly, I'm so sorry for the delay between selecting the Microproject and sending the patch series.
Learning about regular expressions took me a bit longer than I expected, but it was all worth it.
I'm very grateful for the opportunity to work on this project and I'm looking forward to contributing more to Git.
This patch series adds builtin patterns for JavaScript function detection in userdiff, as
my Microproject for GSOC. The first patch adds a regular expression for detecting JavaScript
functions in Git diffs while the second adds a test for JavaScript function detection in Git diffs.
This new pattern looks for lines that start with optional whitespace, followed by 'function' and any
characters (for function declarations), or valid JavaScript identifiers, equals sign '=', 'function'
keyword and any characters (for function expressions). It also considers functions defined inside blocks with '{...}'.
t/t4018-diff-funcname.sh | 25 +++++++++++++++++++++++--
userdiff.c | 17 +++++++++++++++--
2 files changed, 38 insertions(+), 4 deletions(-)
base-commit: c5b454771e6b086f60c7f1f139025f174bcedac9
--
2.43.2
I would greatly appreciate any feedback on the patch series.
Best,
Sergius.
^ permalink raw reply
* [PATCH 1/2] Subject: [GSOC][RFC PATCH 1/2] Add builtin patterns for JavaScript function detection in userdiff.
From: Sergius Nyah @ 2024-02-27 14:19 UTC (permalink / raw)
To: christian.couder, pk; +Cc: git, Sergius Justus Chesami Nyah
In-Reply-To: <20240227141947.71447-1-74214119+Sergius-Nyah@users.noreply.github.com>
From: Sergius Justus Chesami Nyah <74214119+Sergius-Nyah@users.noreply.github.com>
This patch adds the regular expression for detecting JavaScript functions and expressions in Git diffs. The pattern accurately identifies function declerations, expressions, and definitions inside blocks.
---
userdiff.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/userdiff.c b/userdiff.c
index e399543823..12e31ff14d 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -1,7 +1,7 @@
#include "git-compat-util.h"
#include "config.h"
#include "userdiff.h"
-#include "attr.h"
+#include "attr.h"
#include "strbuf.h"
static struct userdiff_driver *drivers;
@@ -183,6 +183,19 @@ PATTERNS("java",
"|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lL]?"
"|[-+*/<>%&^|=!]="
"|--|\\+\\+|<<=?|>>>?=?|&&|\\|\\|"),
+PATTERNS("javascript",
+ /*
+ * Looks for lines that start with optional whitespace, followed
+ * by 'function'* and any characters (for function declarations),
+ * or valid JavaScript identifiers, equals sign '=', 'function' keyword
+ * and any characters (for function expressions).
+ * Also considers functions defined inside blocks with '{...}'.
+ */
+ "^[ \t]*(function[ \t]*.*|[a-zA-Z_$][0-9a-zA-Z_$]*[ \t]*=[ \t]*function[ \t]*.*|(\\{[ \t]*)?)\n",
+ /* This pattern matches JavaScript identifiers */
+ "[a-zA-Z_$][0-9a-zA-Z_$]*"
+ "|[-+0-9.eE]+|0[xX][0-9a-fA-F]+"
+ "|[-+*/<>%&^|=!:]=|--|\\+\\+|<<=?|>>=?|&&|\\|\\|"),
PATTERNS("kotlin",
"^[ \t]*(([a-z]+[ \t]+)*(fun|class|interface)[ \t]+.*)$",
/* -- */
@@ -192,7 +205,7 @@ PATTERNS("kotlin",
/* integers and floats */
"|[0-9][0-9_]*([.][0-9_]*)?([Ee][-+]?[0-9]+)?[fFlLuU]*"
/* floating point numbers beginning with decimal point */
- "|[.][0-9][0-9_]*([Ee][-+]?[0-9]+)?[fFlLuU]?"
+ "|[.][0-9][0-9_]*([Ee][-+]?[0-9]+ )?[fFlLuU]?"
/* unary and binary operators */
"|[-+*/<>%&^|=!]==?|--|\\+\\+|<<=|>>=|&&|\\|\\||->|\\.\\*|!!|[?:.][.:]"),
PATTERNS("markdown",
--
2.43.2
^ permalink raw reply related
* [PATCH 2/2] Subject:[GSOC] [RFC PATCH 2/2] Add test for JavaScript function detection in Git diffs
From: Sergius Nyah @ 2024-02-27 14:19 UTC (permalink / raw)
To: christian.couder, pk; +Cc: git, Sergius Justus Chesami Nyah
In-Reply-To: <20240227141947.71447-1-74214119+Sergius-Nyah@users.noreply.github.com>
From: Sergius Justus Chesami Nyah <74214119+Sergius-Nyah@users.noreply.github.com>
This commit introduces a new test case in t4018-diff-funcname.sh to verify the enhanced JavaScript function detection in Git diffs. The test creates a JavaScript file with function declarations and expressions, modifies them, and then checks the output of git diff to ensure that the changes are correctly identified. This test validates the changes made to userdiff.c for improved JavaScript function detection.
---
t/t4018-diff-funcname.sh | 25 +++++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index e026fac1f4..e88e63bd1f 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -11,7 +11,7 @@ test_expect_success 'setup' '
# a non-trivial custom pattern
git config diff.custom1.funcname "!static
!String
-[^ ].*s.*" &&
+[^ ].*s.*" &&
# a custom pattern which matches to end of line
git config diff.custom2.funcname "......Beer\$" &&
@@ -119,4 +119,25 @@ do
"
done
-test_done
+test_expect_success 'identify builtin patterns in Javascript' '
+ # setup
+ echo "function myFunction() { return true; }" > test.js &&
+ echo "var myVar = function() { return false; }" >> test.js &&
+ git add test.js &&
+ git commit -m "add test.js" &&
+
+ # modify the file
+ echo "function myFunction() { return false; }" > test.js &&
+ echo "var myVar = function() { return true; }" >> test.js &&
+
+ # command under test
+ git diff >output &&
+
+ # check results
+ test_i18ngrep "function myFunction() { return true; }" output &&
+ test_i18ngrep "function myFunction() { return false; }" output &&
+ test_i18ngrep "var myVar = function() { return false; }" output &&
+ test_i18ngrep "var myVar = function() { return true; }" output
+'
+
+test_done
\ No newline at end of file
--
2.43.2
^ permalink raw reply related
* [GSOC][PATCH 0/2] Add builtin patterns for userdiff in JavaScript, as Microproject.
From: Sergius Nyah @ 2024-02-27 14:21 UTC (permalink / raw)
To: christian.couder, pk; +Cc: git, Sergius Nyah
In-Reply-To: <CANAnif-OganZLi0Cu_uq=nveC+u5n14c=o_DQHT-wFOqQ9Vs0Q@mail.gmail.com>
Firstly, I'm so sorry for the delay between selecting the Microproject and sending the patch series.
Learning about regular expressions took me a bit longer than I expected, but it was all worth it.
I'm very grateful for the opportunity to work on this project and I'm looking forward to contributing more to Git.
This patch series adds builtin patterns for JavaScript function detection in userdiff, as
my Microproject for GSOC. The first patch adds a regular expression for detecting JavaScript
functions in Git diffs while the second adds a test for JavaScript function detection in Git diffs.
This new pattern looks for lines that start with optional whitespace, followed by 'function' and any
characters (for function declarations), or valid JavaScript identifiers, equals sign '=', 'function'
keyword and any characters (for function expressions). It also considers functions defined inside blocks with '{...}'.
t/t4018-diff-funcname.sh | 25 +++++++++++++++++++++++--
userdiff.c | 17 +++++++++++++++--
2 files changed, 38 insertions(+), 4 deletions(-)
base-commit: c5b454771e6b086f60c7f1f139025f174bcedac9
--
2.43.2
I would greatly appreciate any feedback on the patch series.
Best,
Sergius.
^ permalink raw reply
* [PATCH 1/2] Subject: [GSOC][RFC PATCH 1/2] Add builtin patterns for JavaScript function detection in userdiff.
From: Sergius Nyah @ 2024-02-27 14:21 UTC (permalink / raw)
To: christian.couder, pk; +Cc: git, Sergius Justus Chesami Nyah
In-Reply-To: <20240227142121.72518-1-74214119+Sergius-Nyah@users.noreply.github.com>
From: Sergius Justus Chesami Nyah <74214119+Sergius-Nyah@users.noreply.github.com>
This patch adds the regular expression for detecting JavaScript functions and expressions in Git diffs. The pattern accurately identifies function declerations, expressions, and definitions inside blocks.
---
userdiff.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/userdiff.c b/userdiff.c
index e399543823..12e31ff14d 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -1,7 +1,7 @@
#include "git-compat-util.h"
#include "config.h"
#include "userdiff.h"
-#include "attr.h"
+#include "attr.h"
#include "strbuf.h"
static struct userdiff_driver *drivers;
@@ -183,6 +183,19 @@ PATTERNS("java",
"|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lL]?"
"|[-+*/<>%&^|=!]="
"|--|\\+\\+|<<=?|>>>?=?|&&|\\|\\|"),
+PATTERNS("javascript",
+ /*
+ * Looks for lines that start with optional whitespace, followed
+ * by 'function'* and any characters (for function declarations),
+ * or valid JavaScript identifiers, equals sign '=', 'function' keyword
+ * and any characters (for function expressions).
+ * Also considers functions defined inside blocks with '{...}'.
+ */
+ "^[ \t]*(function[ \t]*.*|[a-zA-Z_$][0-9a-zA-Z_$]*[ \t]*=[ \t]*function[ \t]*.*|(\\{[ \t]*)?)\n",
+ /* This pattern matches JavaScript identifiers */
+ "[a-zA-Z_$][0-9a-zA-Z_$]*"
+ "|[-+0-9.eE]+|0[xX][0-9a-fA-F]+"
+ "|[-+*/<>%&^|=!:]=|--|\\+\\+|<<=?|>>=?|&&|\\|\\|"),
PATTERNS("kotlin",
"^[ \t]*(([a-z]+[ \t]+)*(fun|class|interface)[ \t]+.*)$",
/* -- */
@@ -192,7 +205,7 @@ PATTERNS("kotlin",
/* integers and floats */
"|[0-9][0-9_]*([.][0-9_]*)?([Ee][-+]?[0-9]+)?[fFlLuU]*"
/* floating point numbers beginning with decimal point */
- "|[.][0-9][0-9_]*([Ee][-+]?[0-9]+)?[fFlLuU]?"
+ "|[.][0-9][0-9_]*([Ee][-+]?[0-9]+ )?[fFlLuU]?"
/* unary and binary operators */
"|[-+*/<>%&^|=!]==?|--|\\+\\+|<<=|>>=|&&|\\|\\||->|\\.\\*|!!|[?:.][.:]"),
PATTERNS("markdown",
--
2.43.2
^ permalink raw reply related
* [PATCH 2/2] Subject:[GSOC] [RFC PATCH 2/2] Add test for JavaScript function detection in Git diffs
From: Sergius Nyah @ 2024-02-27 14:21 UTC (permalink / raw)
To: christian.couder, pk; +Cc: git, Sergius Justus Chesami Nyah
In-Reply-To: <20240227142121.72518-1-74214119+Sergius-Nyah@users.noreply.github.com>
From: Sergius Justus Chesami Nyah <74214119+Sergius-Nyah@users.noreply.github.com>
This commit introduces a new test case in t4018-diff-funcname.sh to verify the enhanced JavaScript function detection in Git diffs. The test creates a JavaScript file with function declarations and expressions, modifies them, and then checks the output of git diff to ensure that the changes are correctly identified. This test validates the changes made to userdiff.c for improved JavaScript function detection.
---
t/t4018-diff-funcname.sh | 25 +++++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index e026fac1f4..e88e63bd1f 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -11,7 +11,7 @@ test_expect_success 'setup' '
# a non-trivial custom pattern
git config diff.custom1.funcname "!static
!String
-[^ ].*s.*" &&
+[^ ].*s.*" &&
# a custom pattern which matches to end of line
git config diff.custom2.funcname "......Beer\$" &&
@@ -119,4 +119,25 @@ do
"
done
-test_done
+test_expect_success 'identify builtin patterns in Javascript' '
+ # setup
+ echo "function myFunction() { return true; }" > test.js &&
+ echo "var myVar = function() { return false; }" >> test.js &&
+ git add test.js &&
+ git commit -m "add test.js" &&
+
+ # modify the file
+ echo "function myFunction() { return false; }" > test.js &&
+ echo "var myVar = function() { return true; }" >> test.js &&
+
+ # command under test
+ git diff >output &&
+
+ # check results
+ test_i18ngrep "function myFunction() { return true; }" output &&
+ test_i18ngrep "function myFunction() { return false; }" output &&
+ test_i18ngrep "var myVar = function() { return false; }" output &&
+ test_i18ngrep "var myVar = function() { return true; }" output
+'
+
+test_done
\ No newline at end of file
--
2.43.2
^ permalink raw reply related
* Re: [BUG] 2.44.0 t7704.9 Fails on NonStop ia64
From: Patrick Steinhardt @ 2024-02-27 14:22 UTC (permalink / raw)
To: rsbecker; +Cc: phillip.wood, 'Torsten Bögershausen', git
In-Reply-To: <02c501da6986$cb7c5c30$62751490$@nexbridge.com>
[-- Attachment #1: Type: text/plain, Size: 1489 bytes --]
On Tue, Feb 27, 2024 at 09:10:55AM -0500, rsbecker@nexbridge.com wrote:
> On Tuesday, February 27, 2024 3:46 AM, Patrick Steinhardt wrote:
> >On Mon, Feb 26, 2024 at 03:32:14PM +0000, Phillip Wood wrote:
> >> Hi Randal
> >>
> >> [cc'ing Patrick for the reftable writer]
> >>
> >> > The question is which call is bad? The cruft stuff is relatively new
> >> > and I don't know the code.
> >> >
> >> > > > reftable/writer.c: int n = w->write(w->write_arg,
> zeroed,
> >> > > > w->pending_padding);
> >> > > > reftable/writer.c: n = w->write(w->write_arg, data, len);
> >>
> >> Neither of these appear to check for short writes and
> >> reftable_fd_write() is a thin wrapper around write(). Maybe
> >> reftable_fd_write() should be using write_in_full()?
> >
> >It already does starting with 85a8c899ce (reftable: handle interrupted
> writes, 2023-12-11):
> >
> >```
> >static ssize_t reftable_fd_write(void *arg, const void *data, size_t sz) {
> > int *fdp = (int *)arg;
> > return write_in_full(*fdp, data, sz);
> >}
>
> Unfortunately, this fix is included in what I am testing but does not impact
> the issue I am seeing one way or another, but thank you.
I didn't expect it to :) The mentioned commit only fixes things with the
reftable backend, which is not tested by default. I assume that you
didn't run tests with GIT_TEST_DEFAULT_REF_FORMAT=reftable, and thus
t7704 wouldn't use the reftable code in the first place.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [GSOC][PATCH 0/2] Add builtin patterns for userdiff in JavaScript, as Microproject.
From: Sergius Nyah @ 2024-02-27 14:25 UTC (permalink / raw)
To: christian.couder, pk; +Cc: git, Sergius Nyah
In-Reply-To: <CANAnif-OganZLi0Cu_uq=nveC+u5n14c=o_DQHT-wFOqQ9Vs0Q@mail.gmail.com>
From: Sergius Nyah <sergiusnyah@gmail.com>
Firstly, I'm so sorry for the delay between selecting the Microproject and sending the patch series.
Learning about regular expressions took me a bit longer than I expected, but it was all worth it.
I'm very grateful for the opportunity to work on this project and I'm looking forward to contributing more to Git.
This patch series adds builtin patterns for JavaScript function detection in userdiff, as
my Microproject for GSOC. The first patch adds a regular expression for detecting JavaScript
functions in Git diffs while the second adds a test for JavaScript function detection in Git diffs.
This new pattern looks for lines that start with optional whitespace, followed by 'function' and any
characters (for function declarations), or valid JavaScript identifiers, equals sign '=', 'function'
keyword and any characters (for function expressions). It also considers functions defined inside blocks with '{...}'.
t/t4018-diff-funcname.sh | 25 +++++++++++++++++++++++--
userdiff.c | 17 +++++++++++++++--
2 files changed, 38 insertions(+), 4 deletions(-)
base-commit: c5b454771e6b086f60c7f1f139025f174bcedac9
--
2.43.2
I would greatly appreciate any feedback on the patch series.
Best,
Sergius.
^ permalink raw reply
* [PATCH 1/2] Subject: [GSOC][RFC PATCH 1/2] Add builtin patterns for JavaScript function detection in userdiff.
From: Sergius Nyah @ 2024-02-27 14:25 UTC (permalink / raw)
To: christian.couder, pk; +Cc: git, Sergius Justus Chesami Nyah
In-Reply-To: <20240227142540.73972-1-74214119+Sergius-Nyah@users.noreply.github.com>
From: Sergius Justus Chesami Nyah <74214119+Sergius-Nyah@users.noreply.github.com>
This patch adds the regular expression for detecting JavaScript functions and expressions in Git diffs. The pattern accurately identifies function declerations, expressions, and definitions inside blocks.
---
userdiff.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/userdiff.c b/userdiff.c
index e399543823..12e31ff14d 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -1,7 +1,7 @@
#include "git-compat-util.h"
#include "config.h"
#include "userdiff.h"
-#include "attr.h"
+#include "attr.h"
#include "strbuf.h"
static struct userdiff_driver *drivers;
@@ -183,6 +183,19 @@ PATTERNS("java",
"|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lL]?"
"|[-+*/<>%&^|=!]="
"|--|\\+\\+|<<=?|>>>?=?|&&|\\|\\|"),
+PATTERNS("javascript",
+ /*
+ * Looks for lines that start with optional whitespace, followed
+ * by 'function'* and any characters (for function declarations),
+ * or valid JavaScript identifiers, equals sign '=', 'function' keyword
+ * and any characters (for function expressions).
+ * Also considers functions defined inside blocks with '{...}'.
+ */
+ "^[ \t]*(function[ \t]*.*|[a-zA-Z_$][0-9a-zA-Z_$]*[ \t]*=[ \t]*function[ \t]*.*|(\\{[ \t]*)?)\n",
+ /* This pattern matches JavaScript identifiers */
+ "[a-zA-Z_$][0-9a-zA-Z_$]*"
+ "|[-+0-9.eE]+|0[xX][0-9a-fA-F]+"
+ "|[-+*/<>%&^|=!:]=|--|\\+\\+|<<=?|>>=?|&&|\\|\\|"),
PATTERNS("kotlin",
"^[ \t]*(([a-z]+[ \t]+)*(fun|class|interface)[ \t]+.*)$",
/* -- */
@@ -192,7 +205,7 @@ PATTERNS("kotlin",
/* integers and floats */
"|[0-9][0-9_]*([.][0-9_]*)?([Ee][-+]?[0-9]+)?[fFlLuU]*"
/* floating point numbers beginning with decimal point */
- "|[.][0-9][0-9_]*([Ee][-+]?[0-9]+)?[fFlLuU]?"
+ "|[.][0-9][0-9_]*([Ee][-+]?[0-9]+ )?[fFlLuU]?"
/* unary and binary operators */
"|[-+*/<>%&^|=!]==?|--|\\+\\+|<<=|>>=|&&|\\|\\||->|\\.\\*|!!|[?:.][.:]"),
PATTERNS("markdown",
--
2.43.2
^ permalink raw reply related
* [PATCH 2/2] Subject:[GSOC] [RFC PATCH 2/2] Add test for JavaScript function detection in Git diffs
From: Sergius Nyah @ 2024-02-27 14:25 UTC (permalink / raw)
To: christian.couder, pk; +Cc: git, Sergius Justus Chesami Nyah
In-Reply-To: <20240227142540.73972-1-74214119+Sergius-Nyah@users.noreply.github.com>
From: Sergius Justus Chesami Nyah <74214119+Sergius-Nyah@users.noreply.github.com>
This commit introduces a new test case in t4018-diff-funcname.sh to verify the enhanced JavaScript function detection in Git diffs. The test creates a JavaScript file with function declarations and expressions, modifies them, and then checks the output of git diff to ensure that the changes are correctly identified. This test validates the changes made to userdiff.c for improved JavaScript function detection.
---
t/t4018-diff-funcname.sh | 25 +++++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index e026fac1f4..e88e63bd1f 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -11,7 +11,7 @@ test_expect_success 'setup' '
# a non-trivial custom pattern
git config diff.custom1.funcname "!static
!String
-[^ ].*s.*" &&
+[^ ].*s.*" &&
# a custom pattern which matches to end of line
git config diff.custom2.funcname "......Beer\$" &&
@@ -119,4 +119,25 @@ do
"
done
-test_done
+test_expect_success 'identify builtin patterns in Javascript' '
+ # setup
+ echo "function myFunction() { return true; }" > test.js &&
+ echo "var myVar = function() { return false; }" >> test.js &&
+ git add test.js &&
+ git commit -m "add test.js" &&
+
+ # modify the file
+ echo "function myFunction() { return false; }" > test.js &&
+ echo "var myVar = function() { return true; }" >> test.js &&
+
+ # command under test
+ git diff >output &&
+
+ # check results
+ test_i18ngrep "function myFunction() { return true; }" output &&
+ test_i18ngrep "function myFunction() { return false; }" output &&
+ test_i18ngrep "var myVar = function() { return false; }" output &&
+ test_i18ngrep "var myVar = function() { return true; }" output
+'
+
+test_done
\ No newline at end of file
--
2.43.2
^ permalink raw reply related
* [PATCH 0/2] builtin/clone: allow remote helpers to detect repo
From: Patrick Steinhardt @ 2024-02-27 14:27 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Mike Hommey
[-- Attachment #1: Type: text/plain, Size: 1212 bytes --]
Hi,
this patch series addresses a regression reported by Mike in Git v2.44
where remote helpers cannot access the Git repository anymore when
running git-clone(1).
The root cause of this is that we have started to initialize the refdb
at a later point, after the helper is spawned. This is required such
that we can initialize it with the correct object format, which fixes
clones of SHA256 repositories with the reftable format and when using
bundles.
The proposed fix here is to partially initialize the refdb with just
enough data such that it can be discovered. The fix isn't pretty, but
addresses the issue. I also couldn't come up with a better idea than
this.
Patrick
Patrick Steinhardt (2):
refs/reftable: don't fail empty transactions in repo without HEAD
builtin/clone: allow remote helpers to detect repo
builtin/clone.c | 46 ++++++++++++++++++++++++++++++++++++++
refs/reftable-backend.c | 1 +
setup.c | 9 +++++++-
t/t0610-reftable-basics.sh | 13 +++++++++++
t/t5801/git-remote-testgit | 5 +++++
5 files changed, 73 insertions(+), 1 deletion(-)
base-commit: a2082dbdd315aa4dd3f315545e5b3ab3b3e2d894
--
2.44.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH 1/2] refs/reftable: don't fail empty transactions in repo without HEAD
From: Patrick Steinhardt @ 2024-02-27 14:27 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Mike Hommey
In-Reply-To: <cover.1709041721.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 2409 bytes --]
Under normal circumstances, it shouldn't ever happen that a repository
has no HEAD reference. In fact, git-update-ref(1) would fail any request
to delete the HEAD reference, and a newly initialized repository always
pre-creates it, too.
But in the next commit, we are going to change git-clone(1) to partially
initialize the refdb just up to the point where remote helpers can find
the repository. With that change, we are going to run into a situation
where repositories have no refs at all.
Now there is a very particular edge case in this situation: when
preparing an empty ref transacton, we end up returning whatever value
`read_ref_without_reload()` returned to the caller. Under normal
conditions this would be fine: "HEAD" should usually exist, and thus the
function would return `0`. But if "HEAD" doesn't exist, the function
returns a positive value which we end up returning to the caller.
Fix this bug by resetting the return code to `0` and add a test.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
refs/reftable-backend.c | 1 +
t/t0610-reftable-basics.sh | 13 +++++++++++++
2 files changed, 14 insertions(+)
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index a14f2ad7f4..45568818f0 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -821,6 +821,7 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,
&head_referent, &head_type);
if (ret < 0)
goto done;
+ ret = 0;
for (i = 0; i < transaction->nr; i++) {
struct ref_update *u = transaction->updates[i];
diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh
index 6a131e40b8..c5f4d23433 100755
--- a/t/t0610-reftable-basics.sh
+++ b/t/t0610-reftable-basics.sh
@@ -328,6 +328,19 @@ test_expect_success 'ref transaction: writes are synced' '
EOF
'
+test_expect_success 'ref transaction: empty transaction in empty repo' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ test_commit -C repo --no-tag A &&
+ COMMIT=$(git -C repo rev-parse HEAD) &&
+ git -C repo update-ref -d refs/heads/main &&
+ test-tool -C repo ref-store main delete-refs REF_NO_DEREF msg HEAD &&
+ git -C repo update-ref --stdin <<-EOF
+ prepare
+ commit
+ EOF
+'
+
test_expect_success 'pack-refs: compacts tables' '
test_when_finished "rm -rf repo" &&
git init repo &&
--
2.44.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 2/2] builtin/clone: allow remote helpers to detect repo
From: Patrick Steinhardt @ 2024-02-27 14:27 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Mike Hommey
In-Reply-To: <cover.1709041721.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 6333 bytes --]
In 18c9cb7524 (builtin/clone: create the refdb with the correct object
format, 2023-12-12), we have changed git-clone(1) so that it delays
creation of the refdb until after it has learned about the remote's
object format. This change was required for the reftable backend, which
encodes the object format into the tables. So if we pre-initialized the
refdb with the default object format, but the remote uses a different
object format than that, then the resulting tables would have encoded
the wrong object format.
This change unfortunately breaks remote helpers which try to access the
repository that is about to be created. Because the refdb has not yet
been initialized at the point where we spawn the remote helper, we also
don't yet have "HEAD" or "refs/". Consequently, any Git commands ran by
the remote helper which try to access the repository would fail because
it cannot be discovered.
This is essentially a chicken-and-egg problem: we cannot initialize the
refdb because we don't know about the object format. But we cannot learn
about the object format because the remote helper may be unable to
access the partially-initialized repository.
Ideally, we would address this issue via capabilities. But the remote
helper protocol is not structured in a way that guarantees that the
capability announcement happens before the remote helper tries to access
the repository.
Instead, fix this issue by partially initializing the refdb up to the
point where it becomes discoverable by Git commands.
Reported-by: Mike Hommey <mh@glandium.org>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/clone.c | 46 ++++++++++++++++++++++++++++++++++++++
setup.c | 9 +++++++-
t/t5801/git-remote-testgit | 5 +++++
3 files changed, 59 insertions(+), 1 deletion(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index bad1b70ce8..5d7f112125 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -926,6 +926,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
struct ref *mapped_refs = NULL;
const struct ref *ref;
struct strbuf key = STRBUF_INIT;
+ struct strbuf buf = STRBUF_INIT;
struct strbuf branch_top = STRBUF_INIT, reflog_msg = STRBUF_INIT;
struct transport *transport = NULL;
const char *src_ref_prefix = "refs/heads/";
@@ -1125,6 +1126,50 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
git_dir = real_git_dir;
}
+ /*
+ * We have a chicken-and-egg situation between initializing the refdb
+ * and spawning transport helpers:
+ *
+ * - Initializing the refdb requires us to know about the object
+ * format. We thus have to spawn the transport helper to learn
+ * about it.
+ *
+ * - The transport helper may want to access the Git repository. But
+ * because the refdb has not been initialized, we don't have "HEAD"
+ * or "refs/". Thus, the helper cannot find the Git repository.
+ *
+ * Ideally, we would have structured the helper protocol such that it's
+ * mandatory for the helper to first announce its capabilities without
+ * yet assuming a fully initialized repository. Like that, we could
+ * have added a "lazy-refdb-init" capability that announces whether the
+ * helper is ready to handle not-yet-initialized refdbs. If any helper
+ * didn't support them, we would have fully initialized the refdb with
+ * the SHA1 object format, but later on bailed out if we found out that
+ * the remote repository used a different object format.
+ *
+ * But we didn't, and thus we use the following workaround to partially
+ * initialize the repository's refdb such that it can be discovered by
+ * Git commands. To do so, we:
+ *
+ * - Create an invalid HEAD ref pointing at "refs/heads/.invalid".
+ *
+ * - Create the "refs/" directory.
+ *
+ * - Set up the ref storage format and repository version as
+ * required.
+ *
+ * This is sufficient for Git commands to discover the Git directory.
+ */
+ initialize_repository_version(GIT_HASH_UNKNOWN,
+ the_repository->ref_storage_format, 1);
+
+ strbuf_addf(&buf, "%s/HEAD", git_dir);
+ write_file(buf.buf, "ref: refs/heads/.invalid");
+
+ strbuf_reset(&buf);
+ strbuf_addf(&buf, "%s/refs", git_dir);
+ safe_create_dir(buf.buf, 1);
+
/*
* additional config can be injected with -c, make sure it's included
* after init_db, which clears the entire config environment.
@@ -1453,6 +1498,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
free(remote_name);
strbuf_release(&reflog_msg);
strbuf_release(&branch_top);
+ strbuf_release(&buf);
strbuf_release(&key);
free_refs(mapped_refs);
free_refs(remote_head_points_at);
diff --git a/setup.c b/setup.c
index b69b1cbc2a..e3b76e84b5 100644
--- a/setup.c
+++ b/setup.c
@@ -1889,6 +1889,13 @@ void initialize_repository_version(int hash_algo,
char repo_version_string[10];
int repo_version = GIT_REPO_VERSION;
+ /*
+ * Note that we initialize the repository version to 1 when the ref
+ * storage format is unknown. This is on purpose so that we can add the
+ * correct object format to the config during git-clone(1). The format
+ * version will get adjusted by git-clone(1) once it has learned about
+ * the remote repository's format.
+ */
if (hash_algo != GIT_HASH_SHA1 ||
ref_storage_format != REF_STORAGE_FORMAT_FILES)
repo_version = GIT_REPO_VERSION_READ;
@@ -1898,7 +1905,7 @@ void initialize_repository_version(int hash_algo,
"%d", repo_version);
git_config_set("core.repositoryformatversion", repo_version_string);
- if (hash_algo != GIT_HASH_SHA1)
+ if (hash_algo != GIT_HASH_SHA1 && hash_algo != GIT_HASH_UNKNOWN)
git_config_set("extensions.objectformat",
hash_algos[hash_algo].name);
else if (reinit)
diff --git a/t/t5801/git-remote-testgit b/t/t5801/git-remote-testgit
index 1544d6dc6b..bcfb358c51 100755
--- a/t/t5801/git-remote-testgit
+++ b/t/t5801/git-remote-testgit
@@ -12,6 +12,11 @@ url=$2
dir="$GIT_DIR/testgit/$alias"
+if ! git rev-parse --is-inside-git-dir
+then
+ exit 1
+fi
+
h_refspec="refs/heads/*:refs/testgit/$alias/heads/*"
t_refspec="refs/tags/*:refs/testgit/$alias/tags/*"
--
2.44.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ 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