[GSoC][PATCH v3 05/13] sequencer: add a new function to silence a command, except if it fails

2018-07-10 Thread Alban Gruin
This adds a new function, run_command_silent_on_success(), to
redirect the stdout and stderr of a command to a strbuf, and then to run
that command. This strbuf is printed only if the command fails. It is
functionnaly similar to output() from git-rebase.sh.

run_git_commit() is then refactored to use of
run_command_silent_on_success().

Signed-off-by: Alban Gruin 
---
 sequencer.c | 51 +--
 1 file changed, 25 insertions(+), 26 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 57fd58bc1..1b5d50298 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -768,6 +768,23 @@ N_("you have staged changes in your working tree\n"
 #define VERIFY_MSG  (1<<4)
 #define CREATE_ROOT_COMMIT (1<<5)
 
+static int run_command_silent_on_success(struct child_process *cmd)
+{
+   struct strbuf buf = STRBUF_INIT;
+   int rc;
+
+   cmd->stdout_to_stderr = 1;
+   rc = pipe_command(cmd,
+ NULL, 0,
+ NULL, 0,
+ , 0);
+
+   if (rc)
+   fputs(buf.buf, stderr);
+   strbuf_release();
+   return rc;
+}
+
 /*
  * If we are cherry-pick, and if the merge did not result in
  * hand-editing, we will hit this commit and inherit the original
@@ -822,18 +839,11 @@ static int run_git_commit(const char *defmsg, struct 
replay_opts *opts,
 
cmd.git_cmd = 1;
 
-   if (is_rebase_i(opts)) {
-   if (!(flags & EDIT_MSG)) {
-   cmd.stdout_to_stderr = 1;
-   cmd.err = -1;
-   }
-
-   if (read_env_script(_array)) {
-   const char *gpg_opt = gpg_sign_opt_quoted(opts);
+   if (is_rebase_i(opts) && read_env_script(_array)) {
+   const char *gpg_opt = gpg_sign_opt_quoted(opts);
 
-   return error(_(staged_changes_advice),
-gpg_opt, gpg_opt);
-   }
+   return error(_(staged_changes_advice),
+gpg_opt, gpg_opt);
}
 
argv_array_push(, "commit");
@@ -863,21 +873,10 @@ static int run_git_commit(const char *defmsg, struct 
replay_opts *opts,
if (opts->allow_empty_message)
argv_array_push(, "--allow-empty-message");
 
-   if (cmd.err == -1) {
-   /* hide stderr on success */
-   struct strbuf buf = STRBUF_INIT;
-   int rc = pipe_command(,
- NULL, 0,
- /* stdout is already redirected */
- NULL, 0,
- , 0);
-   if (rc)
-   fputs(buf.buf, stderr);
-   strbuf_release();
-   return rc;
-   }
-
-   return run_command();
+   if (is_rebase_i(opts) && !(flags & EDIT_MSG))
+   return run_command_silent_on_success();
+   else
+   return run_command();
 }
 
 static int rest_is_empty(const struct strbuf *sb, int start)
-- 
2.18.0



[GSoC][PATCH v3 03/13] editor: add a function to launch the sequence editor

2018-07-10 Thread Alban Gruin
As part of the rewrite of interactive rebase, the sequencer will need to
open the sequence editor to allow the user to edit the todo list.
Instead of duplicating the existing launch_editor() function, this
refactors it to a new function, launch_specified_editor(), which takes
the editor as a parameter, in addition to the path, the buffer and the
environment variables.  launch_sequence_editor() is then added to launch
the sequence editor.

Signed-off-by: Alban Gruin 
---
 cache.h  |  1 +
 editor.c | 27 +--
 strbuf.h |  2 ++
 3 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/cache.h b/cache.h
index d49092d94..33fa70f55 100644
--- a/cache.h
+++ b/cache.h
@@ -1472,6 +1472,7 @@ extern const char *fmt_name(const char *name, const char 
*email);
 extern const char *ident_default_name(void);
 extern const char *ident_default_email(void);
 extern const char *git_editor(void);
+extern const char *git_sequence_editor(void);
 extern const char *git_pager(int stdout_is_tty);
 extern int is_terminal_dumb(void);
 extern int git_ident_config(const char *, const char *, void *);
diff --git a/editor.c b/editor.c
index 9a9b4e12d..c985eee1f 100644
--- a/editor.c
+++ b/editor.c
@@ -1,4 +1,5 @@
 #include "cache.h"
+#include "config.h"
 #include "strbuf.h"
 #include "run-command.h"
 #include "sigchain.h"
@@ -34,10 +35,21 @@ const char *git_editor(void)
return editor;
 }
 
-int launch_editor(const char *path, struct strbuf *buffer, const char *const 
*env)
+const char *git_sequence_editor(void)
 {
-   const char *editor = git_editor();
+   const char *editor = getenv("GIT_SEQUENCE_EDITOR");
+
+   if (!editor)
+   git_config_get_string_const("sequence.editor", );
+   if (!editor)
+   editor = git_editor();
 
+   return editor;
+}
+
+static int launch_specified_editor(const char *editor, const char *path,
+  struct strbuf *buffer, const char *const 
*env)
+{
if (!editor)
return error("Terminal is dumb, but EDITOR unset");
 
@@ -95,3 +107,14 @@ int launch_editor(const char *path, struct strbuf *buffer, 
const char *const *en
return error_errno("could not read file '%s'", path);
return 0;
 }
+
+int launch_editor(const char *path, struct strbuf *buffer, const char *const 
*env)
+{
+   return launch_specified_editor(git_editor(), path, buffer, env);
+}
+
+int launch_sequence_editor(const char *path, struct strbuf *buffer,
+  const char *const *env)
+{
+   return launch_specified_editor(git_sequence_editor(), path, buffer, 
env);
+}
diff --git a/strbuf.h b/strbuf.h
index 60a35aef1..66da9822f 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -575,6 +575,8 @@ extern void strbuf_add_unique_abbrev(struct strbuf *sb,
  * file's contents are not read into the buffer upon completion.
  */
 extern int launch_editor(const char *path, struct strbuf *buffer, const char 
*const *env);
+extern int launch_sequence_editor(const char *path, struct strbuf *buffer,
+ const char *const *env);
 
 extern void strbuf_add_lines(struct strbuf *sb, const char *prefix, const char 
*buf, size_t size);
 
-- 
2.18.0



Re: [GSoC][PATCH v2 1/7] sequencer: make two functions and an enum from sequencer.c public

2018-07-05 Thread Alban Gruin
Hi Junio,

Le 03/07/2018 à 22:20, Junio C Hamano a écrit :
> Alban Gruin  writes:
> 
>> -enum check_level {
>> -CHECK_IGNORE = 0, CHECK_WARN, CHECK_ERROR
>> -};
>> -
>> -static enum check_level get_missing_commit_check_level(void)
>> +enum missing_commit_check_level get_missing_commit_check_level(void)
> 
> The new name definitely is better than "check_level" in the global
> context, but "missing_commit" is much less important thing to say
> than "this symbol is to be used when driving 'rebase' (or even
> 'rebase-i')", I think.  "enum rebase_i_drop_commit_check" with
> "get_rebase_i_drop_commit_check()" perhaps?
> 

I don’t really like those names, but the function and the enum should
eventually move to rebase-interactive.c and become static again, so we
could revert their names in due course.

Cheers,
Alban



[GSoC] GSoC with git, week 9

2018-07-02 Thread Alban Gruin
Hi,

I just published a blog post about last week:

https://blog.pa1ch.fr/posts/2018/07/02/en/gsoc2018-week-9.html

Cheers,
Alban


[GSoC][PATCH v2 7/7] rebase -i: rewrite checkout_onto() in C

2018-07-02 Thread Alban Gruin
This rewrites checkout_onto() from shell to C.

A new command (“checkout-onto”) is added to rebase--helper.c. The shell
version is then stripped.

Signed-off-by: Alban Gruin 
---
 builtin/rebase--helper.c   |  7 ++-
 git-rebase--interactive.sh | 25 -
 sequencer.c| 19 +++
 sequencer.h|  3 +++
 4 files changed, 32 insertions(+), 22 deletions(-)

diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index 6c789e78b..0091e094b 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -18,7 +18,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
enum {
CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,
-   ADD_EXEC, APPEND_TODO_HELP, EDIT_TODO, PREPARE_BRANCH
+   ADD_EXEC, APPEND_TODO_HELP, EDIT_TODO, PREPARE_BRANCH,
+   CHECKOUT_ONTO
} command = 0;
struct option options[] = {
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
@@ -54,6 +55,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
EDIT_TODO),
OPT_CMDMODE(0, "prepare-branch", ,
N_("prepare the branch to be rebased"), 
PREPARE_BRANCH),
+   OPT_CMDMODE(0, "checkout-onto", ,
+   N_("checkout a commit"), CHECKOUT_ONTO),
OPT_END()
};
 
@@ -99,5 +102,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
return !!edit_todo_list(flags);
if (command == PREPARE_BRANCH && argc == 2)
return !!prepare_branch_to_be_rebased(, argv[1], verbose);
+   if (command == CHECKOUT_ONTO && argc == 4)
+   return !!checkout_onto(, argv[1], argv[2], argv[3], 
verbose);
usage_with_options(builtin_rebase_helper_usage, options);
 }
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 77e972bb6..b68f108f2 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -28,17 +28,6 @@ case "$comment_char" in
;;
 esac
 
