git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v3 06/11] merge_bases_many(): pass on errors from `paint_down_to_common()`

From
Junio C Hamano <gitster@pobox.com>
Date
Feb 27, 2024, 18:29 UTC
Message-ID
<xmqqr0gxsqvm.fsf@gitster.g>
In-Reply-To
<2ae6a54dd596c3192b5be32f0e4e2605f4aac6bd.1709040499.git.gitgitgadget@gmail.com>

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 20 quoted lines
> 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(-)

This is the true gem of the series. The next steps may have to be noisy due to many callers adjusting to the new function signatures, but split into these logical steps in a bottom-up way makes them easier to follow. Nice.

Show 80 quoted lines
>
> 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;
Previous: Johannes Schindelin via GitGitGadgetNext: Johannes Schindelin via GitGitGadget
Message 47 of 70 in “The merge-base logic vs missing commit objects”
  1. 00/12 The merge-base logic vs missing commit objectsJohannes Schindelin via GitGitGadget, Feb 13, 2024
  2. 01/12 paint_down_to_common: plug a memory leakJohannes Schindelin via GitGitGadget, Feb 13, 2024
  3. 02/12 Let `repo_in_merge_bases()` report missing commitsJohannes Schindelin via GitGitGadget, Feb 13, 2024
  4. Patrick SteinhardtFeb 15, 2024
  5. 03/12 Prepare `repo_in_merge_bases_many()` to optionally expect missing commitsJohannes Schindelin via GitGitGadget, Feb 13, 2024
  6. 04/12 Prepare `paint_down_to_common()` for handling shallow commitsJohannes Schindelin via GitGitGadget, Feb 13, 2024
  7. 05/12 commit-reach: start reporting errors in `paint_down_to_common()`Johannes Schindelin via GitGitGadget, Feb 13, 2024
  8. 06/12 merge_bases_many(): pass on errors from `paint_down_to_common()`Johannes Schindelin via GitGitGadget, Feb 13, 2024
  9. 07/12 get_merge_bases_many_0(): pass on errors from `merge_bases_many()`Johannes Schindelin via GitGitGadget, Feb 13, 2024
  10. 08/12 repo_get_merge_bases(): pass on errors from `merge_bases_many()`Johannes Schindelin via GitGitGadget, Feb 13, 2024
  11. 09/12 get_octopus_merge_bases(): pass on errors from `merge_bases_many()`Johannes Schindelin via GitGitGadget, Feb 13, 2024
  12. 10/12 repo_get_merge_bases_many(): pass on errors from `merge_bases_many()`Johannes Schindelin via GitGitGadget, Feb 13, 2024
  13. 11/12 repo_get_merge_bases_many_dirty(): pass on errors from `merge_bases_many()`Johannes Schindelin via GitGitGadget, Feb 13, 2024
  14. 12/12 paint_down_to_common(): special-case shallow/partial clonesJohannes Schindelin via GitGitGadget, Feb 13, 2024
  15. Junio C HamanoFeb 13, 2024
  16. 00/11 The merge-base logic vs missing commit objectsJohannes Schindelin via GitGitGadget, Feb 22, 2024
  17. 01/11 paint_down_to_common: plug two memory leaksJohannes Schindelin via GitGitGadget, Feb 22, 2024
  18. 02/11 Prepare `repo_in_merge_bases_many()` to optionally expect missing commitsJohannes Schindelin via GitGitGadget, Feb 22, 2024
  19. 03/11 Start reporting missing commits in `repo_in_merge_bases_many()`Johannes Schindelin via GitGitGadget, Feb 22, 2024
  20. Junio C HamanoFeb 24, 2024
  21. Johannes SchindelinFeb 26, 2024
  22. Junio C HamanoFeb 26, 2024
  23. Jeff KingMar 1, 2024
  24. 04/11 Prepare `paint_down_to_common()` for handling shallow commitsJohannes Schindelin via GitGitGadget, Feb 22, 2024
  25. 05/11 commit-reach: start reporting errors in `paint_down_to_common()`Johannes Schindelin via GitGitGadget, Feb 22, 2024
  26. 06/11 merge_bases_many(): pass on errors from `paint_down_to_common()`Johannes Schindelin via GitGitGadget, Feb 22, 2024
  27. 07/11 get_merge_bases_many_0(): pass on errors from `merge_bases_many()`Johannes Schindelin via GitGitGadget, Feb 22, 2024
  28. 08/11 repo_get_merge_bases(): pass on errors from `merge_bases_many()`Johannes Schindelin via GitGitGadget, Feb 22, 2024
  29. 10/11 repo_get_merge_bases_many(): pass on errors from `merge_bases_many()`Johannes Schindelin via GitGitGadget, Feb 22, 2024
  30. 09/11 get_octopus_merge_bases(): pass on errors from `merge_bases_many()`Johannes Schindelin via GitGitGadget, Feb 22, 2024
  31. 11/11 repo_get_merge_bases_many_dirty(): pass on errors from `merge_bases_many()`Johannes Schindelin via GitGitGadget, Feb 22, 2024
  32. 00/11 The merge-base logic vs missing commit objectsJohannes Schindelin via GitGitGadget, Feb 27, 2024
  33. 01/11 paint_down_to_common: plug two memory leaksJohannes Schindelin via GitGitGadget, Feb 27, 2024
  34. 02/11 Prepare `repo_in_merge_bases_many()` to optionally expect missing commitsJohannes Schindelin via GitGitGadget, Feb 27, 2024
  35. 04/11 Prepare `paint_down_to_common()` for handling shallow commitsJohannes Schindelin via GitGitGadget, Feb 27, 2024
  36. Dirk GoudersFeb 27, 2024
  37. Johannes SchindelinFeb 27, 2024
  38. Junio C HamanoFeb 27, 2024
  39. Junio C HamanoFeb 27, 2024
  40. Dirk GoudersFeb 27, 2024
  41. 03/11 Start reporting missing commits in `repo_in_merge_bases_many()`Johannes Schindelin via GitGitGadget, Feb 27, 2024
  42. 05/11 commit-reach: start reporting errors in `paint_down_to_common()`Johannes Schindelin via GitGitGadget, Feb 27, 2024
  43. Dirk GoudersFeb 27, 2024
  44. Johannes SchindelinFeb 27, 2024
  45. Junio C HamanoFeb 27, 2024
  46. 06/11 merge_bases_many(): pass on errors from `paint_down_to_common()`Johannes Schindelin via GitGitGadget, Feb 27, 2024
  47. Junio C HamanoFeb 27, 2024
  48. 07/11 get_merge_bases_many_0(): pass on errors from `merge_bases_many()`Johannes Schindelin via GitGitGadget, Feb 27, 2024
  49. 08/11 repo_get_merge_bases(): pass on errors from `merge_bases_many()`Johannes Schindelin via GitGitGadget, Feb 27, 2024
  50. 09/11 get_octopus_merge_bases(): pass on errors from `merge_bases_many()`Johannes Schindelin via GitGitGadget, Feb 27, 2024
  51. 10/11 repo_get_merge_bases_many(): pass on errors from `merge_bases_many()`Johannes Schindelin via GitGitGadget, Feb 27, 2024
  52. 11/11 repo_get_merge_bases_many_dirty(): pass on errors from `merge_bases_many()`Johannes Schindelin via GitGitGadget, Feb 27, 2024
  53. 00/11 The merge-base logic vs missing commit objectsJohannes Schindelin via GitGitGadget, Feb 28, 2024
  54. 01/11 commit-reach(paint_down_to_common): plug two memory leaksJohannes Schindelin via GitGitGadget, Feb 28, 2024
  55. 02/11 commit-reach(repo_in_merge_bases_many): optionally expect missing commitsJohannes Schindelin via GitGitGadget, Feb 28, 2024
  56. 03/11 commit-reach(repo_in_merge_bases_many): report missing commitsJohannes Schindelin via GitGitGadget, Feb 28, 2024
  57. Jeff KingMar 1, 2024
  58. Junio C HamanoMar 1, 2024
  59. 04/11 commit-reach(paint_down_to_common): prepare for handling shallow commitsJohannes Schindelin via GitGitGadget, Feb 28, 2024
  60. Junio C HamanoFeb 28, 2024
  61. Dirk GoudersFeb 28, 2024
  62. Johannes SchindelinFeb 29, 2024
  63. Johannes SchindelinFeb 29, 2024
  64. 05/11 commit-reach(paint_down_to_common): start reporting errorsJohannes Schindelin via GitGitGadget, Feb 28, 2024
  65. 06/11 commit-reach(merge_bases_many): pass on "missing commits" errorsJohannes Schindelin via GitGitGadget, Feb 28, 2024
  66. 07/11 commit-reach(get_merge_bases_many_0): pass on "missing commits" errorsJohannes Schindelin via GitGitGadget, Feb 28, 2024
  67. 08/11 commit-reach(repo_get_merge_bases): pass on "missing commits" errorsJohannes Schindelin via GitGitGadget, Feb 28, 2024
  68. 09/11 commit-reach(get_octopus_merge_bases): pass on "missing commits" errorsJohannes Schindelin via GitGitGadget, Feb 28, 2024
  69. 10/11 commit-reach(repo_get_merge_bases_many): pass on "missing commits" errorsJohannes Schindelin via GitGitGadget, Feb 28, 2024
  70. 11/11 commit-reach(repo_get_merge_bases_many_dirty): pass on errorsJohannes Schindelin via GitGitGadget, Feb 28, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.