Re: [PATCH 4/9] rebase -i: also expand/collapse the SHA-1s via the rebase--helper

[Johannes Schindelin]

Hi Dennis,

On Fri, 2 Sep 2016, Dennis Kaarsemaker wrote:

> On vr, 2016-09-02 at 18:23 +0200, Johannes Schindelin wrote:
> > This is crucial to improve performance on Windows, as the speed is now
> > mostly dominated by the SHA-1 transformation (because it spawns a new
> > rev-parse process for *every* line, and spawning processes is pretty
> > slow from Git for Windows' MSYS2 Bash).
> 
> I see these functions only used as part of an shorten-edit-expand
> sequence. Why not do a git rebase-helper --edit-todo instead? Saves
> another few process spawnings.

It would make sense to consolidate the steps, yes. I just tried to be
careful in my incremental approach and am fairly certain about the current
revision faithfully replicating the previous behavior.

> Something for yet another later followup patch?

Sure. Probably more than one patch, though: I could imagine that a minor
refactoring would allow us to read in the todo script once, then apply the
individual processing steps in-memory, and then write out the new todo
script.

And then we can implement an --edit-todo with an optional --initial flag
that triggers the check for validity and the rearranging of the
fixup/squash commands (when the user calls `git rebase --edit-todo`,
neither of those steps should be run).

Maybe you will want to have a look into that while I am mostly offline?

Ciao,
Dscho

Re: [PATCH 4/9] rebase -i: also expand/collapse the SHA-1s via the rebase--helper

[Dennis Kaarsemaker]

On vr, 2016-09-02 at 18:23 +0200, Johannes Schindelin wrote:
> This is crucial to improve performance on Windows, as the speed is now
> mostly dominated by the SHA-1 transformation (because it spawns a new
> rev-parse process for *every* line, and spawning processes is pretty
> slow from Git for Windows' MSYS2 Bash).

I see these functions only used as part of an shorten-edit-expand
sequence. Why not do a git rebase-helper --edit-todo instead? Saves
another few process spawnings.

Something for yet another later followup patch?

D.

[PATCH 4/9] rebase -i: also expand/collapse the SHA-1s via the rebase--helper

[Johannes Schindelin]

This is crucial to improve performance on Windows, as the speed is now
mostly dominated by the SHA-1 transformation (because it spawns a new
rev-parse process for *every* line, and spawning processes is pretty
slow from Git for Windows' MSYS2 Bash).

Signed-off-by: Johannes Schindelin 
---
 builtin/rebase--helper.c   | 10 +++-
 git-rebase--interactive.sh |  4 ++--
 sequencer.c| 59 ++
 sequencer.h|  2 ++
 4 files changed, 72 insertions(+), 3 deletions(-)

diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index 821058d..9444c8d 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -13,7 +13,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
struct replay_opts opts = REPLAY_OPTS_INIT;
int keep_empty = 0;
enum {
-   CONTINUE = 1, ABORT, MAKE_SCRIPT
+   CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_SHA1S, EXPAND_SHA1S
} command = 0;
struct option options[] = {
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
@@ -24,6 +24,10 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
ABORT),
OPT_CMDMODE(0, "make-script", ,
N_("make rebase script"), MAKE_SCRIPT),
+   OPT_CMDMODE(0, "shorten-sha1s", ,
+   N_("shorten SHA-1s in the todo list"), SHORTEN_SHA1S),
+   OPT_CMDMODE(0, "expand-sha1s", ,
+   N_("expand SHA-1s in the todo list"), EXPAND_SHA1S),
OPT_END()
};
 
@@ -42,5 +46,9 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
return !!sequencer_remove_state();
if (command == MAKE_SCRIPT && argc > 1)
return !!sequencer_make_script(keep_empty, stdout, argc, argv);
+   if (command == SHORTEN_SHA1S && argc == 1)
+   return !!transform_todo_ids(1);
+   if (command == EXPAND_SHA1S && argc == 1)
+   return !!transform_todo_ids(0);
usage_with_options(builtin_rebase_helper_usage, options);
 }
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0eb5583..f642ec2 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -764,11 +764,11 @@ transform_todo_ids () {
 }
 
 expand_todo_ids() {
-   transform_todo_ids
+   git rebase--helper --expand-sha1s
 }
 
 collapse_todo_ids() {
-   transform_todo_ids --short
+   git rebase--helper --shorten-sha1s
 }
 
 # Rearrange the todo list that has both "pick sha1 msg" and
diff --git a/sequencer.c b/sequencer.c
index 43e078a..ee4fdb0 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2391,3 +2391,62 @@ int sequencer_make_script(int keep_empty, FILE *out,
strbuf_release();
return 0;
 }
+
+
+int transform_todo_ids(int shorten_sha1s)
+{
+   const char *todo_file = rebase_path_todo();
+   struct todo_list todo_list = TODO_LIST_INIT;
+   int fd, res, i;
+   FILE *out;
+
+   strbuf_reset(_list.buf);
+   fd = open(todo_file, O_RDONLY);
+   if (fd < 0)
+   return error_errno(_("Could not open '%s'"), todo_file);
+   if (strbuf_read(_list.buf, fd, 0) < 0) {
+   close(fd);
+   return error(_("Could not read %s."), todo_file);
+   }
+   close(fd);
+
+   res = parse_insn_buffer(todo_list.buf.buf, _list);
+   if (res) {
+   todo_list_release(_list);
+   return error(_("Unusable instruction sheet: %s"), todo_file);
+   }
+
+   out = fopen(todo_file, "w");
+   if (!out) {
+   todo_list_release(_list);
+   return error(_("Unable to open '%s' for writing"), todo_file);
+   }
+   for (i = 0; i < todo_list.nr; i++) {
+   struct todo_item *item = todo_list.items + i;
+   int bol = item->offset_in_buf;
+   const char *p = todo_list.buf.buf + bol;
+   int eol = i + 1 < todo_list.nr ?
+   todo_list.items[i + 1].offset_in_buf :
+   todo_list.buf.len;
+
+   if (item->command >= TODO_EXEC && item->command != TODO_DROP)
+   fwrite(p, eol - bol, 1, out);
+   else {
+   int eoc = strcspn(p, " \t");
+   const char *sha1 = shorten_sha1s ?
+   short_commit_name(item->commit) :
+   oid_to_hex(>commit->object.oid);
+
+   if (!eoc) {
+   p += strspn(p, " \t");
+   eoc = strcspn(p, " \t");
+   }
+
+   fprintf(out, "%.*s %s %.*s\n",
+   eoc, p, sha1, item->arg_len, item->arg);
+   }
+   }
+