[v3,09/19] rebase: use apply_autostash() from sequencer.c
diff mbox series

Message ID 03bdaeebc993058bf14a0d196f5a1a68800e538e.1584782450.git.liu.denton@gmail.com
State New
Headers show
Series
  • merge: learn --autostash
Related show

Commit Message

Denton Liu March 21, 2020, 9:21 a.m. UTC
The apply_autostash() function in builtin/rebase.c is similar enough to
the apply_autostash() function in sequencer.c that they are almost
interchangeable. Make the sequencer.c version extern and use it in
rebase.

The rebase version was introduced in 6defce2b02 (builtin rebase: support
`--autostash` option, 2018-09-04) as part of the shell to C conversion.
It opted to duplicate the function because, at the time, there was
another in-progress project converting interactive rebase from shell to
C as well and they did not want to clash with them by refactoring
sequencer.c version of apply_autostash(). Since both efforts are long
done, we can freely combine them together now.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 builtin/rebase.c | 49 ++----------------------------------------------
 sequencer.c      |  2 +-
 sequencer.h      |  2 ++
 3 files changed, 5 insertions(+), 48 deletions(-)

Comments

Phillip Wood April 2, 2020, 2:59 p.m. UTC | #1
Hi Denton

On 21/03/2020 09:21, Denton Liu wrote:
> The apply_autostash() function in builtin/rebase.c is similar enough to
> the apply_autostash() function in sequencer.c that they are almost
> interchangeable.

You say they are almost interchangeable and then use them as if they are 
completely interchangeable. I think they are interchangeable, the only 
difference I could see is that the code in rebase.c redirects stdin when 
forking 'git stash apply' but the sequencer does not, I don't think this 
matters.

Best Wishes

Phillip

  Make the sequencer.c version extern and use it in
