| Message ID | 8f7fd8f5941426c3ed7fc28c6e4afd62bcd4bb8d.1592414670.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Accelerate "git merge-base --is-ancestor" | expand |
On Wed, Jun 17, 2020 at 05:24:28PM +0000, Derrick Stolee via GitGitGadget wrote: > From: Derrick Stolee <dstolee@microsoft.com> > > The next change will make repo_in_merge_bases() depend on the logic in > is_descendant_of(), but we need to make the method independent of > the_repository first. > > Signed-off-by: Derrick Stolee <dstolee@microsoft.com> > --- > commit-reach.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/commit-reach.c b/commit-reach.c > index 4ca7e706a18..13722430aa5 100644 > --- a/commit-reach.c > +++ b/commit-reach.c > @@ -283,7 +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? > */ > -int is_descendant_of(struct commit *commit, struct commit_list *with_commit) > +static int repo_is_descendant_of(struct repository *r, > + struct commit *commit, > + struct commit_list *with_commit) > { > if (!with_commit) > return 1; > @@ -301,13 +303,18 @@ int is_descendant_of(struct commit *commit, struct commit_list *with_commit) > > other = with_commit->item; > with_commit = with_commit->next; > - if (in_merge_bases(other, commit)) > + if (repo_in_merge_bases(r, other, commit)) > return 1; > } > return 0; > } > } > > +int is_descendant_of(struct commit *commit, struct commit_list *with_commit) > +{ > + return repo_is_descendant_of(the_repository, commit, with_commit); > +} > + I don't think that it makes a big deal either way, but I wonder about moving 'repo_is_descendant_of' to the header file, and making 'is_descendant_of' be 'static inline int' as you defined it here. Since this has already graduated up to master already, I don't think that it's worth going back just to shuffle this code around, but I was wondering if you had any specific reason for doing it this way. > /* > * Is "commit" an ancestor of one of the "references"? > */ > -- > gitgitgadget > Thanks, Taylor
On 6/29/2020 9:40 AM, Taylor Blau wrote: > On Wed, Jun 17, 2020 at 05:24:28PM +0000, Derrick Stolee via GitGitGadget wrote: >> +int is_descendant_of(struct commit *commit, struct commit_list *with_commit) >> +{ >> + return repo_is_descendant_of(the_repository, commit, with_commit); >> +} >> + > > I don't think that it makes a big deal either way, but I wonder about > moving 'repo_is_descendant_of' to the header file, and making > 'is_descendant_of' be 'static inline int' as you defined it here. > > Since this has already graduated up to master already, I don't think > that it's worth going back just to shuffle this code around, but I was > wondering if you had any specific reason for doing it this way. I have good news for you. [1] [1] https://lore.kernel.org/git/20200623184222.54201-1-carenas@gmail.com/ Thanks, -Stolee
diff --git a/commit-reach.c b/commit-reach.c index 4ca7e706a18..13722430aa5 100644 --- a/commit-reach.c +++ b/commit-reach.c @@ -283,7 +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? */ -int is_descendant_of(struct commit *commit, struct commit_list *with_commit) +static int repo_is_descendant_of(struct repository *r, + struct commit *commit, + struct commit_list *with_commit) { if (!with_commit) return 1; @@ -301,13 +303,18 @@ int is_descendant_of(struct commit *commit, struct commit_list *with_commit) other = with_commit->item; with_commit = with_commit->next; - if (in_merge_bases(other, commit)) + if (repo_in_merge_bases(r, other, commit)) return 1; } return 0; } } +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"? */