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

Re: [PATCH 06/29] bisect: libify `exit_if_skipped_commits` to `error_if_skipped*` and its dependents

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Jan 20, 2020, 21:57 UTC
Message-ID
<nycvar.QRO.7.76.6.2001202253450.46@tvgsbejvaqbjf.bet>
In-Reply-To
<20200120143800.900-7-mirucam@gmail.com>
Hi Miriam,
On Mon, 20 Jan 2020, Miriam Rubio wrote:
Show 9 quoted lines
> From: Pranit Bauva <pranit.bauva@gmail.com>
>
> Since we want to get rid of git-bisect.sh it would be necessary to
> convert those exit() calls to return statements so that errors can be
> reported.
>
> Emulate try catch in C by converting `exit(<positive-value>)` to
> `return <negative-value>`. Follow POSIX conventions to return
> <negative-value> to indicate error.
Good.
> Modify `cmd_bisect_helper()` to handle these negative returns.
>
> Turn `exit()` to `return` calls in `exit_if_skipped_commits()` and rename
> the method to `error_if_skipped_commits()`.

I would remove these two sentences, as I deem it obvious from the rationale above that this needs to be done, and the patch repeats it.

Show 42 quoted lines
>
> Handle this return in dependant function `bisect_next_all()`.
>
> Mentored-by: Christian Couder <chriscool@tuxfamily.org>
> Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
> Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com>
> Signed-off-by: Miriam Rubio <mirucam@gmail.com>
> ---
>  bisect.c | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/bisect.c b/bisect.c
> index 83cb5b3a98..2ac0463327 100644
> --- a/bisect.c
> +++ b/bisect.c
> @@ -661,11 +661,11 @@ static void bisect_common(struct rev_info *revs)
>  		mark_edges_uninteresting(revs, NULL, 0);
>  }
>
> -static void exit_if_skipped_commits(struct commit_list *tried,
> +static int error_if_skipped_commits(struct commit_list *tried,
>  				    const struct object_id *bad)
>  {
>  	if (!tried)
> -		return;
> +		return 0;
>
>  	printf("There are only 'skip'ped commits left to test.\n"
>  	       "The first %s commit could be any of:\n", term_bad);
> @@ -676,7 +676,13 @@ static void exit_if_skipped_commits(struct commit_list *tried,
>  	if (bad)
>  		printf("%s\n", oid_to_hex(bad));
>  	printf(_("We cannot bisect more!\n"));
> -	exit(2);
> +
> +	/*
> +	 * We don't want to clean the bisection state
> +	 * as we need to get back to where we started
> +	 * by using `git bisect reset`.
> +	 */
> +	return -2;

This comment is a good indicator that the constant `-2` here is a "magic" number and it would most likely make sense to turn the return type from an `int` into an `enum` instead.

Show 21 quoted lines
>  }
>
>  static int is_expected_rev(const struct object_id *oid)
> @@ -949,7 +955,7 @@ int bisect_next_all(struct repository *r, const char *prefix, int no_checkout)
>  {
>  	struct rev_info revs;
>  	struct commit_list *tried;
> -	int reaches = 0, all = 0, nr, steps;
> +	int reaches = 0, all = 0, nr, steps, res;
>  	struct object_id *bisect_rev;
>  	char *steps_msg;
>
> @@ -972,8 +978,9 @@ int bisect_next_all(struct repository *r, const char *prefix, int no_checkout)
>  		 * We should exit here only if the "bad"
>  		 * commit is also a "skip" commit.
>  		 */
> -		exit_if_skipped_commits(tried, NULL);
> -
> +		res = error_if_skipped_commits(tried, NULL);
> +		if (res)
> +			exit(-res);

So we still `exit()` in `libgit.a`? I hoped for a more thorough libification.

Besides, the `if (res)` probably wants to be an `if (res < 0)`, right?

Ciao, Johannes

Show 18 quoted lines
>  		printf(_("%s was both %s and %s\n"),
>  		       oid_to_hex(current_bad_oid),
>  		       term_good,
> @@ -990,7 +997,9 @@ int bisect_next_all(struct repository *r, const char *prefix, int no_checkout)
>  	bisect_rev = &revs.commits->item->object.oid;
>
>  	if (oideq(bisect_rev, current_bad_oid)) {
> -		exit_if_skipped_commits(tried, current_bad_oid);
> +		res = error_if_skipped_commits(tried, current_bad_oid);
> +		if (res)
> +			exit(-res);
>  		printf("%s is the first %s commit\n", oid_to_hex(bisect_rev),
>  			term_bad);
>  		show_diff_tree(r, prefix, revs.commits->item);
> --
> 2.21.1 (Apple Git-122.3)
>
>
Previous: Miriam RubioNext: Christian Couder
Message 30 of 54 in “Finish converting git bisect to C part 1”
  1. Miriam RubioJan 20, 2020
  2. 03/29 bisect: use the standard 'if (!var)' way to check for 0Miriam Rubio, Jan 20, 2020
  3. 01/29 bisect--helper: convert `vocab_*` char pointers to char arraysMiriam Rubio, Jan 20, 2020
  4. 02/29 bisect--helper: change `retval` to `res`Miriam Rubio, Jan 20, 2020
  5. 07/29 bisect: libify `bisect_checkout`Miriam Rubio, Jan 20, 2020
  6. 08/29 bisect: libify `check_merge_bases` and its dependentsMiriam Rubio, Jan 20, 2020
  7. Johannes SchindelinJan 20, 2020
  8. Miriam R.Jan 21, 2020
  9. 04/29 run-command: make `exists_in_PATH()` non-staticMiriam Rubio, Jan 20, 2020
  10. 09/29 bisect: libify `check_good_are_ancestors_of_bad` and its dependentsMiriam Rubio, Jan 20, 2020
  11. Johannes SchindelinJan 20, 2020
  12. Christian CouderJan 21, 2020
  13. Miriam R.Jan 21, 2020
  14. 05/29 bisect--helper: introduce new `decide_next()` functionMiriam Rubio, Jan 20, 2020
  15. 10/29 bisect: libify `handle_bad_merge_base` and its dependentsMiriam Rubio, Jan 20, 2020
  16. Johannes SchindelinJan 20, 2020
  17. Christian CouderJan 21, 2020
  18. Miriam R.Jan 21, 2020
  19. 11/29 bisect: libify `bisect_next_all`Miriam Rubio, Jan 20, 2020
  20. Johannes SchindelinJan 20, 2020
  21. Christian CouderJan 21, 2020
  22. Johannes SchindelinJan 30, 2020
  23. 12/29 bisect--helper: reimplement `bisect_next` and `bisect_auto_next` shell functions in CMiriam Rubio, Jan 20, 2020
  24. Johannes SchindelinJan 30, 2020
  25. Miriam R.Jan 31, 2020
  26. Christian CouderFeb 17, 2020
  27. Johannes SchindelinFeb 17, 2020
  28. 13/29 bisect--helper: finish porting `bisect_start()` to CMiriam Rubio, Jan 20, 2020
  29. 06/29 bisect: libify `exit_if_skipped_commits` to `error_if_skipped*` and its dependentsMiriam Rubio, Jan 20, 2020
  30. Johannes SchindelinJan 20, 2020
  31. Christian CouderJan 21, 2020
  32. Miriam R.Jan 21, 2020
  33. 14/29 bisect--helper: retire `--bisect-clean-state` subcommandMiriam Rubio, Jan 20, 2020
  34. 15/29 bisect--helper: retire `--next-all` subcommandMiriam Rubio, Jan 20, 2020
  35. 17/29 bisect--helper: reimplement `bisect_state` & `bisect_head` shell functions in CMiriam Rubio, Jan 20, 2020
  36. 16/29 bisect--helper: reimplement `bisect_autostart` shell function in CMiriam Rubio, Jan 20, 2020
  37. 19/29 bisect--helper: retire `--write-terms` subcommandMiriam Rubio, Jan 20, 2020
  38. 18/29 bisect--helper: retire `--check-expected-revs` subcommandMiriam Rubio, Jan 20, 2020
  39. 20/29 bisect--helper: reimplement `bisect_log` shell function in CMiriam Rubio, Jan 20, 2020
  40. 21/29 bisect--helper: reimplement `bisect_replay` shell function in CMiriam Rubio, Jan 20, 2020
  41. 22/29 bisect--helper: retire `--bisect-write` subcommandMiriam Rubio, Jan 20, 2020
  42. 24/29 bisect--helper: retire `--bisect-autostart` subcommandMiriam Rubio, Jan 20, 2020
  43. 25/29 bisect--helper: retire `--bisect-auto-next` subcommandMiriam Rubio, Jan 20, 2020
  44. 23/29 bisect--helper: use `res` instead of return in BISECT_RESET case optionMiriam Rubio, Jan 20, 2020
  45. 26/29 bisect--helper: reimplement `bisect_skip` shell function in CMiriam Rubio, Jan 20, 2020
  46. 27/29 bisect--helper: retire `--check-and-set-terms` subcommandMiriam Rubio, Jan 20, 2020
  47. 28/29 bisect--helper: reimplement `bisect_visualize()`shell function in CMiriam Rubio, Jan 20, 2020
  48. 29/29 bisect--helper: reimplement `bisect_run` shell function in CMiriam Rubio, Jan 20, 2020
  49. Johannes SchindelinJan 20, 2020
  50. Christian CouderJan 20, 2020
  51. Johannes SchindelinJan 30, 2020
  52. Junio C HamanoJan 30, 2020
  53. Miriam R.Jan 21, 2020
  54. Johannes SchindelinJan 30, 2020

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.