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

Re: [PATCH v2 04/17] sequencer: configurably warn on non-existent files

From
Junio C Hamano <gitster@pobox.com>
Date
Dec 26, 2019, 20:39 UTC
Message-ID
<xmqqwoaikehz.fsf@gitster-ct.c.googlers.com>
In-Reply-To
<fd547aab491b24a47f683d717c6b77255ac16901.1577185374.git.liu.denton@gmail.com>
Denton Liu <liu.denton@gmail.com> writes:
> -static int read_oneliner(struct strbuf *buf,
> -	const char *path, int skip_if_empty)
> +static int read_oneliner(struct strbuf *buf, const char *path,
> +			 int skip_if_empty, int warn_nonexistence)

I would have to say that this results in a rather poor API in the end, and also forces an awkward API evolution, and we need to be careful especially if we plan to make this an external API in a later step in the series.

There is a topic in flight that introduces additional callsites of read_oneliner(). It is a selfish move that breaks codebase unnecessarily to unilaterally change the function signature.

Two possible ways to improve are:
 * keep read_oneliner() with existing function signature working for
   other topics in flight, by renaming your extended variant and
   making read_oneliner() a thin wrapper that calls yours with the
   extended feature disabled (i.e. warn_nonexistence==0); or
 * update skip_if_empty that wastes an int parameter to pass only a
   single bit into an unsigned that is a collection of bits, after
   vetting that new callsites contemporary topics add always pass
   either 0 or 1 as the parameter.  Then
	#define READ_ONELINER_SKIP_IF_EMPTY 01
	#define READ_ONELINER_WARN_MISSING  02
	static int read_oneliner(struct strbuf *buf, const char *path,
   				 unsigned flags)
   would transparently work well for those other topics.

The latter probably is more preferrable, as the end result would be a cleaner API than the former or the one presented in this series.