-orig_reflog_action="$GIT_REFLOG_ACTION"
-
-comment_for_reflog () {
-   case "$orig_reflog_action" in
-   ''|rebase*)
-   GIT_REFLOG_ACTION="rebase -i ($1)"
-   export GIT_REFLOG_ACTION
-   ;;
-   esac
-}
-
 die_abort () {
apply_autostash
rm -rf "$state_dir"
@@ -70,14 +59,6 @@ collapse_todo_ids() {
git rebase--helper --shorten-ids
 }
 
-# Switch to the branch in $into and notify it in the reflog
-checkout_onto () {
-   comment_for_reflog start
-   GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $onto_name"
-   output git checkout $onto || die_abort "$(gettext "could not detach 
HEAD")"
-   git update-ref ORIG_HEAD $orig_head
-}
-
 get_missing_commit_check_level () {
check_level=$(git config --get rebase.missingCommitsCheck)
check_level=${check_level:-ignore}
@@ -176,7 +157,8 @@ EOF
 
git rebase--helper --check-todo-list || {
ret=$?
-   checkout_onto
+   git rebase--helper --checkout-onto "$onto_name" "$onto" \
+   "$orig_head" ${verbose:+--verbose}
exit $ret
}
 
@@ -186,7 +168,8 @@ EOF
onto="$(git rebase--helper --skip-unnecessary-picks)" ||
die "Could not skip unnecessary pick commands"
 
-   checkout_onto
+   git rebase--helper --checkout-onto "$onto_name" "$onto" "$orig_head" \
+   ${verbose:+--verbose}
require_clean_work_tree "rebase"
exec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \
 --continue
diff --git a/sequencer.c b/sequencer.c
index a61276d2c..bc0836ac9 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3162,6 +3162,25 @@ int prepare_branch_to_be_rebased(struct replay_opts 
*opts, const char *commit,
return 0;
 }
 
+int checkout_onto(struct replay_opts *opts,
+ const char *onto_name, const char *onto,
+ const char *orig_head, unsigned verbose)
+{
+   struct object_id oid;
+   const char *action = reflog_message(opts, "start", "checkout %s", 
onto_name);
+
+   if (get_oid(orig_head, ))
+   return error(_("%s: not a valid OID"), orig_head);
+
+   if (run_git_checkout(opts, onto, verbose, action)) {
+   apply_autostash(opts);
+   sequencer_remove_state(opts);
+   return error(_("could not detach HEAD"));
+   }
+
+   return up

[GSoC][PATCH v2 6/7] rebase -i: rewrite setup_reflog_action() in C

2018-07-02 Thread Alban Gruin
This rewrites (the misnamed) setup_reflog_action() from shell to C. The
new version is called prepare_branch_to_be_rebased().

A new command is added to rebase--helper.c, “checkout-base”, as well as
a new flag, “verbose”, to avoid silencing the output of the checkout
operation called by checkout_base_commit().

The function `run_git_checkout()` will also be used in the next commit,
therefore its code is not part of `checkout_base_commit()`.

The shell version is then stripped in favour of a call to the helper.

As $GIT_REFLOG_ACTION is no longer set at the first call of
checkout_onto(), a call to comment_for_reflog() is added at the
beginning of this function.

Signed-off-by: Alban Gruin 
---
 builtin/rebase--helper.c   |  9 +++--
 git-rebase--interactive.sh | 16 ++--
 sequencer.c| 30 ++
 sequencer.h|  3 +++
 4 files changed, 42 insertions(+), 16 deletions(-)

diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index 731a64971..6c789e78b 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -13,12 +13,12 @@ static const char * const builtin_rebase_helper_usage[] = {
 int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
 {
struct replay_opts opts = REPLAY_OPTS_INIT;
-   unsigned flags = 0, keep_empty = 0, rebase_merges = 0;
+   unsigned flags = 0, keep_empty = 0, rebase_merges = 0, verbose = 0;
int abbreviate_commands = 0, rebase_cousins = -1;
enum {
CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,
-   ADD_EXEC, APPEND_TODO_HELP, EDIT_TODO
+   ADD_EXEC, APPEND_TODO_HELP, EDIT_TODO, PREPARE_BRANCH
} command = 0;
struct option options[] = {
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
@@ -28,6 +28,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
OPT_BOOL(0, "rebase-merges", _merges, N_("rebase merge 
commits")),
OPT_BOOL(0, "rebase-cousins", _cousins,
 N_("keep original branch points of cousins")),
+   OPT__VERBOSE(, N_("be verbose")),
OPT_CMDMODE(0, "continue", , N_("continue rebase"),
CONTINUE),
OPT_CMDMODE(0, "abort", , N_("abort rebase"),
@@ -51,6 +52,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
OPT_CMDMODE(0, "edit-todo", ,
N_("edit the todo list during an interactive 
rebase"),
EDIT_TODO),
+   OPT_CMDMODE(0, "prepare-branch", ,
+   N_("prepare the branch to be rebased"), 
PREPARE_BRANCH),
OPT_END()
};
 
@@ -94,5 +97,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
return !!append_todo_help(0, keep_empty);
if (command == EDIT_TODO && argc == 1)
return !!edit_todo_list(flags);
+   if (command == PREPARE_BRANCH && argc == 2)
+   return !!prepare_branch_to_be_rebased(, argv[1], verbose);
usage_with_options(builtin_rebase_helper_usage, options);
 }
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 2defe607f..77e972bb6 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -72,6 +72,7 @@ collapse_todo_ids() {
 
 # Switch to the branch in $into and notify it in the reflog
 checkout_onto () {
+   comment_for_reflog start
GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $onto_name"
output git checkout $onto || die_abort "$(gettext "could not detach 
HEAD")"
git update-ref ORIG_HEAD $orig_head
@@ -119,19 +120,6 @@ initiate_action () {
esac
 }
 
-setup_reflog_action () {
-   comment_for_reflog start
-
-   if test ! -z "$switch_to"
-   then
-   GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $switch_to"
-   output git checkout "$switch_to" -- ||
-   die "$(eval_gettext "Could not checkout \$switch_to")"
-
-   comment_for_reflog start
-   fi
-}
-
 init_basic_state () {
orig_head=$(git rev-parse --verify HEAD) || die "$(gettext "No HEAD?")"
mkdir -p "$state_dir" || die "$(eval_gettext "Could not create 
temporary \$state_dir")"
@@ -211,7 +199,7 @@ git_rebase__interactive () {
return 0
fi
 
-   setup_reflog_action
+   git rebase--helper --prepare-branch "$switch_to" ${verbose:+--verbo

[GSoC][PATCH v2 4/7] rebase-interactive: rewrite the edit-todo functionality in C

2018-07-02 Thread Alban Gruin
This rewrites the edit-todo functionality from shell to C.

To achieve that, a new command mode, `edit-todo`, is added, and the
`write-edit-todo` flag is removed, as the shell script does not need to
write the edit todo help message to the todo list anymore.

The shell version is then stripped in favour of a call to the helper.

Signed-off-by: Alban Gruin 
---
 builtin/rebase--helper.c   | 13 -
 git-rebase--interactive.sh | 11 +--
 rebase-interactive.c   | 31 +++
 rebase-interactive.h   |  1 +
 4 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index 05e73e71d..731a64971 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -13,12 +13,12 @@ static const char * const builtin_rebase_helper_usage[] = {
 int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
 {
struct replay_opts opts = REPLAY_OPTS_INIT;
-   unsigned flags = 0, keep_empty = 0, rebase_merges = 0, write_edit_todo 
= 0;
+   unsigned flags = 0, keep_empty = 0, rebase_merges = 0;
int abbreviate_commands = 0, rebase_cousins = -1;
enum {
CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,
-   ADD_EXEC, APPEND_TODO_HELP
+   ADD_EXEC, APPEND_TODO_HELP, EDIT_TODO
} command = 0;
struct option options[] = {
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
@@ -28,8 +28,6 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
OPT_BOOL(0, "rebase-merges", _merges, N_("rebase merge 
commits")),
OPT_BOOL(0, "rebase-cousins", _cousins,
 N_("keep original branch points of cousins")),
-   OPT_BOOL(0, "write-edit-todo", _edit_todo,
-N_("append the edit-todo message to the todo-list")),
OPT_CMDMODE(0, "continue", , N_("continue rebase"),
CONTINUE),
OPT_CMDMODE(0, "abort", , N_("abort rebase"),
@@ -50,6 +48,9 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
N_("insert exec commands in todo list"), ADD_EXEC),
OPT_CMDMODE(0, "append-todo-help", ,
N_("insert the help in the todo list"), 
APPEND_TODO_HELP),
+   OPT_CMDMODE(0, "edit-todo", ,
+   N_("edit the todo list during an interactive 
rebase"),
+   EDIT_TODO),
OPT_END()
};
 
@@ -90,6 +91,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
if (command == ADD_EXEC && argc == 2)
return !!sequencer_add_exec_commands(argv[1]);
if (command == APPEND_TODO_HELP && argc == 1)
-   return !!append_todo_help(write_edit_todo, keep_empty);
+   return !!append_todo_help(0, keep_empty);
+   if (command == EDIT_TODO && argc == 1)
+   return !!edit_todo_list(flags);
usage_with_options(builtin_rebase_helper_usage, options);
 }
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 94c23a7af..2defe607f 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -108,16 +108,7 @@ initiate_action () {
 --continue
;;
edit-todo)
-   git stripspace --strip-comments <"$todo" >"$todo".new
-   mv -f "$todo".new "$todo"
-   collapse_todo_ids
-   git rebase--helper --append-todo-help --write-edit-todo
-
-   git_sequence_editor "$todo" ||
-   die "$(gettext "Could not execute editor")"
-   expand_todo_ids
-
-   exit
+   exec git rebase--helper --edit-todo
;;
show-current-patch)
exec git show REBASE_HEAD --
diff --git a/rebase-interactive.c b/rebase-interactive.c
index 015e08cd0..fb7ad401a 100644
--- a/rebase-interactive.c
+++ b/rebase-interactive.c
@@ -66,3 +66,34 @@ int append_todo_help(unsigned edit_todo, unsigned keep_empty)
 
return ret;
 }
+
+int edit_todo_list(unsigned flags)
+{
+   struct strbuf buf = STRBUF_INIT;
+   const char *todo_file = rebase_path_todo();
+   FILE *todo;
+
+   if (strbuf_read_file(, todo_file, 0) < 0)
+   return error_errno(_("could not read '%s'."), todo_file);
+
+   strbuf_stripspace(, 1);
+   todo = fopen_or_warn(todo_file, "w");
+   if (!todo) {
+

[GSoC][PATCH v2 3/7] editor: add a function to launch the sequence editor

2018-07-02 Thread Alban Gruin
As part of the rewrite of interactive rebase, the sequencer will need to
open the sequence editor to allow the user to edit the todo list.
Instead of duplicating the existing launch_editor() function, this
refactors it to a new function, launch_specified_editor(), which takes
the editor as a parameter, in addition to the path, the buffer and the
environment variables.  launch_sequence_editor() is then added to launch
the sequence editor.

Signed-off-by: Alban Gruin 
---
 cache.h  |  1 +
 editor.c | 27 +--
 strbuf.h |  2 ++
 3 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/cache.h b/cache.h
index d49092d94..33fa70f55 100644
--- a/cache.h
+++ b/cache.h
@@ -1472,6 +1472,7 @@ extern const char *fmt_name(const char *name, const char 
*email);
 extern const char *ident_default_name(void);
 extern const char *ident_default_email(void);
 extern const char *git_editor(void);
+extern const char *git_sequence_editor(void);
 extern const char *git_pager(int stdout_is_tty);
 extern int is_terminal_dumb(void);
 extern int git_ident_config(const char *, const char *, void *);
diff --git a/editor.c b/editor.c
index 9a9b4e12d..c985eee1f 100644
--- a/editor.c
+++ b/editor.c
@@ -1,4 +1,5 @@
 #include "cache.h"
+#include "config.h"
 #include "strbuf.h"
 #include "run-command.h"
 #include "sigchain.h"
@@ -34,10 +35,21 @@ const char *git_editor(void)
return editor;
 }
 
-int launch_editor(const char *path, struct strbuf *buffer, const char *const 
*env)
+const char *git_sequence_editor(void)
 {
-   const char *editor = git_editor();
+   const char *editor = getenv("GIT_SEQUENCE_EDITOR");
+
+   if (!editor)
+   git_config_get_string_const("sequence.editor", );
+   if (!editor)
+   editor = git_editor();
 
+   return editor;
+}
+
+static int launch_specified_editor(const char *editor, const char *path,
+  struct strbuf *buffer, const char *const 
*env)
+{
if (!editor)
return error("Terminal is dumb, but EDITOR unset");
 
@@ -95,3 +107,14 @@ int launch_editor(const char *path, struct strbuf *buffer, 
const char *const *en
return error_errno("could not read file '%s'", path);
return 0;
 }
+
+int launch_editor(const char *path, struct strbuf *buffer, const char *const 
*env)
+{
+   return launch_specified_editor(git_editor(), path, buffer, env);
+}
+
+int launch_sequence_editor(const char *path, struct strbuf *buffer,
+  const char *const *env)
+{
+   return launch_specified_editor(git_sequence_editor(), path, buffer, 
env);
+}
diff --git a/strbuf.h b/strbuf.h
index 60a35aef1..66da9822f 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -575,6 +575,8 @@ extern void strbuf_add_unique_abbrev(struct strbuf *sb,
  * file's contents are not read into the buffer upon completion.
  */
 extern int launch_editor(const char *path, struct strbuf *buffer, const char 
*const *env);
+extern int launch_sequence_editor(const char *path, struct strbuf *buffer,
+ const char *const *env);
 
 extern void strbuf_add_lines(struct strbuf *sb, const char *prefix, const char 
*buf, size_t size);
 
-- 
2.18.0



[GSoC][PATCH v2 5/7] sequencer: add a new function to silence a command, except if it fails.

2018-07-02 Thread Alban Gruin
This adds a new function, run_command_silent_on_success(), to
redirect the stdout and stderr of a command to a strbuf, and then to run
that command. This strbuf is printed only if the command fails. It is
functionnaly similar to output() from git-rebase.sh.

run_git_commit() is then refactored to use of
run_command_silent_on_success().

Signed-off-by: Alban Gruin 
---
 sequencer.c | 49 -
 1 file changed, 24 insertions(+), 25 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 57fd58bc1..9e2b34a49 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -768,6 +768,24 @@ N_("you have staged changes in your working tree\n"
 #define VERIFY_MSG  (1<<4)
 #define CREATE_ROOT_COMMIT (1<<5)
 
+static int run_command_silent_on_success(struct child_process *cmd)
+{
+   struct strbuf buf = STRBUF_INIT;
+   int rc;
+
+   cmd->stdout_to_stderr = 1;
+   rc = pipe_command(cmd,
+ NULL, 0,
+ /* stdout is already redirected */
+ NULL, 0,
+ , 0);
+
+   if (rc)
+   fputs(buf.buf, stderr);
+   strbuf_release();
+   return rc;
+}
+
 /*
  * If we are cherry-pick, and if the merge did not result in
  * hand-editing, we will hit this commit and inherit the original
@@ -822,18 +840,11 @@ static int run_git_commit(const char *defmsg, struct 
replay_opts *opts,
 
cmd.git_cmd = 1;
 
-   if (is_rebase_i(opts)) {
-   if (!(flags & EDIT_MSG)) {
-   cmd.stdout_to_stderr = 1;
-   cmd.err = -1;
-   }
+   if (is_rebase_i(opts) && read_env_script(_array)) {
+   const char *gpg_opt = gpg_sign_opt_quoted(opts);
 
-   if (read_env_script(_array)) {
-   const char *gpg_opt = gpg_sign_opt_quoted(opts);
-
-   return error(_(staged_changes_advice),
-gpg_opt, gpg_opt);
-   }
+   return error(_(staged_changes_advice),
+gpg_opt, gpg_opt);
}
 
argv_array_push(, "commit");
@@ -863,20 +874,8 @@ static int run_git_commit(const char *defmsg, struct 
replay_opts *opts,
if (opts->allow_empty_message)
argv_array_push(, "--allow-empty-message");
 
-   if (cmd.err == -1) {
-   /* hide stderr on success */
-   struct strbuf buf = STRBUF_INIT;
-   int rc = pipe_command(,
- NULL, 0,
- /* stdout is already redirected */
- NULL, 0,
- , 0);
-   if (rc)
-   fputs(buf.buf, stderr);
-   strbuf_release();
-   return rc;
-   }
-
+   if (is_rebase_i(opts) && !(flags & EDIT_MSG))
+   return run_command_silent_on_success();
return run_command();
 }
 
-- 
2.18.0



[GSoC][PATCH v2 2/7] rebase--interactive: rewrite append_todo_help() in C

2018-07-02 Thread Alban Gruin
This rewrites append_todo_help() from shell to C. It also incorporates
some parts of initiate_action() and complete_action() that also write
help texts to the todo file.

This also introduces the source file rebase-interactive.c. This file
will contain functions necessary for interactive rebase that are too
specific for the sequencer, and is part of libgit.a.

Two flags are added to rebase--helper.c: one to call
append_todo_help() (`--append-todo-help`), and another one to tell
append_todo_help() to write the help text suited for the edit-todo
mode (`--write-edit-todo`).

Finally, append_todo_help() is removed from git-rebase--interactive.sh
to use `rebase--helper --append-todo-help` instead.

Signed-off-by: Alban Gruin 
---
 Makefile   |  1 +
 builtin/rebase--helper.c   | 11 --
 git-rebase--interactive.sh | 52 ++---
 rebase-interactive.c   | 68 ++
 rebase-interactive.h   |  6 
 5 files changed, 86 insertions(+), 52 deletions(-)
 create mode 100644 rebase-interactive.c
 create mode 100644 rebase-interactive.h

diff --git a/Makefile b/Makefile
index 0cb6590f2..a281139ef 100644
--- a/Makefile
+++ b/Makefile
@@ -922,6 +922,7 @@ LIB_OBJS += protocol.o
 LIB_OBJS += quote.o
 LIB_OBJS += reachable.o
 LIB_OBJS += read-cache.o
+LIB_OBJS += rebase-interactive.o
 LIB_OBJS += reflog-walk.o
 LIB_OBJS += refs.o
 LIB_OBJS += refs/files-backend.o
diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index f7c2a5fdc..05e73e71d 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -3,6 +3,7 @@
 #include "config.h"
 #include "parse-options.h"
 #include "sequencer.h"
+#include "rebase-interactive.h"
 
 static const char * const builtin_rebase_helper_usage[] = {
N_("git rebase--helper []"),
@@ -12,12 +13,12 @@ static const char * const builtin_rebase_helper_usage[] = {
 int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
 {
struct replay_opts opts = REPLAY_OPTS_INIT;
-   unsigned flags = 0, keep_empty = 0, rebase_merges = 0;
+   unsigned flags = 0, keep_empty = 0, rebase_merges = 0, write_edit_todo 
= 0;
int abbreviate_commands = 0, rebase_cousins = -1;
enum {
CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,
-   ADD_EXEC
+   ADD_EXEC, APPEND_TODO_HELP
} command = 0;
struct option options[] = {
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
@@ -27,6 +28,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
OPT_BOOL(0, "rebase-merges", _merges, N_("rebase merge 
commits")),
OPT_BOOL(0, "rebase-cousins", _cousins,
 N_("keep original branch points of cousins")),
+   OPT_BOOL(0, "write-edit-todo", _edit_todo,
+N_("append the edit-todo message to the todo-list")),
OPT_CMDMODE(0, "continue", , N_("continue rebase"),
CONTINUE),
OPT_CMDMODE(0, "abort", , N_("abort rebase"),
@@ -45,6 +48,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
N_("rearrange fixup/squash lines"), REARRANGE_SQUASH),
OPT_CMDMODE(0, "add-exec-commands", ,
N_("insert exec commands in todo list"), ADD_EXEC),
+   OPT_CMDMODE(0, "append-todo-help", ,
+   N_("insert the help in the todo list"), 
APPEND_TODO_HELP),
OPT_END()
};
 
@@ -84,5 +89,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
return !!rearrange_squash();
if (command == ADD_EXEC && argc == 2)
return !!sequencer_add_exec_commands(argv[1]);
+   if (command == APPEND_TODO_HELP && argc == 1)
+   return !!append_todo_help(write_edit_todo, keep_empty);
usage_with_options(builtin_rebase_helper_usage, options);
 }
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 299ded213..94c23a7af 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -39,38 +39,6 @@ comment_for_reflog () {
esac
 }
 
-append_todo_help () {
-   gettext "
-Commands:
-p, pick  = use commit
-r, reword  = use commit, but edit the commit message
-e, edit  = use commit, but stop for amending
-s, squash  = use commit, but meld into previous commit
-f, fixup  = like \"squash\", but discard this commit's log message
-x, exec  = run command (the rest of the line) using shell
-d, drop

[GSoC][PATCH v2 1/7] sequencer: make two functions and an enum from sequencer.c public

2018-07-02 Thread Alban Gruin
This makes rebase_path_todo(), get_missing_commit_check_level() and the
enum check_level accessible outside sequencer.c.  check_level is renamed
missing_commit_check_level, and its value names are prefixed by
MISSING_COMMIT_ to avoid namespace pollution.

This will be needed for the rewrite of append_todo_help() from shell to
C, as it will be in a new library source file, rebase-interactive.c.

Signed-off-by: Alban Gruin 
---
 sequencer.c | 22 +-
 sequencer.h |  8 
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 5354d4d51..57fd58bc1 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -51,7 +51,7 @@ static GIT_PATH_FUNC(rebase_path, "rebase-merge")
  * the lines are processed, they are removed from the front of this
  * file and written to the tail of 'done'.
  */
-static GIT_PATH_FUNC(rebase_path_todo, "rebase-merge/git-rebase-todo")
+GIT_PATH_FUNC(rebase_path_todo, "rebase-merge/git-rebase-todo")
 /*
  * The rebase command lines that have already been processed. A line
  * is moved here when it is first handled, before any associated user
@@ -4221,24 +4221,20 @@ int transform_todos(unsigned flags)
return i;
 }
 
-enum check_level {
-   CHECK_IGNORE = 0, CHECK_WARN, CHECK_ERROR
-};
-
-static enum check_level get_missing_commit_check_level(void)
+enum missing_commit_check_level get_missing_commit_check_level(void)
 {
const char *value;
 
if (git_config_get_value("rebase.missingcommitscheck", ) ||
!strcasecmp("ignore", value))
-   return CHECK_IGNORE;
+   return MISSING_COMMIT_CHECK_IGNORE;
if (!strcasecmp("warn", value))
-   return CHECK_WARN;
+   return MISSING_COMMIT_CHECK_WARN;
if (!strcasecmp("error", value))
-   return CHECK_ERROR;
+   return MISSING_COMMIT_CHECK_ERROR;
warning(_("unrecognized setting %s for option "
  "rebase.missingCommitsCheck. Ignoring."), value);
-   return CHECK_IGNORE;
+   return MISSING_COMMIT_CHECK_IGNORE;
 }
 
 define_commit_slab(commit_seen, unsigned char);
@@ -4250,7 +4246,7 @@ define_commit_slab(commit_seen, unsigned char);
  */
 int check_todo_list(void)
 {
-   enum check_level check_level = get_missing_commit_check_level();
+   enum missing_commit_check_level check_level = 
get_missing_commit_check_level();
struct strbuf todo_file = STRBUF_INIT;
struct todo_list todo_list = TODO_LIST_INIT;
struct strbuf missing = STRBUF_INIT;
@@ -4267,7 +4263,7 @@ int check_todo_list(void)
advise_to_edit_todo = res =
parse_insn_buffer(todo_list.buf.buf, _list);
 
-   if (res || check_level == CHECK_IGNORE)
+   if (res || check_level == MISSING_COMMIT_CHECK_IGNORE)
goto leave_check;
 
/* Mark the commits in git-rebase-todo as seen */
@@ -4302,7 +4298,7 @@ int check_todo_list(void)
if (!missing.len)
goto leave_check;
 
-   if (check_level == CHECK_ERROR)
+   if (check_level == MISSING_COMMIT_CHECK_ERROR)
advise_to_edit_todo = res = 1;
 
fprintf(stderr,
diff --git a/sequencer.h b/sequencer.h
index c5787c6b5..ffab798f1 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -3,6 +3,7 @@
 
 const char *git_path_commit_editmsg(void);
 const char *git_path_seq_dir(void);
+const char *rebase_path_todo(void);
 
 #define APPEND_SIGNOFF_DEDUP (1u << 0)
 
@@ -57,6 +58,12 @@ struct replay_opts {
 };
 #define REPLAY_OPTS_INIT { .action = -1, .current_fixups = STRBUF_INIT }
 
+enum missing_commit_check_level {
+   MISSING_COMMIT_CHECK_IGNORE = 0,
+   MISSING_COMMIT_CHECK_WARN,
+   MISSING_COMMIT_CHECK_ERROR
+};
+
 /* Call this to setup defaults before parsing command line options */
 void sequencer_init_config(struct replay_opts *opts);
 int sequencer_pick_revisions(struct replay_opts *opts);
@@ -79,6 +86,7 @@ int sequencer_make_script(FILE *out, int argc, const char 
**argv,
 
 int sequencer_add_exec_commands(const char *command);
 int transform_todos(unsigned flags);
+enum missing_commit_check_level get_missing_commit_check_level(void);
 int check_todo_list(void);
 int skip_unnecessary_picks(void);
 int rearrange_squash(void);
-- 
2.18.0



[GSoC][PATCH v2 0/7] rebase -i: rewrite some parts in C

2018-07-02 Thread Alban Gruin
This patch series rewrites some parts of interactive rebase from shell
to C:

 - append_todo_help(). The C version covers a bit more than the shell
   version.

 - The edit-todo functionnality.

 - The reflog operations.

The v1 of this series is an aggregate made by Junio of my patch series
about append_todo_help() (v3), edit-todo (v2) and reflog (v5), and can
be found in the branch `ag/rebase-i-in-c`.

This branch is based on master (as of 2018-07-02).

Changes since v1:

 - Introducing rebase-interactive.c to contain functions necessary for
   interactive rebase.

 - Show an error message when append_todo_help() fails to edit the todo
   list.

 - Renaming enumeration check_level and its values to avoid namespace
   pollution.

 - Moving append_todo_help() and edit_todo() from sequencer.c to
   interactive-rebase.c.

Alban Gruin (7):
  sequencer: make two functions and an enum from sequencer.c public
  rebase--interactive: rewrite append_todo_help() in C
  editor: add a function to launch the sequence editor
  rebase-interactive: rewrite the edit-todo functionality in C
  sequencer: add a new function to silence a command, except if it
fails.
  rebase -i: rewrite setup_reflog_action() in C
  rebase -i: rewrite checkout_onto() in C

 Makefile   |   1 +
 builtin/rebase--helper.c   |  24 +++-
 cache.h|   1 +
 editor.c   |  27 -
 git-rebase--interactive.sh | 100 +++
 rebase-interactive.c   |  99 ++
 rebase-interactive.h   |   7 +++
 sequencer.c| 120 +
 sequencer.h|  14 +
 strbuf.h   |   2 +
 10 files changed, 260 insertions(+), 135 deletions(-)
 create mode 100644 rebase-interactive.c
 create mode 100644 rebase-interactive.h

-- 
2.18.0



Re: [GSoC][PATCH v5 0/3] rebase -i: rewrite reflog operations in C

2018-07-02 Thread Alban Gruin
Hi Junio,

Le 29/06/2018 à 20:23, Junio C Hamano a écrit :
> Junio C Hamano  writes:
> 
>> Let's aggregate these topics into a single topic, and perhaps call
>> it ag/rebase-i-in-c or something like that.  Pretending as if they
>> are separately replaceable does not make much sense, as you are not
>> rerolling the earlier one and keep going forward with producing more
>> parts that depends on the parts that have been submitted earlier.
> 
> So here is what I tentatively did.
> 
> $ git log --oneline --reverse master..ag/rebase-i-in-c
> 4d303fb608 rebase--interactive: rewrite append_todo_help() in C
> b4ffe143a9 editor: add a function to launch the sequence editor
> 4ebe39cef9 rebase--interactive: rewrite the edit-todo functionality in C
> 0ff6bf7646 sequencer: add a new function to silence a command, except if 
> it fails.
> 36784b351f rebase -i: rewrite setup_reflog_action() in C
> 415cac57ee rebase -i: rewrite checkout_onto() in C
> 
> In several hours please fetch from me and look for "Merge branch
> 'ag/rebase-i-in-c' to pu" to see how they exactly look like; some of
> the patches might not be the latest ones, in which case you may need
> to prod me to get them replaced (resending them as a whole with
> incremented v$n header is probably the easiest if we need to do so).
> 
> Thanks.
> 

The patches about append_todo_help() and edit-todo are not up to date,
so I’ll resend them in a few minutes.  Otherwise, this looks good to me.

Cheers,
Alban



[GSoC][PATCH v5 2/3] rebase -i: rewrite setup_reflog_action() in C

2018-06-29 Thread Alban Gruin
This rewrites (the misnamed) setup_reflog_action() from shell to C. The
new version is called prepare_branch_to_be_rebased().

A new command is added to rebase--helper.c, “checkout-base”, as well as
a new flag, “verbose”, to avoid silencing the output of the checkout
operation called by checkout_base_commit().

The function `run_git_checkout()` will also be used in the next commit,
therefore its code is not part of `checkout_base_commit()`.

The shell version is then stripped in favour of a call to the helper.

As $GIT_REFLOG_ACTION is no longer set at the first call of
checkout_onto(), a call to comment_for_reflog() is added at the
beginning of this function.

Signed-off-by: Alban Gruin 
---
 builtin/rebase--helper.c   |  9 +++--
 git-rebase--interactive.sh | 16 ++--
 sequencer.c| 31 +++
 sequencer.h|  3 +++
 4 files changed, 43 insertions(+), 16 deletions(-)

diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index 731a64971..6c789e78b 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -13,12 +13,12 @@ static const char * const builtin_rebase_helper_usage[] = {
 int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
 {
struct replay_opts opts = REPLAY_OPTS_INIT;
-   unsigned flags = 0, keep_empty = 0, rebase_merges = 0;
+   unsigned flags = 0, keep_empty = 0, rebase_merges = 0, verbose = 0;
int abbreviate_commands = 0, rebase_cousins = -1;
enum {
CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,
-   ADD_EXEC, APPEND_TODO_HELP, EDIT_TODO
+   ADD_EXEC, APPEND_TODO_HELP, EDIT_TODO, PREPARE_BRANCH
} command = 0;
struct option options[] = {
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
@@ -28,6 +28,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
OPT_BOOL(0, "rebase-merges", _merges, N_("rebase merge 
commits")),
OPT_BOOL(0, "rebase-cousins", _cousins,
 N_("keep original branch points of cousins")),
+   OPT__VERBOSE(, N_("be verbose")),
OPT_CMDMODE(0, "continue", , N_("continue rebase"),
CONTINUE),
OPT_CMDMODE(0, "abort", , N_("abort rebase"),
@@ -51,6 +52,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
OPT_CMDMODE(0, "edit-todo", ,
N_("edit the todo list during an interactive 
rebase"),
EDIT_TODO),
+   OPT_CMDMODE(0, "prepare-branch", ,
+   N_("prepare the branch to be rebased"), 
PREPARE_BRANCH),
OPT_END()
};
 
@@ -94,5 +97,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
return !!append_todo_help(0, keep_empty);
if (command == EDIT_TODO && argc == 1)
return !!edit_todo_list(flags);
+   if (command == PREPARE_BRANCH && argc == 2)
+   return !!prepare_branch_to_be_rebased(, argv[1], verbose);
usage_with_options(builtin_rebase_helper_usage, options);
 }
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 2defe607f..77e972bb6 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -72,6 +72,7 @@ collapse_todo_ids() {
 
 # Switch to the branch in $into and notify it in the reflog
 checkout_onto () {
+   comment_for_reflog start
GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $onto_name"
output git checkout $onto || die_abort "$(gettext "could not detach 
HEAD")"
git update-ref ORIG_HEAD $orig_head
@@ -119,19 +120,6 @@ initiate_action () {
esac
 }
 
-setup_reflog_action () {
-   comment_for_reflog start
-
-   if test ! -z "$switch_to"
-   then
-   GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $switch_to"
-   output git checkout "$switch_to" -- ||
-   die "$(eval_gettext "Could not checkout \$switch_to")"
-
-   comment_for_reflog start
-   fi
-}
-
 init_basic_state () {
orig_head=$(git rev-parse --verify HEAD) || die "$(gettext "No HEAD?")"
mkdir -p "$state_dir" || die "$(eval_gettext "Could not create 
temporary \$state_dir")"
@@ -211,7 +199,7 @@ git_rebase__interactive () {
return 0
fi
 
-   setup_reflog_action
+   git rebase--helper --prepare-branch "$switch_to" ${verbose:+--verbo

[GSoC][PATCH v5 0/3] rebase -i: rewrite reflog operations in C

2018-06-29 Thread Alban Gruin
This patch series rewrites the reflog operations from shell to C.  This
is part of the effort to rewrite interactive rebase in C.

The first commit is dedicated to creating a function to silence a
command, as the sequencer will do in several places with these patches.

This branch is based on ag/rebase-i-rewrite-todo, and does not conflict
with pu (as of 2018-06-29).

Changes since v4:

 - Changing the order of setup_reflog_action() and checkout_onto()
   rewrites in the series

 - checkout_onto() is no longer renamed in C

 - setup_reflog_action() is renamed to prepare_branch_to_be_rebased(),
   and not to checkout_onto().

Alban Gruin (3):
  sequencer: add a new function to silence a command, except if it
fails.
  rebase -i: rewrite setup_reflog_action() in C
  rebase -i: rewrite checkout_onto() in C

 builtin/rebase--helper.c   |  14 -
 git-rebase--interactive.sh |  39 ++
 sequencer.c| 101 +++--
 sequencer.h|   6 +++
 4 files changed, 98 insertions(+), 62 deletions(-)

-- 
2.18.0



[GSoC][PATCH v5 1/3] sequencer: add a new function to silence a command, except if it fails.

2018-06-29 Thread Alban Gruin
This adds a new function, run_command_silent_on_success(), to
redirect the stdout and stderr of a command to a strbuf, and then to run
that command. This strbuf is printed only if the command fails. It is
functionnaly similar to output() from git-rebase.sh.

run_git_commit() is then refactored to use of
run_command_silent_on_success().

Signed-off-by: Alban Gruin 
---
 sequencer.c | 51 +--
 1 file changed, 25 insertions(+), 26 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index cb7ec9807..d9545b366 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -769,6 +769,23 @@ N_("you have staged changes in your working tree\n"
 #define VERIFY_MSG  (1<<4)
 #define CREATE_ROOT_COMMIT (1<<5)
 
+static int run_command_silent_on_success(struct child_process *cmd)
+{
+   struct strbuf buf = STRBUF_INIT;
+   int rc;
+
+   cmd->stdout_to_stderr = 1;
+   rc = pipe_command(cmd,
+ NULL, 0,
+ NULL, 0,
+ , 0);
+
+   if (rc)
+   fputs(buf.buf, stderr);
+   strbuf_release();
+   return rc;
+}
+
 /*
  * If we are cherry-pick, and if the merge did not result in
  * hand-editing, we will hit this commit and inherit the original
@@ -823,18 +840,11 @@ static int run_git_commit(const char *defmsg, struct 
replay_opts *opts,
 
cmd.git_cmd = 1;
 
-   if (is_rebase_i(opts)) {
-   if (!(flags & EDIT_MSG)) {
-   cmd.stdout_to_stderr = 1;
-   cmd.err = -1;
-   }
-
-   if (read_env_script(_array)) {
-   const char *gpg_opt = gpg_sign_opt_quoted(opts);
+   if (is_rebase_i(opts) && read_env_script(_array)) {
+   const char *gpg_opt = gpg_sign_opt_quoted(opts);
 
-   return error(_(staged_changes_advice),
-gpg_opt, gpg_opt);
-   }
+   return error(_(staged_changes_advice),
+gpg_opt, gpg_opt);
}
 
argv_array_push(, "commit");
@@ -864,21 +874,10 @@ static int run_git_commit(const char *defmsg, struct 
replay_opts *opts,
if (opts->allow_empty_message)
argv_array_push(, "--allow-empty-message");
 
-   if (cmd.err == -1) {
-   /* hide stderr on success */
-   struct strbuf buf = STRBUF_INIT;
-   int rc = pipe_command(,
- NULL, 0,
- /* stdout is already redirected */
- NULL, 0,
- , 0);
-   if (rc)
-   fputs(buf.buf, stderr);
-   strbuf_release();
-   return rc;
-   }
-
-   return run_command();
+   if (is_rebase_i(opts) && !(flags & EDIT_MSG))
+   return run_command_silent_on_success();
+   else
+   return run_command();
 }
 
 static int rest_is_empty(const struct strbuf *sb, int start)
-- 
2.18.0



[GSoC][PATCH v5 3/3] rebase -i: rewrite checkout_onto() in C

2018-06-29 Thread Alban Gruin
This rewrites checkout_onto() from shell to C.

A new command (“checkout-onto”) is added to rebase--helper.c. The shell
version is then stripped.

Signed-off-by: Alban Gruin 
---
 builtin/rebase--helper.c   |  7 ++-
 git-rebase--interactive.sh | 25 -
 sequencer.c| 19 +++
 sequencer.h|  3 +++
 4 files changed, 32 insertions(+), 22 deletions(-)

diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index 6c789e78b..0091e094b 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -18,7 +18,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
enum {
CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,
-   ADD_EXEC, APPEND_TODO_HELP, EDIT_TODO, PREPARE_BRANCH
+   ADD_EXEC, APPEND_TODO_HELP, EDIT_TODO, PREPARE_BRANCH,
+   CHECKOUT_ONTO
} command = 0;
struct option options[] = {
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
@@ -54,6 +55,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
EDIT_TODO),
OPT_CMDMODE(0, "prepare-branch", ,
N_("prepare the branch to be rebased"), 
PREPARE_BRANCH),
+   OPT_CMDMODE(0, "checkout-onto", ,
+   N_("checkout a commit"), CHECKOUT_ONTO),
OPT_END()
};
 
@@ -99,5 +102,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
return !!edit_todo_list(flags);
if (command == PREPARE_BRANCH && argc == 2)
return !!prepare_branch_to_be_rebased(, argv[1], verbose);
+   if (command == CHECKOUT_ONTO && argc == 4)
+   return !!checkout_onto(, argv[1], argv[2], argv[3], 
verbose);
usage_with_options(builtin_rebase_helper_usage, options);
 }
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 77e972bb6..b68f108f2 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -28,17 +28,6 @@ case "$comment_char" in
;;
 esac
 
-orig_reflog_action="$GIT_REFLOG_ACTION"
-
-comment_for_reflog () {
-   case "$orig_reflog_action" in
-   ''|rebase*)
-   GIT_REFLOG_ACTION="rebase -i ($1)"
-   export GIT_REFLOG_ACTION
-   ;;
-   esac
-}
-
 die_abort () {
apply_autostash
rm -rf "$state_dir"
@@ -70,14 +59,6 @@ collapse_todo_ids() {
git rebase--helper --shorten-ids
 }
 
-# Switch to the branch in $into and notify it in the reflog
-checkout_onto () {
-   comment_for_reflog start
-   GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $onto_name"
-   output git checkout $onto || die_abort "$(gettext "could not detach 
HEAD")"
-   git update-ref ORIG_HEAD $orig_head
-}
-
 get_missing_commit_check_level () {
check_level=$(git config --get rebase.missingCommitsCheck)
check_level=${check_level:-ignore}
@@ -176,7 +157,8 @@ EOF
 
git rebase--helper --check-todo-list || {
ret=$?
-   checkout_onto
+   git rebase--helper --checkout-onto "$onto_name" "$onto" \
+   "$orig_head" ${verbose:+--verbose}
exit $ret
}
 
@@ -186,7 +168,8 @@ EOF
onto="$(git rebase--helper --skip-unnecessary-picks)" ||
die "Could not skip unnecessary pick commands"
 
-   checkout_onto
+   git rebase--helper --checkout-onto "$onto_name" "$onto" "$orig_head" \
+   ${verbose:+--verbose}
require_clean_work_tree "rebase"
exec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \
 --continue
diff --git a/sequencer.c b/sequencer.c
index dd0cf3cb5..ec6de4eda 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3165,6 +3165,25 @@ int prepare_branch_to_be_rebased(struct replay_opts 
*opts, const char *commit,
return 0;
 }
 
+int checkout_onto(struct replay_opts *opts,
+ const char *onto_name, const char *onto,
+ const char *orig_head, unsigned verbose)
+{
+   struct object_id oid;
+   const char *action = reflog_message(opts, "start", "checkout %s", 
onto_name);
+
+   if (get_oid(orig_head, ))
+   return error(_("%s: not a valid OID"), orig_head);
+
+   if (run_git_checkout(opts, onto, verbose, action)) {
+   apply_autostash(opts);
+   sequencer_remove_state(opts);
+   return error(_("could not detach HEAD"));
+   }
+
+   return up

ag/rebase-i-append-todo-help, was Re: What's cooking in git.git (Jun 2018, #07; Thu, 28)

2018-06-29 Thread Alban Gruin
Hi Junio,

Le 28/06/2018 à 23:40, Junio C Hamano a écrit :
> * ag/rebase-i-append-todo-help (2018-06-14) 2 commits
>  - rebase--interactive: rewrite append_todo_help() in C
>  - Merge branch 'ag/rebase-p' into ag/rebase-i-append-todo-help
>  (this branch is used by ag/rebase-i-rewrite-todo.)
> 
>  Stepwise rewriting of the machinery of "rebase -i" into C continues.
> 
>  Needs a reroll.
>  cf. 
> 
> 
As I moved append_todo_help() to `rebase-interactive.c`, it should be
because I did not changed `msg = _(…)` to `msg = N_(…)`.

It was pointed out on IRC that it was not necessary, after all[0].
Basically, N_() is needed for static variables, not for "dynamic"
variables like `msg` in append_todo_help().

[0] http://colabti.org/irclogger/irclogger_log/git-devel?date=2018-06-26#l44

Cheers,
Alban



[GSoC][PATCH v5 2/2] rebase--interactive: rewrite append_todo_help() in C

2018-06-28 Thread Alban Gruin
This rewrites append_todo_help() from shell to C. It also incorporates
some parts of initiate_action() and complete_action() that also write
help texts to the todo file.

This also introduces the source file rebase-interactive.c. This file
will contain functions necessary for interactive rebase that are too
specific for the sequencer, and is part of libgit.a.

Two flags are added to rebase--helper.c: one to call
append_todo_help() (`--append-todo-help`), and another one to tell
append_todo_help() to write the help text suited for the edit-todo
mode (`--write-edit-todo`).

Finally, append_todo_help() is removed from git-rebase--interactive.sh
to use `rebase--helper --append-todo-help` instead.

Signed-off-by: Alban Gruin 
---
 Makefile   |  1 +
 builtin/rebase--helper.c   | 11 --
 git-rebase--interactive.sh | 52 ++---
 rebase-interactive.c   | 68 ++
 rebase-interactive.h   |  6 
 5 files changed, 86 insertions(+), 52 deletions(-)
 create mode 100644 rebase-interactive.c
 create mode 100644 rebase-interactive.h

diff --git a/Makefile b/Makefile
index 0cb6590f2..a281139ef 100644
--- a/Makefile
+++ b/Makefile
@@ -922,6 +922,7 @@ LIB_OBJS += protocol.o
 LIB_OBJS += quote.o
 LIB_OBJS += reachable.o
 LIB_OBJS += read-cache.o
+LIB_OBJS += rebase-interactive.o
 LIB_OBJS += reflog-walk.o
 LIB_OBJS += refs.o
 LIB_OBJS += refs/files-backend.o
diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index f7c2a5fdc..05e73e71d 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -3,6 +3,7 @@
 #include "config.h"
 #include "parse-options.h"
 #include "sequencer.h"
+#include "rebase-interactive.h"
 
 static const char * const builtin_rebase_helper_usage[] = {
N_("git rebase--helper []"),
@@ -12,12 +13,12 @@ static const char * const builtin_rebase_helper_usage[] = {
 int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
 {
struct replay_opts opts = REPLAY_OPTS_INIT;
-   unsigned flags = 0, keep_empty = 0, rebase_merges = 0;
+   unsigned flags = 0, keep_empty = 0, rebase_merges = 0, write_edit_todo 
= 0;
int abbreviate_commands = 0, rebase_cousins = -1;
enum {
CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,
-   ADD_EXEC
+   ADD_EXEC, APPEND_TODO_HELP
} command = 0;
struct option options[] = {
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
@@ -27,6 +28,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
OPT_BOOL(0, "rebase-merges", _merges, N_("rebase merge 
commits")),
OPT_BOOL(0, "rebase-cousins", _cousins,
 N_("keep original branch points of cousins")),
+   OPT_BOOL(0, "write-edit-todo", _edit_todo,
+N_("append the edit-todo message to the todo-list")),
OPT_CMDMODE(0, "continue", , N_("continue rebase"),
CONTINUE),
OPT_CMDMODE(0, "abort", , N_("abort rebase"),
@@ -45,6 +48,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
N_("rearrange fixup/squash lines"), REARRANGE_SQUASH),
OPT_CMDMODE(0, "add-exec-commands", ,
N_("insert exec commands in todo list"), ADD_EXEC),
+   OPT_CMDMODE(0, "append-todo-help", ,
+   N_("insert the help in the todo list"), 
APPEND_TODO_HELP),
OPT_END()
};
 
@@ -84,5 +89,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
return !!rearrange_squash();
if (command == ADD_EXEC && argc == 2)
return !!sequencer_add_exec_commands(argv[1]);
+   if (command == APPEND_TODO_HELP && argc == 1)
+   return !!append_todo_help(write_edit_todo, keep_empty);
usage_with_options(builtin_rebase_helper_usage, options);
 }
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 299ded213..94c23a7af 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -39,38 +39,6 @@ comment_for_reflog () {
esac
 }
 
-append_todo_help () {
-   gettext "
-Commands:
-p, pick  = use commit
-r, reword  = use commit, but edit the commit message
-e, edit  = use commit, but stop for amending
-s, squash  = use commit, but meld into previous commit
-f, fixup  = like \"squash\", but discard this commit's log message
-x, exec  = run command (the rest of the line) using shell
-d, drop

[GSoC][PATCH v5 1/2] sequencer: make two functions and an enum from sequencer.c public

2018-06-28 Thread Alban Gruin
This makes rebase_path_todo(), get_missing_commit_check_level() and the
enum check_level accessible outside sequencer.c.  check_level is renamed
missing_commit_check_level, and its value names are prefixed by
MISSING_COMMIT_ to avoid namespace pollution.

This will be needed for the rewrite of append_todo_help() from shell to
C, as it will be in a new library source file, rebase-interactive.c.

Signed-off-by: Alban Gruin 
---
 sequencer.c | 22 +-
 sequencer.h |  8 
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 0a291c91f..cb7ec9807 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -52,7 +52,7 @@ static GIT_PATH_FUNC(rebase_path, "rebase-merge")
  * the lines are processed, they are removed from the front of this
  * file and written to the tail of 'done'.
  */
-static GIT_PATH_FUNC(rebase_path_todo, "rebase-merge/git-rebase-todo")
+GIT_PATH_FUNC(rebase_path_todo, "rebase-merge/git-rebase-todo")
 /*
  * The rebase command lines that have already been processed. A line
  * is moved here when it is first handled, before any associated user
@@ -4223,24 +4223,20 @@ int transform_todos(unsigned flags)
return i;
 }
 
-enum check_level {
-   CHECK_IGNORE = 0, CHECK_WARN, CHECK_ERROR
-};
-
-static enum check_level get_missing_commit_check_level(void)
+enum missing_commit_check_level get_missing_commit_check_level(void)
 {
const char *value;
 
if (git_config_get_value("rebase.missingcommitscheck", ) ||
!strcasecmp("ignore", value))
-   return CHECK_IGNORE;
+   return MISSING_COMMIT_CHECK_IGNORE;
if (!strcasecmp("warn", value))
-   return CHECK_WARN;
+   return MISSING_COMMIT_CHECK_WARN;
if (!strcasecmp("error", value))
-   return CHECK_ERROR;
+   return MISSING_COMMIT_CHECK_ERROR;
warning(_("unrecognized setting %s for option "
  "rebase.missingCommitsCheck. Ignoring."), value);
-   return CHECK_IGNORE;
+   return MISSING_COMMIT_CHECK_IGNORE;
 }
 
 define_commit_slab(commit_seen, unsigned char);
@@ -4252,7 +4248,7 @@ define_commit_slab(commit_seen, unsigned char);
  */
 int check_todo_list(void)
 {
-   enum check_level check_level = get_missing_commit_check_level();
+   enum missing_commit_check_level check_level = 
get_missing_commit_check_level();
struct strbuf todo_file = STRBUF_INIT;
struct todo_list todo_list = TODO_LIST_INIT;
struct strbuf missing = STRBUF_INIT;
@@ -4269,7 +4265,7 @@ int check_todo_list(void)
advise_to_edit_todo = res =
parse_insn_buffer(todo_list.buf.buf, _list);
 
-   if (res || check_level == CHECK_IGNORE)
+   if (res || check_level == MISSING_COMMIT_CHECK_IGNORE)
goto leave_check;
 
/* Mark the commits in git-rebase-todo as seen */
@@ -4304,7 +4300,7 @@ int check_todo_list(void)
if (!missing.len)
goto leave_check;
 
-   if (check_level == CHECK_ERROR)
+   if (check_level == MISSING_COMMIT_CHECK_ERROR)
advise_to_edit_todo = res = 1;
 
fprintf(stderr,
diff --git a/sequencer.h b/sequencer.h
index c5787c6b5..ffab798f1 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -3,6 +3,7 @@
 
 const char *git_path_commit_editmsg(void);
 const char *git_path_seq_dir(void);
+const char *rebase_path_todo(void);
 
 #define APPEND_SIGNOFF_DEDUP (1u << 0)
 
@@ -57,6 +58,12 @@ struct replay_opts {
 };
 #define REPLAY_OPTS_INIT { .action = -1, .current_fixups = STRBUF_INIT }
 
+enum missing_commit_check_level {
+   MISSING_COMMIT_CHECK_IGNORE = 0,
+   MISSING_COMMIT_CHECK_WARN,
+   MISSING_COMMIT_CHECK_ERROR
+};
+
 /* Call this to setup defaults before parsing command line options */
 void sequencer_init_config(struct replay_opts *opts);
 int sequencer_pick_revisions(struct replay_opts *opts);
@@ -79,6 +86,7 @@ int sequencer_make_script(FILE *out, int argc, const char 
**argv,
 
 int sequencer_add_exec_commands(const char *command);
 int transform_todos(unsigned flags);
+enum missing_commit_check_level get_missing_commit_check_level(void);
 int check_todo_list(void);
 int skip_unnecessary_picks(void);
 int rearrange_squash(void);
-- 
2.18.0



[GSoC][PATCH v5 0/2] rebase -i: rewrite append_todo_help() in C

2018-06-28 Thread Alban Gruin
This patch rewrites append_todo_help() from shell to C. The C version
covers a bit more than the old shell version. To achieve that, some
parameters were added to rebase--helper.

This also introduce a new source file, rebase-interactive.c.

This is part of the effort to rewrite interactive rebase in C.

This is based on next, as of 2018-06-28.

Changes since v4:

 - Renaming enumeration check_level and its values to avoid namespace
   pollution.

Alban Gruin (2):
  sequencer: make two functions and an enum from sequencer.c public
  rebase--interactive: rewrite append_todo_help() in C

 Makefile   |  1 +
 builtin/rebase--helper.c   | 11 --
 git-rebase--interactive.sh | 52 ++---
 rebase-interactive.c   | 68 ++
 rebase-interactive.h   |  6 
 sequencer.c| 22 +---
 sequencer.h|  8 +
 7 files changed, 103 insertions(+), 65 deletions(-)
 create mode 100644 rebase-interactive.c
 create mode 100644 rebase-interactive.h

-- 
2.18.0



Re: [GSoC][PATCH v4 0/2] rebase -i: rewrite append_todo_help() in C

2018-06-27 Thread Alban Gruin
Hi Johannes,

Le 26/06/2018 à 23:37, Johannes Schindelin a écrit :
> Hi Alban,
> 
> On Tue, 26 Jun 2018, Alban Gruin wrote:
> 
>> This patch rewrites append_todo_help() from shell to C. The C version
>> covers a bit more than the old shell version. To achieve that, some
>> parameters were added to rebase--helper.
>>
>> This also introduce a new source file, rebase-interactive.c.
>>
>> This is part of the effort to rewrite interactive rebase in C.
>>
>> This is based on next, as of 2018-06-26.
> 
> Out of curiosity: which commits that are not yet in `master` are required?
> 

None, actually.

Cheers,
Alban



Re: [GSoC][PATCH v4 1/2] sequencer: make two functions and an enum from sequencer.c public

2018-06-27 Thread Alban Gruin
Hi Johannes,

Le 26/06/2018 à 23:41, Johannes Schindelin a écrit :
> Hi Alban,
> 
> On Tue, 26 Jun 2018, Alban Gruin wrote:
> 
>> diff --git a/sequencer.h b/sequencer.h
>> index c5787c6b5..08397b0d1 100644
>> --- a/sequencer.h
>> +++ b/sequencer.h
>> @@ -3,6 +3,7 @@
>>  
>>  const char *git_path_commit_editmsg(void);
>>  const char *git_path_seq_dir(void);
>> +const char *rebase_path_todo(void);
>>  
>>  #define APPEND_SIGNOFF_DEDUP (1u << 0)
>>  
>> @@ -57,6 +58,10 @@ struct replay_opts {
>>  };
>>  #define REPLAY_OPTS_INIT { .action = -1, .current_fixups = STRBUF_INIT }
>>  
>> +enum check_level {
>> +CHECK_IGNORE = 0, CHECK_WARN, CHECK_ERROR
>> +};
>> +
> 
> While this is contained within scopes that include `sequencer.h`, it *is*
> public now, so I am slightly uneasy about keeping this enum so generic.
> Maybe we want to use
> 
> enum missing_commit_check_level {
>   MISSING_COMMIT_CHECK_IGNORE = 0,
>   MISSING_COMMIT_CHECK_WARN,
>   MISSING_COMMIT_CHECK_ERROR
> };
> 
> instead?
> 

You’re right, this would be better.

Cheers,
Alban



[GSoC][PATCH v3 0/2] rebase -i: rewrite the edit-todo functionality in C

2018-06-26 Thread Alban Gruin
This patch rewrites the edit-todo functionality from shell to C. This is
part of the effort to rewrite interactive rebase in C.

This patch is based on the fourth iteration of my series rewriting
append_todo_help() in C.

Changes since v2:

 - Moving edit_todo() from sequencer.c to interactive-rebase.c.

Alban Gruin (2):
  editor: add a function to launch the sequence editor
  rebase-interactive: rewrite the edit-todo functionality in C

 builtin/rebase--helper.c   | 13 -
 cache.h|  1 +
 editor.c   | 27 +--
 git-rebase--interactive.sh | 11 +--
 rebase-interactive.c   | 31 +++
 rebase-interactive.h   |  1 +
 strbuf.h   |  2 ++
 7 files changed, 69 insertions(+), 17 deletions(-)

-- 
2.18.0



[GSoC][PATCH v3 1/2] editor: add a function to launch the sequence editor

2018-06-26 Thread Alban Gruin
As part of the rewrite of interactive rebase, the sequencer will need to
open the sequence editor to allow the user to edit the todo list.
Instead of duplicating the existing launch_editor() function, this
refactors it to a new function, launch_specified_editor(), which takes
the editor as a parameter, in addition to the path, the buffer and the
environment variables.  launch_sequence_editor() is then added to launch
the sequence editor.

Signed-off-by: Alban Gruin 
---
 cache.h  |  1 +
 editor.c | 27 +--
 strbuf.h |  2 ++
 3 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/cache.h b/cache.h
index 8b447652a..d70ae49ca 100644
--- a/cache.h
+++ b/cache.h
@@ -1409,6 +1409,7 @@ extern const char *fmt_name(const char *name, const char 
*email);
 extern const char *ident_default_name(void);
 extern const char *ident_default_email(void);
 extern const char *git_editor(void);
+extern const char *git_sequence_editor(void);
 extern const char *git_pager(int stdout_is_tty);
 extern int is_terminal_dumb(void);
 extern int git_ident_config(const char *, const char *, void *);
diff --git a/editor.c b/editor.c
index 9a9b4e12d..c985eee1f 100644
--- a/editor.c
+++ b/editor.c
@@ -1,4 +1,5 @@
 #include "cache.h"
+#include "config.h"
 #include "strbuf.h"
 #include "run-command.h"
 #include "sigchain.h"
@@ -34,10 +35,21 @@ const char *git_editor(void)
return editor;
 }
 
-int launch_editor(const char *path, struct strbuf *buffer, const char *const 
*env)
+const char *git_sequence_editor(void)
 {
-   const char *editor = git_editor();
+   const char *editor = getenv("GIT_SEQUENCE_EDITOR");
+
+   if (!editor)
+   git_config_get_string_const("sequence.editor", );
+   if (!editor)
+   editor = git_editor();
 
+   return editor;
+}
+
+static int launch_specified_editor(const char *editor, const char *path,
+  struct strbuf *buffer, const char *const 
*env)
+{
if (!editor)
return error("Terminal is dumb, but EDITOR unset");
 
@@ -95,3 +107,14 @@ int launch_editor(const char *path, struct strbuf *buffer, 
const char *const *en
return error_errno("could not read file '%s'", path);
return 0;
 }
+
+int launch_editor(const char *path, struct strbuf *buffer, const char *const 
*env)
+{
+   return launch_specified_editor(git_editor(), path, buffer, env);
+}
+
+int launch_sequence_editor(const char *path, struct strbuf *buffer,
+  const char *const *env)
+{
+   return launch_specified_editor(git_sequence_editor(), path, buffer, 
env);
+}
diff --git a/strbuf.h b/strbuf.h
index 60a35aef1..66da9822f 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -575,6 +575,8 @@ extern void strbuf_add_unique_abbrev(struct strbuf *sb,
  * file's contents are not read into the buffer upon completion.
  */
 extern int launch_editor(const char *path, struct strbuf *buffer, const char 
*const *env);
+extern int launch_sequence_editor(const char *path, struct strbuf *buffer,
+ const char *const *env);
 
 extern void strbuf_add_lines(struct strbuf *sb, const char *prefix, const char 
*buf, size_t size);
 
-- 
2.18.0



[GSoC][PATCH v3 2/2] rebase-interactive: rewrite the edit-todo functionality in C

2018-06-26 Thread Alban Gruin
This rewrites the edit-todo functionality from shell to C.

To achieve that, a new command mode, `edit-todo`, is added, and the
`write-edit-todo` flag is removed, as the shell script does not need to
write the edit todo help message to the todo list anymore.

The shell version is then stripped in favour of a call to the helper.

Signed-off-by: Alban Gruin 
---
 builtin/rebase--helper.c   | 13 -
 git-rebase--interactive.sh | 11 +--
 rebase-interactive.c   | 31 +++
 rebase-interactive.h   |  1 +
 4 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index 05e73e71d..731a64971 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -13,12 +13,12 @@ static const char * const builtin_rebase_helper_usage[] = {
 int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
 {
struct replay_opts opts = REPLAY_OPTS_INIT;
-   unsigned flags = 0, keep_empty = 0, rebase_merges = 0, write_edit_todo 
= 0;
+   unsigned flags = 0, keep_empty = 0, rebase_merges = 0;
int abbreviate_commands = 0, rebase_cousins = -1;
enum {
CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,
-   ADD_EXEC, APPEND_TODO_HELP
+   ADD_EXEC, APPEND_TODO_HELP, EDIT_TODO
} command = 0;
struct option options[] = {
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
@@ -28,8 +28,6 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
OPT_BOOL(0, "rebase-merges", _merges, N_("rebase merge 
commits")),
OPT_BOOL(0, "rebase-cousins", _cousins,
 N_("keep original branch points of cousins")),
-   OPT_BOOL(0, "write-edit-todo", _edit_todo,
-N_("append the edit-todo message to the todo-list")),
OPT_CMDMODE(0, "continue", , N_("continue rebase"),
CONTINUE),
OPT_CMDMODE(0, "abort", , N_("abort rebase"),
@@ -50,6 +48,9 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
N_("insert exec commands in todo list"), ADD_EXEC),
OPT_CMDMODE(0, "append-todo-help", ,
N_("insert the help in the todo list"), 
APPEND_TODO_HELP),
+   OPT_CMDMODE(0, "edit-todo", ,
+   N_("edit the todo list during an interactive 
rebase"),
+   EDIT_TODO),
OPT_END()
};
 
@@ -90,6 +91,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
if (command == ADD_EXEC && argc == 2)
return !!sequencer_add_exec_commands(argv[1]);
if (command == APPEND_TODO_HELP && argc == 1)
-   return !!append_todo_help(write_edit_todo, keep_empty);
+   return !!append_todo_help(0, keep_empty);
+   if (command == EDIT_TODO && argc == 1)
+   return !!edit_todo_list(flags);
usage_with_options(builtin_rebase_helper_usage, options);
 }
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 94c23a7af..2defe607f 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -108,16 +108,7 @@ initiate_action () {
 --continue
;;
edit-todo)
-   git stripspace --strip-comments <"$todo" >"$todo".new
-   mv -f "$todo".new "$todo"
-   collapse_todo_ids
-   git rebase--helper --append-todo-help --write-edit-todo
-
-   git_sequence_editor "$todo" ||
-   die "$(gettext "Could not execute editor")"
-   expand_todo_ids
-
-   exit
+   exec git rebase--helper --edit-todo
;;
show-current-patch)
exec git show REBASE_HEAD --
diff --git a/rebase-interactive.c b/rebase-interactive.c
index c79c819b9..ace8e095b 100644
--- a/rebase-interactive.c
+++ b/rebase-interactive.c
@@ -66,3 +66,34 @@ int append_todo_help(unsigned edit_todo, unsigned keep_empty)
 
return ret;
 }
+
+int edit_todo_list(unsigned flags)
+{
+   struct strbuf buf = STRBUF_INIT;
+   const char *todo_file = rebase_path_todo();
+   FILE *todo;
+
+   if (strbuf_read_file(, todo_file, 0) < 0)
+   return error_errno(_("could not read '%s'."), todo_file);
+
+   strbuf_stripspace(, 1);
+   todo = fopen_or_warn(todo_file, "w");
+   if (!todo) {
+

[GSoC][PATCH v4 2/2] rebase--interactive: rewrite append_todo_help() in C

2018-06-26 Thread Alban Gruin
This rewrites append_todo_help() from shell to C. It also incorporates
some parts of initiate_action() and complete_action() that also write
help texts to the todo file.

This also introduces the source file rebase-interactive.c. This file
will contain functions necessary for interactive rebase that are too
specific for the sequencer, and is part of libgit.a.

Two flags are added to rebase--helper.c: one to call
append_todo_help() (`--append-todo-help`), and another one to tell
append_todo_help() to write the help text suited for the edit-todo
mode (`--write-edit-todo`).

Finally, append_todo_help() is removed from git-rebase--interactive.sh
to use `rebase--helper --append-todo-help` instead.

Signed-off-by: Alban Gruin 
---
 Makefile   |  1 +
 builtin/rebase--helper.c   | 11 --
 git-rebase--interactive.sh | 52 ++---
 rebase-interactive.c   | 68 ++
 rebase-interactive.h   |  6 
 5 files changed, 86 insertions(+), 52 deletions(-)
 create mode 100644 rebase-interactive.c
 create mode 100644 rebase-interactive.h

diff --git a/Makefile b/Makefile
index 0cb6590f2..a281139ef 100644
--- a/Makefile
+++ b/Makefile
@@ -922,6 +922,7 @@ LIB_OBJS += protocol.o
 LIB_OBJS += quote.o
 LIB_OBJS += reachable.o
 LIB_OBJS += read-cache.o
+LIB_OBJS += rebase-interactive.o
 LIB_OBJS += reflog-walk.o
 LIB_OBJS += refs.o
 LIB_OBJS += refs/files-backend.o
diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index f7c2a5fdc..05e73e71d 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -3,6 +3,7 @@
 #include "config.h"
 #include "parse-options.h"
 #include "sequencer.h"
+#include "rebase-interactive.h"
 
 static const char * const builtin_rebase_helper_usage[] = {
N_("git rebase--helper []"),
@@ -12,12 +13,12 @@ static const char * const builtin_rebase_helper_usage[] = {
 int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
 {
struct replay_opts opts = REPLAY_OPTS_INIT;
-   unsigned flags = 0, keep_empty = 0, rebase_merges = 0;
+   unsigned flags = 0, keep_empty = 0, rebase_merges = 0, write_edit_todo 
= 0;
int abbreviate_commands = 0, rebase_cousins = -1;
enum {
CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,
-   ADD_EXEC
+   ADD_EXEC, APPEND_TODO_HELP
} command = 0;
struct option options[] = {
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
@@ -27,6 +28,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
OPT_BOOL(0, "rebase-merges", _merges, N_("rebase merge 
commits")),
OPT_BOOL(0, "rebase-cousins", _cousins,
 N_("keep original branch points of cousins")),
+   OPT_BOOL(0, "write-edit-todo", _edit_todo,
+N_("append the edit-todo message to the todo-list")),
OPT_CMDMODE(0, "continue", , N_("continue rebase"),
CONTINUE),
OPT_CMDMODE(0, "abort", , N_("abort rebase"),
@@ -45,6 +48,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
N_("rearrange fixup/squash lines"), REARRANGE_SQUASH),
OPT_CMDMODE(0, "add-exec-commands", ,
N_("insert exec commands in todo list"), ADD_EXEC),
+   OPT_CMDMODE(0, "append-todo-help", ,
+   N_("insert the help in the todo list"), 
APPEND_TODO_HELP),
OPT_END()
};
 
@@ -84,5 +89,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
return !!rearrange_squash();
if (command == ADD_EXEC && argc == 2)
return !!sequencer_add_exec_commands(argv[1]);
+   if (command == APPEND_TODO_HELP && argc == 1)
+   return !!append_todo_help(write_edit_todo, keep_empty);
usage_with_options(builtin_rebase_helper_usage, options);
 }
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 299ded213..94c23a7af 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -39,38 +39,6 @@ comment_for_reflog () {
esac
 }
 
-append_todo_help () {
-   gettext "
-Commands:
-p, pick  = use commit
-r, reword  = use commit, but edit the commit message
-e, edit  = use commit, but stop for amending
-s, squash  = use commit, but meld into previous commit
-f, fixup  = like \"squash\", but discard this commit's log message
-x, exec  = run command (the rest of the line) using shell
-d, drop

[GSoC][PATCH v4 1/2] sequencer: make two functions and an enum from sequencer.c public

2018-06-26 Thread Alban Gruin
This makes rebase_path_todo(), get_missign_commit_check_level() and the
enum check_level accessible outside sequencer.c.  This will be needed
for the rewrite of append_todo_help() from shell to C, as it will be in
a new library source file, rebase-interactive.c.

Signed-off-by: Alban Gruin 
---
 sequencer.c | 8 ++--
 sequencer.h | 6 ++
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 0a291c91f..881a4f7f4 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -52,7 +52,7 @@ static GIT_PATH_FUNC(rebase_path, "rebase-merge")
  * the lines are processed, they are removed from the front of this
  * file and written to the tail of 'done'.
  */
-static GIT_PATH_FUNC(rebase_path_todo, "rebase-merge/git-rebase-todo")
+GIT_PATH_FUNC(rebase_path_todo, "rebase-merge/git-rebase-todo")
 /*
  * The rebase command lines that have already been processed. A line
  * is moved here when it is first handled, before any associated user
@@ -4223,11 +4223,7 @@ int transform_todos(unsigned flags)
return i;
 }
 
-enum check_level {
-   CHECK_IGNORE = 0, CHECK_WARN, CHECK_ERROR
-};
-
-static enum check_level get_missing_commit_check_level(void)
+enum check_level get_missing_commit_check_level(void)
 {
const char *value;
 
diff --git a/sequencer.h b/sequencer.h
index c5787c6b5..08397b0d1 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -3,6 +3,7 @@
 
 const char *git_path_commit_editmsg(void);
 const char *git_path_seq_dir(void);
+const char *rebase_path_todo(void);
 
 #define APPEND_SIGNOFF_DEDUP (1u << 0)
 
@@ -57,6 +58,10 @@ struct replay_opts {
 };
 #define REPLAY_OPTS_INIT { .action = -1, .current_fixups = STRBUF_INIT }
 
+enum check_level {
+   CHECK_IGNORE = 0, CHECK_WARN, CHECK_ERROR
+};
+
 /* Call this to setup defaults before parsing command line options */
 void sequencer_init_config(struct replay_opts *opts);
 int sequencer_pick_revisions(struct replay_opts *opts);
@@ -79,6 +84,7 @@ int sequencer_make_script(FILE *out, int argc, const char 
**argv,
 
 int sequencer_add_exec_commands(const char *command);
 int transform_todos(unsigned flags);
+enum check_level get_missing_commit_check_level(void);
 int check_todo_list(void);
 int skip_unnecessary_picks(void);
 int rearrange_squash(void);
-- 
2.18.0



[GSoC][PATCH v4 0/2] rebase -i: rewrite append_todo_help() in C

2018-06-26 Thread Alban Gruin
This patch rewrites append_todo_help() from shell to C. The C version
covers a bit more than the old shell version. To achieve that, some
parameters were added to rebase--helper.

This also introduce a new source file, rebase-interactive.c.

This is part of the effort to rewrite interactive rebase in C.

This is based on next, as of 2018-06-26.

Changes since v3:

 - Show an error message when append_todo_help() fails to edit the todo
   list.

 - Introducing rebase-interactive.c to contain functions necessary for
   interactive rebase.

Alban Gruin (2):
  sequencer: make two functions and an enum from sequencer.c public
  rebase--interactive: rewrite append_todo_help() in C

 Makefile   |  1 +
 builtin/rebase--helper.c   | 11 --
 git-rebase--interactive.sh | 52 ++---
 rebase-interactive.c   | 68 ++
 rebase-interactive.h   |  6 
 sequencer.c|  8 ++---
 sequencer.h|  6 
 7 files changed, 94 insertions(+), 58 deletions(-)
 create mode 100644 rebase-interactive.c
 create mode 100644 rebase-interactive.h

-- 
2.18.0



[GSoC][PATCH 1/1] sequencer: print an error message if append_todo_help() fails

2018-06-26 Thread Alban Gruin
This adds an error when append_todo_help() fails to write its message to
the todo file.

Signed-off-by: Alban Gruin 
---
 sequencer.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sequencer.c b/sequencer.c
index 7cc76332e..7c4bdbb99 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4380,6 +4380,9 @@ int append_todo_help(unsigned edit_todo, unsigned 
keep_empty)
}
 
ret = fputs(buf.buf, todo);
+   if (ret)
+   error_errno(_("Could not append help text to '%s'"), 
rebase_path_todo());
+
fclose(todo);
strbuf_release();
 
-- 
2.18.0



[GSoC][PATCH 0/1] sequencer: print an error message if append_todo_help() fails

2018-06-26 Thread Alban Gruin
Currently, append_todo_help() does not warn the user if an error occurs
when trying to write to the todo file.  This patch addresses this
problem.

This patch is based on ag/rebase-i-append-todo-help.

Alban Gruin (1):
  sequencer: print an error message if append_todo_help() fails

 sequencer.c | 3 +++
 1 file changed, 3 insertions(+)

-- 
2.18.0



[GSoC] GSoC with git, week 8

2018-06-25 Thread Alban Gruin
Hi,

I published my blog post about last week:

  https://blog.pa1ch.fr/posts/2018/06/25/en/gsoc2018-week-8.html

Cheers,
Alban



Re: [GSoC][PATCH v3 2/3] rebase -i: rewrite setup_reflog_action() in C

2018-06-25 Thread Alban Gruin
Hi Junio,

Le 25/06/2018 à 17:34, Junio C Hamano a écrit :
> Alban Gruin  writes:
> 
>> Hi Junio,
>>
>> Le 22/06/2018 à 18:27, Junio C Hamano a écrit :
>>> Alban Gruin  writes:
>>>> This rewrites (the misnamed) setup_reflog_action() from shell to C. The
>>>> new version is called checkout_base_commit().
>>>
>>> ;-) on the "misnamed" part.  Indeed, setting up the comment for the
>>> reflog entry is secondary to what this function wants to do, which
>>> is to check out the branch to be rebased.
>>>
>>> I do not think "base_commit" is a good name, either, though.  When I
>>> hear 'base' in the context of 'rebase', I would imagine that the
>>> speaker is talking about the bottom of the range of the commits to
>>> be rebased (i.e. "rebase --onto ONTO BASE BRANCH", which replays
>>> commits BASE..BRANCH on top of ONTO and then points BRANCH at the
>>> result), not the top of the range or the branch these commits are
>>> taken from.
>>>
>>
>> Perhaps should I name this function checkout_onto(), and rename 
>> checkout_onto() to "detach_onto()"?  And I would reorder those two commits 
>> in 
>> the series, as this would be very confusing.
> 
> I may be misunderstanding what is happening in the function, but I
> think it is checking out neither the onto or the base commit.  The
> function instead is about checking out the branch to be rebased
> before anything else happens when the optional  argument is
> given (and when the optional argument is not given, then we rebase
> the current branch so there is no need to check it out upfront), no?
> 
> 

Yes, you’re right.

Now I really don’t know how to call this function.
checkout_top_of_range(), perhaps?

Cheers,
Alban



[GSoC][PATCH v4 3/3] rebase -i: rewrite setup_reflog_action() in C

2018-06-25 Thread Alban Gruin
This rewrites (the misnamed) setup_reflog_action() from shell to C. The
new version is called checkout_onto().

A new command is added to rebase--helper.c, “checkout-base”. The shell
version is then stripped.

As $GIT_REFLOG_ACTION is no longer set at the first call of
checkout_onto(), a call to comment_for_reflog() is added at the
beginning of this function.

If the commit OID provided to checkout_onto() is empty, nothing happens
and no errors are returned, otherwise it would break some
tests (t3404.92 and t3404.93).

Signed-off-by: Alban Gruin 
---
 builtin/rebase--helper.c   |  6 +-
 git-rebase--interactive.sh | 15 +--
 sequencer.c| 14 ++
 sequencer.h|  2 ++
 4 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index 2dfd55c76..a00b1091d 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -17,7 +17,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
enum {
CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,
-   ADD_EXEC, APPEND_TODO_HELP, EDIT_TODO, DETACH_ONTO
+   ADD_EXEC, APPEND_TODO_HELP, EDIT_TODO, DETACH_ONTO, 
CHECKOUT_ONTO
} command = 0;
struct option options[] = {
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
@@ -53,6 +53,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
EDIT_TODO),
OPT_CMDMODE(0, "detach-onto", ,
N_("checkout a commit"), DETACH_ONTO),
+   OPT_CMDMODE(0, "checkout-onto", ,
+   N_("checkout the base commit"), CHECKOUT_ONTO),
OPT_END()
};
 
@@ -98,5 +100,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
return !!edit_todo_list(flags);
if (command == DETACH_ONTO && argc == 4)
return !!detach_onto(, argv[1], argv[2], argv[3], verbose);
+   if (command == CHECKOUT_ONTO && argc == 2)
+   return !!checkout_onto(, argv[1], verbose);
usage_with_options(builtin_rebase_helper_usage, options);
 }
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 2f5761d49..375c36fb5 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -112,19 +112,6 @@ initiate_action () {
esac
 }
 
-setup_reflog_action () {
-   comment_for_reflog start
-
-   if test ! -z "$switch_to"
-   then
-   GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $switch_to"
-   output git checkout "$switch_to" -- ||
-   die "$(eval_gettext "Could not checkout \$switch_to")"
-
-   comment_for_reflog start
-   fi
-}
-
 init_basic_state () {
orig_head=$(git rev-parse --verify HEAD) || die "$(gettext "No HEAD?")"
mkdir -p "$state_dir" || die "$(eval_gettext "Could not create 
temporary \$state_dir")"
@@ -206,7 +193,7 @@ git_rebase__interactive () {
return 0
fi
 
-   setup_reflog_action
+   git rebase--helper --checkout-onto "$switch_to" ${verbose:+--verbose}
init_basic_state
 
init_revisions_and_shortrevisions
diff --git a/sequencer.c b/sequencer.c
index ee6374921..620b41e96 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3165,6 +3165,20 @@ int detach_onto(struct replay_opts *opts,
return update_ref(NULL, "ORIG_HEAD", , NULL, 0, 
UPDATE_REFS_MSG_ON_ERR);
 }
 
+int checkout_onto(struct replay_opts *opts, const char *commit,
+ int verbose)
+{
+   const char *action;
+
+   if (commit && *commit) {
+   action = reflog_message(opts, "start", "checkout %s", commit);
+   if (run_git_checkout(opts, commit, verbose, action))
+   return error(_("Could not checkout %s"), commit);
+   }
+
+   return 0;
+}
+
 static const char rescheduled_advice[] =
 N_("Could not execute the todo command\n"
 "\n"
diff --git a/sequencer.h b/sequencer.h
index 9f0ac5e75..afbb2edf1 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -103,6 +103,8 @@ void commit_post_rewrite(const struct commit *current_head,
 int detach_onto(struct replay_opts *opts,
const char *onto_name, const char *onto,
const char *orig_head, unsigned verbose);
+int checkout_onto(struct replay_opts *opts, const char *commit,
+ int verbose);
 
 #define SUMMARY_INITIAL_COMMIT   (1 << 0)
 #define SUMMARY_SHOW_AUTHOR_DATE (1 << 1)
-- 
2.18.0



[GSoC][PATCH v4 0/3] rebase -i: rewrite reflog operations in C

2018-06-25 Thread Alban Gruin
This patch series rewrites the reflog operations from shell to C.  This
is part of the effort to rewrite interactive rebase in C.

The first commit is dedicated to creating a function to silence a
command, as the sequencer will do in several places with these patches.

This branch is based on ag/rebase-i-rewrite-todo, and does not conflict
with pu (as of 2018-06-25).

Changes since v3:

 - Removing a comment from run_command_silent_on_success()

 - Changing the order of setup_reflog_action() and checkout_onto()
   rewrites in the series

 - Renaming checkout_onto() to detach_onto()

 - Renaming checkout_base_commit() (rewrite of setup_reflog_action()) to
   checkout_onto()

 - Using the `else` keyword to call run_command_silent_on_success() or
   run_command() in run_git_commit() and run_git_checkout().

Alban Gruin (3):
  sequencer: extract a function to silence a command, except if it fails
  rebase -i: rewrite checkout_onto() in C
  rebase -i: rewrite setup_reflog_action() in C

 builtin/rebase--helper.c   |  13 -
 git-rebase--interactive.sh |  28 ++
 sequencer.c| 101 +++--
 sequencer.h|   6 +++
 4 files changed, 97 insertions(+), 51 deletions(-)

-- 
2.18.0



[GSoC][PATCH v4 2/3] rebase -i: rewrite checkout_onto() in C

2018-06-25 Thread Alban Gruin
This rewrites checkout_onto() from shell to C. The new version is called
detach_onto(), given its role.

A new command (“detach-onto”) is added to rebase--helper.c, as well as
a new flag, “verbose”, to avoid silencing the output of the checkout
operation called by detach_onto().

The function `run_git_checkout()` will also be used in the next commit,
therefore its code is not part of `detach_onto()`.

The shell version is then stripped in favour of a call to the helper.

Signed-off-by: Alban Gruin 
---
 builtin/rebase--helper.c   |  9 +++--
 git-rebase--interactive.sh | 13 -
 sequencer.c| 36 
 sequencer.h|  4 
 4 files changed, 51 insertions(+), 11 deletions(-)

diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index d2990b210..2dfd55c76 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -12,12 +12,12 @@ static const char * const builtin_rebase_helper_usage[] = {
 int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
 {
struct replay_opts opts = REPLAY_OPTS_INIT;
-   unsigned flags = 0, keep_empty = 0, rebase_merges = 0;
+   unsigned flags = 0, keep_empty = 0, rebase_merges = 0, verbose = 0;
int abbreviate_commands = 0, rebase_cousins = -1;
enum {
CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,
-   ADD_EXEC, APPEND_TODO_HELP, EDIT_TODO
+   ADD_EXEC, APPEND_TODO_HELP, EDIT_TODO, DETACH_ONTO
} command = 0;
struct option options[] = {
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
@@ -27,6 +27,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
OPT_BOOL(0, "rebase-merges", _merges, N_("rebase merge 
commits")),
OPT_BOOL(0, "rebase-cousins", _cousins,
 N_("keep original branch points of cousins")),
+   OPT__VERBOSE(, N_("be verbose")),
OPT_CMDMODE(0, "continue", , N_("continue rebase"),
CONTINUE),
OPT_CMDMODE(0, "abort", , N_("abort rebase"),
@@ -50,6 +51,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
OPT_CMDMODE(0, "edit-todo", ,
N_("edit the todo list during an interactive 
rebase"),
EDIT_TODO),
+   OPT_CMDMODE(0, "detach-onto", ,
+   N_("checkout a commit"), DETACH_ONTO),
OPT_END()
};
 
@@ -93,5 +96,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
return !!append_todo_help(0, keep_empty);
if (command == EDIT_TODO && argc == 1)
return !!edit_todo_list(flags);
+   if (command == DETACH_ONTO && argc == 4)
+   return !!detach_onto(, argv[1], argv[2], argv[3], verbose);
usage_with_options(builtin_rebase_helper_usage, options);
 }
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 2defe607f..2f5761d49 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -70,13 +70,6 @@ collapse_todo_ids() {
git rebase--helper --shorten-ids
 }
 
-# Switch to the branch in $into and notify it in the reflog
-checkout_onto () {
-   GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $onto_name"
-   output git checkout $onto || die_abort "$(gettext "could not detach 
HEAD")"
-   git update-ref ORIG_HEAD $orig_head
-}
-
 get_missing_commit_check_level () {
check_level=$(git config --get rebase.missingCommitsCheck)
check_level=${check_level:-ignore}
@@ -188,7 +181,8 @@ EOF
 
git rebase--helper --check-todo-list || {
ret=$?
-   checkout_onto
+   git rebase--helper --detach-onto "$onto_name" "$onto" \
+   "$orig_head" ${verbose:+--verbose}
exit $ret
}
 
@@ -198,7 +192,8 @@ EOF
onto="$(git rebase--helper --skip-unnecessary-picks)" ||
die "Could not skip unnecessary pick commands"
 
-   checkout_onto
+   git rebase--helper --detach-onto "$onto_name" "$onto" "$orig_head" \
+   ${verbose:+--verbose}
require_clean_work_tree "rebase"
exec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \
 --continue
diff --git a/sequencer.c b/sequencer.c
index 9a9725e23..ee6374921 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3129,6 +3129,42 @@ static const char *reflog_message(struct replay_opts 

[GSoC][PATCH v4 1/3] sequencer: extract a function to silence a command, except if it fails

2018-06-25 Thread Alban Gruin
This extracts the part of run_git_commit() that redirects the stdout and
stderr of a command to a strbuf, and prints it if the command fails, to
a helper function: run_command_silent_on_success(). It is intended to
replace output() from git-rebase.sh in the rewrite of
setup_reflog_action() and checkout_onto().

Signed-off-by: Alban Gruin 
---
 sequencer.c | 51 +--
 1 file changed, 25 insertions(+), 26 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 7cc76332e..9a9725e23 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -766,6 +766,23 @@ N_("you have staged changes in your working tree\n"
 #define VERIFY_MSG  (1<<4)
 #define CREATE_ROOT_COMMIT (1<<5)
 
+static int run_command_silent_on_success(struct child_process *cmd)
+{
+   struct strbuf buf = STRBUF_INIT;
+   int rc;
+
+   cmd->stdout_to_stderr = 1;
+   rc = pipe_command(cmd,
+ NULL, 0,
+ NULL, 0,
+ , 0);
+
+   if (rc)
+   fputs(buf.buf, stderr);
+   strbuf_release();
+   return rc;
+}
+
 /*
  * If we are cherry-pick, and if the merge did not result in
  * hand-editing, we will hit this commit and inherit the original
@@ -820,18 +837,11 @@ static int run_git_commit(const char *defmsg, struct 
replay_opts *opts,
 
cmd.git_cmd = 1;
 
-   if (is_rebase_i(opts)) {
-   if (!(flags & EDIT_MSG)) {
-   cmd.stdout_to_stderr = 1;
-   cmd.err = -1;
-   }
-
-   if (read_env_script(_array)) {
-   const char *gpg_opt = gpg_sign_opt_quoted(opts);
+   if (is_rebase_i(opts) && read_env_script(_array)) {
+   const char *gpg_opt = gpg_sign_opt_quoted(opts);
 
-   return error(_(staged_changes_advice),
-gpg_opt, gpg_opt);
-   }
+   return error(_(staged_changes_advice),
+gpg_opt, gpg_opt);
}
 
argv_array_push(, "commit");
@@ -861,21 +871,10 @@ static int run_git_commit(const char *defmsg, struct 
replay_opts *opts,
if (opts->allow_empty_message)
argv_array_push(, "--allow-empty-message");
 
-   if (cmd.err == -1) {
-   /* hide stderr on success */
-   struct strbuf buf = STRBUF_INIT;
-   int rc = pipe_command(,
- NULL, 0,
- /* stdout is already redirected */
- NULL, 0,
- , 0);
-   if (rc)
-   fputs(buf.buf, stderr);
-   strbuf_release();
-   return rc;
-   }
-
-   return run_command();
+   if (is_rebase_i(opts) && !(flags & EDIT_MSG))
+   return run_command_silent_on_success();
+   else
+   return run_command();
 }
 
 static int rest_is_empty(const struct strbuf *sb, int start)
-- 
2.18.0



Re: [GSoC][PATCH v3 2/3] rebase -i: rewrite setup_reflog_action() in C

2018-06-22 Thread Alban Gruin
Hi Junio,

Le 22/06/2018 à 18:27, Junio C Hamano a écrit :
> Alban Gruin  writes:
> > This rewrites (the misnamed) setup_reflog_action() from shell to C. The
> > new version is called checkout_base_commit().
> 
> ;-) on the "misnamed" part.  Indeed, setting up the comment for the
> reflog entry is secondary to what this function wants to do, which
> is to check out the branch to be rebased.
> 
> I do not think "base_commit" is a good name, either, though.  When I
> hear 'base' in the context of 'rebase', I would imagine that the
> speaker is talking about the bottom of the range of the commits to
> be rebased (i.e. "rebase --onto ONTO BASE BRANCH", which replays
> commits BASE..BRANCH on top of ONTO and then points BRANCH at the
> result), not the top of the range or the branch these commits are
> taken from.
> 

Perhaps should I name this function checkout_onto(), and rename 
checkout_onto() to "detach_onto()"?  And I would reorder those two commits in 
the series, as this would be very confusing.

> > index 51c8ab7ac..27f8453fe 100644
> > --- a/sequencer.c
> > +++ b/sequencer.c
> > @@ -3129,6 +3129,36 @@ static const char *reflog_message(struct
> > replay_opts *opts,> 
> > return buf.buf;
> >  
> >  }
> > 
> > +static int run_git_checkout(struct replay_opts *opts, const char *commit,
> > +   int verbose, const char *action)
> > +{
> > +   struct child_process cmd = CHILD_PROCESS_INIT;
> > +
> > +   cmd.git_cmd = 1;
> > +
> > +   argv_array_push(, "checkout");
> > +   argv_array_push(, commit);
> > +   argv_array_pushf(_array, GIT_REFLOG_ACTION "=%s", action);
> > +
> > +   if (verbose)
> > +   return run_command();
> > +   return run_command_silent_on_success();
> 
> For the same reason as 1/3, I think it makes more sense to have
> "else" here.
> 

Right.

> > +int checkout_base_commit(struct replay_opts *opts, const char *commit,
> > +int verbose)
> > +{
> > +   const char *action;
> > +
> > +   if (commit && *commit) {
> 
> Hmm, isn't it a programming error to feed !commit or !*commit here?
> I offhand do not think of a reason why making such an input a silent
> no-op success, instead of making it error out, would be a good idea.
> 

I think it’s correct.  

> > +   action = reflog_message(opts, "start", "checkout %s", commit);
> > +   if (run_git_checkout(opts, commit, verbose, action))
> > +   return error(_("Could not checkout %s"), commit);
> > +   }
> > +
> > +   return 0;
> > +}
> > +
> > 
> >  static const char rescheduled_advice[] =
> >  N_("Could not execute the todo command\n"
> >  "\n"
> > 
> > diff --git a/sequencer.h b/sequencer.h
> > index 35730b13e..42c3dda81 100644
> > --- a/sequencer.h
> > +++ b/sequencer.h
> > @@ -100,6 +100,9 @@ int update_head_with_reflog(const struct commit
> > *old_head,> 
> >  void commit_post_rewrite(const struct commit *current_head,
> >  
> >  const struct object_id *new_head);
> > 
> > +int checkout_base_commit(struct replay_opts *opts, const char *commit,
> > +int verbose);
> > +
> > 
> >  #define SUMMARY_INITIAL_COMMIT   (1 << 0)
> >  #define SUMMARY_SHOW_AUTHOR_DATE (1 << 1)
> >  void print_commit_summary(const char *prefix, const struct object_id
> >  *oid,

Cheers,
Alban






Re: [GSoC][PATCH v3 1/3] sequencer: add a new function to silence a command, except if it fails.

2018-06-22 Thread Alban Gruin
Hi Junio,

Le 22/06/2018 à 00:03, Junio C Hamano a écrit :
> Alban Gruin  writes:
> > This adds a new function, run_command_silent_on_success(), to
> > redirect the stdout and stderr of a command to a strbuf, and then to run
> > that command. This strbuf is printed only if the command fails. It is
> > functionnaly similar to output() from git-rebase.sh.
> > 
> > run_git_commit() is then refactored to use of
> > run_command_silent_on_success().
> 
> It might be just a difference in viewpoint, but I think it is more
> customary in this project (hence it will be easier to understand and
> accept by readers of the patch) if a change like this is explained
> NOT as "introducing a new function and then rewrite an existing code
> to use it", and instead as "extract a helper function from an
> existing code so that future callers can reuse it."
> 

I like your wording.  It’s not a difference of point of view, I’m just bad at 
writing commit messages ;)

> > +static int run_command_silent_on_success(struct child_process *cmd)
> > +{
> > +   struct strbuf buf = STRBUF_INIT;
> > +   int rc;
> > +
> > +   cmd->stdout_to_stderr = 1;
> > +   rc = pipe_command(cmd,
> > + NULL, 0,
> > + /* stdout is already redirected */
> 
> I can see that this comment was inherited from the original place
> this code was lifted from (and that is why I say this is not "adding
> a new helper and rewriting an existing piece of code to use it" but
> is "extracting this function out of an existing code for future
> reuse"), but does it still make sense in the new context to keep the
> same comment?
> 
> The original in run_git_commit() made the .stdout_to_stderr=1
> assignment many lines before it called pipe_command(), and it made
> sense to remind readers why we are passing NULL to the out buffer
> and only passing the err buffer here.  But in the context of this
> new helper function, the redirection that may make such a reminder
> necessary sits immediately before the pipe_command() call for
> everybody to see.
> 

Yeah, you’re right.

> > @@ -861,20 +872,8 @@ static int run_git_commit(const char *defmsg, struct
> > replay_opts *opts,> 
> > if (opts->allow_empty_message)
> > 
> > argv_array_push(, "--allow-empty-message");
> > 
> > -   if (cmd.err == -1) {
> > -   /* hide stderr on success */
> > -   struct strbuf buf = STRBUF_INIT;
> > -   int rc = pipe_command(,
> > - NULL, 0,
> > - /* stdout is already redirected */
> > - NULL, 0,
> > - , 0);
> > -   if (rc)
> > -   fputs(buf.buf, stderr);
> > -   strbuf_release();
> > -   return rc;
> > -   }
> > -
> > +   if (is_rebase_i(opts) && !(flags & EDIT_MSG))
> > +   return run_command_silent_on_success();
> > 
> > return run_command();
> >  
> >  }
> 
> It probably is easier to understand the code if you added
> an "else" after, to highlight the fact that this is choosing one out
> of two possible ways to run "cmd", i.e.
> 
>   if (is_rebase_i(opts) && !(flags & EDIT_MSG))
>   return run_command_silent_on_success();
>   else
>   return run_command();

Okay.

Cheers,
Alban






[GSoC][PATCH v3 3/3] rebase -i: rewrite checkout_onto() in C

2018-06-21 Thread Alban Gruin
This rewrites checkout_onto() from shell to C.

A new command (“checkout-onto”) is added to rebase--helper.c. The shell
version is then stripped.

Signed-off-by: Alban Gruin 
---
 builtin/rebase--helper.c   |  7 ++-
 git-rebase--interactive.sh | 25 -
 sequencer.c| 19 +++
 sequencer.h|  3 +++
 4 files changed, 32 insertions(+), 22 deletions(-)

diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index fb5996a2c..20dea4b3a 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -17,7 +17,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
enum {
CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,
-   ADD_EXEC, APPEND_TODO_HELP, EDIT_TODO, CHECKOUT_BASE
+   ADD_EXEC, APPEND_TODO_HELP, EDIT_TODO, CHECKOUT_BASE,
+   CHECKOUT_ONTO
} command = 0;
struct option options[] = {
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
@@ -53,6 +54,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
EDIT_TODO),
OPT_CMDMODE(0, "checkout-base", ,
N_("checkout the base commit"), CHECKOUT_BASE),
+   OPT_CMDMODE(0, "checkout-onto", ,
+   N_("checkout a commit"), CHECKOUT_ONTO),
OPT_END()
};
 
@@ -98,5 +101,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
return !!edit_todo_list(flags);
if (command == CHECKOUT_BASE && argc == 2)
return !!checkout_base_commit(, argv[1], verbose);
+   if (command == CHECKOUT_ONTO && argc == 4)
+   return !!checkout_onto(, argv[1], argv[2], argv[3], 
verbose);
usage_with_options(builtin_rebase_helper_usage, options);
 }
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index af46cf9c2..7b6142a76 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -28,17 +28,6 @@ case "$comment_char" in
;;
 esac
 
-orig_reflog_action="$GIT_REFLOG_ACTION"
-
-comment_for_reflog () {
-   case "$orig_reflog_action" in
-   ''|rebase*)
-   GIT_REFLOG_ACTION="rebase -i ($1)"
-   export GIT_REFLOG_ACTION
-   ;;
-   esac
-}
-
 die_abort () {
apply_autostash
rm -rf "$state_dir"
@@ -70,14 +59,6 @@ collapse_todo_ids() {
git rebase--helper --shorten-ids
 }
 
-# Switch to the branch in $into and notify it in the reflog
-checkout_onto () {
-   comment_for_reflog start
-   GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $onto_name"
-   output git checkout $onto || die_abort "$(gettext "could not detach 
HEAD")"
-   git update-ref ORIG_HEAD $orig_head
-}
-
 get_missing_commit_check_level () {
check_level=$(git config --get rebase.missingCommitsCheck)
check_level=${check_level:-ignore}
@@ -176,7 +157,8 @@ EOF
 
git rebase--helper --check-todo-list || {
ret=$?
-   checkout_onto
+   git rebase--helper --checkout-onto "$onto_name" "$onto" \
+   "$orig_head" ${verbose:+--verbose}
exit $ret
}
 
@@ -186,7 +168,8 @@ EOF
onto="$(git rebase--helper --skip-unnecessary-picks)" ||
die "Could not skip unnecessary pick commands"
 
-   checkout_onto
+   git rebase--helper --checkout-onto "$onto_name" "$onto" "$orig_head" \
+   ${verbose:+--verbose}
require_clean_work_tree "rebase"
exec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \
 --continue
diff --git a/sequencer.c b/sequencer.c
index 27f8453fe..b3b1a2e18 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3159,6 +3159,25 @@ int checkout_base_commit(struct replay_opts *opts, const 
char *commit,
return 0;
 }
 
+int checkout_onto(struct replay_opts *opts,
+ const char *onto_name, const char *onto,
+ const char *orig_head, unsigned verbose)
+{
+   struct object_id oid;
+   const char *action = reflog_message(opts, "start", "checkout %s", 
onto_name);
+
+   if (get_oid(orig_head, ))
+   return error(_("%s: not a valid OID"), orig_head);
+
+   if (run_git_checkout(opts, onto, verbose, action)) {
+   apply_autostash(opts);
+   sequencer_remove_state(opts);
+   return error(_("could not detach HEAD"));
+   }
+
+   return update_ref(NULL, "ORIG_HE

[GSoC][PATCH v3 1/3] sequencer: add a new function to silence a command, except if it fails.

2018-06-21 Thread Alban Gruin
This adds a new function, run_command_silent_on_success(), to
redirect the stdout and stderr of a command to a strbuf, and then to run
that command. This strbuf is printed only if the command fails. It is
functionnaly similar to output() from git-rebase.sh.

run_git_commit() is then refactored to use of
run_command_silent_on_success().

Signed-off-by: Alban Gruin 
---
 sequencer.c | 49 -
 1 file changed, 24 insertions(+), 25 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 7cc76332e..51c8ab7ac 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -766,6 +766,24 @@ N_("you have staged changes in your working tree\n"
 #define VERIFY_MSG  (1<<4)
 #define CREATE_ROOT_COMMIT (1<<5)
 
+static int run_command_silent_on_success(struct child_process *cmd)
+{
+   struct strbuf buf = STRBUF_INIT;
+   int rc;
+
+   cmd->stdout_to_stderr = 1;
+   rc = pipe_command(cmd,
+ NULL, 0,
+ /* stdout is already redirected */
+ NULL, 0,
+ , 0);
+
+   if (rc)
+   fputs(buf.buf, stderr);
+   strbuf_release();
+   return rc;
+}
+
 /*
  * If we are cherry-pick, and if the merge did not result in
  * hand-editing, we will hit this commit and inherit the original
@@ -820,18 +838,11 @@ static int run_git_commit(const char *defmsg, struct 
replay_opts *opts,
 
cmd.git_cmd = 1;
 
-   if (is_rebase_i(opts)) {
-   if (!(flags & EDIT_MSG)) {
-   cmd.stdout_to_stderr = 1;
-   cmd.err = -1;
-   }
+   if (is_rebase_i(opts) && read_env_script(_array)) {
+   const char *gpg_opt = gpg_sign_opt_quoted(opts);
 
-   if (read_env_script(_array)) {
-   const char *gpg_opt = gpg_sign_opt_quoted(opts);
-
-   return error(_(staged_changes_advice),
-gpg_opt, gpg_opt);
-   }
+   return error(_(staged_changes_advice),
+gpg_opt, gpg_opt);
}
 
argv_array_push(, "commit");
@@ -861,20 +872,8 @@ static int run_git_commit(const char *defmsg, struct 
replay_opts *opts,
if (opts->allow_empty_message)
argv_array_push(, "--allow-empty-message");
 
-   if (cmd.err == -1) {
-   /* hide stderr on success */
-   struct strbuf buf = STRBUF_INIT;
-   int rc = pipe_command(,
- NULL, 0,
- /* stdout is already redirected */
- NULL, 0,
- , 0);
-   if (rc)
-   fputs(buf.buf, stderr);
-   strbuf_release();
-   return rc;
-   }
-
+   if (is_rebase_i(opts) && !(flags & EDIT_MSG))
+   return run_command_silent_on_success();
return run_command();
 }
 
-- 
2.17.1



[GSoC][PATCH v3 2/3] rebase -i: rewrite setup_reflog_action() in C

2018-06-21 Thread Alban Gruin
This rewrites (the misnamed) setup_reflog_action() from shell to C. The
new version is called checkout_base_commit().

A new command is added to rebase--helper.c, “checkout-base”, as well as
a new flag, “verbose”, to avoid silencing the output of the checkout
operation called by checkout_base_commit().

The function `run_git_checkout()` will also be used in the next commit,
therefore its code is not part of `checkout_base_commit()`.

The shell version is then stripped in favour of a call to the helper.

As $GIT_REFLOG_ACTION is no longer set at the first call of
checkout_onto(), a call to comment_for_reflog() is added at the
beginning of this function.

Signed-off-by: Alban Gruin 
---
 builtin/rebase--helper.c   |  9 +++--
 git-rebase--interactive.sh | 16 ++--
 sequencer.c| 30 ++
 sequencer.h|  3 +++
 4 files changed, 42 insertions(+), 16 deletions(-)

diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index d2990b210..fb5996a2c 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -12,12 +12,12 @@ static const char * const builtin_rebase_helper_usage[] = {
 int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
 {
struct replay_opts opts = REPLAY_OPTS_INIT;
-   unsigned flags = 0, keep_empty = 0, rebase_merges = 0;
+   unsigned flags = 0, keep_empty = 0, rebase_merges = 0, verbose = 0;
int abbreviate_commands = 0, rebase_cousins = -1;
enum {
CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,
-   ADD_EXEC, APPEND_TODO_HELP, EDIT_TODO
+   ADD_EXEC, APPEND_TODO_HELP, EDIT_TODO, CHECKOUT_BASE
} command = 0;
struct option options[] = {
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
@@ -27,6 +27,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
OPT_BOOL(0, "rebase-merges", _merges, N_("rebase merge 
commits")),
OPT_BOOL(0, "rebase-cousins", _cousins,
 N_("keep original branch points of cousins")),
+   OPT__VERBOSE(, N_("be verbose")),
OPT_CMDMODE(0, "continue", , N_("continue rebase"),
CONTINUE),
OPT_CMDMODE(0, "abort", , N_("abort rebase"),
@@ -50,6 +51,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
OPT_CMDMODE(0, "edit-todo", ,
N_("edit the todo list during an interactive 
rebase"),
EDIT_TODO),
+   OPT_CMDMODE(0, "checkout-base", ,
+   N_("checkout the base commit"), CHECKOUT_BASE),
OPT_END()
};
 
@@ -93,5 +96,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
return !!append_todo_help(0, keep_empty);
if (command == EDIT_TODO && argc == 1)
return !!edit_todo_list(flags);
+   if (command == CHECKOUT_BASE && argc == 2)
+   return !!checkout_base_commit(, argv[1], verbose);
usage_with_options(builtin_rebase_helper_usage, options);
 }
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 2defe607f..af46cf9c2 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -72,6 +72,7 @@ collapse_todo_ids() {
 
 # Switch to the branch in $into and notify it in the reflog
 checkout_onto () {
+   comment_for_reflog start
GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $onto_name"
output git checkout $onto || die_abort "$(gettext "could not detach 
HEAD")"
git update-ref ORIG_HEAD $orig_head
@@ -119,19 +120,6 @@ initiate_action () {
esac
 }
 
-setup_reflog_action () {
-   comment_for_reflog start
-
-   if test ! -z "$switch_to"
-   then
-   GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $switch_to"
-   output git checkout "$switch_to" -- ||
-   die "$(eval_gettext "Could not checkout \$switch_to")"
-
-   comment_for_reflog start
-   fi
-}
-
 init_basic_state () {
orig_head=$(git rev-parse --verify HEAD) || die "$(gettext "No HEAD?")"
mkdir -p "$state_dir" || die "$(eval_gettext "Could not create 
temporary \$state_dir")"
@@ -211,7 +199,7 @@ git_rebase__interactive () {
return 0
fi
 
-   setup_reflog_action
+   git rebase--helper --checkout-base "$switch_to" ${verbose:+--verbose}
init_basic_state
 
 

[GSoC][PATCH v3 0/3] rebase -i: rewrite reflog operations in C

2018-06-21 Thread Alban Gruin
Thes patch series rewrites the reflog operations from shell to C.  This
is part of the effort to rewrite interactive rebase in C.

The first commit is dedicated to creating a function to silence a
command, as the sequencer will do in several places with these patches.

This branch is based on ag/rebase-i-rewrite-todo, and does not conflict
with pu (as of 2018-06-21).

Changes since v2:

 - Removing the “verbose” parameter to run_command_silent_on_success()

 - Rewording some parts of the second commit

 - Changing the help for the “--verbose” flag

Alban Gruin (3):
  sequencer: add a new function to silence a command, except if it
fails.
  rebase -i: rewrite setup_reflog_action() in C
  rebase -i: rewrite checkout_onto() in C

 builtin/rebase--helper.c   | 14 +-
 git-rebase--interactive.sh | 39 ++-
 sequencer.c| 98 --
 sequencer.h|  6 +++
 4 files changed, 96 insertions(+), 61 deletions(-)

-- 
2.17.1



Re: [GSoC][PATCH v2 1/3] sequencer: add a new function to silence a command, except if it fails.

2018-06-21 Thread Alban Gruin
Hi Johannes,

Le 21/06/2018 à 11:37, Johannes Schindelin a écrit :
> Hi Alban,
> 
> On Tue, 19 Jun 2018, Alban Gruin wrote:
> 
>> diff --git a/sequencer.c b/sequencer.c
>> index 7cc76332e..9aa7ddb33 100644
>> --- a/sequencer.c
>> +++ b/sequencer.c
>> @@ -766,6 +766,29 @@ N_("you have staged changes in your working tree\n"
>>  #define VERIFY_MSG  (1<<4)
>>  #define CREATE_ROOT_COMMIT (1<<5)
>>  
>> +static int run_command_silent_on_success(struct child_process *cmd,
>> + unsigned verbose)
>> +{
>> +struct strbuf buf = STRBUF_INIT;
>> +int rc;
>> +
>> +if (verbose)
>> +return run_command(cmd);
>> +
>> +/* hide stderr on success */
>> +cmd->stdout_to_stderr = 1;
> 
> This comment is a bit misleading: that line does not hide stderr on
> success, it redirects stdout to stderr instead.
> 
>> +rc = pipe_command(cmd,
>> +  NULL, 0,
>> +  /* stdout is already redirected */
>> +  NULL, 0,
>> +  , 0);
>> +
>> +if (rc)
>> +fputs(buf.buf, stderr);
>> +strbuf_release();
>> +return rc;
>> +}
>> +
>>  /*
>>   * If we are cherry-pick, and if the merge did not result in
>>   * hand-editing, we will hit this commit and inherit the original
>> @@ -820,18 +843,11 @@ static int run_git_commit(const char *defmsg, struct 
>> replay_opts *opts,
>>  
>>  cmd.git_cmd = 1;
>>  
>> -if (is_rebase_i(opts)) {
>> -if (!(flags & EDIT_MSG)) {
>> -cmd.stdout_to_stderr = 1;
>> -cmd.err = -1;
>> -}
> 
> This code made sure that we *only* do this redirection, and stderr
> buffering, if `git commit` is called non-interactively. When it is called
> interactively, redirecting stdout and stderr is absolutely not what we
> want.
> 
>> +if (is_rebase_i(opts) && read_env_script(_array)) {
>> +const char *gpg_opt = gpg_sign_opt_quoted(opts);
>>  
>> -if (read_env_script(_array)) {
>> -const char *gpg_opt = gpg_sign_opt_quoted(opts);
>> -
>> -return error(_(staged_changes_advice),
>> - gpg_opt, gpg_opt);
>> -}
>> +return error(_(staged_changes_advice),
>> + gpg_opt, gpg_opt);
>>  }
>>  
>>  argv_array_push(, "commit");
>> @@ -861,21 +877,8 @@ static int run_git_commit(const char *defmsg, struct 
>> replay_opts *opts,
>>  if (opts->allow_empty_message)
>>  argv_array_push(, "--allow-empty-message");
>>  
>> -if (cmd.err == -1) {
>> -/* hide stderr on success */
>> -struct strbuf buf = STRBUF_INIT;
>> -int rc = pipe_command(,
>> -  NULL, 0,
>> -  /* stdout is already redirected */
>> -  NULL, 0,
>> -  , 0);
>> -if (rc)
>> -fputs(buf.buf, stderr);
>> -strbuf_release();
>> -return rc;
>> -}
>> -
>> -return run_command();
>> +return run_command_silent_on_success(,
>> + !(is_rebase_i(opts) && !(flags & 
>> EDIT_MSG)));
> 
> It would probably make more sense to change the signature of
> `run_command_silent_on_success()` to not even take the `int verbose`
> parameter: why call it "silent on success" when we can ask it *not* to be
> silent on success?
> 
> And then you can avoid this overly-long line (as well as the quite
> convoluted Boolean logic that took me a couple of seconds to verify) very
> elegantly by this code:
> 
>   if (is_rebase_i(opts) && !(flags & EDIT_MSG))
>   return run_command_silent_on_success();
>   return run_command();
> 
> I vaguely recall that I wanted to make this an option in the `struct
> child_process` when I originally introduced this code, but I think it was
> Peff who suggested that doing it "by hand" was the more appropriate way
> here because I use it only once.
> 
> My recollection might fail me, but if it is correct, maybe that would be a
> good way forward, to make this an `int silent_on_success:1;` field?
> 

I think I found it:

https://public-inbox.org/git/1e82aeabb906a35175362418b2b4957fae50c3b0.1481642927.git.johannes.schinde...@gmx.de/

Apparently, you wanted to introduce a new RUN_ flag for
run_command_v_opt_cd_env(), but the change was qualified as a “bolted-on
feature” by Johannes Sixt.

So, I will remove the “verbose” argument in the reroll.

Cheers,
Alban



[GSoC][PATCH v2 3/3] rebase -i: rewrite checkout_onto() in C

2018-06-19 Thread Alban Gruin
This rewrites checkout_onto() from shell to C.

A new command (“checkout-onto”) is added to rebase--helper.c. The shell
version is then stripped.

Signed-off-by: Alban Gruin 
---
 builtin/rebase--helper.c   |  7 ++-
 git-rebase--interactive.sh | 25 -
 sequencer.c| 19 +++
 sequencer.h|  3 +++
 4 files changed, 32 insertions(+), 22 deletions(-)

diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index 7cd74da2e..19c1dba9a 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -17,7 +17,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
enum {
CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,
-   ADD_EXEC, APPEND_TODO_HELP, EDIT_TODO, CHECKOUT_BASE
+   ADD_EXEC, APPEND_TODO_HELP, EDIT_TODO, CHECKOUT_BASE,
+   CHECKOUT_ONTO
} command = 0;
struct option options[] = {
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
@@ -53,6 +54,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
EDIT_TODO),
OPT_CMDMODE(0, "checkout-base", ,
N_("checkout the base commit"), CHECKOUT_BASE),
+   OPT_CMDMODE(0, "checkout-onto", ,
+   N_("checkout a commit"), CHECKOUT_ONTO),
OPT_END()
};
 
@@ -98,5 +101,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
return !!edit_todo_list(flags);
if (command == CHECKOUT_BASE && argc == 2)
return !!checkout_base_commit(, argv[1], verbose);
+   if (command == CHECKOUT_ONTO && argc == 4)
+   return !!checkout_onto(, argv[1], argv[2], argv[3], 
verbose);
usage_with_options(builtin_rebase_helper_usage, options);
 }
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index af46cf9c2..7b6142a76 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -28,17 +28,6 @@ case "$comment_char" in
;;
 esac
 
-orig_reflog_action="$GIT_REFLOG_ACTION"
-
-comment_for_reflog () {
-   case "$orig_reflog_action" in
-   ''|rebase*)
-   GIT_REFLOG_ACTION="rebase -i ($1)"
-   export GIT_REFLOG_ACTION
-   ;;
-   esac
-}
-
 die_abort () {
apply_autostash
rm -rf "$state_dir"
@@ -70,14 +59,6 @@ collapse_todo_ids() {
git rebase--helper --shorten-ids
 }
 
-# Switch to the branch in $into and notify it in the reflog
-checkout_onto () {
-   comment_for_reflog start
-   GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $onto_name"
-   output git checkout $onto || die_abort "$(gettext "could not detach 
HEAD")"
-   git update-ref ORIG_HEAD $orig_head
-}
-
 get_missing_commit_check_level () {
check_level=$(git config --get rebase.missingCommitsCheck)
check_level=${check_level:-ignore}
@@ -176,7 +157,8 @@ EOF
 
git rebase--helper --check-todo-list || {
ret=$?
-   checkout_onto
+   git rebase--helper --checkout-onto "$onto_name" "$onto" \
+   "$orig_head" ${verbose:+--verbose}
exit $ret
}
 
@@ -186,7 +168,8 @@ EOF
onto="$(git rebase--helper --skip-unnecessary-picks)" ||
die "Could not skip unnecessary pick commands"
 
-   checkout_onto
+   git rebase--helper --checkout-onto "$onto_name" "$onto" "$orig_head" \
+   ${verbose:+--verbose}
require_clean_work_tree "rebase"
exec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \
 --continue
diff --git a/sequencer.c b/sequencer.c
index a7a73e3ef..9165bf96c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3161,6 +3161,25 @@ int checkout_base_commit(struct replay_opts *opts, const 
char *commit,
return 0;
 }
 
+int checkout_onto(struct replay_opts *opts,
+ const char *onto_name, const char *onto,
+ const char *orig_head, unsigned verbose)
+{
+   struct object_id oid;
+   const char *action = reflog_message(opts, "start", "checkout %s", 
onto_name);
+
+   if (get_oid(orig_head, ))
+   return error(_("%s: not a valid OID"), orig_head);
+
+   if (run_git_checkout(opts, onto, verbose, action)) {
+   apply_autostash(opts);
+   sequencer_remove_state(opts);
+   return error(_("could not detach HEAD"));
+   }
+
+   return update_ref(NULL, "ORIG_HE

[GSoC][PATCH v2 2/3] rebase -i: rewrite setup_reflog_action() in C

2018-06-19 Thread Alban Gruin
This rewrites setup_reflog_action() from shell to C. The new version is
called checkout_base_commit().

A new command is added to rebase--helper.c, “checkout-base”, as such as
a new flag, “verbose”, to avoid silencing the output of the checkout
operation called by checkout_base_commit().

The shell version is then stripped in favour of a call to the helper. As
$GIT_REFLOG_ACTION is not longer set at the first call of
checkout_onto(), a call to comment_for_reflog() is added at the
beginning of this function.

Signed-off-by: Alban Gruin 
---
 builtin/rebase--helper.c   |  9 +++--
 git-rebase--interactive.sh | 16 ++--
 sequencer.c| 28 
 sequencer.h|  3 +++
 4 files changed, 40 insertions(+), 16 deletions(-)

diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index d2990b210..7cd74da2e 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -12,12 +12,12 @@ static const char * const builtin_rebase_helper_usage[] = {
 int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
 {
struct replay_opts opts = REPLAY_OPTS_INIT;
-   unsigned flags = 0, keep_empty = 0, rebase_merges = 0;
+   unsigned flags = 0, keep_empty = 0, rebase_merges = 0, verbose = 0;
int abbreviate_commands = 0, rebase_cousins = -1;
enum {
CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,
-   ADD_EXEC, APPEND_TODO_HELP, EDIT_TODO
+   ADD_EXEC, APPEND_TODO_HELP, EDIT_TODO, CHECKOUT_BASE
} command = 0;
struct option options[] = {
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
@@ -27,6 +27,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
OPT_BOOL(0, "rebase-merges", _merges, N_("rebase merge 
commits")),
OPT_BOOL(0, "rebase-cousins", _cousins,
 N_("keep original branch points of cousins")),
+   OPT__VERBOSE(, N_("print subcommands output even if 
they succeed")),
OPT_CMDMODE(0, "continue", , N_("continue rebase"),
CONTINUE),
OPT_CMDMODE(0, "abort", , N_("abort rebase"),
@@ -50,6 +51,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
OPT_CMDMODE(0, "edit-todo", ,
N_("edit the todo list during an interactive 
rebase"),
EDIT_TODO),
+   OPT_CMDMODE(0, "checkout-base", ,
+   N_("checkout the base commit"), CHECKOUT_BASE),
OPT_END()
};
 
@@ -93,5 +96,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
return !!append_todo_help(0, keep_empty);
if (command == EDIT_TODO && argc == 1)
return !!edit_todo_list(flags);
+   if (command == CHECKOUT_BASE && argc == 2)
+   return !!checkout_base_commit(, argv[1], verbose);
usage_with_options(builtin_rebase_helper_usage, options);
 }
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 2defe607f..af46cf9c2 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -72,6 +72,7 @@ collapse_todo_ids() {
 
 # Switch to the branch in $into and notify it in the reflog
 checkout_onto () {
+   comment_for_reflog start
GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $onto_name"
output git checkout $onto || die_abort "$(gettext "could not detach 
HEAD")"
git update-ref ORIG_HEAD $orig_head
@@ -119,19 +120,6 @@ initiate_action () {
esac
 }
 
-setup_reflog_action () {
-   comment_for_reflog start
-
-   if test ! -z "$switch_to"
-   then
-   GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $switch_to"
-   output git checkout "$switch_to" -- ||
-   die "$(eval_gettext "Could not checkout \$switch_to")"
-
-   comment_for_reflog start
-   fi
-}
-
 init_basic_state () {
orig_head=$(git rev-parse --verify HEAD) || die "$(gettext "No HEAD?")"
mkdir -p "$state_dir" || die "$(eval_gettext "Could not create 
temporary \$state_dir")"
@@ -211,7 +199,7 @@ git_rebase__interactive () {
return 0
fi
 
-   setup_reflog_action
+   git rebase--helper --checkout-base "$switch_to" ${verbose:+--verbose}
init_basic_state
 
init_revisions_and_shortrevisions
diff --git a/sequencer.c b/sequencer.c
index 9aa7ddb33..a7a73e3ef 100644

[GSoC][PATCH v2 1/3] sequencer: add a new function to silence a command, except if it fails.

2018-06-19 Thread Alban Gruin
This adds a new function, run_command_silent_on_success(), to redirect
the stdout and stderr of a command to a strbuf, and then to run that
command. This strbuf is printed only if the command fails. It also takes
a parameter, “verbose”. When true, the command is executed without
redirecting its output. It is functionnally similar to output() from
git-rebase.sh.

run_git_commit() is then refactored to use of
run_command_silent_on_success().

Signed-off-by: Alban Gruin 
---
 sequencer.c | 55 +--
 1 file changed, 29 insertions(+), 26 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 7cc76332e..9aa7ddb33 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -766,6 +766,29 @@ N_("you have staged changes in your working tree\n"
 #define VERIFY_MSG  (1<<4)
 #define CREATE_ROOT_COMMIT (1<<5)
 
+static int run_command_silent_on_success(struct child_process *cmd,
+unsigned verbose)
+{
+   struct strbuf buf = STRBUF_INIT;
+   int rc;
+
+   if (verbose)
+   return run_command(cmd);
+
+   /* hide stderr on success */
+   cmd->stdout_to_stderr = 1;
+   rc = pipe_command(cmd,
+ NULL, 0,
+ /* stdout is already redirected */
+ NULL, 0,
+ , 0);
+
+   if (rc)
+   fputs(buf.buf, stderr);
+   strbuf_release();
+   return rc;
+}
+
 /*
  * If we are cherry-pick, and if the merge did not result in
  * hand-editing, we will hit this commit and inherit the original
@@ -820,18 +843,11 @@ static int run_git_commit(const char *defmsg, struct 
replay_opts *opts,
 
cmd.git_cmd = 1;
 
-   if (is_rebase_i(opts)) {
-   if (!(flags & EDIT_MSG)) {
-   cmd.stdout_to_stderr = 1;
-   cmd.err = -1;
-   }
+   if (is_rebase_i(opts) && read_env_script(_array)) {
+   const char *gpg_opt = gpg_sign_opt_quoted(opts);
 
-   if (read_env_script(_array)) {
-   const char *gpg_opt = gpg_sign_opt_quoted(opts);
-
-   return error(_(staged_changes_advice),
-gpg_opt, gpg_opt);
-   }
+   return error(_(staged_changes_advice),
+gpg_opt, gpg_opt);
}
 
argv_array_push(, "commit");
@@ -861,21 +877,8 @@ static int run_git_commit(const char *defmsg, struct 
replay_opts *opts,
if (opts->allow_empty_message)
argv_array_push(, "--allow-empty-message");
 
-   if (cmd.err == -1) {
-   /* hide stderr on success */
-   struct strbuf buf = STRBUF_INIT;
-   int rc = pipe_command(,
- NULL, 0,
- /* stdout is already redirected */
- NULL, 0,
- , 0);
-   if (rc)
-   fputs(buf.buf, stderr);
-   strbuf_release();
-   return rc;
-   }
-
-   return run_command();
+   return run_command_silent_on_success(,
+!(is_rebase_i(opts) && !(flags & 
EDIT_MSG)));
 }
 
 static int rest_is_empty(const struct strbuf *sb, int start)
-- 
2.16.4



[GSoC][PATCH v2 0/3] rebase -i: rewrite reflog operations in C

2018-06-19 Thread Alban Gruin
This patch series rewrites the reflog operations from shell to C.  This
is part of the effort to rewrite interactive rebase in C.

The first commit is dedicated to creating a function to silence a
command, as the sequencer will do in several places with these patches.

This branch is based on ag/rebase-i-rewrite-todo, and does not conflict
with pu (as of 2018-06-19).

Changes since v1:

 - Replacing run_command_silent_if_successful() by
   run_command_silent_on_success() in the first commit’s message (thanks
   Christian!)

 - Adding a “verbose” parameter to run_command_silent_on_success()
   (thanks Phillip!)

 - Using OPT__VERBOSE to parse the “--verbose” flag (thanks Stefan!)

 - Fixing some typos and errors in the commit messages

 - Renaming “setup-reflog” to “checkout-base”

 - Renaming checkout_base_commit() to run_git_checkout()

 - Replacing calls to die() by error()

Alban Gruin (3):
  sequencer: add a new function to silence a command, except if it
fails.
  rebase -i: rewrite setup_reflog_action() in C
  rebase -i: rewrite checkout_onto() in C

 builtin/rebase--helper.c   |  14 ++-
 git-rebase--interactive.sh |  39 +++--
 sequencer.c| 102 +
 sequencer.h|   6 +++
 4 files changed, 99 insertions(+), 62 deletions(-)

-- 
2.16.4



[GSoC] GSoC with git, week 7

2018-06-18 Thread Alban Gruin
Hi,

I published a new blog post here:

  https://blog.pa1ch.fr/posts/2018/06/18/en/gsoc2018-week-7.html

It’s shorter than the last one, but feel free to tell me what you think
about it!  :)

Cheers,
Alban



Re: [GSoC][PATCH 1/3] sequencer: add a new function to silence a command, except if it fails.

2018-06-18 Thread Alban Gruin
Hi Christian,

Le 18/06/2018 à 18:26, Christian Couder a écrit :
> Hi Alban,
> 
> On Mon, Jun 18, 2018 at 3:18 PM, Alban Gruin  wrote:
>> This adds a new function, run_command_silent_if_successful(),
> 
> He re the function is called run_command_silent_if_successful()...
> 
>> to
>> redirect the stdout and stderr of a command to a strbuf, and then to run
>> that command. This strbuf is printed only if the command fails. It is
>> functionnaly similar to output() from git-rebase.sh.
>>
>> run_git_commit() is then refactored to use of
>> run_command_silent_if_successful().
> 
> ...here also...
> 
> [...]
> 
>> +static int run_command_silent_on_success(struct child_process *cmd)
> 
> ...but here it is called run_command_silent_on_success().
> 
> Thanks,
> Christian.
> 

Oops, my bad.  I will fix this in a reroll.

Cheers,
Alban



Re: [GSoC][PATCH 2/3] rebase -i: rewrite setup_reflog_action() in C

2018-06-18 Thread Alban Gruin
Hi Phillip,

Le 18/06/2018 à 17:34, Phillip Wood a écrit :
> On 18/06/18 14:18, Alban Gruin wrote:
>> This rewrites setup_reflog_action() from shell to C.
>>
>> A new command is added to rebase--helper.c, “setup-reflog”, as such as a
>> new flag, “verbose”, to silence the output of the checkout operation
>> called by setup_reflog_action().
> 
> I'm having difficulty understanding what that means, surely the verbose
> flag is to stop the output from being silenced.
> 

I reversed the meaning of the flag, I will correct it in a reroll.

> I'm not that keen on the naming in this patch, if it's only a staging
> post and will be removed it probably doesn't matter too much. I can see
> why you've based the function and flag names on the shell version, but
> the C version is not setting up the reflog it is checking out the branch
> we're rebasing. --checkout-base or something like that would be clearer.
> 
> Also the name of the function that does the checkout is tied to checking
> out the base revision, but then reused in the next patch to checkout the
> 'onto' commit. As such I think it would be clearer if it was called
> run_git_checkout() or something like that.
> 

Right.

> One further thought - how easy would it be to refactor the code in
> do_reset() to handle the checkouts in this patch series, rather than
> having to fork 'git checkout'
> 

Good remark, I did not notice do_reset().  I intend to refactor and
optimize after I have done (most of) the conversion, so this goes into
my todo-list, but it does not really seem difficult at first sight.

Cheers,
Alban



Re: [GSoC][PATCH 3/3] rebase -i: rewrite checkout_onto() in C

2018-06-18 Thread Alban Gruin
Hi Phillip,

Le 18/06/2018 à 18:09, Phillip Wood a écrit :
>> +    if (get_oid(orig_head, ))
>> +    die(_("%s: not a valid OID"), orig_head);
> 
> If this code is going to live long-term in sequencer.c then it would be
> better not to die, but return an error instead as it's part of libgit.
> 

Right, I will fix this.

Cheers,
Alban



Re: [GSoC][PATCH 1/3] sequencer: add a new function to silence a command, except if it fails.

2018-06-18 Thread Alban Gruin
Hi Phillip,

Le 18/06/2018 à 17:26, Phillip Wood a écrit :
> Hi Alban
> 
> On 18/06/18 14:18, Alban Gruin wrote:
>> This adds a new function, run_command_silent_if_successful(), to
>> redirect the stdout and stderr of a command to a strbuf, and then to run
>> that command. This strbuf is printed only if the command fails. It is
>> functionnaly similar to output() from git-rebase.sh.
> 
> s/functionnaly/functionally/
> 
> It's not quite the same though because the shell versions handles
> --verbose where as here the caller has to put that check in every call
> site. I wonder if it would simplify the callers if the C version did the
> --verbose handling it's self.
> 

That’s what I did first, but removed it because I thought it would be
less confusing.  I’m fine with this solution, though.  Do you want me to
do this instead?

Cheers,
Alban



[GSoC][PATCH 3/3] rebase -i: rewrite checkout_onto() in C

2018-06-18 Thread Alban Gruin
This rewrites checkout_onto() from shell to C.

A new command (“checkout-onto”) is added to rebase--helper.c. The shell
version is then stripped.

Signed-off-by: Alban Gruin 
---
 builtin/rebase--helper.c   |  7 ++-
 git-rebase--interactive.sh | 25 -
 sequencer.c| 19 +++
 sequencer.h|  3 +++
 4 files changed, 32 insertions(+), 22 deletions(-)

diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index d677fb663..f9fffba96 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -17,7 +17,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
enum {
CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,
-   ADD_EXEC, APPEND_TODO_HELP, EDIT_TODO, SETUP_REFLOG
+   ADD_EXEC, APPEND_TODO_HELP, EDIT_TODO, SETUP_REFLOG,
+   CHECKOUT_ONTO
} command = 0;
struct option options[] = {
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
@@ -53,6 +54,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
EDIT_TODO),
OPT_CMDMODE(0, "setup-reflog", ,
N_("setup the reflog action"), SETUP_REFLOG),
+   OPT_CMDMODE(0, "checkout-onto", ,
+   N_("checkout a commit"), CHECKOUT_ONTO),
OPT_END()
};
 
@@ -98,5 +101,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
return !!edit_todo_list(flags);
if (command == SETUP_REFLOG && argc == 2)
return !!setup_reflog_action(, argv[1], verbose);
+   if (command == CHECKOUT_ONTO && argc == 4)
+   return !!checkout_onto(, argv[1], argv[2], argv[3], 
verbose);
usage_with_options(builtin_rebase_helper_usage, options);
 }
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 048bbf041..0ae053291 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -28,17 +28,6 @@ case "$comment_char" in
;;
 esac
 
-orig_reflog_action="$GIT_REFLOG_ACTION"
-
-comment_for_reflog () {
-   case "$orig_reflog_action" in
-   ''|rebase*)
-   GIT_REFLOG_ACTION="rebase -i ($1)"
-   export GIT_REFLOG_ACTION
-   ;;
-   esac
-}
-
 die_abort () {
apply_autostash
rm -rf "$state_dir"
@@ -70,14 +59,6 @@ collapse_todo_ids() {
git rebase--helper --shorten-ids
 }
 
-# Switch to the branch in $into and notify it in the reflog
-checkout_onto () {
-   comment_for_reflog start
-   GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $onto_name"
-   output git checkout $onto || die_abort "$(gettext "could not detach 
HEAD")"
-   git update-ref ORIG_HEAD $orig_head
-}
-
 get_missing_commit_check_level () {
check_level=$(git config --get rebase.missingCommitsCheck)
check_level=${check_level:-ignore}
@@ -176,7 +157,8 @@ EOF
 
git rebase--helper --check-todo-list || {
ret=$?
-   checkout_onto
+   git rebase--helper --checkout-onto "$onto_name" "$onto" \
+   "$orig_head" ${verbose:+--verbose}
exit $ret
}
 
@@ -186,7 +168,8 @@ EOF
onto="$(git rebase--helper --skip-unnecessary-picks)" ||
die "Could not skip unnecessary pick commands"
 
-   checkout_onto
+   git rebase--helper --checkout-onto "$onto_name" "$onto" "$orig_head" \
+   ${verbose:+--verbose}
require_clean_work_tree "rebase"
exec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \
 --continue
diff --git a/sequencer.c b/sequencer.c
index 4bfe29c7b..d149cbf92 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3162,6 +3162,25 @@ int setup_reflog_action(struct replay_opts *opts, const 
char *commit,
return 0;
 }
 
+int checkout_onto(struct replay_opts *opts,
+ const char *onto_name, const char *onto,
+ const char *orig_head, unsigned verbose)
+{
+   struct object_id oid;
+   const char *action = reflog_message(opts, "start", "checkout %s", 
onto_name);
+
+   if (get_oid(orig_head, ))
+   die(_("%s: not a valid OID"), orig_head);
+
+   if (checkout_base_commit(opts, onto, verbose, action)) {
+   apply_autostash(opts);
+   sequencer_remove_state(opts);
+   die(_("could not detach HEAD"));
+   }
+
+   return update_ref(NULL, "ORIG_HEAD", , NULL, 0,

[GSoC][PATCH 2/3] rebase -i: rewrite setup_reflog_action() in C

2018-06-18 Thread Alban Gruin
This rewrites setup_reflog_action() from shell to C.

A new command is added to rebase--helper.c, “setup-reflog”, as such as a
new flag, “verbose”, to silence the output of the checkout operation
called by setup_reflog_action().

The shell version is then stripped in favour of a call to the helper. As
$GIT_REFLOG_ACTION is not longer set at the first call of
checkout_onto(), a call to comment_for_reflog() is added at the
beginning of this function.

Signed-off-by: Alban Gruin 
---
 builtin/rebase--helper.c   |  9 +++--
 git-rebase--interactive.sh | 16 ++--
 sequencer.c| 31 +++
 sequencer.h|  3 +++
 4 files changed, 43 insertions(+), 16 deletions(-)

diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index d2990b210..d677fb663 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -12,12 +12,12 @@ static const char * const builtin_rebase_helper_usage[] = {
 int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
 {
struct replay_opts opts = REPLAY_OPTS_INIT;
-   unsigned flags = 0, keep_empty = 0, rebase_merges = 0;
+   unsigned flags = 0, keep_empty = 0, rebase_merges = 0, verbose = 0;
int abbreviate_commands = 0, rebase_cousins = -1;
enum {
CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,
-   ADD_EXEC, APPEND_TODO_HELP, EDIT_TODO
+   ADD_EXEC, APPEND_TODO_HELP, EDIT_TODO, SETUP_REFLOG
} command = 0;
struct option options[] = {
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
@@ -27,6 +27,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
OPT_BOOL(0, "rebase-merges", _merges, N_("rebase merge 
commits")),
OPT_BOOL(0, "rebase-cousins", _cousins,
 N_("keep original branch points of cousins")),
+   OPT_BOOL(0, "verbose", , N_("verbose")),
OPT_CMDMODE(0, "continue", , N_("continue rebase"),
CONTINUE),
OPT_CMDMODE(0, "abort", , N_("abort rebase"),
@@ -50,6 +51,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
OPT_CMDMODE(0, "edit-todo", ,
N_("edit the todo list during an interactive 
rebase"),
EDIT_TODO),
+   OPT_CMDMODE(0, "setup-reflog", ,
+   N_("setup the reflog action"), SETUP_REFLOG),
OPT_END()
};
 
@@ -93,5 +96,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
return !!append_todo_help(0, keep_empty);
if (command == EDIT_TODO && argc == 1)
return !!edit_todo_list(flags);
+   if (command == SETUP_REFLOG && argc == 2)
+   return !!setup_reflog_action(, argv[1], verbose);
usage_with_options(builtin_rebase_helper_usage, options);
 }
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 2defe607f..048bbf041 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -72,6 +72,7 @@ collapse_todo_ids() {
 
 # Switch to the branch in $into and notify it in the reflog
 checkout_onto () {
+   comment_for_reflog start
GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $onto_name"
output git checkout $onto || die_abort "$(gettext "could not detach 
HEAD")"
git update-ref ORIG_HEAD $orig_head
@@ -119,19 +120,6 @@ initiate_action () {
esac
 }
 
-setup_reflog_action () {
-   comment_for_reflog start
-
-   if test ! -z "$switch_to"
-   then
-   GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $switch_to"
-   output git checkout "$switch_to" -- ||
-   die "$(eval_gettext "Could not checkout \$switch_to")"
-
-   comment_for_reflog start
-   fi
-}
-
 init_basic_state () {
orig_head=$(git rev-parse --verify HEAD) || die "$(gettext "No HEAD?")"
mkdir -p "$state_dir" || die "$(eval_gettext "Could not create 
temporary \$state_dir")"
@@ -211,7 +199,7 @@ git_rebase__interactive () {
return 0
fi
 
-   setup_reflog_action
+   git rebase--helper --setup-reflog "$switch_to" ${verbose:+--verbose}
init_basic_state
 
init_revisions_and_shortrevisions
diff --git a/sequencer.c b/sequencer.c
index 3437673d2..4bfe29c7b 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3131,6 +3131,37 @@ static const char *reflog_me

[GSoC][PATCH 0/3] rebase -i: rewrite reflog operations in C

2018-06-18 Thread Alban Gruin
This patch series rewrites the reflog operations from shell to C.  This
is part of the effort to rewrite interactive rebase in C.

The first commit is dedicated to creating a function to silence a
command, as the sequencer will do in several places with these patches.

This branch is based on ag/rebase-i-rewrite-todo, and does not conflict
with pu (as of 2018-06-18).

Alban Gruin (3):
  sequencer: add a new function to silence a command, except if it
fails.
  rebase -i: rewrite setup_reflog_action() in C
  rebase -i: rewrite checkout_onto() in C

 builtin/rebase--helper.c   |  14 +-
 git-rebase--interactive.sh |  39 +++--
 sequencer.c| 103 +
 sequencer.h|   6 +++
 4 files changed, 100 insertions(+), 62 deletions(-)

-- 
2.16.4



[GSoC][PATCH 1/3] sequencer: add a new function to silence a command, except if it fails.

2018-06-18 Thread Alban Gruin
This adds a new function, run_command_silent_if_successful(), to
redirect the stdout and stderr of a command to a strbuf, and then to run
that command. This strbuf is printed only if the command fails. It is
functionnaly similar to output() from git-rebase.sh.

run_git_commit() is then refactored to use of
run_command_silent_if_successful().

Signed-off-by: Alban Gruin 
---
 sequencer.c | 53 +++--
 1 file changed, 27 insertions(+), 26 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 7cc76332e..3437673d2 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -766,6 +766,25 @@ N_("you have staged changes in your working tree\n"
 #define VERIFY_MSG  (1<<4)
 #define CREATE_ROOT_COMMIT (1<<5)
 
+static int run_command_silent_on_success(struct child_process *cmd)
+{
+   struct strbuf buf = STRBUF_INIT;
+   int rc;
+
+   /* hide stderr on success */
+   cmd->stdout_to_stderr = 1;
+   rc = pipe_command(cmd,
+ NULL, 0,
+ /* stdout is already redirected */
+ NULL, 0,
+ , 0);
+
+   if (rc)
+   fputs(buf.buf, stderr);
+   strbuf_release();
+   return rc;
+}
+
 /*
  * If we are cherry-pick, and if the merge did not result in
  * hand-editing, we will hit this commit and inherit the original
@@ -820,18 +839,11 @@ static int run_git_commit(const char *defmsg, struct 
replay_opts *opts,
 
cmd.git_cmd = 1;
 
-   if (is_rebase_i(opts)) {
-   if (!(flags & EDIT_MSG)) {
-   cmd.stdout_to_stderr = 1;
-   cmd.err = -1;
-   }
+   if (is_rebase_i(opts) && read_env_script(_array)) {
+   const char *gpg_opt = gpg_sign_opt_quoted(opts);
 
-   if (read_env_script(_array)) {
-   const char *gpg_opt = gpg_sign_opt_quoted(opts);
-
-   return error(_(staged_changes_advice),
-gpg_opt, gpg_opt);
-   }
+   return error(_(staged_changes_advice),
+gpg_opt, gpg_opt);
}
 
argv_array_push(, "commit");
@@ -861,21 +873,10 @@ static int run_git_commit(const char *defmsg, struct 
replay_opts *opts,
if (opts->allow_empty_message)
argv_array_push(, "--allow-empty-message");
 
-   if (cmd.err == -1) {
-   /* hide stderr on success */
-   struct strbuf buf = STRBUF_INIT;
-   int rc = pipe_command(,
- NULL, 0,
- /* stdout is already redirected */
- NULL, 0,
- , 0);
-   if (rc)
-   fputs(buf.buf, stderr);
-   strbuf_release();
-   return rc;
-   }
-
-   return run_command();
+   if (is_rebase_i(opts) && !(flags & EDIT_MSG))
+   return run_command_silent_on_success();
+   else
+   return run_command();
 }
 
 static int rest_is_empty(const struct strbuf *sb, int start)
-- 
2.16.4



Re: [GSoC][PATCH v2 2/2] rebase--interactive: rewrite the edit-todo functionality in C

2018-06-14 Thread Alban Gruin
Hi Junio,

Le 14/06/2018 à 22:14, Junio C Hamano a écrit :
> Alban Gruin  writes:
> 
>> diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
>> index ded5e291d..d2990b210 100644
>> --- a/builtin/rebase--helper.c
>> +++ b/builtin/rebase--helper.c
>> @@ -12,12 +12,12 @@ static const char * const builtin_rebase_helper_usage[] 
>> = {
>>  int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
>>  {
>>  struct replay_opts opts = REPLAY_OPTS_INIT;
>> -unsigned flags = 0, keep_empty = 0, rebase_merges = 0, write_edit_todo 
>> = 0;
>> +unsigned flags = 0, keep_empty = 0, rebase_merges = 0;
> 
> Sorry, but where does "write_edit_todo = 0" in the preimage come from?
> 

It comes from my conversion of append_todo_help()[0], on which this
series is based.

[0]
https://public-inbox.org/git/20180607103012.22981-1-alban.gr...@gmail.com/

Cheers,
Alban



[GSoC][PATCH v2 0/2] rebase -i: rewrite the edit-todo functionality in C

2018-06-13 Thread Alban Gruin
This patch rewrites the edit-todo functionality from shell to C. This is
part of the effort to rewrite interactive rebase in C.

Changes since v1:

 - Add a new function to launch the sequence editor, as advised by
   Phillip Wood[0]

[0] 
https://public-inbox.org/git/3bfd3470-4482-fe6a-2cd9-08311a0bb...@talktalk.net/

Alban Gruin (2):
  editor: add a function to launch the sequence editor
  rebase--interactive: rewrite the edit-todo functionality in C

 builtin/rebase--helper.c   | 13 -
 cache.h|  1 +
 editor.c   | 27 +--
 git-rebase--interactive.sh | 11 +--
 sequencer.c| 31 +++
 sequencer.h|  1 +
 strbuf.h   |  2 ++
 7 files changed, 69 insertions(+), 17 deletions(-)

-- 
2.16.4



[GSoC][PATCH v2 2/2] rebase--interactive: rewrite the edit-todo functionality in C

2018-06-13 Thread Alban Gruin
This rewrites the edit-todo functionality from shell to C.

To achieve that, a new command mode, `edit-todo`, is added, and the
`write-edit-todo` flag is removed, as the shell script does not need to
write the edit todo help message to the todo list anymore.

The shell version is then stripped in favour of a call to the helper.

Signed-off-by: Alban Gruin 
---
 builtin/rebase--helper.c   | 13 -
 git-rebase--interactive.sh | 11 +--
 sequencer.c| 31 +++
 sequencer.h|  1 +
 4 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index ded5e291d..d2990b210 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -12,12 +12,12 @@ static const char * const builtin_rebase_helper_usage[] = {
 int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
 {
struct replay_opts opts = REPLAY_OPTS_INIT;
-   unsigned flags = 0, keep_empty = 0, rebase_merges = 0, write_edit_todo 
= 0;
+   unsigned flags = 0, keep_empty = 0, rebase_merges = 0;
int abbreviate_commands = 0, rebase_cousins = -1;
enum {
CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,
-   ADD_EXEC, APPEND_TODO_HELP
+   ADD_EXEC, APPEND_TODO_HELP, EDIT_TODO
} command = 0;
struct option options[] = {
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
@@ -27,8 +27,6 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
OPT_BOOL(0, "rebase-merges", _merges, N_("rebase merge 
commits")),
OPT_BOOL(0, "rebase-cousins", _cousins,
 N_("keep original branch points of cousins")),
-   OPT_BOOL(0, "write-edit-todo", _edit_todo,
-N_("append the edit-todo message to the todo-list")),
OPT_CMDMODE(0, "continue", , N_("continue rebase"),
CONTINUE),
OPT_CMDMODE(0, "abort", , N_("abort rebase"),
@@ -49,6 +47,9 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
N_("insert exec commands in todo list"), ADD_EXEC),
OPT_CMDMODE(0, "append-todo-help", ,
N_("insert the help in the todo list"), 
APPEND_TODO_HELP),
+   OPT_CMDMODE(0, "edit-todo", ,
+   N_("edit the todo list during an interactive 
rebase"),
+   EDIT_TODO),
OPT_END()
};
 
@@ -89,6 +90,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
if (command == ADD_EXEC && argc == 2)
return !!sequencer_add_exec_commands(argv[1]);
if (command == APPEND_TODO_HELP && argc == 1)
-   return !!append_todo_help(write_edit_todo, keep_empty);
+   return !!append_todo_help(0, keep_empty);
+   if (command == EDIT_TODO && argc == 1)
+   return !!edit_todo_list(flags);
usage_with_options(builtin_rebase_helper_usage, options);
 }
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 94c23a7af..2defe607f 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -108,16 +108,7 @@ initiate_action () {
 --continue
;;
edit-todo)
-   git stripspace --strip-comments <"$todo" >"$todo".new
-   mv -f "$todo".new "$todo"
-   collapse_todo_ids
-   git rebase--helper --append-todo-help --write-edit-todo
-
-   git_sequence_editor "$todo" ||
-   die "$(gettext "Could not execute editor")"
-   expand_todo_ids
-
-   exit
+   exec git rebase--helper --edit-todo
;;
show-current-patch)
exec git show REBASE_HEAD --
diff --git a/sequencer.c b/sequencer.c
index 1ffd990f7..7cc76332e 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4386,6 +4386,37 @@ int append_todo_help(unsigned edit_todo, unsigned 
keep_empty)
return ret;
 }
 
+int edit_todo_list(unsigned flags)
+{
+   struct strbuf buf = STRBUF_INIT;
+   const char *todo_file = rebase_path_todo();
+   FILE *todo;
+
+   if (strbuf_read_file(, todo_file, 0) < 0)
+   return error_errno(_("could not read '%s'."), todo_file);
+
+   strbuf_stripspace(, 1);
+   todo = fopen_or_warn(todo_file, "w");
+   if (!todo) {
+   strbuf_release()

[GSoC][PATCH v2 1/2] editor: add a function to launch the sequence editor

2018-06-13 Thread Alban Gruin
As part of the rewrite of interactive rebase, the sequencer will need to
open the sequence editor to allow the user to edit the todo list.
Instead of duplicating the existing launch_editor() function, this
refactors it to a new function, launch_specified_editor(), which takes
the editor as a parameter, in addition to the path, the buffer and the
environment variables.  launch_sequence_editor() is then added to launch
the sequence editor.

Signed-off-by: Alban Gruin 
---
 cache.h  |  1 +
 editor.c | 27 +--
 strbuf.h |  2 ++
 3 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/cache.h b/cache.h
index 89a107a7f..c124849a1 100644
--- a/cache.h
+++ b/cache.h
@@ -1472,6 +1472,7 @@ extern const char *fmt_name(const char *name, const char 
*email);
 extern const char *ident_default_name(void);
 extern const char *ident_default_email(void);
 extern const char *git_editor(void);
+extern const char *git_sequence_editor(void);
 extern const char *git_pager(int stdout_is_tty);
 extern int is_terminal_dumb(void);
 extern int git_ident_config(const char *, const char *, void *);
diff --git a/editor.c b/editor.c
index 9a9b4e12d..c985eee1f 100644
--- a/editor.c
+++ b/editor.c
@@ -1,4 +1,5 @@
 #include "cache.h"
+#include "config.h"
 #include "strbuf.h"
 #include "run-command.h"
 #include "sigchain.h"
@@ -34,10 +35,21 @@ const char *git_editor(void)
return editor;
 }
 
-int launch_editor(const char *path, struct strbuf *buffer, const char *const 
*env)
+const char *git_sequence_editor(void)
 {
-   const char *editor = git_editor();
+   const char *editor = getenv("GIT_SEQUENCE_EDITOR");
+
+   if (!editor)
+   git_config_get_string_const("sequence.editor", );
+   if (!editor)
+   editor = git_editor();
 
+   return editor;
+}
+
+static int launch_specified_editor(const char *editor, const char *path,
+  struct strbuf *buffer, const char *const 
*env)
+{
if (!editor)
return error("Terminal is dumb, but EDITOR unset");
 
@@ -95,3 +107,14 @@ int launch_editor(const char *path, struct strbuf *buffer, 
const char *const *en
return error_errno("could not read file '%s'", path);
return 0;
 }
+
+int launch_editor(const char *path, struct strbuf *buffer, const char *const 
*env)
+{
+   return launch_specified_editor(git_editor(), path, buffer, env);
+}
+
+int launch_sequence_editor(const char *path, struct strbuf *buffer,
+  const char *const *env)
+{
+   return launch_specified_editor(git_sequence_editor(), path, buffer, 
env);
+}
diff --git a/strbuf.h b/strbuf.h
index 60a35aef1..66da9822f 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -575,6 +575,8 @@ extern void strbuf_add_unique_abbrev(struct strbuf *sb,
  * file's contents are not read into the buffer upon completion.
  */
 extern int launch_editor(const char *path, struct strbuf *buffer, const char 
*const *env);
+extern int launch_sequence_editor(const char *path, struct strbuf *buffer,
+ const char *const *env);
 
 extern void strbuf_add_lines(struct strbuf *sb, const char *prefix, const char 
*buf, size_t size);
 
-- 
2.16.4



Re: [GSoC][PATCH 1/1] rebase--interactive: rewrite the edit-todo functionality in C

2018-06-12 Thread Alban Gruin
Hi Elijah,

Le 12/06/2018 à 17:46, Elijah Newren a écrit :
> Hi Alban,
> 
> On Mon, Jun 11, 2018 at 6:57 AM, Alban Gruin  wrote:
>> This rewrites the edit-todo functionality from shell to C.
>>
>> To achieve that, a new command mode, `edit-todo`, is added, and the
>> `write-edit-todo` flag is removed, as the shell script does not need to
>> write the edit todo help message to the todo list anymore.
>>
>> The shell version is then stripped in favour of a call to the helper.
> 
> I looked over the patch and didn't see any problems (though I haven't
> worked with rebase--helper before, or the code for todo list editing),
> but when I went to apply the patch it failed telling me:
> 
> Applying: rebase--interactive: rewrite the edit-todo functionality in C
> error: sha1 information is lacking or useless (builtin/rebase--helper.c).
> error: could not build fake ancestor
> Patch failed at 0001 rebase--interactive: rewrite the edit-todo
> functionality in C
> Use 'git am --show-current-patch' to see the failed patch
> 
> I tried each of master, next, and pu (as of today) to see if it might
> apply there.  On which commit is this patch based?  (Do you have other
> local commits that this was based on top of?)
> 
> 
> Elijah
> 

It can be applied on top of pu with my patch that rewrites
append_todo_help() in C
(https://public-inbox.org/git/20180607103012.22981-1-alban.gr...@gmail.com/)

Cheers,
Alban



Re: [GSoC][PATCH 1/1] rebase--interactive: rewrite the edit-todo functionality in C

2018-06-12 Thread Alban Gruin
Hi Phillip,

Le 11/06/2018 à 17:32, Phillip Wood a écrit :
>> +    if (launch_editor(todo_file, NULL, NULL))
> 
> I'm not sure that this will respect GIT_SEQUENCE_EDITOR, it would be
> nice to have a launch_sequence_editor() function that shared as much
> code as possible with launch_editor()
> 

It could be done by making launch_editor() and launch_sequence_editor()
some kind of wrapper around a function like launch_specified_editor()
(or something like that), that would take the editor as a parameter, in
addition to the path, the buffer and environment variables.  It would be
also very trivial to implement your first point above on top of that.

>>   int append_todo_help(unsigned edit_todo, unsigned keep_empty);
> 
> Can this declaration be removed now?

No, it’s still used in rebase--helper.c for now.

> 
>> +int edit_todo_list(unsigned flags);
>>   int skip_unnecessary_picks(void);
>>   int rearrange_squash(void);
> 
> Best Wishes
> 
> Phillip
> 

Cheers,
Alban



[GSoC] GSoC with git, week 6

2018-06-11 Thread Alban Gruin
Hi,

I published a new blog post about last week:

  https://blog.pa1ch.fr/posts/2018/06/11/en/gsoc2018-week-6.html

Any feedback is welcome! :)

Cheers,
Alban



[GSoC][PATCH 0/1] rebase -i: rewrite the edit-todo functionality in C

2018-06-11 Thread Alban Gruin
This patch rewrites the edit-todo functionality from shell to C. This is
part of the effort to rewrite interactive rebase in C.

Alban Gruin (1):
  rebase--interactive: rewrite the edit-todo functionality in C

 builtin/rebase--helper.c   | 13 -
 git-rebase--interactive.sh | 11 +--
 sequencer.c| 31 +++
 sequencer.h|  1 +
 4 files changed, 41 insertions(+), 15 deletions(-)

-- 
2.16.4



[GSoC][PATCH 1/1] rebase--interactive: rewrite the edit-todo functionality in C

2018-06-11 Thread Alban Gruin
This rewrites the edit-todo functionality from shell to C.

To achieve that, a new command mode, `edit-todo`, is added, and the
`write-edit-todo` flag is removed, as the shell script does not need to
write the edit todo help message to the todo list anymore.

The shell version is then stripped in favour of a call to the helper.

Signed-off-by: Alban Gruin 
---
 builtin/rebase--helper.c   | 13 -
 git-rebase--interactive.sh | 11 +--
 sequencer.c| 31 +++
 sequencer.h|  1 +
 4 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index ded5e291d..d2990b210 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -12,12 +12,12 @@ static const char * const builtin_rebase_helper_usage[] = {
 int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
 {
struct replay_opts opts = REPLAY_OPTS_INIT;
-   unsigned flags = 0, keep_empty = 0, rebase_merges = 0, write_edit_todo 
= 0;
+   unsigned flags = 0, keep_empty = 0, rebase_merges = 0;
int abbreviate_commands = 0, rebase_cousins = -1;
enum {
CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,
-   ADD_EXEC, APPEND_TODO_HELP
+   ADD_EXEC, APPEND_TODO_HELP, EDIT_TODO
} command = 0;
struct option options[] = {
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
@@ -27,8 +27,6 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
OPT_BOOL(0, "rebase-merges", _merges, N_("rebase merge 
commits")),
OPT_BOOL(0, "rebase-cousins", _cousins,
 N_("keep original branch points of cousins")),
-   OPT_BOOL(0, "write-edit-todo", _edit_todo,
-N_("append the edit-todo message to the todo-list")),
OPT_CMDMODE(0, "continue", , N_("continue rebase"),
CONTINUE),
OPT_CMDMODE(0, "abort", , N_("abort rebase"),
@@ -49,6 +47,9 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
N_("insert exec commands in todo list"), ADD_EXEC),
OPT_CMDMODE(0, "append-todo-help", ,
N_("insert the help in the todo list"), 
APPEND_TODO_HELP),
+   OPT_CMDMODE(0, "edit-todo", ,
+   N_("edit the todo list during an interactive 
rebase"),
+   EDIT_TODO),
OPT_END()
};
 
@@ -89,6 +90,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
if (command == ADD_EXEC && argc == 2)
return !!sequencer_add_exec_commands(argv[1]);
if (command == APPEND_TODO_HELP && argc == 1)
-   return !!append_todo_help(write_edit_todo, keep_empty);
+   return !!append_todo_help(0, keep_empty);
+   if (command == EDIT_TODO && argc == 1)
+   return !!edit_todo_list(flags);
usage_with_options(builtin_rebase_helper_usage, options);
 }
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 94c23a7af..2defe607f 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -108,16 +108,7 @@ initiate_action () {
 --continue
;;
edit-todo)
-   git stripspace --strip-comments <"$todo" >"$todo".new
-   mv -f "$todo".new "$todo"
-   collapse_todo_ids
-   git rebase--helper --append-todo-help --write-edit-todo
-
-   git_sequence_editor "$todo" ||
-   die "$(gettext "Could not execute editor")"
-   expand_todo_ids
-
-   exit
+   exec git rebase--helper --edit-todo
;;
show-current-patch)
exec git show REBASE_HEAD --
diff --git a/sequencer.c b/sequencer.c
index 1ffd990f7..1c1799c91 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4386,6 +4386,37 @@ int append_todo_help(unsigned edit_todo, unsigned 
keep_empty)
return ret;
 }
 
+int edit_todo_list(unsigned flags)
+{
+   struct strbuf buf = STRBUF_INIT;
+   const char *todo_file = rebase_path_todo();
+   FILE *todo;
+
+   if (strbuf_read_file(, todo_file, 0) < 0)
+   return error_errno(_("could not read '%s'."), todo_file);
+
+   strbuf_stripspace(, 1);
+   todo = fopen_or_warn(todo_file, "w");
+   if (!todo) {
+   strbuf_

[GSoC][PATCH v3 1/1] rebase--interactive: rewrite append_todo_help() in C

2018-06-07 Thread Alban Gruin
This rewrites append_todo_help() from shell to C. It also incorporates
some parts of initiate_action() and complete_action() that also write
help texts to the todo file.

Two flags are added to rebase--helper.c: one to call
append_todo_help() (`--append-todo-help`), and another one to tell
append_todo_help() to write the help text suited for the edit-todo
mode (`--write-edit-todo`).

Finally, append_todo_help() is removed from git-rebase--interactive.sh
to use `rebase--helper --append-todo-help` instead.

Signed-off-by: Alban Gruin 
---
 builtin/rebase--helper.c   | 10 ++--
 git-rebase--interactive.sh | 52 ++--
 sequencer.c| 60 ++
 sequencer.h|  1 +
 4 files changed, 71 insertions(+), 52 deletions(-)

diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index f7c2a5fdc..ded5e291d 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -12,12 +12,12 @@ static const char * const builtin_rebase_helper_usage[] = {
 int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
 {
struct replay_opts opts = REPLAY_OPTS_INIT;
-   unsigned flags = 0, keep_empty = 0, rebase_merges = 0;
+   unsigned flags = 0, keep_empty = 0, rebase_merges = 0, write_edit_todo 
= 0;
int abbreviate_commands = 0, rebase_cousins = -1;
enum {
CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,
-   ADD_EXEC
+   ADD_EXEC, APPEND_TODO_HELP
} command = 0;
struct option options[] = {
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
@@ -27,6 +27,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
OPT_BOOL(0, "rebase-merges", _merges, N_("rebase merge 
commits")),
OPT_BOOL(0, "rebase-cousins", _cousins,
 N_("keep original branch points of cousins")),
+   OPT_BOOL(0, "write-edit-todo", _edit_todo,
+N_("append the edit-todo message to the todo-list")),
OPT_CMDMODE(0, "continue", , N_("continue rebase"),
CONTINUE),
OPT_CMDMODE(0, "abort", , N_("abort rebase"),
@@ -45,6 +47,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
N_("rearrange fixup/squash lines"), REARRANGE_SQUASH),
OPT_CMDMODE(0, "add-exec-commands", ,
N_("insert exec commands in todo list"), ADD_EXEC),
+   OPT_CMDMODE(0, "append-todo-help", ,
+   N_("insert the help in the todo list"), 
APPEND_TODO_HELP),
OPT_END()
};
 
@@ -84,5 +88,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
return !!rearrange_squash();
if (command == ADD_EXEC && argc == 2)
return !!sequencer_add_exec_commands(argv[1]);
+   if (command == APPEND_TODO_HELP && argc == 1)
+   return !!append_todo_help(write_edit_todo, keep_empty);
usage_with_options(builtin_rebase_helper_usage, options);
 }
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 299ded213..94c23a7af 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -39,38 +39,6 @@ comment_for_reflog () {
esac
 }
 
-append_todo_help () {
-   gettext "
-Commands:
-p, pick  = use commit
-r, reword  = use commit, but edit the commit message
-e, edit  = use commit, but stop for amending
-s, squash  = use commit, but meld into previous commit
-f, fixup  = like \"squash\", but discard this commit's log message
-x, exec  = run command (the rest of the line) using shell
-d, drop  = remove commit
-l, label  = label current HEAD with a name
-t, reset  = reset HEAD to a label
-m, merge [-C  | -c ]  [# ]
-.   create a merge commit using the original merge commit's
-.   message (or the oneline, if no original merge commit was
-.   specified). Use -c  to reword the commit message.
-
-These lines can be re-ordered; they are executed from top to bottom.
-" | git stripspace --comment-lines >>"$todo"
-
-   if test $(get_missing_commit_check_level) = error
-   then
-   gettext "
-Do not remove any line. Use 'drop' explicitly to remove a commit.
-" | git stripspace --comment-lines >>"$todo"
-   else
-   gettext "
-If you remove a line here THAT COMMIT WILL BE LOST.
-" | git stripspace --comment-lines >>"$todo"
-   fi
-}
-
 die_abort () 

[GSoC][PATCH v3 0/1] rebase -i: rewrite append_todo_help() in C

2018-06-07 Thread Alban Gruin
This patch rewrites append_todo_help() from shell to C. The C version
covers a bit more than the old shell version. To achieve that, some
parameters were added to rebase--helper.

This is part of the effort to rewrite interactive rebase in C.

Changes since v2:

 - Renaming the variable `edit_todo` to `write_edit_todo` to avoid
   confusions, after a comment by Christian Couder[1].

[1] https://github.com/git/git/pull/503#discussion_r193392270

Alban Gruin (1):
  rebase--interactive: rewrite append_todo_help() in C

 builtin/rebase--helper.c   | 10 ++--
 git-rebase--interactive.sh | 52 ++--
 sequencer.c| 60 ++
 sequencer.h|  1 +
 4 files changed, 71 insertions(+), 52 deletions(-)

-- 
2.16.4



[GSoC][PATCH v2 0/1] rebase -i: rewrite append_todo_help() in C

2018-06-05 Thread Alban Gruin
This patch rewrites append_todo_help() from shell to C. The C version
covers a bit more than the old shell version. To achieve that, some
parameters were added to rebase--helper.

This is part of the effort to rewrite interactive rebase in C.

Changes since v1:

 - Renaming the parameter to insert the edit-todo message from
   `edit-todo` to `write-edit-todo`.

 - Clarifying the `write-edit-todo` help message.

 - Dropping the commit that removed newlines in the messages.

Alban Gruin (1):
  rebase--interactive: rewrite append_todo_help() in C

 builtin/rebase--helper.c   | 10 ++--
 git-rebase--interactive.sh | 52 ++--
 sequencer.c| 60 ++
 sequencer.h|  1 +
 4 files changed, 71 insertions(+), 52 deletions(-)

-- 
2.16.4



[GSoC][PATCH v2 1/1] rebase--interactive: rewrite append_todo_help() in C

2018-06-05 Thread Alban Gruin
This rewrites append_todo_help() from shell to C. It also incorporates
some parts of initiate_action() and complete_action() that also write
help texts to the todo file.

Two flags are added to rebase--helper.c: one to call
append_todo_help() (`--append-todo-help`), and another one to tell
append_todo_help() to write the help text suited for the edit-todo
mode (`--write-edit-todo`).

Finally, append_todo_help() is removed from git-rebase--interactive.sh
to use `rebase--helper --append-todo-help` instead.

Signed-off-by: Alban Gruin 
---
 builtin/rebase--helper.c   | 10 ++--
 git-rebase--interactive.sh | 52 ++--
 sequencer.c| 60 ++
 sequencer.h|  1 +
 4 files changed, 71 insertions(+), 52 deletions(-)

diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index f7c2a5fdc..7ef92fbb6 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -12,12 +12,12 @@ static const char * const builtin_rebase_helper_usage[] = {
 int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
 {
struct replay_opts opts = REPLAY_OPTS_INIT;
-   unsigned flags = 0, keep_empty = 0, rebase_merges = 0;
+   unsigned flags = 0, keep_empty = 0, rebase_merges = 0, edit_todo = 0;
int abbreviate_commands = 0, rebase_cousins = -1;
enum {
CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,
-   ADD_EXEC
+   ADD_EXEC, APPEND_TODO_HELP
} command = 0;
struct option options[] = {
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
@@ -27,6 +27,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
OPT_BOOL(0, "rebase-merges", _merges, N_("rebase merge 
commits")),
OPT_BOOL(0, "rebase-cousins", _cousins,
 N_("keep original branch points of cousins")),
+   OPT_BOOL(0, "write-edit-todo", _todo,
+N_("append the edit-todo message to the todo-list")),
OPT_CMDMODE(0, "continue", , N_("continue rebase"),
CONTINUE),
OPT_CMDMODE(0, "abort", , N_("abort rebase"),
@@ -45,6 +47,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
N_("rearrange fixup/squash lines"), REARRANGE_SQUASH),
OPT_CMDMODE(0, "add-exec-commands", ,
N_("insert exec commands in todo list"), ADD_EXEC),
+   OPT_CMDMODE(0, "append-todo-help", ,
+   N_("insert the help in the todo list"), 
APPEND_TODO_HELP),
OPT_END()
};
 
@@ -84,5 +88,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
return !!rearrange_squash();
if (command == ADD_EXEC && argc == 2)
return !!sequencer_add_exec_commands(argv[1]);
+   if (command == APPEND_TODO_HELP && argc == 1)
+   return !!append_todo_help(edit_todo, keep_empty);
usage_with_options(builtin_rebase_helper_usage, options);
 }
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 299ded213..94c23a7af 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -39,38 +39,6 @@ comment_for_reflog () {
esac
 }
 
-append_todo_help () {
-   gettext "
-Commands:
-p, pick  = use commit
-r, reword  = use commit, but edit the commit message
-e, edit  = use commit, but stop for amending
-s, squash  = use commit, but meld into previous commit
-f, fixup  = like \"squash\", but discard this commit's log message
-x, exec  = run command (the rest of the line) using shell
-d, drop  = remove commit
-l, label  = label current HEAD with a name
-t, reset  = reset HEAD to a label
-m, merge [-C  | -c ]  [# ]
-.   create a merge commit using the original merge commit's
-.   message (or the oneline, if no original merge commit was
-.   specified). Use -c  to reword the commit message.
-
-These lines can be re-ordered; they are executed from top to bottom.
-" | git stripspace --comment-lines >>"$todo"
-
-   if test $(get_missing_commit_check_level) = error
-   then
-   gettext "
-Do not remove any line. Use 'drop' explicitly to remove a commit.
-" | git stripspace --comment-lines >>"$todo"
-   else
-   gettext "
-If you remove a line here THAT COMMIT WILL BE LOST.
-" | git stripspace --comment-lines >>"$todo"
-   fi
-}
-
 die_abort () {
apply_autostash
 

[GSoC] GSoC with git, week 5

2018-06-03 Thread Alban Gruin
Hi,

I wrote a post about this week. Unfortunately, I have a technical issue
with my website, so you can find it right in this email instead. I’m
sorry about that. I will publish it on my blog as soon as it comes back
online.

Feel free to give me your feedback!

Cheers,
Alban


--

As you may know, I made no less than three attempts to upstream my
changes about ``preserve-merges`` last week.

This week began by a fourth one[1].

4th attempt
---

Johannes pointed out some problems with my commit messages on that
series:

 - some lines are too long
 - they do not describe what I wanted to do well.

On the first point, commits messages should wrap at 72 characters, but
I configured Emacs to wrap pretty much anything at 80 characters.

He also wanted to keep the original name of
``git_rebase__interactive__preserve_merges()`` (which I renamed
``git_rebase__preserve_merges()``), but I decided not to, as this
function is now part of ``git-rebase--preserve-merges.sh``.

Also, Friday, Junio Hamano announced[2] that my changes (among many
others) have been merged into the ``pu`` (proposed updates) branch of
git.git. This does not mean that it will necessarily hit ``master``,
but it’s a start.

TODO: help
--

When you start an interactive rebase, you’re greeted with your
``$EDITOR``, containing a list of commits to edit, and an help text
explaining you how this works.

So, this week, I rewrote the code writing this message from shell
to C. You can find the branch here[3], and the discussion on the list
here[4].

The conversion itself is quite trivial, but strings are a rather
curious case here: some of them begins with one or two newlines. As
this becomes useless in C, Stefan advised me to remove them, as this
would probably cause less confusion for the translators, but it
implies changing the translations, as pointed out by Johannes and
Phillip Wood. Right now, no clear opinion has emerged from the
discussion.

Some additionnal work and refactoring could be made once more code has
been converted. For instance, the todo-list file is opened, modified,
and closed several times. Instead, we could create a ``strbuf``, build
it progressively, then write it once to the todo file.

What’s next?


I’ve started to work on converting the ``edit-todo`` functionnality of
interactive rebase. This is also a straightforward change, but it
would also require some future work once the conversion is completed.

[1]
https://public-inbox.org/git/20180528123422.6718-1-alban.gr...@gmail.com/
[2] https://public-inbox.org/git/xmqqa7sf7yzs@gitster-ct.c.googlers.com/
[3] https://github.com/agrn/git/tree/ag/append-todo-help-c
[4]
https://public-inbox.org/git/20180531110130.18839-1-alban.gr...@gmail.com/


Re: [GSoC][PATCH 0/2] rebase -i: rewrite append_todo_help() in C

2018-05-31 Thread Alban Gruin
Hi Stefan,

Le 31/05/2018 à 20:44, Stefan Beller a écrit :
> On Thu, May 31, 2018 at 10:48 AM, Phillip Wood
>  wrote:
>> Hi Alban, it's great to see you working on this
>>
>> On 31/05/18 12:01, Alban Gruin wrote:
>>> This series rewrites append_todo_help() from shell to C. This is part
>>> of the effort to rewrite interactive rebase in C.
>>>
>>> The first commit rewrites append_todo_help() in C (the C version
>>> covers a bit more than the old shell version), adds some parameters to
>>> rebase--helper, etc.
>>
>> I've had a read of the first patch and I think it looks fine, my only
>> comment would be that the help for '--edit-todo' is a bit misleading at
>> the moment as currently it's just a flag to tell rebase-helper that the
>> todo list is being edited rather than actually implementing the
>> functionality to edit the list (but hopefully that will follow in the
>> future).
> 
> Would you have better suggestions for the name of the flag?
> Of the top of my head:
>   --write-edit-todo
>   --hint-todo-edit
>   --include-todo-edit-hint
> not sure I like these names, though they seem to reflect the
> nature of that flag a little bit better.
> 

As my next patch series will probably be about rewriting edit-todo in C,
do you really think I should rename the flag?

> If you feel strongly, I'd rather see Alban drop this second patch and
> move on instead of waiting for our argument to settle. ( I do not feel
> strongly about it, but put it out as a suggestion as that seemed like
> it would lead to a better end state for the project).
> 

Okay, so I drop this patch and reroll the other?


Cheers,
Alban



Re: [GSoC][PATCH 0/2] rebase -i: rewrite append_todo_help() in C

2018-05-31 Thread Alban Gruin
Hi Phillip,

Le 31/05/2018 à 19:48, Phillip Wood a écrit :
> Hi Alban, it's great to see you working on this
> 
> On 31/05/18 12:01, Alban Gruin wrote:
>> This series rewrites append_todo_help() from shell to C. This is part
>> of the effort to rewrite interactive rebase in C.
>>
>> The first commit rewrites append_todo_help() in C (the C version
>> covers a bit more than the old shell version), adds some parameters to
>> rebase--helper, etc.
> 
> I've had a read of the first patch and I think it looks fine, my only
> comment would be that the help for '--edit-todo' is a bit misleading at
> the moment as currently it's just a flag to tell rebase-helper that the
> todo list is being edited rather than actually implementing the
> functionality to edit the list

Right, what do you think about something like “appends the edit-todo
message to the todo list”?

> (but hopefully that will follow in the
> future).
> 

This is the next step :)

Cheers,
Alban



[GSoC][PATCH 0/2] rebase -i: rewrite append_todo_help() in C

2018-05-31 Thread Alban Gruin
This series rewrites append_todo_help() from shell to C. This is part
of the effort to rewrite interactive rebase in C.

The first commit rewrites append_todo_help() in C (the C version
covers a bit more than the old shell version), adds some parameters to
rebase--helper, etc.

The second one strips newlines from append_todo_help() messages, which
require to update the translations. This change was advised to me by
Stefan Beller, but Johannes Schindelin voiced concerns. I don’t really
have a strong opinion about it, so feel free to give yours.

Alban Gruin (2):
  rebase--interactive: rewrite append_todo_help() in C
  sequencer: remove newlines from append_todo_help() messages

 builtin/rebase--helper.c   | 10 ++--
 git-rebase--interactive.sh | 52 ++---
 sequencer.c| 64 ++
 sequencer.h|  1 +
 4 files changed, 75 insertions(+), 52 deletions(-)

-- 
2.16.4



[GSoC][PATCH 1/2] rebase--interactive: rewrite append_todo_help() in C

2018-05-31 Thread Alban Gruin
This rewrites append_todo_help() from shell to C. It also incorporates
some parts of initiate_action() and complete_action() that also write
help texts to the todo file.

Two flags are added to rebase--helper.c: one to call
append_todo_help() (`--append-todo-help`), and another one to tell
append_todo_help() to write the help text suited for the edit-todo
mode (`--edit-todo`).

Finally, append_todo_help() is removed from git-rebase--interactive.sh
to use `rebase--helper --append-todo-help` instead.

Signed-off-by: Alban Gruin 
---
 builtin/rebase--helper.c   | 10 ++--
 git-rebase--interactive.sh | 52 ++--
 sequencer.c| 60 ++
 sequencer.h|  1 +
 4 files changed, 71 insertions(+), 52 deletions(-)

diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index f7c2a5fdc..ad3a3a7eb 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -12,12 +12,12 @@ static const char * const builtin_rebase_helper_usage[] = {
 int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
 {
struct replay_opts opts = REPLAY_OPTS_INIT;
-   unsigned flags = 0, keep_empty = 0, rebase_merges = 0;
+   unsigned flags = 0, keep_empty = 0, rebase_merges = 0, edit_todo = 0;
int abbreviate_commands = 0, rebase_cousins = -1;
enum {
CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,
-   ADD_EXEC
+   ADD_EXEC, APPEND_TODO_HELP
} command = 0;
struct option options[] = {
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
@@ -27,6 +27,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
OPT_BOOL(0, "rebase-merges", _merges, N_("rebase merge 
commits")),
OPT_BOOL(0, "rebase-cousins", _cousins,
 N_("keep original branch points of cousins")),
+   OPT_BOOL(0, "edit-todo", _todo,
+N_("edit the todo list during an interactive rebase")),
OPT_CMDMODE(0, "continue", , N_("continue rebase"),
CONTINUE),
OPT_CMDMODE(0, "abort", , N_("abort rebase"),
@@ -45,6 +47,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
N_("rearrange fixup/squash lines"), REARRANGE_SQUASH),
OPT_CMDMODE(0, "add-exec-commands", ,
N_("insert exec commands in todo list"), ADD_EXEC),
+   OPT_CMDMODE(0, "append-todo-help", ,
+   N_("insert the help in the todo list"), 
APPEND_TODO_HELP),
OPT_END()
};
 
@@ -84,5 +88,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
return !!rearrange_squash();
if (command == ADD_EXEC && argc == 2)
return !!sequencer_add_exec_commands(argv[1]);
+   if (command == APPEND_TODO_HELP && argc == 1)
+   return !!append_todo_help(edit_todo, keep_empty);
usage_with_options(builtin_rebase_helper_usage, options);
 }
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 9884ecd71..419c54068 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -39,38 +39,6 @@ comment_for_reflog () {
esac
 }
 
-append_todo_help () {
-   gettext "
-Commands:
-p, pick  = use commit
-r, reword  = use commit, but edit the commit message
-e, edit  = use commit, but stop for amending
-s, squash  = use commit, but meld into previous commit
-f, fixup  = like \"squash\", but discard this commit's log message
-x, exec  = run command (the rest of the line) using shell
-d, drop  = remove commit
-l, label  = label current HEAD with a name
-t, reset  = reset HEAD to a label
-m, merge [-C  | -c ]  [# ]
-.   create a merge commit using the original merge commit's
-.   message (or the oneline, if no original merge commit was
-.   specified). Use -c  to reword the commit message.
-
-These lines can be re-ordered; they are executed from top to bottom.
-" | git stripspace --comment-lines >>"$todo"
-
-   if test $(get_missing_commit_check_level) = error
-   then
-   gettext "
-Do not remove any line. Use 'drop' explicitly to remove a commit.
-" | git stripspace --comment-lines >>"$todo"
-   else
-   gettext "
-If you remove a line here THAT COMMIT WILL BE LOST.
-" | git stripspace --comment-lines >>"$todo"
-   fi
-}
-
 die_abort () {
apply_autostash
rm -rf &q

[GSoC][PATCH 2/2] sequencer: remove newlines from append_todo_help() messages

2018-05-31 Thread Alban Gruin
This removes newlines before and after the messages in
append_todo_help(). This is done to avoid confusions from
translators.

These newlines are now inserted with
`strbuf_add_commented_lines()`. Messages were ended by two newlines
characters, but here we only insert one at a time. This is because
`strbuf_add_commented_lines()` automatically inserts a newline at the
end of the input, and ignore the last from the input.

Signed-off-by: Alban Gruin 
---
 sequencer.c | 24 ++--
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 9b291845e..9ab6c28d7 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4228,7 +4228,7 @@ int append_todo_help(unsigned edit_todo, unsigned 
keep_empty)
struct strbuf buf = STRBUF_INIT;
FILE *todo;
int ret;
-   const char *msg = _("\nCommands:\n"
+   const char *msg = _("Commands:\n"
 "p, pick  = use commit\n"
 "r, reword  = use commit, but edit the commit message\n"
 "e, edit  = use commit, but stop for amending\n"
@@ -4243,33 +4243,37 @@ int append_todo_help(unsigned edit_todo, unsigned 
keep_empty)
 ".   message (or the oneline, if no original merge commit was\n"
 ".   specified). Use -c  to reword the commit message.\n"
 "\n"
-"These lines can be re-ordered; they are executed from top to bottom.\n");
+"These lines can be re-ordered; they are executed from top to bottom.");
 
todo = fopen_or_warn(rebase_path_todo(), "a");
if (!todo)
return 1;
 
+   strbuf_add_commented_lines(, "\n", 1);
strbuf_add_commented_lines(, msg, strlen(msg));
 
if (get_missing_commit_check_level() == CHECK_ERROR)
-   msg = _("\nDo not remove any line. Use 'drop' "
-"explicitly to remove a commit.\n");
+   msg = _("Do not remove any line. Use 'drop' "
+"explicitly to remove a commit.");
else
-   msg = _("\nIf you remove a line here "
-"THAT COMMIT WILL BE LOST.\n");
+   msg = _("If you remove a line here "
+"THAT COMMIT WILL BE LOST.");
 
+   strbuf_add_commented_lines(, "\n", 1);
strbuf_add_commented_lines(, msg, strlen(msg));
 
if (edit_todo)
-   msg = _("\nYou are editing the todo file "
+   msg = _("You are editing the todo file "
"of an ongoing interactive rebase.\n"
"To continue rebase after editing, run:\n"
-   "git rebase --continue\n\n");
+   "git rebase --continue");
else
-   msg = _("\nHowever, if you remove everything, "
-   "the rebase will be aborted.\n\n");
+   msg = _("However, if you remove everything, "
+   "the rebase will be aborted.");
 
+   strbuf_add_commented_lines(, "\n", 1);
strbuf_add_commented_lines(, msg, strlen(msg));
+   strbuf_add_commented_lines(, "\n", 1);
 
if (!keep_empty) {
msg = _("Note that empty commits are commented out");
-- 
2.16.4



[GSoC][PATCH v4 1/4] rebase: introduce a dedicated backend for --preserve-merges

2018-05-28 Thread Alban Gruin
This duplicates git-rebase--interactive.sh to
git-rebase--preserve-merges.sh. This is done to split -p from -i. No
modifications are made to this file here, but any code that is not used
by -p will be stripped in the next commit.

Signed-off-by: Alban Gruin <alban.gr...@gmail.com>
---
 .gitignore |1 +
 Makefile   |2 +
 git-rebase--preserve-merges.sh | 1069 
 3 files changed, 1072 insertions(+)
 create mode 100644 git-rebase--preserve-merges.sh

diff --git a/.gitignore b/.gitignore
index 833ef3b0b..ef4925485 100644
--- a/.gitignore
+++ b/.gitignore
@@ -117,6 +117,7 @@
 /git-rebase--helper
 /git-rebase--interactive
 /git-rebase--merge
+/git-rebase--preserve-merges
 /git-receive-pack
 /git-reflog
 /git-remote
diff --git a/Makefile b/Makefile
index 50da82b01..810a0d0f4 100644
--- a/Makefile
+++ b/Makefile
@@ -582,6 +582,7 @@ SCRIPT_LIB += git-mergetool--lib
 SCRIPT_LIB += git-parse-remote
 SCRIPT_LIB += git-rebase--am
 SCRIPT_LIB += git-rebase--interactive
+SCRIPT_LIB += git-rebase--preserve-merges
 SCRIPT_LIB += git-rebase--merge
 SCRIPT_LIB += git-sh-setup
 SCRIPT_LIB += git-sh-i18n
@@ -2271,6 +2272,7 @@ LOCALIZED_C = $(C_OBJ:o=c) $(LIB_H) $(GENERATED_H)
 LOCALIZED_SH = $(SCRIPT_SH)
 LOCALIZED_SH += git-parse-remote.sh
 LOCALIZED_SH += git-rebase--interactive.sh
+LOCALIZED_SH += git-rebase--preserve-merges.sh
 LOCALIZED_SH += git-sh-setup.sh
 LOCALIZED_PERL = $(SCRIPT_PERL)
 
diff --git a/git-rebase--preserve-merges.sh b/git-rebase--preserve-merges.sh
new file mode 100644
index 0..2f4941d0f
--- /dev/null
+++ b/git-rebase--preserve-merges.sh
@@ -0,0 +1,1069 @@
+# This shell script fragment is sourced by git-rebase to implement
+# its interactive mode.  "git rebase --interactive" makes it easy
+# to fix up commits in the middle of a series and rearrange commits.
+#
+# Copyright (c) 2006 Johannes E. Schindelin
+#
+# The original idea comes from Eric W. Biederman, in
+# https://public-inbox.org/git/m1odwkyuf5.fsf...@ebiederm.dsl.xmission.com/
+#
+# The file containing rebase commands, comments, and empty lines.
+# This file is created by "git rebase -i" then edited by the user.  As
+# the lines are processed, they are removed from the front of this
+# file and written to the tail of $done.
+todo="$state_dir"/git-rebase-todo
+
+# The rebase command lines that have already been processed.  A line
+# is moved here when it is first handled, before any associated user
+# actions.
+done="$state_dir"/done
+
+# The commit message that is planned to be used for any changes that
+# need to be committed following a user interaction.
+msg="$state_dir"/message
+
+# The file into which is accumulated the suggested commit message for
+# squash/fixup commands.  When the first of a series of squash/fixups
+# is seen, the file is created and the commit message from the
+# previous commit and from the first squash/fixup commit are written
+# to it.  The commit message for each subsequent squash/fixup commit
+# is appended to the file as it is processed.
+#
+# The first line of the file is of the form
+# # This is a combination of $count commits.
+# where $count is the number of commits whose messages have been
+# written to the file so far (including the initial "pick" commit).
+# Each time that a commit message is processed, this line is read and
+# updated.  It is deleted just before the combined commit is made.
+squash_msg="$state_dir"/message-squash
+
+# If the current series of squash/fixups has not yet included a squash
+# command, then this file exists and holds the commit message of the
+# original "pick" commit.  (If the series ends without a "squash"
+# command, then this can be used as the commit message of the combined
+# commit without opening the editor.)
+fixup_msg="$state_dir"/message-fixup
+
+# $rewritten is the name of a directory containing files for each
+# commit that is reachable by at least one merge base of $head and
+# $upstream. They are not necessarily rewritten, but their children
+# might be.  This ensures that commits on merged, but otherwise
+# unrelated side branches are left alone. (Think "X" in the man page's
+# example.)
+rewritten="$state_dir"/rewritten
+
+dropped="$state_dir"/dropped
+
+end="$state_dir"/end
+msgnum="$state_dir"/msgnum
+
+# A script to set the GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and
+# GIT_AUTHOR_DATE that will be used for the commit that is currently
+# being rebased.
+author_script="$state_dir"/author-script
+
+# When an "edit" rebase command is being processed, the SHA1 of the
+# commit to be edited is recorded in this file.  When "git rebase
+# --continue" is executed, if there are any staged changes then they
+# will be amended to the HEAD commit, but only provided the HEAD
+# commit is 

[GSoC][PATCH v4 2/4] rebase: strip unused code in git-rebase--preserve-merges.sh

2018-05-28 Thread Alban Gruin
This removes the code coming from git-rebase--interactive.sh that is not
needed by preserve-merges, and changes the header comment accordingly.

In a following commit, the -p code from git-rebase--interactive.sh will
be stripped out. As preserve-merges’ successor is already in the works,
this will be the only script to be converted.

This also seems to fix a bug where a failure in
`pick_one_preserving_merges()` would fallback to the non-preserve-merges
`pick_one()`.

Signed-off-by: Alban Gruin <alban.gr...@gmail.com>
---
 git-rebase--preserve-merges.sh | 65 +++---
 1 file changed, 4 insertions(+), 61 deletions(-)

diff --git a/git-rebase--preserve-merges.sh b/git-rebase--preserve-merges.sh
index 2f4941d0f..c51c7828e 100644
--- a/git-rebase--preserve-merges.sh
+++ b/git-rebase--preserve-merges.sh
@@ -1,12 +1,8 @@
-# This shell script fragment is sourced by git-rebase to implement
-# its interactive mode.  "git rebase --interactive" makes it easy
-# to fix up commits in the middle of a series and rearrange commits.
+# This shell script fragment is sourced by git-rebase to implement its
+# preserve-merges mode.
 #
 # Copyright (c) 2006 Johannes E. Schindelin
 #
-# The original idea comes from Eric W. Biederman, in
-# https://public-inbox.org/git/m1odwkyuf5.fsf...@ebiederm.dsl.xmission.com/
-#
 # The file containing rebase commands, comments, and empty lines.
 # This file is created by "git rebase -i" then edited by the user.  As
 # the lines are processed, they are removed from the front of this
@@ -287,17 +283,7 @@ pick_one () {
empty_args="--allow-empty"
fi
 
-   test -d "$rewritten" &&
-   pick_one_preserving_merges "$@" && return
-   output eval git cherry-pick $allow_rerere_autoupdate 
$allow_empty_message \
-   ${gpg_sign_opt:+$(git rev-parse --sq-quote 
"$gpg_sign_opt")} \
-   $signoff "$strategy_args" $empty_args $ff "$@"
-
-   # If cherry-pick dies it leaves the to-be-picked commit unrecorded. 
Reschedule
-   # previous task so this commit is not lost.
-   ret=$?
-   case "$ret" in [01]) ;; *) reschedule_last_action ;; esac
-   return $ret
+   pick_one_preserving_merges "$@"
 }
 
 pick_one_preserving_merges () {
@@ -761,11 +747,6 @@ get_missing_commit_check_level () {
 initiate_action () {
case "$1" in
continue)
-   if test ! -d "$rewritten"
-   then
-   exec git rebase--helper ${force_rebase:+--no-ff} 
$allow_empty_message \
-   --continue
-   fi
# do we have anything to commit?
if git diff-index --cached --quiet HEAD --
then
@@ -824,12 +805,6 @@ first and then run 'git rebase --continue' again.")"
;;
skip)
git rerere clear
-
-   if test ! -d "$rewritten"
-   then
-   exec git rebase--helper ${force_rebase:+--no-ff} 
$allow_empty_message \
-   --continue
-   fi
do_rest
return 0
;;
@@ -944,43 +919,11 @@ EOF
}
 
expand_todo_ids
-
-   test -d "$rewritten" || test -n "$force_rebase" ||
-   onto="$(git rebase--helper --skip-unnecessary-picks)" ||
-   die "Could not skip unnecessary pick commands"
-
checkout_onto
-   if test ! -d "$rewritten"
-   then
-   require_clean_work_tree "rebase"
-   exec git rebase--helper ${force_rebase:+--no-ff} 
$allow_empty_message \
-   --continue
-   fi
do_rest
 }
 
-git_rebase__interactive () {
-   initiate_action "$action"
-   ret=$?
-   if test $ret = 0; then
-   return 0
-   fi
-
-   setup_reflog_action
-   init_basic_state
-
-   init_revisions_and_shortrevisions
-
-   git rebase--helper --make-script ${keep_empty:+--keep-empty} \
-   ${rebase_merges:+--rebase-merges} \
-   ${rebase_cousins:+--rebase-cousins} \
-   $revisions ${restrict_revision+^$restrict_revision} >"$todo" ||
-   die "$(gettext "Could not generate todo list")"
-
-   complete_action
-}
-
-git_rebase__interactive__preserve_merges () {
+git_rebase__preserve_merges () {
initiate_action "$action"
ret=$?
if test $ret = 0; then
-- 
2.16.1



[GSoC][PATCH v4 3/4] rebase: use the new git-rebase--preserve-merges.sh

2018-05-28 Thread Alban Gruin
Create a new type of rebase, "preserve-merges", used when rebase is
called with -p.

Before that, the type for preserve-merges was "interactive", and some
places of this script compared $type to "interactive". Instead, the code
now checks if $interactive_rebase is empty or not, as it is set to
"explicit" when calling an interactive rebase (and, possibly, one of its
submodes), and "implied" when calling one of its
submodes (eg. preserve-merges) *without* interactive rebase.

It also detects the presence of the directory "$merge_dir"/rewritten
left by the preserve-merges script when calling rebase --continue,
--skip, etc., and, if it exists, sets the rebase mode to
preserve-merges. In this case, interactive_rebase is set to "explicit",
as "implied" would break some tests.

Signed-off-by: Alban Gruin <alban.gr...@gmail.com>
---
 git-rebase.sh | 32 +---
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 40be59ecc..19bdebb48 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -207,7 +207,14 @@ run_specific_rebase () {
autosquash=
fi
. git-rebase--$type
-   git_rebase__$type${preserve_merges:+__preserve_merges}
+
+   if test -z "$preserve_merges"
+   then
+   git_rebase__$type
+   else
+   git_rebase__preserve_merges
+   fi
+
ret=$?
if test $ret -eq 0
then
@@ -239,7 +246,12 @@ then
state_dir="$apply_dir"
 elif test -d "$merge_dir"
 then
-   if test -f "$merge_dir"/interactive
+   if test -d "$merge_dir"/rewritten
+   then
+   type=preserve-merges
+   interactive_rebase=explicit
+   preserve_merges=t
+   elif test -f "$merge_dir"/interactive
then
type=interactive
interactive_rebase=explicit
@@ -402,14 +414,14 @@ if test -n "$action"
 then
test -z "$in_progress" && die "$(gettext "No rebase in progress?")"
# Only interactive rebase uses detailed reflog messages
-   if test "$type" = interactive && test "$GIT_REFLOG_ACTION" = rebase
+   if test -n "$interactive_rebase" && test "$GIT_REFLOG_ACTION" = rebase
then
GIT_REFLOG_ACTION="rebase -i ($action)"
export GIT_REFLOG_ACTION
fi
 fi
 
-if test "$action" = "edit-todo" && test "$type" != "interactive"
+if test "$action" = "edit-todo" && test -z "$interactive_rebase"
 then
die "$(gettext "The --edit-todo action can only be used during 
interactive rebase.")"
 fi
@@ -487,7 +499,13 @@ fi
 
 if test -n "$interactive_rebase"
 then
-   type=interactive
+   if test -z "$preserve_merges"
+   then
+   type=interactive
+   else
+   type=preserve-merges
+   fi
+
state_dir="$merge_dir"
 elif test -n "$do_merge"
 then
@@ -647,7 +665,7 @@ require_clean_work_tree "rebase" "$(gettext "Please commit 
or stash them.")"
 # but this should be done only when upstream and onto are the same
 # and if this is not an interactive rebase.
 mb=$(git merge-base "$onto" "$orig_head")
-if test "$type" != interactive && test "$upstream" = "$onto" &&
+if test -z "$interactive_rebase" && test "$upstream" = "$onto" &&
test "$mb" = "$onto" && test -z "$restrict_revision" &&
# linear history?
! (git rev-list --parents "$onto".."$orig_head" | sane_grep " .* ") > 
/dev/null
@@ -691,7 +709,7 @@ then
GIT_PAGER='' git diff --stat --summary "$mb" "$onto"
 fi
 
-test "$type" = interactive && run_specific_rebase
+test -n "$interactive_rebase" && run_specific_rebase
 
 # Detach HEAD and reset the tree
 say "$(gettext "First, rewinding head to replay your work on top of it...")"
-- 
2.16.1



[GSoC][PATCH v4 4/4] rebase: remove -p code from git-rebase--interactive.sh

2018-05-28 Thread Alban Gruin
All the code specific to preserve-merges was moved to
git-rebase--preserve-merges.sh, and so it’s useless to keep it
here.

The intent of this commit is to clean this script as much as possible to
prepare a peaceful conversion as a builtin written in C.

Signed-off-by: Alban Gruin <alban.gr...@gmail.com>
---
 git-rebase--interactive.sh | 802 +
 1 file changed, 8 insertions(+), 794 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 2f4941d0f..9884ecd71 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -13,83 +13,6 @@
 # file and written to the tail of $done.
 todo="$state_dir"/git-rebase-todo
 
-# The rebase command lines that have already been processed.  A line
-# is moved here when it is first handled, before any associated user
-# actions.
-done="$state_dir"/done
-
-# The commit message that is planned to be used for any changes that
-# need to be committed following a user interaction.
-msg="$state_dir"/message
-
-# The file into which is accumulated the suggested commit message for
-# squash/fixup commands.  When the first of a series of squash/fixups
-# is seen, the file is created and the commit message from the
-# previous commit and from the first squash/fixup commit are written
-# to it.  The commit message for each subsequent squash/fixup commit
-# is appended to the file as it is processed.
-#
-# The first line of the file is of the form
-# # This is a combination of $count commits.
-# where $count is the number of commits whose messages have been
-# written to the file so far (including the initial "pick" commit).
-# Each time that a commit message is processed, this line is read and
-# updated.  It is deleted just before the combined commit is made.
-squash_msg="$state_dir"/message-squash
-
-# If the current series of squash/fixups has not yet included a squash
-# command, then this file exists and holds the commit message of the
-# original "pick" commit.  (If the series ends without a "squash"
-# command, then this can be used as the commit message of the combined
-# commit without opening the editor.)
-fixup_msg="$state_dir"/message-fixup
-
-# $rewritten is the name of a directory containing files for each
-# commit that is reachable by at least one merge base of $head and
-# $upstream. They are not necessarily rewritten, but their children
-# might be.  This ensures that commits on merged, but otherwise
-# unrelated side branches are left alone. (Think "X" in the man page's
-# example.)
-rewritten="$state_dir"/rewritten
-
-dropped="$state_dir"/dropped
-
-end="$state_dir"/end
-msgnum="$state_dir"/msgnum
-
-# A script to set the GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and
-# GIT_AUTHOR_DATE that will be used for the commit that is currently
-# being rebased.
-author_script="$state_dir"/author-script
-
-# When an "edit" rebase command is being processed, the SHA1 of the
-# commit to be edited is recorded in this file.  When "git rebase
-# --continue" is executed, if there are any staged changes then they
-# will be amended to the HEAD commit, but only provided the HEAD
-# commit is still the commit to be edited.  When any other rebase
-# command is processed, this file is deleted.
-amend="$state_dir"/amend
-
-# For the post-rewrite hook, we make a list of rewritten commits and
-# their new sha1s.  The rewritten-pending list keeps the sha1s of
-# commits that have been processed, but not committed yet,
-# e.g. because they are waiting for a 'squash' command.
-rewritten_list="$state_dir"/rewritten-list
-rewritten_pending="$state_dir"/rewritten-pending
-
-# Work around Git for Windows' Bash whose "read" does not strip CRLF
-# and leaves CR at the end instead.
-cr=$(printf "\015")
-
-strategy_args=${strategy:+--strategy=$strategy}
-test -n "$strategy_opts" &&
-eval '
-   for strategy_opt in '"$strategy_opts"'
-   do
-   strategy_args="$strategy_args -X$(git rev-parse --sq-quote 
"${strategy_opt#--}")"
-   done
-'
-
 GIT_CHERRY_PICK_HELP="$resolvemsg"
 export GIT_CHERRY_PICK_HELP
 
@@ -105,15 +28,6 @@ case "$comment_char" in
;;
 esac
 
-warn () {
-   printf '%s\n' "$*" >&2
-}
-
-# Output the commit message for the specified commit.
-commit_message () {
-   git cat-file commit "$1" | sed "1,/^$/d"
-}
-
 orig_reflog_action="$GIT_REFLOG_ACTION"
 
 comment_for_reflog () {
@@ -125,33 +39,6 @@ comment_for_reflog () {
esac
 }
 
-last_count=
-mark_action_done () {
-   sed -e 1q < "$todo" >> "$done"
-   sed -e 1d < "$todo" >> "$todo".new
-   mv -f &quo

[GSoC][PATCH v4 0/4] rebase: split rebase -p from rebase -i

2018-05-28 Thread Alban Gruin
This splits the `rebase --preserve-merges` functionnality from
git-rebase--interactive.sh. All the dead code left by the duplication of
git-rebase--interactive.sh is also removed. This will make it easier to rewrite
this script in C.

This patch series is based of js/sequencer-and-root-commits.

Changes since v3:

 - Rewording the commits to be more descriptive, and to respect the 72
   characters limit.

Alban Gruin (4):
  rebase: introduce a dedicated backend for --preserve-merges
  rebase: strip unused code in git-rebase--preserve-merges.sh
  rebase: use the new git-rebase--preserve-merges.sh
  rebase: remove -p code from git-rebase--interactive.sh

 .gitignore |1 +
 Makefile   |2 +
 git-rebase--interactive.sh |  802 +--
 git-rebase--preserve-merges.sh | 1012 
 git-rebase.sh  |   32 +-
 5 files changed, 1048 insertions(+), 801 deletions(-)
 create mode 100644 git-rebase--preserve-merges.sh

-- 
2.16.1



[GSoC] GSoC with git, week 4

2018-05-26 Thread Alban Gruin
Hi,

I published my blog post about this week. You can read it here:

https://blog.pa1ch.fr/posts/2018/05/26/en/gsoc2018-week-4.html

All comments are welcome!

Cheers,
Alban



Re: [GSoC][PATCH v3 0/4] rebase: split rebase -p from rebase -i

2018-05-24 Thread Alban Gruin
Hi,

Le 24/05/2018 à 13:49, Alban Gruin a écrit :
> Changes since v2:

 - Removing `mark_action_done()` from git-rebase--interactive.sh

 - Removing unused variables from git-rebase--interactive.sh


Cheers,
Alban



[GSoC][PATCH v3 1/4] rebase: duplicate git-rebase--interactive.sh to git-rebase--preserve-merges.sh

2018-05-24 Thread Alban Gruin
This duplicates git-rebase--interactive.sh to
git-rebase--preserve-merges.sh. This is done to split -p from -i. No
modifications are made to this file here, but any code that is not used by -p
will be stripped in the next commit.

Signed-off-by: Alban Gruin <alban.gr...@gmail.com>
---
 .gitignore |1 +
 Makefile   |2 +
 git-rebase--preserve-merges.sh | 1069 
 3 files changed, 1072 insertions(+)
 create mode 100644 git-rebase--preserve-merges.sh

diff --git a/.gitignore b/.gitignore
index 833ef3b0b..ef4925485 100644
--- a/.gitignore
+++ b/.gitignore
@@ -117,6 +117,7 @@
 /git-rebase--helper
 /git-rebase--interactive
 /git-rebase--merge
+/git-rebase--preserve-merges
 /git-receive-pack
 /git-reflog
 /git-remote
diff --git a/Makefile b/Makefile
index 50da82b01..810a0d0f4 100644
--- a/Makefile
+++ b/Makefile
@@ -582,6 +582,7 @@ SCRIPT_LIB += git-mergetool--lib
 SCRIPT_LIB += git-parse-remote
 SCRIPT_LIB += git-rebase--am
 SCRIPT_LIB += git-rebase--interactive
+SCRIPT_LIB += git-rebase--preserve-merges
 SCRIPT_LIB += git-rebase--merge
 SCRIPT_LIB += git-sh-setup
 SCRIPT_LIB += git-sh-i18n
@@ -2271,6 +2272,7 @@ LOCALIZED_C = $(C_OBJ:o=c) $(LIB_H) $(GENERATED_H)
 LOCALIZED_SH = $(SCRIPT_SH)
 LOCALIZED_SH += git-parse-remote.sh
 LOCALIZED_SH += git-rebase--interactive.sh
+LOCALIZED_SH += git-rebase--preserve-merges.sh
 LOCALIZED_SH += git-sh-setup.sh
 LOCALIZED_PERL = $(SCRIPT_PERL)
 
diff --git a/git-rebase--preserve-merges.sh b/git-rebase--preserve-merges.sh
new file mode 100644
index 0..2f4941d0f
--- /dev/null
+++ b/git-rebase--preserve-merges.sh
@@ -0,0 +1,1069 @@
+# This shell script fragment is sourced by git-rebase to implement
+# its interactive mode.  "git rebase --interactive" makes it easy
+# to fix up commits in the middle of a series and rearrange commits.
+#
+# Copyright (c) 2006 Johannes E. Schindelin
+#
+# The original idea comes from Eric W. Biederman, in
+# https://public-inbox.org/git/m1odwkyuf5.fsf...@ebiederm.dsl.xmission.com/
+#
+# The file containing rebase commands, comments, and empty lines.
+# This file is created by "git rebase -i" then edited by the user.  As
+# the lines are processed, they are removed from the front of this
+# file and written to the tail of $done.
+todo="$state_dir"/git-rebase-todo
+
+# The rebase command lines that have already been processed.  A line
+# is moved here when it is first handled, before any associated user
+# actions.
+done="$state_dir"/done
+
+# The commit message that is planned to be used for any changes that
+# need to be committed following a user interaction.
+msg="$state_dir"/message
+
+# The file into which is accumulated the suggested commit message for
+# squash/fixup commands.  When the first of a series of squash/fixups
+# is seen, the file is created and the commit message from the
+# previous commit and from the first squash/fixup commit are written
+# to it.  The commit message for each subsequent squash/fixup commit
+# is appended to the file as it is processed.
+#
+# The first line of the file is of the form
+# # This is a combination of $count commits.
+# where $count is the number of commits whose messages have been
+# written to the file so far (including the initial "pick" commit).
+# Each time that a commit message is processed, this line is read and
+# updated.  It is deleted just before the combined commit is made.
+squash_msg="$state_dir"/message-squash
+
+# If the current series of squash/fixups has not yet included a squash
+# command, then this file exists and holds the commit message of the
+# original "pick" commit.  (If the series ends without a "squash"
+# command, then this can be used as the commit message of the combined
+# commit without opening the editor.)
+fixup_msg="$state_dir"/message-fixup
+
+# $rewritten is the name of a directory containing files for each
+# commit that is reachable by at least one merge base of $head and
+# $upstream. They are not necessarily rewritten, but their children
+# might be.  This ensures that commits on merged, but otherwise
+# unrelated side branches are left alone. (Think "X" in the man page's
+# example.)
+rewritten="$state_dir"/rewritten
+
+dropped="$state_dir"/dropped
+
+end="$state_dir"/end
+msgnum="$state_dir"/msgnum
+
+# A script to set the GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and
+# GIT_AUTHOR_DATE that will be used for the commit that is currently
+# being rebased.
+author_script="$state_dir"/author-script
+
+# When an "edit" rebase command is being processed, the SHA1 of the
+# commit to be edited is recorded in this file.  When "git rebase
+# --continue" is executed, if there are any staged changes then they
+# will be amended to the HEAD commit, but only provided the HEAD
+# commit is 

[GSoC][PATCH v3 2/4] rebase: strip unused code in git-rebase--preserve-merges.sh

2018-05-24 Thread Alban Gruin
This removes the code coming from git-rebase--interactive.sh that is not needed
by preserve-merges.

Signed-off-by: Alban Gruin <alban.gr...@gmail.com>
---
 git-rebase--preserve-merges.sh | 65 +++---
 1 file changed, 4 insertions(+), 61 deletions(-)

diff --git a/git-rebase--preserve-merges.sh b/git-rebase--preserve-merges.sh
index 2f4941d0f..c51c7828e 100644
--- a/git-rebase--preserve-merges.sh
+++ b/git-rebase--preserve-merges.sh
@@ -1,12 +1,8 @@
-# This shell script fragment is sourced by git-rebase to implement
-# its interactive mode.  "git rebase --interactive" makes it easy
-# to fix up commits in the middle of a series and rearrange commits.
+# This shell script fragment is sourced by git-rebase to implement its
+# preserve-merges mode.
 #
 # Copyright (c) 2006 Johannes E. Schindelin
 #
-# The original idea comes from Eric W. Biederman, in
-# https://public-inbox.org/git/m1odwkyuf5.fsf...@ebiederm.dsl.xmission.com/
-#
 # The file containing rebase commands, comments, and empty lines.
 # This file is created by "git rebase -i" then edited by the user.  As
 # the lines are processed, they are removed from the front of this
@@ -287,17 +283,7 @@ pick_one () {
empty_args="--allow-empty"
fi
 
-   test -d "$rewritten" &&
-   pick_one_preserving_merges "$@" && return
-   output eval git cherry-pick $allow_rerere_autoupdate 
$allow_empty_message \
-   ${gpg_sign_opt:+$(git rev-parse --sq-quote 
"$gpg_sign_opt")} \
-   $signoff "$strategy_args" $empty_args $ff "$@"
-
-   # If cherry-pick dies it leaves the to-be-picked commit unrecorded. 
Reschedule
-   # previous task so this commit is not lost.
-   ret=$?
-   case "$ret" in [01]) ;; *) reschedule_last_action ;; esac
-   return $ret
+   pick_one_preserving_merges "$@"
 }
 
 pick_one_preserving_merges () {
@@ -761,11 +747,6 @@ get_missing_commit_check_level () {
 initiate_action () {
case "$1" in
continue)
-   if test ! -d "$rewritten"
-   then
-   exec git rebase--helper ${force_rebase:+--no-ff} 
$allow_empty_message \
-   --continue
-   fi
# do we have anything to commit?
if git diff-index --cached --quiet HEAD --
then
@@ -824,12 +805,6 @@ first and then run 'git rebase --continue' again.")"
;;
skip)
git rerere clear
-
-   if test ! -d "$rewritten"
-   then
-   exec git rebase--helper ${force_rebase:+--no-ff} 
$allow_empty_message \
-   --continue
-   fi
do_rest
return 0
;;
@@ -944,43 +919,11 @@ EOF
}
 
expand_todo_ids
-
-   test -d "$rewritten" || test -n "$force_rebase" ||
-   onto="$(git rebase--helper --skip-unnecessary-picks)" ||
-   die "Could not skip unnecessary pick commands"
-
checkout_onto
-   if test ! -d "$rewritten"
-   then
-   require_clean_work_tree "rebase"
-   exec git rebase--helper ${force_rebase:+--no-ff} 
$allow_empty_message \
-   --continue
-   fi
do_rest
 }
 
-git_rebase__interactive () {
-   initiate_action "$action"
-   ret=$?
-   if test $ret = 0; then
-   return 0
-   fi
-
-   setup_reflog_action
-   init_basic_state
-
-   init_revisions_and_shortrevisions
-
-   git rebase--helper --make-script ${keep_empty:+--keep-empty} \
-   ${rebase_merges:+--rebase-merges} \
-   ${rebase_cousins:+--rebase-cousins} \
-   $revisions ${restrict_revision+^$restrict_revision} >"$todo" ||
-   die "$(gettext "Could not generate todo list")"
-
-   complete_action
-}
-
-git_rebase__interactive__preserve_merges () {
+git_rebase__preserve_merges () {
initiate_action "$action"
ret=$?
if test $ret = 0; then
-- 
2.16.1



[GSoC][PATCH v3 0/4] rebase: split rebase -p from rebase -i

2018-05-24 Thread Alban Gruin
This splits the `rebase --preserve-merges` functionnality from
git-rebase--interactive.sh. All the dead code left by the duplication of
git-rebase--interactive.sh is also removed. This will make it easier to rewrite
this script in C.

This patch series is based of js/sequencer-and-root-commits.

Changes since v2:

 - Removing `

Alban Gruin (4):
  rebase: duplicate git-rebase--interactive.sh to
git-rebase--preserve-merges.sh
  rebase: strip unused code in git-rebase--preserve-merges.sh
  rebase: use the new git-rebase--preserve-merges.sh
  rebase: remove -p code from git-rebase--interactive.sh

 .gitignore |1 +
 Makefile   |2 +
 git-rebase--interactive.sh |  802 +--
 git-rebase--preserve-merges.sh | 1012 
 git-rebase.sh  |   32 +-
 5 files changed, 1048 insertions(+), 801 deletions(-)
 create mode 100644 git-rebase--preserve-merges.sh

-- 
2.16.1



[GSoC][PATCH v3 4/4] rebase: remove -p code from git-rebase--interactive.sh

2018-05-24 Thread Alban Gruin
All the code specific to preserve-merges was moved to
git-rebase--preserve-merges.sh, and so it’s useless to keep it here.

Signed-off-by: Alban Gruin <alban.gr...@gmail.com>
---
 git-rebase--interactive.sh | 802 +
 1 file changed, 8 insertions(+), 794 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 2f4941d0f..9884ecd71 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -13,83 +13,6 @@
 # file and written to the tail of $done.
 todo="$state_dir"/git-rebase-todo
 
-# The rebase command lines that have already been processed.  A line
-# is moved here when it is first handled, before any associated user
-# actions.
-done="$state_dir"/done
-
-# The commit message that is planned to be used for any changes that
-# need to be committed following a user interaction.
-msg="$state_dir"/message
-
-# The file into which is accumulated the suggested commit message for
-# squash/fixup commands.  When the first of a series of squash/fixups
-# is seen, the file is created and the commit message from the
-# previous commit and from the first squash/fixup commit are written
-# to it.  The commit message for each subsequent squash/fixup commit
-# is appended to the file as it is processed.
-#
-# The first line of the file is of the form
-# # This is a combination of $count commits.
-# where $count is the number of commits whose messages have been
-# written to the file so far (including the initial "pick" commit).
-# Each time that a commit message is processed, this line is read and
-# updated.  It is deleted just before the combined commit is made.
-squash_msg="$state_dir"/message-squash
-
-# If the current series of squash/fixups has not yet included a squash
-# command, then this file exists and holds the commit message of the
-# original "pick" commit.  (If the series ends without a "squash"
-# command, then this can be used as the commit message of the combined
-# commit without opening the editor.)
-fixup_msg="$state_dir"/message-fixup
-
-# $rewritten is the name of a directory containing files for each
-# commit that is reachable by at least one merge base of $head and
-# $upstream. They are not necessarily rewritten, but their children
-# might be.  This ensures that commits on merged, but otherwise
-# unrelated side branches are left alone. (Think "X" in the man page's
-# example.)
-rewritten="$state_dir"/rewritten
-
-dropped="$state_dir"/dropped
-
-end="$state_dir"/end
-msgnum="$state_dir"/msgnum
-
-# A script to set the GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and
-# GIT_AUTHOR_DATE that will be used for the commit that is currently
-# being rebased.
-author_script="$state_dir"/author-script
-
-# When an "edit" rebase command is being processed, the SHA1 of the
-# commit to be edited is recorded in this file.  When "git rebase
-# --continue" is executed, if there are any staged changes then they
-# will be amended to the HEAD commit, but only provided the HEAD
-# commit is still the commit to be edited.  When any other rebase
-# command is processed, this file is deleted.
-amend="$state_dir"/amend
-
-# For the post-rewrite hook, we make a list of rewritten commits and
-# their new sha1s.  The rewritten-pending list keeps the sha1s of
-# commits that have been processed, but not committed yet,
-# e.g. because they are waiting for a 'squash' command.
-rewritten_list="$state_dir"/rewritten-list
-rewritten_pending="$state_dir"/rewritten-pending
-
-# Work around Git for Windows' Bash whose "read" does not strip CRLF
-# and leaves CR at the end instead.
-cr=$(printf "\015")
-
-strategy_args=${strategy:+--strategy=$strategy}
-test -n "$strategy_opts" &&
-eval '
-   for strategy_opt in '"$strategy_opts"'
-   do
-   strategy_args="$strategy_args -X$(git rev-parse --sq-quote 
"${strategy_opt#--}")"
-   done
-'
-
 GIT_CHERRY_PICK_HELP="$resolvemsg"
 export GIT_CHERRY_PICK_HELP
 
@@ -105,15 +28,6 @@ case "$comment_char" in
;;
 esac
 
-warn () {
-   printf '%s\n' "$*" >&2
-}
-
-# Output the commit message for the specified commit.
-commit_message () {
-   git cat-file commit "$1" | sed "1,/^$/d"
-}
-
 orig_reflog_action="$GIT_REFLOG_ACTION"
 
 comment_for_reflog () {
@@ -125,33 +39,6 @@ comment_for_reflog () {
esac
 }
 
-last_count=
-mark_action_done () {
-   sed -e 1q < "$todo" >> "$done"
-   sed -e 1d < "$todo" >> "$todo".new
-   mv -f "$todo".new "$todo"
-   new_count=$(( $(git stripspace --strip-comments <"$done" | wc -l) ))
-   

[GSoC][PATCH v3 3/4] rebase: use the new git-rebase--preserve-merges.sh

2018-05-24 Thread Alban Gruin
Creates a new type of rebase, "preserve-merges", used when rebase is called with
-p.

Before that, the type for preserve-merges was "interactive", and some places of
this script compared $type to "interactive". Instead, the code now checks if
$interactive_rebase is empty or not, as it is set to "explicit" when calling an
interactive rebase (and, possibly, one of its submodes), and "implied" when
calling one of its submodes (eg. preserve-merges) *without* interactive rebase.

It also detects the presence of the directory "$merge_dir"/rewritten left by the
preserve-merges script when calling rebase --continue, --skip, etc., and, if it
exists, sets the rebase mode to preserve-merges. In this case,
interactive_rebase is set to "explicit", as "implied" would break some tests.

Signed-off-by: Alban Gruin <alban.gr...@gmail.com>
---
 git-rebase.sh | 32 +---
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 40be59ecc..19bdebb48 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -207,7 +207,14 @@ run_specific_rebase () {
autosquash=
fi
. git-rebase--$type
-   git_rebase__$type${preserve_merges:+__preserve_merges}
+
+   if test -z "$preserve_merges"
+   then
+   git_rebase__$type
+   else
+   git_rebase__preserve_merges
+   fi
+
ret=$?
if test $ret -eq 0
then
@@ -239,7 +246,12 @@ then
state_dir="$apply_dir"
 elif test -d "$merge_dir"
 then
-   if test -f "$merge_dir"/interactive
+   if test -d "$merge_dir"/rewritten
+   then
+   type=preserve-merges
+   interactive_rebase=explicit
+   preserve_merges=t
+   elif test -f "$merge_dir"/interactive
then
type=interactive
interactive_rebase=explicit
@@ -402,14 +414,14 @@ if test -n "$action"
 then
test -z "$in_progress" && die "$(gettext "No rebase in progress?")"
# Only interactive rebase uses detailed reflog messages
-   if test "$type" = interactive && test "$GIT_REFLOG_ACTION" = rebase
+   if test -n "$interactive_rebase" && test "$GIT_REFLOG_ACTION" = rebase
then
GIT_REFLOG_ACTION="rebase -i ($action)"
export GIT_REFLOG_ACTION
fi
 fi
 
-if test "$action" = "edit-todo" && test "$type" != "interactive"
+if test "$action" = "edit-todo" && test -z "$interactive_rebase"
 then
die "$(gettext "The --edit-todo action can only be used during 
interactive rebase.")"
 fi
@@ -487,7 +499,13 @@ fi
 
 if test -n "$interactive_rebase"
 then
-   type=interactive
+   if test -z "$preserve_merges"
+   then
+   type=interactive
+   else
+   type=preserve-merges
+   fi
+
state_dir="$merge_dir"
 elif test -n "$do_merge"
 then
@@ -647,7 +665,7 @@ require_clean_work_tree "rebase" "$(gettext "Please commit 
or stash them.")"
 # but this should be done only when upstream and onto are the same
 # and if this is not an interactive rebase.
 mb=$(git merge-base "$onto" "$orig_head")
-if test "$type" != interactive && test "$upstream" = "$onto" &&
+if test -z "$interactive_rebase" && test "$upstream" = "$onto" &&
test "$mb" = "$onto" && test -z "$restrict_revision" &&
# linear history?
! (git rev-list --parents "$onto".."$orig_head" | sane_grep " .* ") > 
/dev/null
@@ -691,7 +709,7 @@ then
GIT_PAGER='' git diff --stat --summary "$mb" "$onto"
 fi
 
-test "$type" = interactive && run_specific_rebase
+test -n "$interactive_rebase" && run_specific_rebase
 
 # Detach HEAD and reset the tree
 say "$(gettext "First, rewinding head to replay your work on top of it...")"
-- 
2.16.1



[GSoC][PATCH v2 4/4] rebase: remove -p code from git-rebase--interactive.sh

2018-05-22 Thread Alban Gruin
All the code specific to preserve-merges was moved to
git-rebase--preserve-merges.sh, and so it’s useless to keep it here.

Signed-off-by: Alban Gruin <alban.gr...@gmail.com>
---
 git-rebase--interactive.sh | 708 +
 1 file changed, 8 insertions(+), 700 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 2f4941d0f..657d32773 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -105,15 +105,6 @@ case "$comment_char" in
;;
 esac
 
-warn () {
-   printf '%s\n' "$*" >&2
-}
-
-# Output the commit message for the specified commit.
-commit_message () {
-   git cat-file commit "$1" | sed "1,/^$/d"
-}
-
 orig_reflog_action="$GIT_REFLOG_ACTION"
 
 comment_for_reflog () {
@@ -142,16 +133,6 @@ mark_action_done () {
fi
 }
 
-# Put the last action marked done at the beginning of the todo list
-# again. If there has not been an action marked done yet, leave the list of
-# items on the todo list unchanged.
-reschedule_last_action () {
-   tail -n 1 "$done" | cat - "$todo" >"$todo".new
-   sed -e \$d <"$done" >"$done".new
-   mv -f "$todo".new "$todo"
-   mv -f "$done".new "$done"
-}
-
 append_todo_help () {
gettext "
 Commands:
@@ -184,50 +165,6 @@ If you remove a line here THAT COMMIT WILL BE LOST.
fi
 }
 
-make_patch () {
-   sha1_and_parents="$(git rev-list --parents -1 "$1")"
-   case "$sha1_and_parents" in
-   ?*' '?*' '?*)
-   git diff --cc $sha1_and_parents
-   ;;
-   ?*' '?*)
-   git diff-tree -p "$1^!"
-   ;;
-   *)
-   echo "Root commit"
-   ;;
-   esac > "$state_dir"/patch
-   test -f "$msg" ||
-   commit_message "$1" > "$msg"
-   test -f "$author_script" ||
-   get_author_ident_from_commit "$1" > "$author_script"
-}
-
-die_with_patch () {
-   echo "$1" > "$state_dir"/stopped-sha
-   git update-ref REBASE_HEAD "$1"
-   make_patch "$1"
-   die "$2"
-}
-
-exit_with_patch () {
-   echo "$1" > "$state_dir"/stopped-sha
-   git update-ref REBASE_HEAD "$1"
-   make_patch $1
-   git rev-parse --verify HEAD > "$amend"
-   gpg_sign_opt_quoted=${gpg_sign_opt:+$(git rev-parse --sq-quote 
"$gpg_sign_opt")}
-   warn "$(eval_gettext "\
-You can amend the commit now, with
-
-   git commit --amend \$gpg_sign_opt_quoted
-
-Once you are satisfied with your changes, run
-
-   git rebase --continue")"
-   warn
-   exit $2
-}
-
 die_abort () {
apply_autostash
rm -rf "$state_dir"
@@ -238,30 +175,6 @@ has_action () {
test -n "$(git stripspace --strip-comments <"$1")"
 }
 
-is_empty_commit() {
-   tree=$(git rev-parse -q --verify "$1"^{tree} 2>/dev/null) || {
-   sha1=$1
-   die "$(eval_gettext "\$sha1: not a commit that can be picked")"
-   }
-   ptree=$(git rev-parse -q --verify "$1"^^{tree} 2>/dev/null) ||
-   ptree=4b825dc642cb6eb9a060e54bf8d69288fbee4904
-   test "$tree" = "$ptree"
-}
-
-is_merge_commit()
-{
-   git rev-parse --verify --quiet "$1"^2 >/dev/null 2>&1
-}
-
-# Run command with GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and
-# GIT_AUTHOR_DATE exported from the current environment.
-do_with_author () {
-   (
-   export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE
-   "$@"
-   )
-}
-
 git_sequence_editor () {
if test -z "$GIT_SEQUENCE_EDITOR"
then
@@ -275,455 +188,6 @@ git_sequence_editor () {
eval "$GIT_SEQUENCE_EDITOR" '"$@"'
 }
 
-pick_one () {
-   ff=--ff
-
-   case "$1" in -n) sha1=$2; ff= ;; *) sha1=$1 ;; esac
-   case "$force_rebase" in '') ;; ?*) ff= ;; esac
-   output git rev-parse --verify $sha1 || die "$(eval_gettext "Invalid 
commit name: \$sha1")"
-
-   if is_empty_commit "$sha1"
-   then
-   empty_args="--allow-empty"
-   fi
-
-   test -d "$rewritten" &&
-   pick_one_preserving_merges "$@" && return
-   output eval git cherry-pick $allow_rerere_autoupdate 
$allow_empty_message \
-   ${gpg_sign_opt:+$(git rev-parse --sq-quote 
"$gpg_sign_opt")} \
-   $s

[GSoC][PATCH v2 3/4] rebase: use the new git-rebase--preserve-merges.sh

2018-05-22 Thread Alban Gruin
Creates a new type of rebase, "preserve-merges", used when rebase is called with
-p.

Before that, the type for preserve-merges was "interactive", and some places of
this script compared $type to "interactive". Instead, the code now checks if
$interactive_rebase is empty or not, as it is set to "explicit" when calling an
interactive rebase (and, possibly, one of its submodes), and "implied" when
calling one of its submodes (eg. preserve-merges) *without* interactive rebase.

It also detects the presence of the directory "$merge_dir"/rewritten left by the
preserve-merges script when calling rebase --continue, --skip, etc., and, if it
exists, sets the rebase mode to preserve-merges. In this case,
interactive_rebase is set to "explicit", as "implied" would break some tests.

Signed-off-by: Alban Gruin <alban.gr...@gmail.com>
---
 git-rebase.sh | 32 +---
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 40be59ecc..19bdebb48 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -207,7 +207,14 @@ run_specific_rebase () {
autosquash=
fi
. git-rebase--$type
-   git_rebase__$type${preserve_merges:+__preserve_merges}
+
+   if test -z "$preserve_merges"
+   then
+   git_rebase__$type
+   else
+   git_rebase__preserve_merges
+   fi
+
ret=$?
if test $ret -eq 0
then
@@ -239,7 +246,12 @@ then
state_dir="$apply_dir"
 elif test -d "$merge_dir"
 then
-   if test -f "$merge_dir"/interactive
+   if test -d "$merge_dir"/rewritten
+   then
+   type=preserve-merges
+   interactive_rebase=explicit
+   preserve_merges=t
+   elif test -f "$merge_dir"/interactive
then
type=interactive
interactive_rebase=explicit
@@ -402,14 +414,14 @@ if test -n "$action"
 then
test -z "$in_progress" && die "$(gettext "No rebase in progress?")"
# Only interactive rebase uses detailed reflog messages
-   if test "$type" = interactive && test "$GIT_REFLOG_ACTION" = rebase
+   if test -n "$interactive_rebase" && test "$GIT_REFLOG_ACTION" = rebase
then
GIT_REFLOG_ACTION="rebase -i ($action)"
export GIT_REFLOG_ACTION
fi
 fi
 
-if test "$action" = "edit-todo" && test "$type" != "interactive"
+if test "$action" = "edit-todo" && test -z "$interactive_rebase"
 then
die "$(gettext "The --edit-todo action can only be used during 
interactive rebase.")"
 fi
@@ -487,7 +499,13 @@ fi
 
 if test -n "$interactive_rebase"
 then
-   type=interactive
+   if test -z "$preserve_merges"
+   then
+   type=interactive
+   else
+   type=preserve-merges
+   fi
+
state_dir="$merge_dir"
 elif test -n "$do_merge"
 then
@@ -647,7 +665,7 @@ require_clean_work_tree "rebase" "$(gettext "Please commit 
or stash them.")"
 # but this should be done only when upstream and onto are the same
 # and if this is not an interactive rebase.
 mb=$(git merge-base "$onto" "$orig_head")
-if test "$type" != interactive && test "$upstream" = "$onto" &&
+if test -z "$interactive_rebase" && test "$upstream" = "$onto" &&
test "$mb" = "$onto" && test -z "$restrict_revision" &&
# linear history?
! (git rev-list --parents "$onto".."$orig_head" | sane_grep " .* ") > 
/dev/null
@@ -691,7 +709,7 @@ then
GIT_PAGER='' git diff --stat --summary "$mb" "$onto"
 fi
 
-test "$type" = interactive && run_specific_rebase
+test -n "$interactive_rebase" && run_specific_rebase
 
 # Detach HEAD and reset the tree
 say "$(gettext "First, rewinding head to replay your work on top of it...")"
-- 
2.16.1



[GSoC][PATCH v2 1/4] rebase: duplicate git-rebase--interactive.sh to git-rebase--preserve-merges.sh

2018-05-22 Thread Alban Gruin
This duplicates git-rebase--interactive.sh to
git-rebase--preserve-merges.sh. This is done to split -p from -i. No
modifications are made to this file here, but any code that is not used by -p
will be stripped in the next commit.

Signed-off-by: Alban Gruin <alban.gr...@gmail.com>
---
 .gitignore |1 +
 Makefile   |2 +
 git-rebase--preserve-merges.sh | 1069 
 3 files changed, 1072 insertions(+)
 create mode 100644 git-rebase--preserve-merges.sh

diff --git a/.gitignore b/.gitignore
index 833ef3b0b..ef4925485 100644
--- a/.gitignore
+++ b/.gitignore
@@ -117,6 +117,7 @@
 /git-rebase--helper
 /git-rebase--interactive
 /git-rebase--merge
+/git-rebase--preserve-merges
 /git-receive-pack
 /git-reflog
 /git-remote
diff --git a/Makefile b/Makefile
index 50da82b01..810a0d0f4 100644
--- a/Makefile
+++ b/Makefile
@@ -582,6 +582,7 @@ SCRIPT_LIB += git-mergetool--lib
 SCRIPT_LIB += git-parse-remote
 SCRIPT_LIB += git-rebase--am
 SCRIPT_LIB += git-rebase--interactive
+SCRIPT_LIB += git-rebase--preserve-merges
 SCRIPT_LIB += git-rebase--merge
 SCRIPT_LIB += git-sh-setup
 SCRIPT_LIB += git-sh-i18n
@@ -2271,6 +2272,7 @@ LOCALIZED_C = $(C_OBJ:o=c) $(LIB_H) $(GENERATED_H)
 LOCALIZED_SH = $(SCRIPT_SH)
 LOCALIZED_SH += git-parse-remote.sh
 LOCALIZED_SH += git-rebase--interactive.sh
+LOCALIZED_SH += git-rebase--preserve-merges.sh
 LOCALIZED_SH += git-sh-setup.sh
 LOCALIZED_PERL = $(SCRIPT_PERL)
 
diff --git a/git-rebase--preserve-merges.sh b/git-rebase--preserve-merges.sh
new file mode 100644
index 0..2f4941d0f
--- /dev/null
+++ b/git-rebase--preserve-merges.sh
@@ -0,0 +1,1069 @@
+# This shell script fragment is sourced by git-rebase to implement
+# its interactive mode.  "git rebase --interactive" makes it easy
+# to fix up commits in the middle of a series and rearrange commits.
+#
+# Copyright (c) 2006 Johannes E. Schindelin
+#
+# The original idea comes from Eric W. Biederman, in
+# https://public-inbox.org/git/m1odwkyuf5.fsf...@ebiederm.dsl.xmission.com/
+#
+# The file containing rebase commands, comments, and empty lines.
+# This file is created by "git rebase -i" then edited by the user.  As
+# the lines are processed, they are removed from the front of this
+# file and written to the tail of $done.
+todo="$state_dir"/git-rebase-todo
+
+# The rebase command lines that have already been processed.  A line
+# is moved here when it is first handled, before any associated user
+# actions.
+done="$state_dir"/done
+
+# The commit message that is planned to be used for any changes that
+# need to be committed following a user interaction.
+msg="$state_dir"/message
+
+# The file into which is accumulated the suggested commit message for
+# squash/fixup commands.  When the first of a series of squash/fixups
+# is seen, the file is created and the commit message from the
+# previous commit and from the first squash/fixup commit are written
+# to it.  The commit message for each subsequent squash/fixup commit
+# is appended to the file as it is processed.
+#
+# The first line of the file is of the form
+# # This is a combination of $count commits.
+# where $count is the number of commits whose messages have been
+# written to the file so far (including the initial "pick" commit).
+# Each time that a commit message is processed, this line is read and
+# updated.  It is deleted just before the combined commit is made.
+squash_msg="$state_dir"/message-squash
+
+# If the current series of squash/fixups has not yet included a squash
+# command, then this file exists and holds the commit message of the
+# original "pick" commit.  (If the series ends without a "squash"
+# command, then this can be used as the commit message of the combined
+# commit without opening the editor.)
+fixup_msg="$state_dir"/message-fixup
+
+# $rewritten is the name of a directory containing files for each
+# commit that is reachable by at least one merge base of $head and
+# $upstream. They are not necessarily rewritten, but their children
+# might be.  This ensures that commits on merged, but otherwise
+# unrelated side branches are left alone. (Think "X" in the man page's
+# example.)
+rewritten="$state_dir"/rewritten
+
+dropped="$state_dir"/dropped
+
+end="$state_dir"/end
+msgnum="$state_dir"/msgnum
+
+# A script to set the GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and
+# GIT_AUTHOR_DATE that will be used for the commit that is currently
+# being rebased.
+author_script="$state_dir"/author-script
+
+# When an "edit" rebase command is being processed, the SHA1 of the
+# commit to be edited is recorded in this file.  When "git rebase
+# --continue" is executed, if there are any staged changes then they
+# will be amended to the HEAD commit, but only provided the HEAD
+# commit is 

[GSoC][PATCH v2 2/4] rebase: strip unused code in git-rebase--preserve-merges.sh

2018-05-22 Thread Alban Gruin
This removes the code coming from git-rebase--interactive.sh that is not needed
by preserve-merges.

Signed-off-by: Alban Gruin <alban.gr...@gmail.com>
---
 git-rebase--preserve-merges.sh | 65 +++---
 1 file changed, 4 insertions(+), 61 deletions(-)

diff --git a/git-rebase--preserve-merges.sh b/git-rebase--preserve-merges.sh
index 2f4941d0f..c51c7828e 100644
--- a/git-rebase--preserve-merges.sh
+++ b/git-rebase--preserve-merges.sh
@@ -1,12 +1,8 @@
-# This shell script fragment is sourced by git-rebase to implement
-# its interactive mode.  "git rebase --interactive" makes it easy
-# to fix up commits in the middle of a series and rearrange commits.
+# This shell script fragment is sourced by git-rebase to implement its
+# preserve-merges mode.
 #
 # Copyright (c) 2006 Johannes E. Schindelin
 #
-# The original idea comes from Eric W. Biederman, in
-# https://public-inbox.org/git/m1odwkyuf5.fsf...@ebiederm.dsl.xmission.com/
-#
 # The file containing rebase commands, comments, and empty lines.
 # This file is created by "git rebase -i" then edited by the user.  As
 # the lines are processed, they are removed from the front of this
@@ -287,17 +283,7 @@ pick_one () {
empty_args="--allow-empty"
fi
 
-   test -d "$rewritten" &&
-   pick_one_preserving_merges "$@" && return
-   output eval git cherry-pick $allow_rerere_autoupdate 
$allow_empty_message \
-   ${gpg_sign_opt:+$(git rev-parse --sq-quote 
"$gpg_sign_opt")} \
-   $signoff "$strategy_args" $empty_args $ff "$@"
-
-   # If cherry-pick dies it leaves the to-be-picked commit unrecorded. 
Reschedule
-   # previous task so this commit is not lost.
-   ret=$?
-   case "$ret" in [01]) ;; *) reschedule_last_action ;; esac
-   return $ret
+   pick_one_preserving_merges "$@"
 }
 
 pick_one_preserving_merges () {
@@ -761,11 +747,6 @@ get_missing_commit_check_level () {
 initiate_action () {
case "$1" in
continue)
-   if test ! -d "$rewritten"
-   then
-   exec git rebase--helper ${force_rebase:+--no-ff} 
$allow_empty_message \
-   --continue
-   fi
# do we have anything to commit?
if git diff-index --cached --quiet HEAD --
then
@@ -824,12 +805,6 @@ first and then run 'git rebase --continue' again.")"
;;
skip)
git rerere clear
-
-   if test ! -d "$rewritten"
-   then
-   exec git rebase--helper ${force_rebase:+--no-ff} 
$allow_empty_message \
-   --continue
-   fi
do_rest
return 0
;;
@@ -944,43 +919,11 @@ EOF
}
 
expand_todo_ids
-
-   test -d "$rewritten" || test -n "$force_rebase" ||
-   onto="$(git rebase--helper --skip-unnecessary-picks)" ||
-   die "Could not skip unnecessary pick commands"
-
checkout_onto
-   if test ! -d "$rewritten"
-   then
-   require_clean_work_tree "rebase"
-   exec git rebase--helper ${force_rebase:+--no-ff} 
$allow_empty_message \
-   --continue
-   fi
do_rest
 }
 
-git_rebase__interactive () {
-   initiate_action "$action"
-   ret=$?
-   if test $ret = 0; then
-   return 0
-   fi
-
-   setup_reflog_action
-   init_basic_state
-
-   init_revisions_and_shortrevisions
-
-   git rebase--helper --make-script ${keep_empty:+--keep-empty} \
-   ${rebase_merges:+--rebase-merges} \
-   ${rebase_cousins:+--rebase-cousins} \
-   $revisions ${restrict_revision+^$restrict_revision} >"$todo" ||
-   die "$(gettext "Could not generate todo list")"
-
-   complete_action
-}
-
-git_rebase__interactive__preserve_merges () {
+git_rebase__preserve_merges () {
initiate_action "$action"
ret=$?
if test $ret = 0; then
-- 
2.16.1



[GSoC][PATCH v2 0/4] rebase: split rebase -p from rebase -i

2018-05-22 Thread Alban Gruin
This splits the `rebase --preserve-merges` functionnality from
git-rebase--interactive.sh. All the dead code left by the duplication of
git-rebase--interactive.sh is also removed. This will make it easier to rewrite
this script in C.

This patch series is based of js/sequencer-and-root-commits.

Changes since v1:

 - Duplicating the correct version of git-rebase--interactive.sh (thanks
   Stefan!)

Alban Gruin (4):
  rebase: duplicate git-rebase--interactive.sh to
git-rebase--preserve-merges.sh
  rebase: strip unused code in git-rebase--preserve-merges.sh
  rebase: use the new git-rebase--preserve-merges.sh
  rebase: remove -p code from git-rebase--interactive.sh

 .gitignore |1 +
 Makefile   |2 +
 git-rebase--interactive.sh |  708 +---
 git-rebase--preserve-merges.sh | 1012 
 git-rebase.sh  |   32 +-
 5 files changed, 1048 insertions(+), 707 deletions(-)
 create mode 100644 git-rebase--preserve-merges.sh

-- 
2.16.1



Re: [GSoC][PATCH 1/4] rebase: duplicate git-rebase--interactive.sh to git-rebase--preserve-merges.sh

2018-05-22 Thread Alban Gruin
Hi Stefan,

Le 22/05/2018 à 20:26, Stefan Beller a écrit :
> On Tue, May 22, 2018 at 6:31 AM, Alban Gruin <alban.gr...@gmail.com> wrote:
>> This duplicates git-rebase--interactive.sh to
>> git-rebase--preserve-merges.sh. This is done to split -p from -i. No
>> modifications are made to this file here, but any code that is not used by -p
>> will be stripped in the next commit.
>>
>> Signed-off-by: Alban Gruin <alban.gr...@gmail.com>
> 
> So how would I best review this?
> 
> I applied the patches locally[1], and ran git-ls-tree on this commit
> hoping to find the same blob id for git-rebase--interactive.sh as for
> git-rebase--preserve-merges.sh; however I did not.
> 
> So I diffed them and had the diff below[2], which looks like that has parts
> of Johannes recent series?
> 

Thanks for the heads-up.

I started to work on that patch set on the master branch of git.git, and
I forgot to update git-rebase--preserve-merges.sh after rebasing on
Johannes’ branch.

So I’ll reroll the patch as soon as possible.

> Thanks,
> Stefan
> 




[GSoC][PATCH 2/4] rebase: strip unused code in git-rebase--preserve-merges.sh

2018-05-22 Thread Alban Gruin
This removes the code coming from git-rebase--interactive.sh that is not needed
by preserve-merges.

Signed-off-by: Alban Gruin <alban.gr...@gmail.com>
---
 git-rebase--preserve-merges.sh | 63 +++---
 1 file changed, 4 insertions(+), 59 deletions(-)

diff --git a/git-rebase--preserve-merges.sh b/git-rebase--preserve-merges.sh
index 9947e6265..72438 100644
--- a/git-rebase--preserve-merges.sh
+++ b/git-rebase--preserve-merges.sh
@@ -1,12 +1,8 @@
-# This shell script fragment is sourced by git-rebase to implement
-# its interactive mode.  "git rebase --interactive" makes it easy
-# to fix up commits in the middle of a series and rearrange commits.
+# This shell script fragment is sourced by git-rebase to implement its
+# preserve-merges mode.
 #
 # Copyright (c) 2006 Johannes E. Schindelin
 #
-# The original idea comes from Eric W. Biederman, in
-# https://public-inbox.org/git/m1odwkyuf5.fsf...@ebiederm.dsl.xmission.com/
-#
 # The file containing rebase commands, comments, and empty lines.
 # This file is created by "git rebase -i" then edited by the user.  As
 # the lines are processed, they are removed from the front of this
@@ -281,17 +277,7 @@ pick_one () {
empty_args="--allow-empty"
fi
 
-   test -d "$rewritten" &&
-   pick_one_preserving_merges "$@" && return
-   output eval git cherry-pick $allow_rerere_autoupdate 
$allow_empty_message \
-   ${gpg_sign_opt:+$(git rev-parse --sq-quote 
"$gpg_sign_opt")} \
-   $signoff "$strategy_args" $empty_args $ff "$@"
-
-   # If cherry-pick dies it leaves the to-be-picked commit unrecorded. 
Reschedule
-   # previous task so this commit is not lost.
-   ret=$?
-   case "$ret" in [01]) ;; *) reschedule_last_action ;; esac
-   return $ret
+   pick_one_preserving_merges "$@"
 }
 
 pick_one_preserving_merges () {
@@ -755,11 +741,6 @@ get_missing_commit_check_level () {
 initiate_action () {
case "$1" in
continue)
-   if test ! -d "$rewritten"
-   then
-   exec git rebase--helper ${force_rebase:+--no-ff} 
$allow_empty_message \
-   --continue
-   fi
# do we have anything to commit?
if git diff-index --cached --quiet HEAD --
then
@@ -818,12 +799,6 @@ first and then run 'git rebase --continue' again.")"
;;
skip)
git rerere clear
-
-   if test ! -d "$rewritten"
-   then
-   exec git rebase--helper ${force_rebase:+--no-ff} 
$allow_empty_message \
-   --continue
-   fi
do_rest
return 0
;;
@@ -936,41 +911,11 @@ EOF
}
 
expand_todo_ids
-
-   test -d "$rewritten" || test -n "$force_rebase" ||
-   onto="$(git rebase--helper --skip-unnecessary-picks)" ||
-   die "Could not skip unnecessary pick commands"
-
checkout_onto
-   if test -z "$rebase_root" && test ! -d "$rewritten"
-   then
-   require_clean_work_tree "rebase"
-   exec git rebase--helper ${force_rebase:+--no-ff} 
$allow_empty_message \
-   --continue
-   fi
do_rest
 }
 
-git_rebase__interactive () {
-   initiate_action "$action"
-   ret=$?
-   if test $ret = 0; then
-   return 0
-   fi
-
-   setup_reflog_action
-   init_basic_state
-
-   init_revisions_and_shortrevisions
-
-   git rebase--helper --make-script ${keep_empty:+--keep-empty} \
-   $revisions ${restrict_revision+^$restrict_revision} >"$todo" ||
-   die "$(gettext "Could not generate todo list")"
-
-   complete_action
-}
-
-git_rebase__interactive__preserve_merges () {
+git_rebase__preserve_merges () {
initiate_action "$action"
ret=$?
if test $ret = 0; then
-- 
2.16.1



[GSoC][PATCH 1/4] rebase: duplicate git-rebase--interactive.sh to git-rebase--preserve-merges.sh

2018-05-22 Thread Alban Gruin
This duplicates git-rebase--interactive.sh to
git-rebase--preserve-merges.sh. This is done to split -p from -i. No
modifications are made to this file here, but any code that is not used by -p
will be stripped in the next commit.

Signed-off-by: Alban Gruin <alban.gr...@gmail.com>
---
 .gitignore |1 +
 Makefile   |2 +
 git-rebase--preserve-merges.sh | 1059 
 3 files changed, 1062 insertions(+)
 create mode 100644 git-rebase--preserve-merges.sh

diff --git a/.gitignore b/.gitignore
index 833ef3b0b..ef4925485 100644
--- a/.gitignore
+++ b/.gitignore
@@ -117,6 +117,7 @@
 /git-rebase--helper
 /git-rebase--interactive
 /git-rebase--merge
+/git-rebase--preserve-merges
 /git-receive-pack
 /git-reflog
 /git-remote
diff --git a/Makefile b/Makefile
index 50da82b01..810a0d0f4 100644
--- a/Makefile
+++ b/Makefile
@@ -582,6 +582,7 @@ SCRIPT_LIB += git-mergetool--lib
 SCRIPT_LIB += git-parse-remote
 SCRIPT_LIB += git-rebase--am
 SCRIPT_LIB += git-rebase--interactive
+SCRIPT_LIB += git-rebase--preserve-merges
 SCRIPT_LIB += git-rebase--merge
 SCRIPT_LIB += git-sh-setup
 SCRIPT_LIB += git-sh-i18n
@@ -2271,6 +2272,7 @@ LOCALIZED_C = $(C_OBJ:o=c) $(LIB_H) $(GENERATED_H)
 LOCALIZED_SH = $(SCRIPT_SH)
 LOCALIZED_SH += git-parse-remote.sh
 LOCALIZED_SH += git-rebase--interactive.sh
+LOCALIZED_SH += git-rebase--preserve-merges.sh
 LOCALIZED_SH += git-sh-setup.sh
 LOCALIZED_PERL = $(SCRIPT_PERL)
 
diff --git a/git-rebase--preserve-merges.sh b/git-rebase--preserve-merges.sh
new file mode 100644
index 0..9947e6265
--- /dev/null
+++ b/git-rebase--preserve-merges.sh
@@ -0,0 +1,1059 @@
+# This shell script fragment is sourced by git-rebase to implement
+# its interactive mode.  "git rebase --interactive" makes it easy
+# to fix up commits in the middle of a series and rearrange commits.
+#
+# Copyright (c) 2006 Johannes E. Schindelin
+#
+# The original idea comes from Eric W. Biederman, in
+# https://public-inbox.org/git/m1odwkyuf5.fsf...@ebiederm.dsl.xmission.com/
+#
+# The file containing rebase commands, comments, and empty lines.
+# This file is created by "git rebase -i" then edited by the user.  As
+# the lines are processed, they are removed from the front of this
+# file and written to the tail of $done.
+todo="$state_dir"/git-rebase-todo
+
+# The rebase command lines that have already been processed.  A line
+# is moved here when it is first handled, before any associated user
+# actions.
+done="$state_dir"/done
+
+# The commit message that is planned to be used for any changes that
+# need to be committed following a user interaction.
+msg="$state_dir"/message
+
+# The file into which is accumulated the suggested commit message for
+# squash/fixup commands.  When the first of a series of squash/fixups
+# is seen, the file is created and the commit message from the
+# previous commit and from the first squash/fixup commit are written
+# to it.  The commit message for each subsequent squash/fixup commit
+# is appended to the file as it is processed.
+#
+# The first line of the file is of the form
+# # This is a combination of $count commits.
+# where $count is the number of commits whose messages have been
+# written to the file so far (including the initial "pick" commit).
+# Each time that a commit message is processed, this line is read and
+# updated.  It is deleted just before the combined commit is made.
+squash_msg="$state_dir"/message-squash
+
+# If the current series of squash/fixups has not yet included a squash
+# command, then this file exists and holds the commit message of the
+# original "pick" commit.  (If the series ends without a "squash"
+# command, then this can be used as the commit message of the combined
+# commit without opening the editor.)
+fixup_msg="$state_dir"/message-fixup
+
+# $rewritten is the name of a directory containing files for each
+# commit that is reachable by at least one merge base of $head and
+# $upstream. They are not necessarily rewritten, but their children
+# might be.  This ensures that commits on merged, but otherwise
+# unrelated side branches are left alone. (Think "X" in the man page's
+# example.)
+rewritten="$state_dir"/rewritten
+
+dropped="$state_dir"/dropped
+
+end="$state_dir"/end
+msgnum="$state_dir"/msgnum
+
+# A script to set the GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and
+# GIT_AUTHOR_DATE that will be used for the commit that is currently
+# being rebased.
+author_script="$state_dir"/author-script
+
+# When an "edit" rebase command is being processed, the SHA1 of the
+# commit to be edited is recorded in this file.  When "git rebase
+# --continue" is executed, if there are any staged changes then they
+# will be amended to the HEAD commit, but only provided the HEAD
+# commit is 

[GSoC][PATCH 4/4] rebase: remove -p code from git-rebase--interactive.sh

2018-05-22 Thread Alban Gruin
All the code specific to preserve-merges was moved to
git-rebase--preserve-merges.sh, and so it’s useless to keep it here.

Signed-off-by: Alban Gruin <alban.gr...@gmail.com>
---
 git-rebase--interactive.sh | 708 +
 1 file changed, 8 insertions(+), 700 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 2f4941d0f..657d32773 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -105,15 +105,6 @@ case "$comment_char" in
;;
 esac
 
-warn () {
-   printf '%s\n' "$*" >&2
-}
-
-# Output the commit message for the specified commit.
-commit_message () {
-   git cat-file commit "$1" | sed "1,/^$/d"
-}
-
 orig_reflog_action="$GIT_REFLOG_ACTION"
 
 comment_for_reflog () {
@@ -142,16 +133,6 @@ mark_action_done () {
fi
 }
 
-# Put the last action marked done at the beginning of the todo list
-# again. If there has not been an action marked done yet, leave the list of
-# items on the todo list unchanged.
-reschedule_last_action () {
-   tail -n 1 "$done" | cat - "$todo" >"$todo".new
-   sed -e \$d <"$done" >"$done".new
-   mv -f "$todo".new "$todo"
-   mv -f "$done".new "$done"
-}
-
 append_todo_help () {
gettext "
 Commands:
@@ -184,50 +165,6 @@ If you remove a line here THAT COMMIT WILL BE LOST.
fi
 }
 
-make_patch () {
-   sha1_and_parents="$(git rev-list --parents -1 "$1")"
-   case "$sha1_and_parents" in
-   ?*' '?*' '?*)
-   git diff --cc $sha1_and_parents
-   ;;
-   ?*' '?*)
-   git diff-tree -p "$1^!"
-   ;;
-   *)
-   echo "Root commit"
-   ;;
-   esac > "$state_dir"/patch
-   test -f "$msg" ||
-   commit_message "$1" > "$msg"
-   test -f "$author_script" ||
-   get_author_ident_from_commit "$1" > "$author_script"
-}
-
-die_with_patch () {
-   echo "$1" > "$state_dir"/stopped-sha
-   git update-ref REBASE_HEAD "$1"
-   make_patch "$1"
-   die "$2"
-}
-
-exit_with_patch () {
-   echo "$1" > "$state_dir"/stopped-sha
-   git update-ref REBASE_HEAD "$1"
-   make_patch $1
-   git rev-parse --verify HEAD > "$amend"
-   gpg_sign_opt_quoted=${gpg_sign_opt:+$(git rev-parse --sq-quote 
"$gpg_sign_opt")}
-   warn "$(eval_gettext "\
-You can amend the commit now, with
-
-   git commit --amend \$gpg_sign_opt_quoted
-
-Once you are satisfied with your changes, run
-
-   git rebase --continue")"
-   warn
-   exit $2
-}
-
 die_abort () {
apply_autostash
rm -rf "$state_dir"
@@ -238,30 +175,6 @@ has_action () {
test -n "$(git stripspace --strip-comments <"$1")"
 }
 
-is_empty_commit() {
-   tree=$(git rev-parse -q --verify "$1"^{tree} 2>/dev/null) || {
-   sha1=$1
-   die "$(eval_gettext "\$sha1: not a commit that can be picked")"
-   }
-   ptree=$(git rev-parse -q --verify "$1"^^{tree} 2>/dev/null) ||
-   ptree=4b825dc642cb6eb9a060e54bf8d69288fbee4904
-   test "$tree" = "$ptree"
-}
-
-is_merge_commit()
-{
-   git rev-parse --verify --quiet "$1"^2 >/dev/null 2>&1
-}
-
-# Run command with GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and
-# GIT_AUTHOR_DATE exported from the current environment.
-do_with_author () {
-   (
-   export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE
-   "$@"
-   )
-}
-
 git_sequence_editor () {
if test -z "$GIT_SEQUENCE_EDITOR"
then
@@ -275,455 +188,6 @@ git_sequence_editor () {
eval "$GIT_SEQUENCE_EDITOR" '"$@"'
 }
 
-pick_one () {
-   ff=--ff
-
-   case "$1" in -n) sha1=$2; ff= ;; *) sha1=$1 ;; esac
-   case "$force_rebase" in '') ;; ?*) ff= ;; esac
-   output git rev-parse --verify $sha1 || die "$(eval_gettext "Invalid 
commit name: \$sha1")"
-
-   if is_empty_commit "$sha1"
-   then
-   empty_args="--allow-empty"
-   fi
-
-   test -d "$rewritten" &&
-   pick_one_preserving_merges "$@" && return
-   output eval git cherry-pick $allow_rerere_autoupdate 
$allow_empty_message \
-   ${gpg_sign_opt:+$(git rev-parse --sq-quote 
"$gpg_sign_opt")} \
-   $s

[GSoC][PATCH 3/4] rebase: use the new git-rebase--preserve-merges.sh

2018-05-22 Thread Alban Gruin
Creates a new type of rebase, "preserve-merges", used when rebase is called with
-p.

Before that, the type for preserve-merges was "interactive", and some places of
this script compared $type to "interactive". Instead, the code now checks if
$interactive_rebase is empty or not, as it is set to "explicit" when calling an
interactive rebase (and, possibly, one of its submodes), and "implied" when
calling one of its submodes (eg. preserve-merges) *without* interactive rebase.

It also detects the presence of the directory "$merge_dir"/rewritten left by the
preserve-merges script when calling rebase --continue, --skip, etc., and, if it
exists, sets the rebase mode to preserve-merges. In this case,
interactive_rebase is set to "explicit", as "implied" would break some tests.

Signed-off-by: Alban Gruin <alban.gr...@gmail.com>
---
 git-rebase.sh | 32 +---
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 40be59ecc..19bdebb48 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -207,7 +207,14 @@ run_specific_rebase () {
autosquash=
fi
. git-rebase--$type
-   git_rebase__$type${preserve_merges:+__preserve_merges}
+
+   if test -z "$preserve_merges"
+   then
+   git_rebase__$type
+   else
+   git_rebase__preserve_merges
+   fi
+
ret=$?
if test $ret -eq 0
then
@@ -239,7 +246,12 @@ then
state_dir="$apply_dir"
 elif test -d "$merge_dir"
 then
-   if test -f "$merge_dir"/interactive
+   if test -d "$merge_dir"/rewritten
+   then
+   type=preserve-merges
+   interactive_rebase=explicit
+   preserve_merges=t
+   elif test -f "$merge_dir"/interactive
then
type=interactive
interactive_rebase=explicit
@@ -402,14 +414,14 @@ if test -n "$action"
 then
test -z "$in_progress" && die "$(gettext "No rebase in progress?")"
# Only interactive rebase uses detailed reflog messages
-   if test "$type" = interactive && test "$GIT_REFLOG_ACTION" = rebase
+   if test -n "$interactive_rebase" && test "$GIT_REFLOG_ACTION" = rebase
then
GIT_REFLOG_ACTION="rebase -i ($action)"
export GIT_REFLOG_ACTION
fi
 fi
 
-if test "$action" = "edit-todo" && test "$type" != "interactive"
+if test "$action" = "edit-todo" && test -z "$interactive_rebase"
 then
die "$(gettext "The --edit-todo action can only be used during 
interactive rebase.")"
 fi
@@ -487,7 +499,13 @@ fi
 
 if test -n "$interactive_rebase"
 then
-   type=interactive
+   if test -z "$preserve_merges"
+   then
+   type=interactive
+   else
+   type=preserve-merges
+   fi
+
state_dir="$merge_dir"
 elif test -n "$do_merge"
 then
@@ -647,7 +665,7 @@ require_clean_work_tree "rebase" "$(gettext "Please commit 
or stash them.")"
 # but this should be done only when upstream and onto are the same
 # and if this is not an interactive rebase.
 mb=$(git merge-base "$onto" "$orig_head")
-if test "$type" != interactive && test "$upstream" = "$onto" &&
+if test -z "$interactive_rebase" && test "$upstream" = "$onto" &&
test "$mb" = "$onto" && test -z "$restrict_revision" &&
# linear history?
! (git rev-list --parents "$onto".."$orig_head" | sane_grep " .* ") > 
/dev/null
@@ -691,7 +709,7 @@ then
GIT_PAGER='' git diff --stat --summary "$mb" "$onto"
 fi
 
-test "$type" = interactive && run_specific_rebase
+test -n "$interactive_rebase" && run_specific_rebase
 
 # Detach HEAD and reset the tree
 say "$(gettext "First, rewinding head to replay your work on top of it...")"
-- 
2.16.1



[GSoC][PATCH 0/4] rebase: split rebase -p from rebase -i

2018-05-22 Thread Alban Gruin
This splits the `rebase --preserve-merges` functionnality from
git-rebase--interactive.sh. This is part of the effort to depreciate
preserve-merges. The new script, git-rebase--preserve-merges.sh, should be left
to bitrot. All the dead code left by the duplication of
git-rebase--interactive.sh is also removed.

This patch series is based of js/sequencer-and-root-commits.

Alban Gruin (4):
  rebase: duplicate git-rebase--interactive.sh to
git-rebase--preserve-merges.sh
  rebase: strip unused code in git-rebase--preserve-merges.sh
  rebase: use the new git-rebase--preserve-merges.sh
  rebase: remove -p code from git-rebase--interactive.sh

 .gitignore |1 +
 Makefile   |2 +
 git-rebase--interactive.sh |  708 +---
 git-rebase--preserve-merges.sh | 1004 
 git-rebase.sh  |   32 +-
 5 files changed, 1040 insertions(+), 707 deletions(-)
 create mode 100644 git-rebase--preserve-merges.sh

-- 
2.16.1



[GSoC] GSoC with git, week 3

2018-05-20 Thread Alban Gruin
Hi,

I published a new post about this week. You can read it here:

https://blog.pa1ch.fr/posts/2018/05/20/en/gsoc2018-week-3.html

Feel free to give me your feedback! :)

Cheers,
Alban






<    1   2   3   4   >