Hi Johannes,
Johannes Schindelin <johannes.schindelin@xxxxxx> writes:
> 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 <johannes.schindelin@xxxxxx>
> ---
> builtin/rebase--helper.c | 10 +++++++-
> git-rebase--interactive.sh | 27 ++--------------------
> sequencer.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++
> sequencer.h | 2 ++
> 4 files changed, 70 insertions(+), 26 deletions(-)
>
> diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
> index 821058d452d..9444c8d6c60 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", &opts.allow_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", &command,
> N_("make rebase script"), MAKE_SCRIPT),
> + OPT_CMDMODE(0, "shorten-sha1s", &command,
> + N_("shorten SHA-1s in the todo list"), SHORTEN_SHA1S),
> + OPT_CMDMODE(0, "expand-sha1s", &command,
> + N_("expand SHA-1s in the todo list"), EXPAND_SHA1S),
Since work is being done to convert to `struct object_id` would it
not be best to use a more generic name instead of 'sha1'?
maybe something like {shorten,expand}-hashs
> OPT_END()
> };
>
> @@ -42,5 +46,9 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
> return !!sequencer_remove_state(&opts);
> 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 214af0372ba..82a1941c42c 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -750,35 +750,12 @@ skip_unnecessary_picks () {
> die "$(gettext "Could not skip unnecessary pick commands")"
> }
>
> -transform_todo_ids () {
> - while read -r command rest
> - do
> - case "$command" in
> - "$comment_char"* | exec)
> - # Be careful for oddball commands like 'exec'
> - # that do not have a SHA-1 at the beginning of $rest.
> - ;;
> - *)
> - sha1=$(git rev-parse --verify --quiet "$@" ${rest%%[ ]*}) &&
> - if test "a$rest" = "a${rest#*[ ]}"
> - then
> - rest=$sha1
> - else
> - rest="$sha1 ${rest#*[ ]}"
> - fi
> - ;;
> - esac
> - printf '%s\n' "$command${rest:+ }$rest"
> - done <"$todo" >"$todo.new" &&
> - mv -f "$todo.new" "$todo"
> -}
> -
> 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 88819a1a2a9..201d45b1677 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -2437,3 +2437,60 @@ int sequencer_make_script(int keep_empty, FILE *out,
> strbuf_release(&buf);
> 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(&todo_list.buf);
> + fd = open(todo_file, O_RDONLY);
> + if (fd < 0)
> + return error_errno(_("could not open '%s'"), todo_file);
> + if (strbuf_read(&todo_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, &todo_list);
> + if (res) {
> + todo_list_release(&todo_list);
> + return error(_("unusable instruction sheet: '%s'"), todo_file);
As you pointed out last time, the name of the "todo script" can be a
source of confusion. The migration to C could be a good opportunity to
clarify this.
I don't know which is the preferred name but we could go with
"todo list" as it is the most common across the code base.
$ git grep 'todo[ -]list' | wc -l
20
$ git grep 'rebase[ -]script' | wc -l
0
$ git grep 'instruction[ -]list' | wc -l
1
$ git grep 'instruction[ -]sheet' | wc -l
20
$ git grep 'instruction[ -]sheet' | grep -v ^po | wc -l
8
> + }
> +
> + out = fopen(todo_file, "w");
> + if (!out) {
> + todo_list_release(&todo_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 {
> + const char *sha1 = shorten_sha1s ?
> + short_commit_name(item->commit) :
> + oid_to_hex(&item->commit->object.oid);
We could also use 'hash' or 'ids' here instead of 'sha1'.
> + int len;
> +
> + p += strspn(p, " \t"); /* left-trim command */
> + len = strcspn(p, " \t"); /* length of command */
> +
> + fprintf(out, "%.*s %s %.*s\n",
> + len, p, sha1, item->arg_len, item->arg);
> + }
> + }
> + fclose(out);
> + todo_list_release(&todo_list);
> + return 0;
> +}
> diff --git a/sequencer.h b/sequencer.h
> index 83f2943b7a9..47a81034e76 100644
> --- a/sequencer.h
> +++ b/sequencer.h
> @@ -48,6 +48,8 @@ int sequencer_remove_state(struct replay_opts *opts);
> int sequencer_make_script(int keep_empty, FILE *out,
> int argc, const char **argv);
>
> +int transform_todo_ids(int shorten_sha1s);
> +
> extern const char sign_off_header[];
>
> void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag);
> --
> 2.12.2.windows.2.800.gede8f145e06
Liam