> rebase.
> 
> The rebase version was introduced in 6defce2b02 (builtin rebase: support
> `--autostash` option, 2018-09-04) as part of the shell to C conversion.
> It opted to duplicate the function because, at the time, there was
> another in-progress project converting interactive rebase from shell to
> C as well and they did not want to clash with them by refactoring
> sequencer.c version of apply_autostash(). Since both efforts are long
> done, we can freely combine them together now.
> 
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
>   builtin/rebase.c | 49 ++----------------------------------------------
>   sequencer.c      |  2 +-
>   sequencer.h      |  2 ++
>   3 files changed, 5 insertions(+), 48 deletions(-)
> 
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index 1146463099..ceb115247a 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -712,51 +712,6 @@ static int rebase_write_basic_state(struct rebase_options *opts)
>   	return 0;
>   }
>   
> -static int apply_autostash(struct rebase_options *opts)
> -{
> -	const char *path = state_dir_path("autostash", opts);
> -	struct strbuf autostash = STRBUF_INIT;
> -	struct child_process stash_apply = CHILD_PROCESS_INIT;
> -
> -	if (!file_exists(path))
> -		return 0;
> -
> -	if (!read_oneliner(&autostash, path, READ_ONELINER_WARN_NON_EXISTENCE))
> -		return error(_("Could not read '%s'"), path);
> -	/* Ensure that the hash is not mistaken for a number */
> -	strbuf_addstr(&autostash, "^0");
> -	argv_array_pushl(&stash_apply.args,
> -			 "stash", "apply", autostash.buf, NULL);
> -	stash_apply.git_cmd = 1;
> -	stash_apply.no_stderr = stash_apply.no_stdout =
> -		stash_apply.no_stdin = 1;
> -	if (!run_command(&stash_apply))
> -		printf(_("Applied autostash.\n"));
> -	else {
> -		struct argv_array args = ARGV_ARRAY_INIT;
> -		int res = 0;
> -
> -		argv_array_pushl(&args,
> -				 "stash", "store", "-m", "autostash", "-q",
> -				 autostash.buf, NULL);
> -		if (run_command_v_opt(args.argv, RUN_GIT_CMD))
> -			res = error(_("Cannot store %s"), autostash.buf);
> -		argv_array_clear(&args);
> -		strbuf_release(&autostash);
> -		if (res)
> -			return res;
> -
> -		fprintf(stderr,
> -			_("Applying autostash resulted in conflicts.\n"
> -			  "Your changes are safe in the stash.\n"
> -			  "You can run \"git stash pop\" or \"git stash drop\" "
> -			  "at any time.\n"));
> -	}
> -
> -	strbuf_release(&autostash);
> -	return 0;
> -}
> -
>   static int finish_rebase(struct rebase_options *opts)
>   {
>   	struct strbuf dir = STRBUF_INIT;
> @@ -764,7 +719,7 @@ static int finish_rebase(struct rebase_options *opts)
>   	int ret = 0;
>   
>   	delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
> -	apply_autostash(opts);
> +	apply_autostash(state_dir_path("autostash", opts));
>   	close_object_store(the_repository->objects);
>   	/*
>   	 * We ignore errors in 'gc --auto', since the
> @@ -1209,7 +1164,7 @@ static int run_specific_rebase(struct rebase_options *opts, enum action action)
>   	} else if (status == 2) {
>   		struct strbuf dir = STRBUF_INIT;
>   
> -		apply_autostash(opts);
> +		apply_autostash(state_dir_path("autostash", opts));
>   		strbuf_addstr(&dir, opts->state_dir);
>   		remove_dir_recursively(&dir, 0);
>   		strbuf_release(&dir);
> diff --git a/sequencer.c b/sequencer.c
> index ad40a8b3fc..b52668f8de 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -3655,7 +3655,7 @@ static enum todo_command peek_command(struct todo_list *todo_list, int offset)
>   	return -1;
>   }
>   
> -static int apply_autostash(const char *path)
> +int apply_autostash(const char *path)
>   {
>   	struct strbuf stash_sha1 = STRBUF_INIT;
>   	struct child_process child = CHILD_PROCESS_INIT;
> diff --git a/sequencer.h b/sequencer.h
> index 5c9662a60a..bab910f012 100644
> --- a/sequencer.h
> +++ b/sequencer.h
> @@ -191,6 +191,8 @@ void commit_post_rewrite(struct repository *r,
>   			 const struct commit *current_head,
>   			 const struct object_id *new_head);
>   
> +int apply_autostash(const char *path);
> +
>   #define SUMMARY_INITIAL_COMMIT   (1 << 0)
>   #define SUMMARY_SHOW_AUTHOR_DATE (1 << 1)
>   void print_commit_summary(struct repository *repo,
>

Patch
diff mbox series

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 1146463099..ceb115247a 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -712,51 +712,6 @@  static int rebase_write_basic_state(struct rebase_options *opts)
 	return 0;
 }
 
-static int apply_autostash(struct rebase_options *opts)
-{
-	const char *path = state_dir_path("autostash", opts);
-	struct strbuf autostash = STRBUF_INIT;
-	struct child_process stash_apply = CHILD_PROCESS_INIT;
-
-	if (!file_exists(path))
-		return 0;
-
-	if (!read_oneliner(&autostash, path, READ_ONELINER_WARN_NON_EXISTENCE))
-		return error(_("Could not read '%s'"), path);
-	/* Ensure that the hash is not mistaken for a number */
-	strbuf_addstr(&autostash, "^0");
-	argv_array_pushl(&stash_apply.args,
-			 "stash", "apply", autostash.buf, NULL);
-	stash_apply.git_cmd = 1;
-	stash_apply.no_stderr = stash_apply.no_stdout =
-		stash_apply.no_stdin = 1;
-	if (!run_command(&stash_apply))
-		printf(_("Applied autostash.\n"));
-	else {
-		struct argv_array args = ARGV_ARRAY_INIT;
-		int res = 0;
-
-		argv_array_pushl(&args,
-				 "stash", "store", "-m", "autostash", "-q",
-				 autostash.buf, NULL);
-		if (run_command_v_opt(args.argv, RUN_GIT_CMD))
-			res = error(_("Cannot store %s"), autostash.buf);
-		argv_array_clear(&args);
-		strbuf_release(&autostash);
-		if (res)
-			return res;
-
-		fprintf(stderr,
-			_("Applying autostash resulted in conflicts.\n"
-			  "Your changes are safe in the stash.\n"
-			  "You can run \"git stash pop\" or \"git stash drop\" "
-			  "at any time.\n"));
-	}
-
-	strbuf_release(&autostash);
-	return 0;
-}
-
 static int finish_rebase(struct rebase_options *opts)
 {
 	struct strbuf dir = STRBUF_INIT;
@@ -764,7 +719,7 @@  static int finish_rebase(struct rebase_options *opts)
 	int ret = 0;
 
 	delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
-	apply_autostash(opts);
+	apply_autostash(state_dir_path("autostash", opts));
 	close_object_store(the_repository->objects);
 	/*
 	 * We ignore errors in 'gc --auto', since the
@@ -1209,7 +1164,7 @@  static int run_specific_rebase(struct rebase_options *opts, enum action action)
 	} else if (status == 2) {
 		struct strbuf dir = STRBUF_INIT;
 
-		apply_autostash(opts);
+		apply_autostash(state_dir_path("autostash", opts));
 		strbuf_addstr(&dir, opts->state_dir);
 		remove_dir_recursively(&dir, 0);
 		strbuf_release(&dir);
diff --git a/sequencer.c b/sequencer.c
index ad40a8b3fc..b52668f8de 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3655,7 +3655,7 @@  static enum todo_command peek_command(struct todo_list *todo_list, int offset)
 	return -1;
 }
 
-static int apply_autostash(const char *path)
+int apply_autostash(const char *path)
 {
 	struct strbuf stash_sha1 = STRBUF_INIT;
 	struct child_process child = CHILD_PROCESS_INIT;
diff --git a/sequencer.h b/sequencer.h
index 5c9662a60a..bab910f012 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -191,6 +191,8 @@  void commit_post_rewrite(struct repository *r,
 			 const struct commit *current_head,
 			 const struct object_id *new_head);
 
+int apply_autostash(const char *path);
+
 #define SUMMARY_INITIAL_COMMIT   (1 << 0)
 #define SUMMARY_SHOW_AUTHOR_DATE (1 << 1)
 void print_commit_summary(struct repository *repo,