git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v2 0/4] Let the builtin rebase call the git am command directly
Date: Fri, 18 Jan 2019 07:09:23 -0800 (PST)	[thread overview]
Message-ID: <pull.24.v2.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.24.git.gitgitgadget@gmail.com>

Especially on Windows, where Unix shell scripting is a foreign endeavor, and
an expensive one at that, we really want to avoid running through the Bash.

This not only makes everything faster, but also more robust, as the Bash we
use on Windows relies on a derivative of the Cygwin runtime, which in turn
has to jump through a couple of hoops that are sometimes a little too tricky
to make things work. Read: the less we rely on Unix shell scripting, the
more likely Windows users will be able to enjoy our software.

Changes since v1:

 * Rebased on top of master to avoid merge conflicts.
 * Adjusted the commit message talking about double entries, to clarify that
   it talks about HEAD's reflog.
 * Replaced a misleading action parameter "checkout" for the reset_head() 
   function by the empty string: we do not check out here, we just update
   the refs, and certainly do not want any checkout functionality (such as
   hooks) to be involved.
 * Reused a just-prepared refs_only variable instead of repeating the value
   assigned to it.
 * Fixed a stale comment about the shell variable "$upstream" (which should
   have been negated to begin with).
 * Fixed error messages when files could not be opened.

Johannes Schindelin (4):
  rebase: move `reset_head()` into a better spot
  rebase: avoid double reflog entry when switching branches
  rebase: teach `reset_head()` to optionally skip the worktree
  built-in rebase: call `git am` directly

 builtin/rebase.c | 424 ++++++++++++++++++++++++++++++++++-------------
 1 file changed, 305 insertions(+), 119 deletions(-)


base-commit: 77556354bb7ac50450e3b28999e3576969869068
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-24%2Fdscho%2Fbuiltin-rebase--am-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-24/dscho/builtin-rebase--am-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/24

