* [PATCH] commit-reach: avoid is_dependant_of() shim
@ 2020-06-23 8:17 Carlo Marcelo Arenas Belón
2020-06-23 12:59 ` Derrick Stolee
2020-06-23 18:42 ` [PATCH v2] commit-reach: avoid is_descendant_of() shim Carlo Marcelo Arenas Belón
0 siblings, 2 replies; 4+ messages in thread
From: Carlo Marcelo Arenas Belón @ 2020-06-23 8:17 UTC (permalink / raw)
To: git; +Cc: dstolee, Carlo Marcelo Arenas Belón
d91d6fbf26 (commit-reach: create repo_is_descendant_of(), 2020-06-17)
adds a repository aware version of is_dependant_of() and a backward
compatibility shim that is barelly used.
Update all callers to directly use the new repo_is_dependant_of()
function instead.
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
builtin/pull.c | 3 ++-
commit-reach.c | 16 ++++++----------
commit-reach.h | 4 +++-
t/helper/test-reach.c | 2 +-
4 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/builtin/pull.c b/builtin/pull.c
index 8e6572d305..babc6a4e36 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -1025,7 +1025,8 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
commit_list_insert(head, &list);
merge_head = lookup_commit_reference(the_repository,
&merge_heads.oid[0]);
- if (is_descendant_of(merge_head, list)) {
+ if (repo_is_descendant_of(the_repository,
+ merge_head, list)) {
/* we can fast-forward this without invoking rebase */
opt_ff = "--ff-only";
ran_ff = 1;
diff --git a/commit-reach.c b/commit-reach.c
index 1761217663..82c73171dd 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -283,9 +283,9 @@ struct commit_list *repo_get_merge_bases(struct repository *r,
/*
* Is "commit" a descendant of one of the elements on the "with_commit" list?
*/
-static int repo_is_descendant_of(struct repository *r,
- struct commit *commit,
- struct commit_list *with_commit)
+int repo_is_descendant_of(struct repository *r,
+ struct commit *commit,
+ struct commit_list *with_commit)
{
if (!with_commit)
return 1;
@@ -310,11 +310,6 @@ static int repo_is_descendant_of(struct repository *r,
}
}
-int is_descendant_of(struct commit *commit, struct commit_list *with_commit)
-{
- return repo_is_descendant_of(the_repository, commit, with_commit);
-}
-
/*
* Is "commit" an ancestor of one of the "references"?
*/
@@ -433,7 +428,8 @@ int ref_newer(const struct object_id *new_oid, const struct object_id *old_oid)
return 0;
commit_list_insert(old_commit, &old_commit_list);
- ret = is_descendant_of(new_commit, old_commit_list);
+ ret = repo_is_descendant_of(the_repository,
+ new_commit, old_commit_list);
free_commit_list(old_commit_list);
return ret;
}
@@ -554,7 +550,7 @@ int commit_contains(struct ref_filter *filter, struct commit *commit,
{
if (filter->with_commit_tag_algo)
return contains_tag_algo(commit, list, cache) == CONTAINS_YES;
- return is_descendant_of(commit, list);
+ return repo_is_descendant_of(the_repository, commit, list);
}
static int compare_commits_by_gen(const void *_a, const void *_b)
diff --git a/commit-reach.h b/commit-reach.h
index 99a43e8b64..b49ad71a31 100644
--- a/commit-reach.h
+++ b/commit-reach.h
@@ -27,7 +27,9 @@ struct commit_list *repo_get_merge_bases_many_dirty(struct repository *r,
struct commit_list *get_octopus_merge_bases(struct commit_list *in);
-int is_descendant_of(struct commit *commit, struct commit_list *with_commit);
+int repo_is_descendant_of(struct repository *r,
+ struct commit *commit,
+ struct commit_list *with_commit);
int repo_in_merge_bases(struct repository *r,
struct commit *commit,
struct commit *reference);
diff --git a/t/helper/test-reach.c b/t/helper/test-reach.c
index a0272178b7..1d640f4757 100644
--- a/t/helper/test-reach.c
+++ b/t/helper/test-reach.c
@@ -108,7 +108,7 @@ int cmd__reach(int ac, const char **av)
else if (!strcmp(av[1], "in_merge_bases"))
printf("%s(A,B):%d\n", av[1], in_merge_bases(A, B));
else if (!strcmp(av[1], "is_descendant_of"))
- printf("%s(A,X):%d\n", av[1], is_descendant_of(A, X));
+ 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 = get_merge_bases_many(A, X_nr, X_array);
printf("%s(A,X):\n", av[1]);
base-commit: 4b34aa94c75220f6f4cd334b08a3fb053128a32d
--
2.27.0.288.g4b34aa94c7
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] commit-reach: avoid is_dependant_of() shim
2020-06-23 8:17 [PATCH] commit-reach: avoid is_dependant_of() shim Carlo Marcelo Arenas Belón
@ 2020-06-23 12:59 ` Derrick Stolee
2020-06-23 18:42 ` [PATCH v2] commit-reach: avoid is_descendant_of() shim Carlo Marcelo Arenas Belón
1 sibling, 0 replies; 4+ messages in thread
From: Derrick Stolee @ 2020-06-23 12:59 UTC (permalink / raw)
To: Carlo Marcelo Arenas Belón, git; +Cc: dstolee
On 6/23/2020 4:17 AM, Carlo Marcelo Arenas Belón wrote:
> d91d6fbf26 (commit-reach: create repo_is_descendant_of(), 2020-06-17)
> adds a repository aware version of is_dependant_of() and a backward
> compatibility shim that is barelly used.>
> Update all callers to directly use the new repo_is_dependant_of()
> function instead.
In your commit message:
s/dependant/descendant/
s/barelly/barely/
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
> builtin/pull.c | 3 ++-
> commit-reach.c | 16 ++++++----------
> commit-reach.h | 4 +++-
> t/helper/test-reach.c | 2 +-
> 4 files changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/builtin/pull.c b/builtin/pull.c
> index 8e6572d305..babc6a4e36 100644
> --- a/builtin/pull.c
> +++ b/builtin/pull.c
> @@ -1025,7 +1025,8 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
> commit_list_insert(head, &list);
> merge_head = lookup_commit_reference(the_repository,
> &merge_heads.oid[0]);
> - if (is_descendant_of(merge_head, list)) {
> + if (repo_is_descendant_of(the_repository,
> + merge_head, list)) {
nit: left-align "merge_head" with "the_repository". Should look like this:
+ if (repo_is_descendant_of(the_repository,
+ merge_head, list)) {
> /* we can fast-forward this without invoking rebase */
> opt_ff = "--ff-only";
> ran_ff = 1;
> diff --git a/commit-reach.c b/commit-reach.c
> index 1761217663..82c73171dd 100644
> --- a/commit-reach.c
> +++ b/commit-reach.c
> @@ -283,9 +283,9 @@ struct commit_list *repo_get_merge_bases(struct repository *r,
> /*
> * Is "commit" a descendant of one of the elements on the "with_commit" list?
> */
> -static int repo_is_descendant_of(struct repository *r,
> - struct commit *commit,
> - struct commit_list *with_commit)
> +int repo_is_descendant_of(struct repository *r,
> + struct commit *commit,
> + struct commit_list *with_commit)
> {
> if (!with_commit)
> return 1;
> @@ -310,11 +310,6 @@ static int repo_is_descendant_of(struct repository *r,
> }
> }
>
> -int is_descendant_of(struct commit *commit, struct commit_list *with_commit)
> -{
> - return repo_is_descendant_of(the_repository, commit, with_commit);
> -}
> -
> /*
> * Is "commit" an ancestor of one of the "references"?
> */
> @@ -433,7 +428,8 @@ int ref_newer(const struct object_id *new_oid, const struct object_id *old_oid)
> return 0;
>
> commit_list_insert(old_commit, &old_commit_list);
> - ret = is_descendant_of(new_commit, old_commit_list);
> + ret = repo_is_descendant_of(the_repository,
> + new_commit, old_commit_list);
nit: align the whitespace so "new_commit" is left-aligned with the start
of "the_repository" in the line above. It should look like this:
+ ret = repo_is_descendant_of(the_repository,
+ new_commit, old_commit_list);
> free_commit_list(old_commit_list);
> return ret;
> }
> @@ -554,7 +550,7 @@ int commit_contains(struct ref_filter *filter, struct commit *commit,
> {
> if (filter->with_commit_tag_algo)
> return contains_tag_algo(commit, list, cache) == CONTAINS_YES;
> - return is_descendant_of(commit, list);
> + return repo_is_descendant_of(the_repository, commit, list);
> }
>
> static int compare_commits_by_gen(const void *_a, const void *_b)
> diff --git a/commit-reach.h b/commit-reach.h
> index 99a43e8b64..b49ad71a31 100644
> --- a/commit-reach.h
> +++ b/commit-reach.h
> @@ -27,7 +27,9 @@ struct commit_list *repo_get_merge_bases_many_dirty(struct repository *r,
>
> struct commit_list *get_octopus_merge_bases(struct commit_list *in);
>
> -int is_descendant_of(struct commit *commit, struct commit_list *with_commit);
> +int repo_is_descendant_of(struct repository *r,
> + struct commit *commit,
> + struct commit_list *with_commit);
> int repo_in_merge_bases(struct repository *r,
> struct commit *commit,
> struct commit *reference);
> diff --git a/t/helper/test-reach.c b/t/helper/test-reach.c
> index a0272178b7..1d640f4757 100644
> --- a/t/helper/test-reach.c
> +++ b/t/helper/test-reach.c
> @@ -108,7 +108,7 @@ int cmd__reach(int ac, const char **av)
> else if (!strcmp(av[1], "in_merge_bases"))
> printf("%s(A,B):%d\n", av[1], in_merge_bases(A, B));
> else if (!strcmp(av[1], "is_descendant_of"))
> - printf("%s(A,X):%d\n", av[1], is_descendant_of(A, X));
> + printf("%s(A,X):%d\n", av[1], repo_is_descendant_of(r, A, X));
This is the most subtle of the changes, but it is done correctly.
This is a worthwhile patch. Some of the callers _look_ more complicated,
but overall this simplifies the codebase. Further: it pushes more
the_repository references higher up the stack.
Thanks,
-Stolee
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2] commit-reach: avoid is_descendant_of() shim
2020-06-23 8:17 [PATCH] commit-reach: avoid is_dependant_of() shim Carlo Marcelo Arenas Belón
2020-06-23 12:59 ` Derrick Stolee
@ 2020-06-23 18:42 ` Carlo Marcelo Arenas Belón
2020-06-23 18:53 ` Derrick Stolee
1 sibling, 1 reply; 4+ messages in thread
From: Carlo Marcelo Arenas Belón @ 2020-06-23 18:42 UTC (permalink / raw)
To: git; +Cc: Carlo Marcelo Arenas Belón, Derrick Stolee
d91d6fbf26 (commit-reach: create repo_is_descendant_of(), 2020-06-17)
adds a repository aware version of is_descendant_of() and a backward
compatibility shim that is barely used.
Update all callers to directly use the new repo_is_descendant_of()
function instead; making the codebase simpler and pushing more
the_repository references higher up the stack.
Helped-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
builtin/pull.c | 3 ++-
commit-reach.c | 16 ++++++----------
commit-reach.h | 4 +++-
t/helper/test-reach.c | 2 +-
4 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/builtin/pull.c b/builtin/pull.c
index 8e6572d305..8159c5d7c9 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -1025,7 +1025,8 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
commit_list_insert(head, &list);
merge_head = lookup_commit_reference(the_repository,
&merge_heads.oid[0]);
- if (is_descendant_of(merge_head, list)) {
+ if (repo_is_descendant_of(the_repository,
+ merge_head, list)) {
/* we can fast-forward this without invoking rebase */
opt_ff = "--ff-only";
ran_ff = 1;
diff --git a/commit-reach.c b/commit-reach.c
index 1761217663..f846d30f22 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -283,9 +283,9 @@ struct commit_list *repo_get_merge_bases(struct repository *r,
/*
* Is "commit" a descendant of one of the elements on the "with_commit" list?
*/
-static int repo_is_descendant_of(struct repository *r,
- struct commit *commit,
- struct commit_list *with_commit)
+int repo_is_descendant_of(struct repository *r,
+ struct commit *commit,
+ struct commit_list *with_commit)
{
if (!with_commit)
return 1;
@@ -310,11 +310,6 @@ static int repo_is_descendant_of(struct repository *r,
}
}
-int is_descendant_of(struct commit *commit, struct commit_list *with_commit)
-{
- return repo_is_descendant_of(the_repository, commit, with_commit);
-}
-
/*
* Is "commit" an ancestor of one of the "references"?
*/
@@ -433,7 +428,8 @@ int ref_newer(const struct object_id *new_oid, const struct object_id *old_oid)
return 0;
commit_list_insert(old_commit, &old_commit_list);
- ret = is_descendant_of(new_commit, old_commit_list);
+ ret = repo_is_descendant_of(the_repository,
+ new_commit, old_commit_list);
free_commit_list(old_commit_list);
return ret;
}
@@ -554,7 +550,7 @@ int commit_contains(struct ref_filter *filter, struct commit *commit,
{
if (filter->with_commit_tag_algo)
return contains_tag_algo(commit, list, cache) == CONTAINS_YES;
- return is_descendant_of(commit, list);
+ return repo_is_descendant_of(the_repository, commit, list);
}
static int compare_commits_by_gen(const void *_a, const void *_b)
diff --git a/commit-reach.h b/commit-reach.h
index 99a43e8b64..b49ad71a31 100644
--- a/commit-reach.h
+++ b/commit-reach.h
@@ -27,7 +27,9 @@ struct commit_list *repo_get_merge_bases_many_dirty(struct repository *r,
struct commit_list *get_octopus_merge_bases(struct commit_list *in);
-int is_descendant_of(struct commit *commit, struct commit_list *with_commit);
+int repo_is_descendant_of(struct repository *r,
+ struct commit *commit,
+ struct commit_list *with_commit);
int repo_in_merge_bases(struct repository *r,
struct commit *commit,
struct commit *reference);
diff --git a/t/helper/test-reach.c b/t/helper/test-reach.c
index a0272178b7..1d640f4757 100644
--- a/t/helper/test-reach.c
+++ b/t/helper/test-reach.c
@@ -108,7 +108,7 @@ int cmd__reach(int ac, const char **av)
else if (!strcmp(av[1], "in_merge_bases"))
printf("%s(A,B):%d\n", av[1], in_merge_bases(A, B));
else if (!strcmp(av[1], "is_descendant_of"))
- printf("%s(A,X):%d\n", av[1], is_descendant_of(A, X));
+ 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 = get_merge_bases_many(A, X_nr, X_array);
printf("%s(A,X):\n", av[1]);
base-commit: 4b34aa94c75220f6f4cd334b08a3fb053128a32d
--
2.27.0.288.g4b34aa94c7
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] commit-reach: avoid is_descendant_of() shim
2020-06-23 18:42 ` [PATCH v2] commit-reach: avoid is_descendant_of() shim Carlo Marcelo Arenas Belón
@ 2020-06-23 18:53 ` Derrick Stolee
0 siblings, 0 replies; 4+ messages in thread
From: Derrick Stolee @ 2020-06-23 18:53 UTC (permalink / raw)
To: Carlo Marcelo Arenas Belón, git; +Cc: Derrick Stolee
On 6/23/2020 2:42 PM, Carlo Marcelo Arenas Belón wrote:
> d91d6fbf26 (commit-reach: create repo_is_descendant_of(), 2020-06-17)
> adds a repository aware version of is_descendant_of() and a backward
> compatibility shim that is barely used.
>
> Update all callers to directly use the new repo_is_descendant_of()
> function instead; making the codebase simpler and pushing more
> the_repository references higher up the stack.
Thanks for the update! This version looks good to me.
-Stolee
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-06-23 18:54 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-23 8:17 [PATCH] commit-reach: avoid is_dependant_of() shim Carlo Marcelo Arenas Belón
2020-06-23 12:59 ` Derrick Stolee
2020-06-23 18:42 ` [PATCH v2] commit-reach: avoid is_descendant_of() shim Carlo Marcelo Arenas Belón
2020-06-23 18:53 ` Derrick Stolee
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).