Thanks.
Show 119 quoted lines
>  {
>  	int ret = 0;
>  	struct strbuf file_buf = STRBUF_INIT;
>  
> -	if (!file_exists(path))
> +	if (!warn_nonexistence && !file_exists(path))
>  		return 0;
>  
>  	if (strbuf_read_file(&file_buf, path, 0) < 0) {
> @@ -2558,10 +2558,10 @@ void parse_strategy_opts(struct replay_opts *opts, char *raw_opts)
>  static void read_strategy_opts(struct replay_opts *opts, struct strbuf *buf)
>  {
>  	strbuf_reset(buf);
> -	if (!read_oneliner(buf, rebase_path_strategy(), 0))
> +	if (!read_oneliner(buf, rebase_path_strategy(), 0, 0))
>  		return;
>  	opts->strategy = strbuf_detach(buf, NULL);
> -	if (!read_oneliner(buf, rebase_path_strategy_opts(), 0))
> +	if (!read_oneliner(buf, rebase_path_strategy_opts(), 0, 0))
>  		return;
>  
>  	parse_strategy_opts(opts, buf->buf);
> @@ -2572,7 +2572,7 @@ static int read_populate_opts(struct replay_opts *opts)
>  	if (is_rebase_i(opts)) {
>  		struct strbuf buf = STRBUF_INIT;
>  
> -		if (read_oneliner(&buf, rebase_path_gpg_sign_opt(), 1)) {
> +		if (read_oneliner(&buf, rebase_path_gpg_sign_opt(), 1, 0)) {
>  			if (!starts_with(buf.buf, "-S"))
>  				strbuf_reset(&buf);
>  			else {
> @@ -2582,7 +2582,7 @@ static int read_populate_opts(struct replay_opts *opts)
>  			strbuf_reset(&buf);
>  		}
>  
> -		if (read_oneliner(&buf, rebase_path_allow_rerere_autoupdate(), 1)) {
> +		if (read_oneliner(&buf, rebase_path_allow_rerere_autoupdate(), 1, 0)) {
>  			if (!strcmp(buf.buf, "--rerere-autoupdate"))
>  				opts->allow_rerere_auto = RERERE_AUTOUPDATE;
>  			else if (!strcmp(buf.buf, "--no-rerere-autoupdate"))
> @@ -2618,7 +2618,7 @@ static int read_populate_opts(struct replay_opts *opts)
>  		strbuf_release(&buf);
>  
>  		if (read_oneliner(&opts->current_fixups,
> -				  rebase_path_current_fixups(), 1)) {
> +				  rebase_path_current_fixups(), 1, 0)) {
>  			const char *p = opts->current_fixups.buf;
>  			opts->current_fixup_count = 1;
>  			while ((p = strchr(p, '\n'))) {
> @@ -2627,7 +2627,7 @@ static int read_populate_opts(struct replay_opts *opts)
>  			}
>  		}
>  
> -		if (read_oneliner(&buf, rebase_path_squash_onto(), 0)) {
> +		if (read_oneliner(&buf, rebase_path_squash_onto(), 0, 0)) {
>  			if (get_oid_hex(buf.buf, &opts->squash_onto) < 0)
>  				return error(_("unusable squash-onto"));
>  			opts->have_squash_onto = 1;
> @@ -3759,7 +3759,7 @@ static int apply_autostash(struct replay_opts *opts)
>  	struct child_process child = CHILD_PROCESS_INIT;
>  	int ret = 0;
>  
> -	if (!read_oneliner(&stash_sha1, rebase_path_autostash(), 1)) {
> +	if (!read_oneliner(&stash_sha1, rebase_path_autostash(), 1, 0)) {
>  		strbuf_release(&stash_sha1);
>  		return 0;
>  	}
> @@ -4093,7 +4093,7 @@ static int pick_commits(struct repository *r,
>  		if (todo_list->current < todo_list->nr)
>  			return 0;
>  
> -		if (read_oneliner(&head_ref, rebase_path_head_name(), 0) &&
> +		if (read_oneliner(&head_ref, rebase_path_head_name(), 0, 0) &&
>  				starts_with(head_ref.buf, "refs/")) {
>  			const char *msg;
>  			struct object_id head, orig;
> @@ -4106,13 +4106,13 @@ static int pick_commits(struct repository *r,
>  				strbuf_release(&buf);
>  				return res;
>  			}
> -			if (!read_oneliner(&buf, rebase_path_orig_head(), 0) ||
> +			if (!read_oneliner(&buf, rebase_path_orig_head(), 0, 0) ||
>  					get_oid_hex(buf.buf, &orig)) {
>  				res = error(_("could not read orig-head"));
>  				goto cleanup_head_ref;
>  			}
>  			strbuf_reset(&buf);
> -			if (!read_oneliner(&buf, rebase_path_onto(), 0)) {
> +			if (!read_oneliner(&buf, rebase_path_onto(), 0, 0)) {
>  				res = error(_("could not read 'onto'"));
>  				goto cleanup_head_ref;
>  			}
> @@ -4145,7 +4145,7 @@ static int pick_commits(struct repository *r,
>  				DIFF_FORMAT_DIFFSTAT;
>  			log_tree_opt.disable_stdin = 1;
>  
> -			if (read_oneliner(&buf, rebase_path_orig_head(), 0) &&
> +			if (read_oneliner(&buf, rebase_path_orig_head(), 0, 0) &&
>  			    !get_oid(buf.buf, &orig) &&
>  			    !get_oid("HEAD", &head)) {
>  				diff_tree_oid(&orig, &head, "",
> @@ -4230,7 +4230,7 @@ static int commit_staged_changes(struct repository *r,
>  
>  		if (get_oid("HEAD", &head))
>  			return error(_("cannot amend non-existing commit"));
> -		if (!read_oneliner(&rev, rebase_path_amend(), 0))
> +		if (!read_oneliner(&rev, rebase_path_amend(), 0, 0))
>  			return error(_("invalid file: '%s'"), rebase_path_amend());
>  		if (get_oid_hex(rev.buf, &to_amend))
>  			return error(_("invalid contents: '%s'"),
> @@ -4391,7 +4391,7 @@ int sequencer_continue(struct repository *r, struct replay_opts *opts)
>  		struct strbuf buf = STRBUF_INIT;
>  		struct object_id oid;
>  
> -		if (read_oneliner(&buf, rebase_path_stopped_sha(), 1) &&
> +		if (read_oneliner(&buf, rebase_path_stopped_sha(), 1, 0) &&
>  		    !get_oid_committish(buf.buf, &oid))
>  			record_in_rewritten(&oid, peek_command(&todo_list, 0));
>  		strbuf_release(&buf);
Previous: Denton LiuNext: Denton Liu
Message 34 of 152 in “merge: learn --autostash”
  1. 0/7 merge: learn --autostashDenton Liu, Oct 16, 2019
  2. 1/7 Makefile: alphabetically sort += listsDenton Liu, Oct 16, 2019
  3. Junio C HamanoOct 17, 2019
  4. Johannes SchindelinOct 21, 2019
  5. Denton LiuOct 21, 2019
  6. Johannes SchindelinOct 22, 2019
  7. Junio C HamanoOct 21, 2019
  8. Jeff KingOct 21, 2019
  9. Junio C HamanoOct 22, 2019
  10. 2/7 autostash: extract read_one() from rebaseDenton Liu, Oct 16, 2019
  11. Phillip WoodOct 18, 2019
  12. Johannes SchindelinOct 21, 2019
  13. 3/7 autostash: extract apply_autostash() from rebaseDenton Liu, Oct 16, 2019
  14. 4/7 autostash: extract reset_head() from rebaseDenton Liu, Oct 16, 2019
  15. Phillip WoodOct 18, 2019
  16. Johannes SchindelinOct 21, 2019
  17. 5/7 autostash: extract perform_autostash() from rebaseDenton Liu, Oct 16, 2019
  18. Johannes SchindelinOct 21, 2019
  19. 6/7 autostash.c: undefine USE_THE_INDEX_COMPATIBILITY_MACROSDenton Liu, Oct 16, 2019
  20. Phillip WoodOct 18, 2019
  21. 7/7 merge: teach --autostash optionDenton Liu, Oct 16, 2019
  22. Phillip WoodOct 18, 2019
  23. Denton LiuDec 24, 2019
  24. Johannes SchindelinOct 21, 2019
  25. Denton LiuDec 24, 2019
  26. Denton LiuDec 24, 2019
  27. 00/17 merge: learn --autostashDenton Liu, Dec 24, 2019
  28. 03/17 sequencer: use file strbuf for read_oneliner()Denton Liu, Dec 24, 2019
  29. 02/17 t7600: use test_write_lines()Denton Liu, Dec 24, 2019
  30. 01/17 Makefile: alphabetically sort += listsDenton Liu, Dec 24, 2019
  31. Junio C HamanoDec 30, 2019
  32. 06/17 rebase: use read_oneliner()Denton Liu, Dec 24, 2019
  33. 04/17 sequencer: configurably warn on non-existent filesDenton Liu, Dec 24, 2019
  34. Junio C HamanoDec 26, 2019
  35. 07/17 sequencer: make apply_rebase() accept a pathDenton Liu, Dec 24, 2019
  36. 08/17 rebase: use apply_autostash() from sequencer.cDenton Liu, Dec 24, 2019
  37. 05/17 sequencer: make read_oneliner() externDenton Liu, Dec 24, 2019
  38. 10/17 reset: extract reset_head() from rebaseDenton Liu, Dec 24, 2019
  39. 12/17 rebase: generify create_autostash()Denton Liu, Dec 24, 2019
  40. 13/17 sequencer: extract perform_autostash() from rebaseDenton Liu, Dec 24, 2019
  41. 09/17 rebase: generify reset_head()Denton Liu, Dec 24, 2019
  42. 11/17 rebase: extract create_autostash()Denton Liu, Dec 24, 2019
  43. 17/17 pull: pass --autostash to mergeDenton Liu, Dec 24, 2019
  44. 14/17 sequencer: unlink autostash in apply_autostash()Denton Liu, Dec 24, 2019
  45. 16/17 t5520: make test_pull_autostash() accept expect_parent_numDenton Liu, Dec 24, 2019
  46. 15/17 merge: teach --autostash optionDenton Liu, Dec 24, 2019
  47. Junio C HamanoDec 30, 2019
  48. Phillip WoodDec 31, 2019
  49. Denton LiuJan 8, 2020
  50. Phillip WoodJan 10, 2020
  51. Denton LiuJan 15, 2020
  52. Denton LiuJan 1, 2020
  53. 00/19 merge: learn --autostashDenton Liu, Mar 21, 2020
  54. 02/19 t7600: use test_write_lines()Denton Liu, Mar 21, 2020
  55. 01/19 Makefile: ASCII-sort += listsDenton Liu, Mar 21, 2020
  56. 03/19 sequencer: use file strbuf for read_oneliner()Denton Liu, Mar 21, 2020
  57. Phillip WoodApr 2, 2020
  58. 04/19 sequencer: make read_oneliner() accept flagsDenton Liu, Mar 21, 2020
  59. 05/19 sequencer: configurably warn on non-existent filesDenton Liu, Mar 21, 2020
  60. Phillip WoodApr 2, 2020
  61. 07/19 rebase: use read_oneliner()Denton Liu, Mar 21, 2020
  62. Phillip WoodApr 2, 2020
  63. 06/19 sequencer: make read_oneliner() externDenton Liu, Mar 21, 2020
  64. 08/19 sequencer: make apply_rebase() accept a pathDenton Liu, Mar 21, 2020
  65. 09/19 rebase: use apply_autostash() from sequencer.cDenton Liu, Mar 21, 2020
  66. Phillip WoodApr 2, 2020
  67. 10/19 rebase: generify reset_head()Denton Liu, Mar 21, 2020
  68. 11/19 reset: extract reset_head() from rebaseDenton Liu, Mar 21, 2020
  69. Phillip WoodApr 2, 2020
  70. 12/19 rebase: extract create_autostash()Denton Liu, Mar 21, 2020
  71. 13/19 rebase: generify create_autostash()Denton Liu, Mar 21, 2020
  72. 14/19 sequencer: extract perform_autostash() from rebaseDenton Liu, Mar 21, 2020
  73. 15/19 sequencer: unlink autostash in apply_autostash()Denton Liu, Mar 21, 2020
  74. 16/19 sequencer: implement save_autostash()Denton Liu, Mar 21, 2020
  75. Phillip WoodApr 2, 2020
  76. 18/19 t5520: make test_pull_autostash() accept expect_parent_numDenton Liu, Mar 21, 2020
  77. 17/19 merge: teach --autostash optionDenton Liu, Mar 21, 2020
  78. Phillip WoodApr 2, 2020
  79. Denton LiuApr 3, 2020
  80. Denton LiuApr 3, 2020
  81. Phillip WoodApr 3, 2020
  82. Denton LiuApr 3, 2020
  83. Phillip WoodApr 3, 2020
  84. Denton LiuApr 3, 2020
  85. 19/19 pull: pass --autostash to mergeDenton Liu, Mar 21, 2020
  86. Phillip WoodApr 2, 2020
  87. 00/23 merge: learn --autostashDenton Liu, Apr 4, 2020
  88. 02/23 t7600: use test_write_lines()Denton Liu, Apr 4, 2020
  89. 03/23 sequencer: stop leaking bufDenton Liu, Apr 4, 2020
  90. Junio C HamanoApr 5, 2020
  91. Junio C HamanoApr 5, 2020
  92. Denton LiuApr 5, 2020
  93. 05/23 sequencer: make file exists check more efficientDenton Liu, Apr 4, 2020
  94. 04/23 sequencer: reuse strbuf_trim() in read_oneliner()Denton Liu, Apr 4, 2020
  95. Junio C HamanoApr 5, 2020
  96. Denton LiuApr 6, 2020
  97. Phillip WoodApr 6, 2020
  98. Phillip WoodApr 6, 2020
  99. Denton LiuApr 7, 2020
  100. 07/23 sequencer: configurably warn on non-existent filesDenton Liu, Apr 4, 2020
  101. Phillip WoodApr 6, 2020
  102. 09/23 rebase: use read_oneliner()Denton Liu, Apr 4, 2020
  103. 01/23 Makefile: ASCII-sort += listsDenton Liu, Apr 4, 2020
  104. 08/23 sequencer: make read_oneliner() externDenton Liu, Apr 4, 2020
  105. 12/23 rebase: use apply_autostash() from sequencer.cDenton Liu, Apr 4, 2020
  106. 11/23 sequencer: rename stash_sha1 to stash_oidDenton Liu, Apr 4, 2020
  107. 15/23 rebase: extract create_autostash()Denton Liu, Apr 4, 2020
  108. 13/23 rebase: generify reset_head()Denton Liu, Apr 4, 2020
  109. 16/23 rebase: generify create_autostash()Denton Liu, Apr 4, 2020
  110. 17/23 sequencer: extract perform_autostash() from rebaseDenton Liu, Apr 4, 2020
  111. 18/23 sequencer: unlink autostash in apply_autostash()Denton Liu, Apr 4, 2020
  112. 20/23 sequencer: implement apply_autostash_oid()Denton Liu, Apr 4, 2020
  113. 14/23 reset: extract reset_head() from rebaseDenton Liu, Apr 4, 2020
  114. 22/23 t5520: make test_pull_autostash() accept expect_parent_numDenton Liu, Apr 4, 2020
  115. 19/23 sequencer: implement save_autostash()Denton Liu, Apr 4, 2020
  116. Phillip WoodApr 6, 2020
  117. 23/23 pull: pass --autostash to mergeDenton Liu, Apr 4, 2020
  118. 10/23 sequencer: make apply_rebase() accept a pathDenton Liu, Apr 4, 2020
  119. Phillip WoodApr 6, 2020
  120. 21/23 merge: teach --autostash optionDenton Liu, Apr 4, 2020
  121. Phillip WoodApr 6, 2020
  122. Denton LiuApr 7, 2020
  123. Phillip WoodApr 7, 2020
  124. 06/23 sequencer: make read_oneliner() accept flagsDenton Liu, Apr 4, 2020
  125. 00/22 merge: learn --autostashDenton Liu, Apr 7, 2020
  126. 02/22 t7600: use test_write_lines()Denton Liu, Apr 7, 2020
  127. 01/22 Makefile: ASCII-sort += listsDenton Liu, Apr 7, 2020
  128. 04/22 sequencer: make file exists check more efficientDenton Liu, Apr 7, 2020
  129. Junio C HamanoApr 8, 2020
  130. 03/22 sequencer: stop leaking bufDenton Liu, Apr 7, 2020
  131. 05/22 sequencer: make read_oneliner() accept flagsDenton Liu, Apr 7, 2020
  132. Junio C HamanoApr 8, 2020
  133. 06/22 sequencer: configurably warn on non-existent filesDenton Liu, Apr 7, 2020
  134. 07/22 sequencer: make read_oneliner() externDenton Liu, Apr 7, 2020
  135. 08/22 rebase: use read_oneliner()Denton Liu, Apr 7, 2020
  136. Junio C HamanoApr 8, 2020
  137. 11/22 rebase: use apply_autostash() from sequencer.cDenton Liu, Apr 7, 2020
  138. 12/22 rebase: generify reset_head()Denton Liu, Apr 7, 2020
  139. 14/22 rebase: extract create_autostash()Denton Liu, Apr 7, 2020
  140. 15/22 rebase: generify create_autostash()Denton Liu, Apr 7, 2020
  141. 13/22 reset: extract reset_head() from rebaseDenton Liu, Apr 7, 2020
  142. 10/22 sequencer: rename stash_sha1 to stash_oidDenton Liu, Apr 7, 2020
  143. 09/22 sequencer: make apply_autostash() accept a pathDenton Liu, Apr 7, 2020
  144. 16/22 sequencer: extract perform_autostash() from rebaseDenton Liu, Apr 7, 2020
  145. 17/22 sequencer: unlink autostash in apply_autostash()Denton Liu, Apr 7, 2020
  146. 21/22 t5520: make test_pull_autostash() accept expect_parent_numDenton Liu, Apr 7, 2020
  147. 18/22 sequencer: implement save_autostash()Denton Liu, Apr 7, 2020
  148. 19/22 sequencer: implement apply_autostash_oid()Denton Liu, Apr 7, 2020
  149. 20/22 merge: teach --autostash optionDenton Liu, Apr 7, 2020
  150. 22/22 pull: pass --autostash to mergeDenton Liu, Apr 7, 2020
  151. Phillip WoodApr 10, 2020
  152. Junio C HamanoApr 10, 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.