Range-diff vs v1:

 1:  175dc7d29a ! 1:  e886ae3de5 rebase: move `reset_head()` into a better spot
     @@ -91,7 +91,7 @@
      +	}
      +
      +	tree = parse_tree_indirect(oid);
     -+	prime_cache_tree(the_repository->index, tree);
     ++	prime_cache_tree(the_repository, the_repository->index, tree);
      +
      +	if (write_locked_index(the_repository->index, &lock, COMMIT_LOCK) < 0) {
      +		ret = error(_("could not write index"));
     @@ -217,7 +217,7 @@
      -	}
      -
      -	tree = parse_tree_indirect(oid);
     --	prime_cache_tree(the_repository->index, tree);
     +-	prime_cache_tree(the_repository, the_repository->index, tree);
      -
      -	if (write_locked_index(the_repository->index, &lock, COMMIT_LOCK) < 0) {
      -		ret = error(_("could not write index"));
 2:  4c5f87b9dc ! 2:  3a68f1c509 rebase: avoid double reflog entry when switching branches
     @@ -3,8 +3,8 @@
          rebase: avoid double reflog entry when switching branches
      
          When switching a branch *and* updating said branch to a different
     -    revision, let's avoid a double entry by first updating the branch and
     -    then adjusting the symbolic ref HEAD.
     +    revision, let's avoid a double entry in HEAD's reflog by first updating
     +    the branch and then adjusting the symbolic ref HEAD.
      
          Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
      
 3:  21939140e0 ! 3:  387071dcd7 rebase: teach `reset_head()` to optionally skip the worktree
     @@ -40,7 +40,7 @@
       	if (!oid)
       		oid = &head_oid;
       
     -+	if (flags & RESET_HEAD_REFS_ONLY)
     ++	if (refs_only)
      +		goto reset_head_refs;
      +
       	memset(&unpack_tree_opts, 0, sizeof(unpack_tree_opts));
 4:  2b5ece8263 ! 4:  3c40318682 built-in rebase: call `git am` directly
     @@ -16,6 +16,10 @@
          itself a derivative of the Cygwin runtime): when no shell script is
          called, the POSIX emulation layer is avoided altogether.
      
     +    Note: we pass an empty action to `reset_head()` here when moving back to
     +    the original branch, as no other action is applicable, really. This
     +    parameter is used to initialize `unpack_trees()`' messages.
     +
          Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
      
       diff --git a/builtin/rebase.c b/builtin/rebase.c
     @@ -78,8 +82,7 @@
      +		    opts->head_name, oid_to_hex(&opts->onto->object.oid));
      +	strbuf_addf(&head_reflog, "rebase finished: returning to %s",
      +		    opts->head_name);
     -+	ret = reset_head(NULL, "checkout", opts->head_name,
     -+			 RESET_HEAD_REFS_ONLY,
     ++	ret = reset_head(NULL, "", opts->head_name, RESET_HEAD_REFS_ONLY,
      +			 orig_head_reflog.buf, head_reflog.buf);
      +
      +	strbuf_release(&orig_head_reflog);
     @@ -114,7 +117,6 @@
      +		if (status)
      +			return status;
      +
     -+		discard_cache();
      +		return move_to_original_branch(opts);
      +	}
      +	if (opts->action && !strcmp("skip", opts->action)) {
     @@ -124,7 +126,6 @@
      +		if (status)
      +			return status;
      +
     -+		discard_cache();
      +		return move_to_original_branch(opts);
      +	}
      +	if (opts->action && !strcmp("show-current-patch", opts->action)) {
     @@ -134,7 +135,7 @@
      +
      +	strbuf_addf(&revisions, "%s...%s",
      +		    oid_to_hex(opts->root ?
     -+			       /* this is now equivalent to ! -z "$upstream" */
     ++			       /* this is now equivalent to !opts->upstream */
      +			       &opts->onto->object.oid :
      +			       &opts->upstream->object.oid),
      +		    oid_to_hex(&opts->orig_head));
     @@ -143,7 +144,7 @@
      +	format_patch.out = open(rebased_patches,
      +				O_WRONLY | O_CREAT | O_TRUNC, 0666);
      +	if (format_patch.out < 0) {
     -+		status = error_errno(_("could not write '%s'"),
     ++		status = error_errno(_("could not open '%s' for writing"),
      +				     rebased_patches);
      +		free(rebased_patches);
      +		argv_array_clear(&am.args);
     @@ -185,7 +186,7 @@
      +
      +	am.in = open(rebased_patches, O_RDONLY);
      +	if (am.in < 0) {
     -+		status = error_errno(_("could not read '%s'"),
     ++		status = error_errno(_("could not open '%s' for reading"),
      +				     rebased_patches);
      +		free(rebased_patches);
      +		argv_array_clear(&am.args);
     @@ -207,7 +208,6 @@
      +	free(rebased_patches);
      +
      +	if (!status) {
     -+		discard_cache();
      +		return move_to_original_branch(opts);
      +	}
      +

-- 
gitgitgadget

  parent reply	other threads:[~2019-01-18 15:09 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-21 13:17 [PATCH 0/4] Let the builtin rebase call the git am command directly Johannes Schindelin via GitGitGadget
2018-12-21 13:17 ` [PATCH 1/4] rebase: move `reset_head()` into a better spot Johannes Schindelin via GitGitGadget
2019-01-04 18:39   ` Junio C Hamano
2018-12-21 13:17 ` [PATCH 2/4] rebase: avoid double reflog entry when switching branches Johannes Schindelin via GitGitGadget
2019-01-04 18:38   ` Junio C Hamano
2019-01-18 14:18     ` Johannes Schindelin
2018-12-21 13:17 ` [PATCH 4/4] built-in rebase: call `git am` directly Johannes Schindelin via GitGitGadget
2019-01-04 18:38   ` Junio C Hamano
2019-01-18 14:15     ` Johannes Schindelin
2019-01-18 17:59       ` Junio C Hamano
2019-01-18 18:14         ` Johannes Schindelin
2019-01-04 23:19   ` Junio C Hamano
2019-01-07 17:19     ` Junio C Hamano
2018-12-21 13:17 ` [PATCH 3/4] rebase: teach `reset_head()` to optionally skip the worktree Johannes Schindelin via GitGitGadget
2019-01-04 18:38   ` Junio C Hamano
2019-01-04 18:40     ` Junio C Hamano
2019-01-18 14:18     ` Johannes Schindelin
2019-01-18 15:09 ` Johannes Schindelin via GitGitGadget [this message]
2019-01-18 15:09   ` [PATCH v2 1/4] rebase: move `reset_head()` into a better spot Johannes Schindelin via GitGitGadget
2019-01-18 15:09   ` [PATCH v2 2/4] rebase: avoid double reflog entry when switching branches Johannes Schindelin via GitGitGadget
2019-01-18 15:09   ` [PATCH v2 3/4] rebase: teach `reset_head()` to optionally skip the worktree Johannes Schindelin via GitGitGadget
2019-01-18 15:09   ` [PATCH v2 4/4] built-in rebase: call `git am` directly Johannes Schindelin via GitGitGadget
2019-01-26 21:13     ` Alban Gruin
2019-01-29  9:46       ` Johannes Schindelin
2019-01-18 18:06   ` [PATCH v2 0/4] Let the builtin rebase call the git am command directly Junio C Hamano
2019-01-18 20:13     ` Junio C Hamano
2019-01-18 20:31     ` Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.24.v2.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).