{"thread":{"id":"41678","subject":"[PATCH/RFC/GSoC 02/17] sha1_name: implement get_oid() and friends","startedAt":"2016-03-12T10:46:20Z","lastAt":"2016-03-21T14:55:09Z","messageCount":59,"participants":["Paul Tan","Duy Nguyen","Christian Couder","Stefan Beller","Junio C Hamano","Johannes Schindelin","Thomas Gummerer"],"isPatch":true,"patchVersion":1,"patchTotal":17},"messages":[{"id":"280664","messageId":"1457779597-6918-1-git-send-email-pyokagan@gmail.com","threadId":"41678","inReplyTo":null,"subject":"[PATCH/RFC/GSoC 00/17] A barebones git-rebase in C","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2016-03-12T10:46:20Z","receivedAt":"2016-03-12T10:46:20Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi all,\n\nLast year I rewrote git-am from shell script to C. This succeeded in speeding\nup a non-interactive git-rebase by 6-7x[1], which is really handly when rebasing\nmultiple topic branches.\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/271967\n\nHowever, it turns out that when working on a topic branch, I frequently use\ninteractive rebase instead to edit and squash commits. Unfortunately, as\ngit-rebase--interactive.sh is still a shell script, it is a bit slower (e.g.\ntaking a few seconds longer compared to non-interactive rebase when rebasing\nbig topic branches).\n\nThe situation is much worse on Windows, as from the invocation of git rebase -i,\nit takes a few seconds before the editor even pops up, and the actual\nrebase proceeds at a snails pace, taking around 3 minutes for a 50-patch\nseries, which is a huge deal-breaker since my workflow depends on frequent\ncommits and squashes.\n\nAs such, this year I would like to apply for GSoC to work on a rewrite of\ngit-rebase to C. It is slightly hefty, as there are three backends (am, merge\nand interactive), along with the git-rebase.sh script.\n\nTo get a gauge of how much code is needed for the rewrite, I explored rewriting\nthe scripts into C, and then extracted some bits out and polished them a bit to\nmake a barebones git-rebase in C, creating this patch series:\n\n[01/17] perf: introduce performance tests for git-rebase\n\nA simple performance test for the three rebase backends so we can compare this\nC version and the shell version below.\n\n[02/17] sha1_name: implement get_oid() and friends\n[03/17] builtin-rebase: implement skeletal builtin rebase\n[04/17] builtin-rebase: parse rebase arguments into a common rebase_options struct\n[05/17] rebase-options: implement rebase_options_load() and rebase_options_save()\n\nThe three rebase backends (am, merge, interactive) have vastly different\ncapabilities, so I did not try to shoehorn them into the same interface.\nHowever, they do share a few common options and functionality, so I introduced\nthe common rebase-common.c library and rebase_options struct.\n\nIn the above patches we implement the essential arguments for a rebase: the\nupstream, branch_name and --onto <newbase>.\n\n[06/17] rebase-am: introduce am backend for builtin rebase\n\nThis patch implements a barebones rebase-am backend.\n\n[07/17] rebase-common: implement refresh_and_write_cache()\n[08/17] rebase-common: let refresh_and_write_cache() take a flags argument\n[09/17] rebase-common: implement cache_has_unstaged_changes()\n[10/17] rebase-common: implement cache_has_uncommitted_changes()\n[11/17] rebase-merge: introduce merge backend for builtin rebase\n\nThese patches implement a barebones rebase-merge backend.\n\n[12/17] rebase-todo: introduce rebase_todo_item\n[13/17] rebase-todo: introduce rebase_todo_list\n[14/17] status: use rebase_todo_list\n[15/17] wrapper: implement append_file()\n[16/17] editor: implement git_sequence_editor() and launch_sequence_editor()\n[17/17] rebase-interactive: introduce interactive backend for builtin rebase\n\nAnd these patches implement a barebones rebase-interactive backend.\n\nWith these patches the performance numbers when rebasing 50 commits on the\ngit.git repository are, on Linux,\n\nBefore patch series:\n\nTest                               this tree\n--------------------------------------------------\n3400.2: rebase --onto master^      1.10(0.84+0.06)\n3402.2: rebase -m --onto master^   2.38(1.38+0.13)\n3404.2: rebase -i --onto master^   3.11(1.37+0.27)\n\nAfter patch series:\n\nTest                               this tree\n--------------------------------------------------\n3400.2: rebase --onto master^      0.74(0.51+0.08)\n3402.2: rebase -m --onto master^   1.72(1.26+0.17)\n3404.2: rebase -i --onto master^   1.74(1.20+0.18)\n\nAnd on Windows,\n\nBefore patch series:\n\nTest                               this tree\n----------------------------------------------------\n3400.2: rebase --onto master^      10.90(0.06+0.47)\n3402.2: rebase -m --onto master^   86.87(0.04+0.47)\n3404.2: rebase -i --onto master^   191.65(0.09+0.44)\n\nAfter patch series:\n\nTest                               this tree\n---------------------------------------------------\n3400.2: rebase --onto master^      6.45(0.13+0.40)\n3402.2: rebase -m --onto master^   12.32(0.13+0.40)\n3404.2: rebase -i --onto master^   14.16(0.15+0.40)\n\n(Thanks to the git-am rewrite, non-interactive rebase on Windows is already\nrelatively fast ;-) )\n\nSo, we have around a 1.4x-1.8x speedup for Linux users, and a 1.7x-13x speedup\nfor Windows users. The annoying long delay before the interactive editor is\nlaunched on Windows is gotten rid of, which I'm very happy about :-)\n\nOn the code side, we do get some nice things with a rewrite to C. For example,\nwe get the rebase-todo library for parsing and writing git-rebase-todo files,\nwhich means that wt-status.c and rebase-interactive.c can share the same\nparsing code. Although not in this patch series, rebase-interactive.c can also\nnow share the same author-script parsing and writing code from builtin/am.c as\nwell.\n\nRegards,\nPaul\n\nPaul Tan (17):\n  perf: introduce performance tests for git-rebase\n  sha1_name: implement get_oid() and friends\n  builtin-rebase: implement skeletal builtin rebase\n  builtin-rebase: parse rebase arguments into a common rebase_options\n    struct\n  rebase-options: implement rebase_options_load() and\n    rebase_options_save()\n  rebase-am: introduce am backend for builtin rebase\n  rebase-common: implement refresh_and_write_cache()\n  rebase-common: let refresh_and_write_cache() take a flags argument\n  rebase-common: implement cache_has_unstaged_changes()\n  rebase-common: implement cache_has_uncommitted_changes()\n  rebase-merge: introduce merge backend for builtin rebase\n  rebase-todo: introduce rebase_todo_item\n  rebase-todo: introduce rebase_todo_list\n  status: use rebase_todo_list\n  wrapper: implement append_file()\n  editor: implement git_sequence_editor() and launch_sequence_editor()\n  rebase-interactive: introduce interactive backend for builtin rebase\n\n Makefile                           |  10 +-\n builtin.h                          |   1 +\n builtin/am.c                       |  16 +-\n builtin/pull.c                     |  41 +---\n builtin/rebase.c                   | 264 ++++++++++++++++++++++++++\n cache.h                            |   8 +\n editor.c                           |  27 ++-\n git.c                              |   1 +\n rebase-am.c                        | 110 +++++++++++\n rebase-am.h                        |  22 +++\n rebase-common.c                    | 220 ++++++++++++++++++++++\n rebase-common.h                    |  48 +++++\n rebase-interactive.c               | 375 +++++++++++++++++++++++++++++++++++++\n rebase-interactive.h               |  33 ++++\n rebase-merge.c                     | 256 +++++++++++++++++++++++++\n rebase-merge.h                     |  28 +++\n rebase-todo.c                      | 251 +++++++++++++++++++++++++\n rebase-todo.h                      |  55 ++++++\n sha1_name.c                        |  30 +++\n strbuf.h                           |   1 +\n t/perf/p3400-rebase.sh             |  25 +++\n t/perf/p3402-rebase-merge.sh       |  25 +++\n t/perf/p3404-rebase-interactive.sh |  26 +++\n wrapper.c                          |  23 +++\n wt-status.c                        | 100 +++-------\n 25 files changed, 1863 insertions(+), 133 deletions(-)\n create mode 100644 builtin/rebase.c\n create mode 100644 rebase-am.c\n create mode 100644 rebase-am.h\n create mode 100644 rebase-common.c\n create mode 100644 rebase-common.h\n create mode 100644 rebase-interactive.c\n create mode 100644 rebase-interactive.h\n create mode 100644 rebase-merge.c\n create mode 100644 rebase-merge.h\n create mode 100644 rebase-todo.c\n create mode 100644 rebase-todo.h\n create mode 100755 t/perf/p3400-rebase.sh\n create mode 100755 t/perf/p3402-rebase-merge.sh\n create mode 100755 t/perf/p3404-rebase-interactive.sh\n\n-- \n2.7.0\n"},{"id":"280662","messageId":"1457779597-6918-2-git-send-email-pyokagan@gmail.com","threadId":"41678","inReplyTo":"1457779597-6918-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH/RFC/GSoC 01/17] perf: introduce performance tests for git-rebase","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2016-03-12T10:46:21Z","receivedAt":"2016-03-12T10:46:21Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"To determine the speedup (or slowdown) of the upcoming git-rebase\nrewrite to C, add a simple performance test for each of the 3 git-rebase\nbackends (am, merge and interactive).\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n t/perf/p3400-rebase.sh             | 25 +++++++++++++++++++++++++\n t/perf/p3402-rebase-merge.sh       | 25 +++++++++++++++++++++++++\n t/perf/p3404-rebase-interactive.sh | 26 ++++++++++++++++++++++++++\n 3 files changed, 76 insertions(+)\n create mode 100755 t/perf/p3400-rebase.sh\n create mode 100755 t/perf/p3402-rebase-merge.sh\n create mode 100755 t/perf/p3404-rebase-interactive.sh\n\ndiff --git a/t/perf/p3400-rebase.sh b/t/perf/p3400-rebase.sh\nnew file mode 100755\nindex 0000000..f172a64\n--- /dev/null\n+++ b/t/perf/p3400-rebase.sh\n@@ -0,0 +1,25 @@\n+#!/bin/sh\n+\n+test_description=\"Tests rebase performance with am backend\"\n+\n+. ./perf-lib.sh\n+\n+test_perf_default_repo\n+test_checkout_worktree\n+\n+# Setup a topic branch with 50 commits\n+test_expect_success 'setup topic branch' '\n+\tgit checkout -b perf-topic-branch master &&\n+\tfor i in $(test_seq 50); do\n+\t\ttest_commit perf-$i file\n+\tdone &&\n+\tgit tag perf-topic-branch-initial\n+'\n+\n+test_perf 'rebase --onto master^' '\n+\tgit checkout perf-topic-branch &&\n+\tgit reset --hard perf-topic-branch-initial &&\n+\tgit rebase --onto master^ master\n+'\n+\n+test_done\ndiff --git a/t/perf/p3402-rebase-merge.sh b/t/perf/p3402-rebase-merge.sh\nnew file mode 100755\nindex 0000000..b71ce12\n--- /dev/null\n+++ b/t/perf/p3402-rebase-merge.sh\n@@ -0,0 +1,25 @@\n+#!/bin/sh\n+\n+test_description=\"Tests rebase performance with merge backend\"\n+\n+. ./perf-lib.sh\n+\n+test_perf_default_repo\n+test_checkout_worktree\n+\n+# Setup a topic branch with 50 commits\n+test_expect_success 'setup topic branch' '\n+\tgit checkout -b perf-topic-branch master &&\n+\tfor i in $(test_seq 50); do\n+\t\ttest_commit perf-$i file\n+\tdone &&\n+\tgit tag perf-topic-branch-initial\n+'\n+\n+test_perf 'rebase -m --onto master^' '\n+\tgit checkout perf-topic-branch &&\n+\tgit reset --hard perf-topic-branch-initial &&\n+\tgit rebase -m --onto master^ master\n+'\n+\n+test_done\ndiff --git a/t/perf/p3404-rebase-interactive.sh b/t/perf/p3404-rebase-interactive.sh\nnew file mode 100755\nindex 0000000..aaca105\n--- /dev/null\n+++ b/t/perf/p3404-rebase-interactive.sh\n@@ -0,0 +1,26 @@\n+#!/bin/sh\n+\n+test_description=\"Tests interactive rebase performance\"\n+\n+. ./perf-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-rebase.sh\n+\n+test_perf_default_repo\n+test_checkout_worktree\n+\n+# Setup a topic branch with 50 commits\n+test_expect_success 'setup topic branch' '\n+\tgit checkout -b perf-topic-branch master &&\n+\tfor i in $(test_seq 50); do\n+\t\ttest_commit perf-$i file\n+\tdone &&\n+\tgit tag perf-topic-branch-initial\n+'\n+\n+test_perf 'rebase -i --onto master^' '\n+\tgit checkout perf-topic-branch &&\n+\tgit reset --hard perf-topic-branch-initial &&\n+\tGIT_SEQUENCE_EDITOR=: git rebase -i --onto master^ master\n+'\n+\n+test_done\n-- \n2.7.0\n"},{"id":"280661","messageId":"1457779597-6918-3-git-send-email-pyokagan@gmail.com","threadId":"41678","inReplyTo":"1457779597-6918-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH/RFC/GSoC 02/17] sha1_name: implement get_oid() and friends","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2016-03-12T10:46:22Z","receivedAt":"2016-03-12T10:46:22Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"5f7817c (define a structure for object IDs, 2015-03-13) introduced the\nobject_id struct to replace the used of unsigned char[] arrays to hold\nobject IDs. This gives us the benefit of compile-time checking for\nmisuse.\n\nTo fully take advantage of compile-time type-checking, introduce the\nget_oid_*() functions which wrap the corresponding get_sha1_*()\nfunctions.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n cache.h     |  6 ++++++\n sha1_name.c | 30 ++++++++++++++++++++++++++++++\n 2 files changed, 36 insertions(+)\n\ndiff --git a/cache.h b/cache.h\nindex b829410..55d443e 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1116,11 +1116,17 @@ struct object_context {\n #define GET_SHA1_ONLY_TO_DIE    04000\n \n extern int get_sha1(const char *str, unsigned char *sha1);\n+extern int get_oid(const char *str, struct object_id *oid);\n extern int get_sha1_commit(const char *str, unsigned char *sha1);\n+extern int get_oid_commit(const char *str, struct object_id *oid);\n extern int get_sha1_committish(const char *str, unsigned char *sha1);\n+extern int get_oid_committish(const char *str, struct object_id *oid);\n extern int get_sha1_tree(const char *str, unsigned char *sha1);\n+extern int get_oid_tree(const char *str, struct object_id *oid);\n extern int get_sha1_treeish(const char *str, unsigned char *sha1);\n+extern int get_oid_treeish(const char *str, struct object_id *oid);\n extern int get_sha1_blob(const char *str, unsigned char *sha1);\n+extern int get_oid_blob(const char *str, struct object_id *oid);\n extern void maybe_die_on_misspelt_object_name(const char *name, const char *prefix);\n extern int get_sha1_with_context(const char *str, unsigned flags, unsigned char *sha1, struct object_context *orc);\n \ndiff --git a/sha1_name.c b/sha1_name.c\nindex 3acf221..307dfad 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -1214,6 +1214,11 @@ int get_sha1(const char *name, unsigned char *sha1)\n \treturn get_sha1_with_context(name, 0, sha1, &unused);\n }\n \n+int get_oid(const char *name, struct object_id *oid)\n+{\n+\treturn get_sha1(name, oid->hash);\n+}\n+\n /*\n  * Many callers know that the user meant to name a commit-ish by\n  * syntactical positions where the object name appears.  Calling this\n@@ -1231,6 +1236,11 @@ int get_sha1_committish(const char *name, unsigned char *sha1)\n \t\t\t\t     sha1, &unused);\n }\n \n+int get_oid_committish(const char *name, struct object_id *oid)\n+{\n+\treturn get_sha1_committish(name, oid->hash);\n+}\n+\n int get_sha1_treeish(const char *name, unsigned char *sha1)\n {\n \tstruct object_context unused;\n@@ -1238,6 +1248,11 @@ int get_sha1_treeish(const char *name, unsigned char *sha1)\n \t\t\t\t     sha1, &unused);\n }\n \n+int get_oid_treeish(const char *name, struct object_id *oid)\n+{\n+\treturn get_sha1_treeish(name, oid->hash);\n+}\n+\n int get_sha1_commit(const char *name, unsigned char *sha1)\n {\n \tstruct object_context unused;\n@@ -1245,6 +1260,11 @@ int get_sha1_commit(const char *name, unsigned char *sha1)\n \t\t\t\t     sha1, &unused);\n }\n \n+int get_oid_commit(const char *name, struct object_id *oid)\n+{\n+\treturn get_sha1_commit(name, oid->hash);\n+}\n+\n int get_sha1_tree(const char *name, unsigned char *sha1)\n {\n \tstruct object_context unused;\n@@ -1252,6 +1272,11 @@ int get_sha1_tree(const char *name, unsigned char *sha1)\n \t\t\t\t     sha1, &unused);\n }\n \n+int get_oid_tree(const char *name, struct object_id *oid)\n+{\n+\treturn get_sha1_tree(name, oid->hash);\n+}\n+\n int get_sha1_blob(const char *name, unsigned char *sha1)\n {\n \tstruct object_context unused;\n@@ -1259,6 +1284,11 @@ int get_sha1_blob(const char *name, unsigned char *sha1)\n \t\t\t\t     sha1, &unused);\n }\n \n+int get_oid_blob(const char *name, struct object_id *oid)\n+{\n+\treturn get_sha1_blob(name, oid->hash);\n+}\n+\n /* Must be called only when object_name:filename doesn't exist. */\n static void diagnose_invalid_sha1_path(const char *prefix,\n \t\t\t\t       const char *filename,\n-- \n2.7.0\n"},{"id":"280666","messageId":"1457779597-6918-4-git-send-email-pyokagan@gmail.com","threadId":"41678","inReplyTo":"1457779597-6918-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH/RFC/GSoC 03/17] builtin-rebase: implement skeletal builtin rebase","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2016-03-12T10:46:23Z","receivedAt":"2016-03-12T10:46:23Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Signed-off-by: Paul Tan <pyokagan@gmail.com>\n---\n Makefile         |  5 +----\n builtin.h        |  1 +\n builtin/rebase.c | 31 +++++++++++++++++++++++++++++++\n git.c            |  1 +\n 4 files changed, 34 insertions(+), 4 deletions(-)\n create mode 100644 builtin/rebase.c\n\ndiff --git a/Makefile b/Makefile\nindex 24bef8d..ad98714 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -496,7 +496,6 @@ SCRIPT_SH += git-merge-one-file.sh\n SCRIPT_SH += git-merge-resolve.sh\n SCRIPT_SH += git-mergetool.sh\n SCRIPT_SH += git-quiltimport.sh\n-SCRIPT_SH += git-rebase.sh\n SCRIPT_SH += git-remote-testgit.sh\n SCRIPT_SH += git-request-pull.sh\n SCRIPT_SH += git-stash.sh\n@@ -505,9 +504,6 @@ SCRIPT_SH += git-web--browse.sh\n \n SCRIPT_LIB += git-mergetool--lib\n SCRIPT_LIB += git-parse-remote\n-SCRIPT_LIB += git-rebase--am\n-SCRIPT_LIB += git-rebase--interactive\n-SCRIPT_LIB += git-rebase--merge\n SCRIPT_LIB += git-sh-setup\n SCRIPT_LIB += git-sh-i18n\n \n@@ -909,6 +905,7 @@ BUILTIN_OBJS += builtin/prune.o\n BUILTIN_OBJS += builtin/pull.o\n BUILTIN_OBJS += builtin/push.o\n BUILTIN_OBJS += builtin/read-tree.o\n+BUILTIN_OBJS += builtin/rebase.o\n BUILTIN_OBJS += builtin/receive-pack.o\n BUILTIN_OBJS += builtin/reflog.o\n BUILTIN_OBJS += builtin/remote.o\ndiff --git a/builtin.h b/builtin.h\nindex 6b95006..a184a58 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -102,6 +102,7 @@ extern int cmd_prune_packed(int argc, const char **argv, const char *prefix);\n extern int cmd_pull(int argc, const char **argv, const char *prefix);\n extern int cmd_push(int argc, const char **argv, const char *prefix);\n extern int cmd_read_tree(int argc, const char **argv, const char *prefix);\n+extern int cmd_rebase(int argc, const char **argv, const char *prefix);\n extern int cmd_receive_pack(int argc, const char **argv, const char *prefix);\n extern int cmd_reflog(int argc, const char **argv, const char *prefix);\n extern int cmd_remote(int argc, const char **argv, const char *prefix);\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nnew file mode 100644\nindex 0000000..04cc1bd\n--- /dev/null\n+++ b/builtin/rebase.c\n@@ -0,0 +1,31 @@\n+/*\n+ * Builtin \"git rebase\"\n+ */\n+#include \"cache.h\"\n+#include \"builtin.h\"\n+#include \"parse-options.h\"\n+\n+static int git_rebase_config(const char *k, const char *v, void *cb)\n+{\n+\treturn git_default_config(k, v, NULL);\n+}\n+\n+int cmd_rebase(int argc, const char **argv, const char *prefix)\n+{\n+\tconst char * const usage[] = {\n+\t\tN_(\"git rebase [options]\"),\n+\t\tNULL\n+\t};\n+\tstruct option options[] = {\n+\t\tOPT_END()\n+\t};\n+\n+\tgit_config(git_rebase_config, NULL);\n+\n+\targc = parse_options(argc, argv, prefix, options, usage, 0);\n+\n+\tif (read_cache_preload(NULL) < 0)\n+\t\tdie(_(\"failed to read the index\"));\n+\n+\treturn 0;\n+}\ndiff --git a/git.c b/git.c\nindex 6cc0c07..f9b7033 100644\n--- a/git.c\n+++ b/git.c\n@@ -452,6 +452,7 @@ static struct cmd_struct commands[] = {\n \t{ \"pull\", cmd_pull, RUN_SETUP | NEED_WORK_TREE },\n \t{ \"push\", cmd_push, RUN_SETUP },\n \t{ \"read-tree\", cmd_read_tree, RUN_SETUP },\n+\t{ \"rebase\", cmd_rebase, RUN_SETUP | NEED_WORK_TREE },\n \t{ \"receive-pack\", cmd_receive_pack },\n \t{ \"reflog\", cmd_reflog, RUN_SETUP },\n \t{ \"remote\", cmd_remote, RUN_SETUP },\n-- \n2.7.0\n"},{"id":"280663","messageId":"1457779597-6918-5-git-send-email-pyokagan@gmail.com","threadId":"41678","inReplyTo":"1457779597-6918-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH/RFC/GSoC 04/17] builtin-rebase: parse rebase arguments into a common rebase_options struct","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2016-03-12T10:46:24Z","receivedAt":"2016-03-12T10:46:24Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"A non-root rebase takes 3 arguments:\n\n* branch_name -- the branch or commit that will be rebased. If it is not\n  specified, the current branch is used.\n\n* upstream -- The upstream commit to compare against. If it is not\n  specified, the configured upstream for the current branch is used.\n\n* onto (or newbase) -- The commit to be used as the starting point to\n  re-apply the commits. If it is not specified, `upstream` is used.\n\nSince these parameters are used by all 3 rebase backends, introduce a\ncommon rebase_options struct to hold all these options. Teach\nbuiltin/rebase.c to handle the above arguments and store them in a\nrebase_options struct. In later patches we will pass the rebase_options\nstruct to the appropriate backend to perform the rebase.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n Makefile         |   1 +\n builtin/rebase.c | 184 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n rebase-common.c  |  28 +++++++++\n rebase-common.h  |  23 +++++++\n 4 files changed, 235 insertions(+), 1 deletion(-)\n create mode 100644 rebase-common.c\n create mode 100644 rebase-common.h\n\ndiff --git a/Makefile b/Makefile\nindex ad98714..b29c672 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -779,6 +779,7 @@ LIB_OBJS += prompt.o\n LIB_OBJS += quote.o\n LIB_OBJS += reachable.o\n LIB_OBJS += read-cache.o\n+LIB_OBJS += rebase-common.o\n LIB_OBJS += reflog-walk.o\n LIB_OBJS += refs.o\n LIB_OBJS += refs/files-backend.o\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 04cc1bd..40176ca 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -4,6 +4,112 @@\n #include \"cache.h\"\n #include \"builtin.h\"\n #include \"parse-options.h\"\n+#include \"rebase-common.h\"\n+#include \"remote.h\"\n+#include \"branch.h\"\n+#include \"refs.h\"\n+\n+/**\n+ * Used by get_curr_branch_upstream_name() as a for_each_remote() callback to\n+ * retrieve the name of the remote if the repository only has one remote.\n+ */\n+static int get_only_remote(struct remote *remote, void *cb_data)\n+{\n+\tconst char **remote_name = cb_data;\n+\n+\tif (*remote_name)\n+\t\treturn -1;\n+\n+\t*remote_name = remote->name;\n+\treturn 0;\n+}\n+\n+const char *get_curr_branch_upstream_name(void)\n+{\n+\tconst char *upstream_name;\n+\tstruct branch *curr_branch;\n+\n+\tcurr_branch = branch_get(\"HEAD\");\n+\tif (!curr_branch) {\n+\t\tfprintf_ln(stderr, _(\"You are not currently on a branch.\"));\n+\t\tfprintf_ln(stderr, _(\"Please specify which branch you want to rebase against.\"));\n+\t\tfprintf_ln(stderr, _(\"See git-rebase(1) for details.\"));\n+\t\tfprintf(stderr, \"\\n\");\n+\t\tfprintf_ln(stderr, \"    git rebase <branch>\");\n+\t\tfprintf(stderr, \"\\n\");\n+\t\texit(1);\n+\t}\n+\n+\tupstream_name = branch_get_upstream(curr_branch, NULL);\n+\tif (!upstream_name) {\n+\t\tconst char *remote_name = NULL;\n+\n+\t\tif (for_each_remote(get_only_remote, &remote_name) || !remote_name)\n+\t\t\tremote_name = \"<remote>\";\n+\n+\t\tfprintf_ln(stderr, _(\"There is no tracking information for the current branch.\"));\n+\t\tfprintf_ln(stderr, _(\"Please specify which branch you want to rebase against.\"));\n+\t\tfprintf_ln(stderr, _(\"See git-rebase(1) for details.\"));\n+\t\tfprintf(stderr, \"\\n\");\n+\t\tfprintf_ln(stderr, \"    git rebase <branch>\");\n+\t\tfprintf(stderr, \"\\n\");\n+\t\tfprintf_ln(stderr, _(\"If you wish to set tracking information for this branch you can do so with:\"));\n+\t\tfprintf(stderr, \"\\n\");\n+\t\tfprintf_ln(stderr, _(\"If you wish to set tracking information for this branch you can do so with:\\n\"\n+\t\t\"\\n\"\n+\t\t\"    git branch --set-upstream-to=%s/<branch> %s\\n\"),\n+\t\tremote_name, curr_branch->name);\n+\t\texit(1);\n+\t}\n+\n+\treturn upstream_name;\n+}\n+\n+/**\n+ * Given the --onto <name>, return the onto hash\n+ */\n+static void get_onto_oid(const char *_onto_name, struct object_id *onto)\n+{\n+\tchar *onto_name = xstrdup(_onto_name);\n+\tstruct commit *onto_commit;\n+\tchar *dotdot;\n+\n+\tdotdot = strstr(onto_name, \"...\");\n+\tif (dotdot) {\n+\t\tconst char *left = onto_name;\n+\t\tconst char *right = dotdot + 3;\n+\t\tstruct commit *left_commit, *right_commit;\n+\t\tstruct commit_list *merge_bases;\n+\n+\t\t*dotdot = 0;\n+\t\tif (!*left)\n+\t\t\tleft = \"HEAD\";\n+\t\tif (!*right)\n+\t\t\tright = \"HEAD\";\n+\n+\t\t/* git merge-base --all $left $right */\n+\t\tleft_commit = lookup_commit_reference_by_name(left);\n+\t\tright_commit = lookup_commit_reference_by_name(right);\n+\t\tif (!left_commit || !right_commit)\n+\t\t\tdie(_(\"%s: there is no merge base\"), _onto_name);\n+\n+\t\tmerge_bases = get_merge_bases(left_commit, right_commit);\n+\t\tif (merge_bases && merge_bases->next)\n+\t\t\tdie(_(\"%s: there are more than one merge bases\"), _onto_name);\n+\t\telse if (!merge_bases)\n+\t\t\tdie(_(\"%s: there is no merge base\"), _onto_name);\n+\n+\t\tonto_commit = merge_bases->item;\n+\t\tfree_commit_list(merge_bases);\n+\t} else {\n+\t\tonto_commit = lookup_commit_reference_by_name(onto_name);\n+\t\tif (!onto_commit)\n+\t\t\tdie(_(\"invalid upstream %s\"), onto_name);\n+\t}\n+\n+\tfree(onto_name);\n+\toidcpy(onto, &onto_commit->object.oid);\n+}\n \n static int git_rebase_config(const char *k, const char *v, void *cb)\n {\n@@ -12,20 +118,96 @@ static int git_rebase_config(const char *k, const char *v, void *cb)\n \n int cmd_rebase(int argc, const char **argv, const char *prefix)\n {\n+\tstruct rebase_options rebase_opts;\n+\tconst char *onto_name = NULL;\n+\tconst char *branch_name;\n+\n \tconst char * const usage[] = {\n-\t\tN_(\"git rebase [options]\"),\n+\t\tN_(\"git rebase [options] [--onto <newbase>] [<upstream>] [<branch>]\"),\n \t\tNULL\n \t};\n \tstruct option options[] = {\n+\t\tOPT_GROUP(N_(\"Available options are\")),\n+\t\tOPT_STRING(0, \"onto\", &onto_name, NULL,\n+\t\t\tN_(\"rebase onto given branch instead of upstream\")),\n \t\tOPT_END()\n \t};\n \n \tgit_config(git_rebase_config, NULL);\n+\trebase_options_init(&rebase_opts);\n+\trebase_opts.resolvemsg = _(\"\\nWhen you have resolved this problem, run \\\"git rebase --continue\\\".\\n\"\n+\t\t\t\"If you prefer to skip this patch, run \\\"git rebase --skip\\\" instead.\\n\"\n+\t\t\t\"To check out the original branch and stop rebasing, run \\\"git rebase --abort\\\".\");\n \n \targc = parse_options(argc, argv, prefix, options, usage, 0);\n \n \tif (read_cache_preload(NULL) < 0)\n \t\tdie(_(\"failed to read the index\"));\n \n+\t/*\n+\t * Parse command-line arguments:\n+\t *    rebase [<options>] [<upstream_name>] [<branch_name>]\n+\t */\n+\n+\t/* Parse <upstream_name> into rebase_opts.upstream */\n+\t{\n+\t\tconst char *upstream_name;\n+\t\tif (argc > 2)\n+\t\t\tusage_with_options(usage, options);\n+\t\tif (!argc) {\n+\t\t\tupstream_name = get_curr_branch_upstream_name();\n+\t\t} else {\n+\t\t\tupstream_name = argv[0];\n+\t\t\targv++, argc--;\n+\t\t\tif (!strcmp(upstream_name, \"-\"))\n+\t\t\t\tupstream_name = \"@{-1}\";\n+\t\t}\n+\t\tif (get_oid_commit(upstream_name, &rebase_opts.upstream))\n+\t\t\tdie(_(\"invalid upstream %s\"), upstream_name);\n+\t\tif (!onto_name)\n+\t\t\tonto_name = upstream_name;\n+\t}\n+\n+\t/*\n+\t * Parse --onto <onto_name> into rebase_opts.onto and\n+\t * rebase_opts.onto_name\n+\t */\n+\tget_onto_oid(onto_name, &rebase_opts.onto);\n+\trebase_opts.onto_name = xstrdup(onto_name);\n+\n+\t/*\n+\t * Parse <branch_name> into rebase_opts.orig_head and\n+\t * rebase_opts.orig_refname\n+\t */\n+\tbranch_name = argv[0];\n+\tif (branch_name) {\n+\t\t/* Is branch_name a branch or commit? */\n+\t\tchar *ref_name = xstrfmt(\"refs/heads/%s\", branch_name);\n+\t\tstruct object_id orig_head_id;\n+\n+\t\tif (!read_ref(ref_name, orig_head_id.hash)) {\n+\t\t\trebase_opts.orig_refname = ref_name;\n+\t\t\tif (get_oid_commit(ref_name, &rebase_opts.orig_head))\n+\t\t\t\tdie(\"get_sha1_commit failed\");\n+\t\t} else if (!get_oid_commit(branch_name, &rebase_opts.orig_head)) {\n+\t\t\trebase_opts.orig_refname = NULL;\n+\t\t\tfree(ref_name);\n+\t\t} else {\n+\t\t\tdie(_(\"no such branch: %s\"), branch_name);\n+\t\t}\n+\t} else {\n+\t\t/* Do not need to switch branches, we are already on it */\n+\t\tstruct branch *curr_branch = branch_get(\"HEAD\");\n+\n+\t\tif (curr_branch)\n+\t\t\trebase_opts.orig_refname = xstrdup(curr_branch->refname);\n+\t\telse\n+\t\t\trebase_opts.orig_refname = NULL;\n+\n+\t\tif (get_oid_commit(\"HEAD\", &rebase_opts.orig_head))\n+\t\t\tdie(_(\"Failed to resolve '%s' as a valid revision.\"), \"HEAD\");\n+\t}\n+\n+\trebase_options_release(&rebase_opts);\n \treturn 0;\n }\ndiff --git a/rebase-common.c b/rebase-common.c\nnew file mode 100644\nindex 0000000..5a49ac4\n--- /dev/null\n+++ b/rebase-common.c\n@@ -0,0 +1,28 @@\n+#include \"cache.h\"\n+#include \"rebase-common.h\"\n+\n+void rebase_options_init(struct rebase_options *opts)\n+{\n+\toidclr(&opts->onto);\n+\topts->onto_name = NULL;\n+\n+\toidclr(&opts->upstream);\n+\n+\toidclr(&opts->orig_head);\n+\topts->orig_refname = NULL;\n+\n+\topts->resolvemsg = NULL;\n+}\n+\n+void rebase_options_release(struct rebase_options *opts)\n+{\n+\tfree(opts->onto_name);\n+\tfree(opts->orig_refname);\n+}\n+\n+void rebase_options_swap(struct rebase_options *dst, struct rebase_options *src)\n+{\n+\tstruct rebase_options tmp = *dst;\n+\t*dst = *src;\n+\t*src = tmp;\n+}\ndiff --git a/rebase-common.h b/rebase-common.h\nnew file mode 100644\nindex 0000000..db5146a\n--- /dev/null\n+++ b/rebase-common.h\n@@ -0,0 +1,23 @@\n+#ifndef REBASE_COMMON_H\n+#define REBASE_COMMON_H\n+\n+/* common rebase backend options */\n+struct rebase_options {\n+\tstruct object_id onto;\n+\tchar *onto_name;\n+\n+\tstruct object_id upstream;\n+\n+\tstruct object_id orig_head;\n+\tchar *orig_refname;\n+\n+\tconst char *resolvemsg;\n+};\n+\n+void rebase_options_init(struct rebase_options *);\n+\n+void rebase_options_release(struct rebase_options *);\n+\n+void rebase_options_swap(struct rebase_options *dst, struct rebase_options *src);\n+\n+#endif /* REBASE_COMMON_H */\n-- \n2.7.0\n"},{"id":"280665","messageId":"1457779597-6918-6-git-send-email-pyokagan@gmail.com","threadId":"41678","inReplyTo":"1457779597-6918-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH/RFC/GSoC 05/17] rebase-options: implement rebase_options_load() and rebase_options_save()","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2016-03-12T10:46:25Z","receivedAt":"2016-03-12T10:46:25Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"These functions can be used for loading and saving common rebase options\ninto a state directory.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n rebase-common.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n rebase-common.h |  4 ++++\n 2 files changed, 73 insertions(+)\n\ndiff --git a/rebase-common.c b/rebase-common.c\nindex 5a49ac4..1835f08 100644\n--- a/rebase-common.c\n+++ b/rebase-common.c\n@@ -26,3 +26,72 @@ void rebase_options_swap(struct rebase_options *dst, struct rebase_options *src)\n \t*dst = *src;\n \t*src = tmp;\n }\n+\n+static int state_file_exists(const char *dir, const char *file)\n+{\n+\treturn file_exists(mkpath(\"%s/%s\", dir, file));\n+}\n+\n+static int read_state_file(struct strbuf *sb, const char *dir, const char *file)\n+{\n+\tconst char *path = mkpath(\"%s/%s\", dir, file);\n+\tstrbuf_reset(sb);\n+\tif (strbuf_read_file(sb, path, 0) >= 0)\n+\t\treturn sb->len;\n+\telse\n+\t\treturn error(_(\"could not read '%s'\"), path);\n+}\n+\n+int rebase_options_load(struct rebase_options *opts, const char *dir)\n+{\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tconst char *filename;\n+\n+\t/* opts->orig_refname */\n+\tif (read_state_file(&sb, dir, \"head-name\") < 0)\n+\t\treturn -1;\n+\tstrbuf_trim(&sb);\n+\tif (starts_with(sb.buf, \"refs/heads/\"))\n+\t\topts->orig_refname = strbuf_detach(&sb, NULL);\n+\telse if (!strcmp(sb.buf, \"detached HEAD\"))\n+\t\topts->orig_refname = NULL;\n+\telse\n+\t\treturn error(_(\"could not parse %s\"), mkpath(\"%s/%s\", dir, \"head-name\"));\n+\n+\t/* opts->onto */\n+\tif (read_state_file(&sb, dir, \"onto\") < 0)\n+\t\treturn -1;\n+\tstrbuf_trim(&sb);\n+\tif (get_oid_hex(sb.buf, &opts->onto) < 0)\n+\t\treturn error(_(\"could not parse %s\"), mkpath(\"%s/%s\", dir, \"onto\"));\n+\n+\t/*\n+\t * We always write to orig-head, but interactive rebase used to write\n+\t * to head. Fall back to reading from head to cover for the case that\n+\t * the user upgraded git with an ongoing interactive rebase.\n+\t */\n+\tfilename = state_file_exists(dir, \"orig-head\") ? \"orig-head\" : \"head\";\n+\tif (read_state_file(&sb, dir, filename) < 0)\n+\t\treturn -1;\n+\tstrbuf_trim(&sb);\n+\tif (get_oid_hex(sb.buf, &opts->orig_head) < 0)\n+\t\treturn error(_(\"could not parse %s\"), mkpath(\"%s/%s\", dir, filename));\n+\n+\tstrbuf_release(&sb);\n+\treturn 0;\n+}\n+\n+static int write_state_text(const char *dir, const char *file, const char *string)\n+{\n+\treturn write_file(mkpath(\"%s/%s\", dir, file), \"%s\", string);\n+}\n+\n+void rebase_options_save(const struct rebase_options *opts, const char *dir)\n+{\n+\tconst char *head_name = opts->orig_refname;\n+\tif (!head_name)\n+\t\thead_name = \"detached HEAD\";\n+\twrite_state_text(dir, \"head-name\", head_name);\n+\twrite_state_text(dir, \"onto\", oid_to_hex(&opts->onto));\n+\twrite_state_text(dir, \"orig-head\", oid_to_hex(&opts->orig_head));\n+}\ndiff --git a/rebase-common.h b/rebase-common.h\nindex db5146a..051c056 100644\n--- a/rebase-common.h\n+++ b/rebase-common.h\n@@ -20,4 +20,8 @@ void rebase_options_release(struct rebase_options *);\n \n void rebase_options_swap(struct rebase_options *dst, struct rebase_options *src);\n \n+int rebase_options_load(struct rebase_options *, const char *dir);\n+\n+void rebase_options_save(const struct rebase_options *, const char *dir);\n+\n #endif /* REBASE_COMMON_H */\n-- \n2.7.0\n"},{"id":"280667","messageId":"1457779597-6918-7-git-send-email-pyokagan@gmail.com","threadId":"41678","inReplyTo":"1457779597-6918-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH/RFC/GSoC 06/17] rebase-am: introduce am backend for builtin rebase","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2016-03-12T10:46:26Z","receivedAt":"2016-03-12T10:46:26Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Since 7f59dbb (Rewrite rebase to use git-format-patch piped to git-am.,\n2005-11-14), git-rebase will by default use \"git am\" to rebase commits.\nThis is done by first checking out to the new base commit, generating a\nseries of patches with the commits to replay, and then applying them\nwith git-am. Finally, if orig_head is a branch, it is updated to point\nto the tip of the new rebased commit history.\n\nImplement a skeletal version of this method of rebasing commits by\nintroducing a new rebase-am backend for our builtin-rebase. This\nskeletal version can only call git-format-patch and git-am to perform a\nrebase, and is unable to resume from a failed rebase. Subsequent\npatches will re-implement all the missing features.\n\nThe symmetric difference between upstream...orig_head is used because in\na later patch, we will add an additional exclusion revision in order to\nhandle fork points correctly.  See b6266dc (rebase--am: use\n--cherry-pick instead of --ignore-if-in-upstream, 2014-07-15).\n\nThe initial steps of checking out the new base commit, and the final\ncleanup steps of updating refs are common between the am backend and\nmerge backend. As such, we implement the common setup and teardown\nsequence in the shared functions rebase_common_setup() and\nrebase_common_finish(), so we can share code with the merge backend when\nit is implemented in a later patch.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n Makefile         |   1 +\n builtin/rebase.c |  25 +++++++++++++\n rebase-am.c      | 110 +++++++++++++++++++++++++++++++++++++++++++++++++++++++\n rebase-am.h      |  22 +++++++++++\n rebase-common.c  |  81 ++++++++++++++++++++++++++++++++++++++++\n rebase-common.h  |   6 +++\n 6 files changed, 245 insertions(+)\n create mode 100644 rebase-am.c\n create mode 100644 rebase-am.h\n\ndiff --git a/Makefile b/Makefile\nindex b29c672..a2618ea 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -779,6 +779,7 @@ LIB_OBJS += prompt.o\n LIB_OBJS += quote.o\n LIB_OBJS += reachable.o\n LIB_OBJS += read-cache.o\n+LIB_OBJS += rebase-am.o\n LIB_OBJS += rebase-common.o\n LIB_OBJS += reflog-walk.o\n LIB_OBJS += refs.o\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 40176ca..ec63d3b 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -8,6 +8,22 @@\n #include \"remote.h\"\n #include \"branch.h\"\n #include \"refs.h\"\n+#include \"rebase-am.h\"\n+\n+enum rebase_type {\n+\tREBASE_TYPE_NONE = 0,\n+\tREBASE_TYPE_AM\n+};\n+\n+static const char *rebase_dir(enum rebase_type type)\n+{\n+\tswitch (type) {\n+\tcase REBASE_TYPE_AM:\n+\t\treturn git_path_rebase_am_dir();\n+\tdefault:\n+\t\tdie(\"BUG: invalid rebase_type %d\", type);\n+\t}\n+}\n \n /**\n  * Used by get_curr_branch_upstream_name() as a for_each_remote() callback to\n@@ -208,6 +224,15 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t\t\tdie(_(\"Failed to resolve '%s' as a valid revision.\"), \"HEAD\");\n \t}\n \n+\t/* Run the appropriate rebase backend */\n+\t{\n+\t\tstruct rebase_am state;\n+\t\trebase_am_init(&state, rebase_dir(REBASE_TYPE_AM));\n+\t\trebase_options_swap(&state.opts, &rebase_opts);\n+\t\trebase_am_run(&state);\n+\t\trebase_am_release(&state);\n+\t}\n+\n \trebase_options_release(&rebase_opts);\n \treturn 0;\n }\ndiff --git a/rebase-am.c b/rebase-am.c\nnew file mode 100644\nindex 0000000..53e8798\n--- /dev/null\n+++ b/rebase-am.c\n@@ -0,0 +1,110 @@\n+#include \"cache.h\"\n+#include \"rebase-am.h\"\n+#include \"run-command.h\"\n+\n+GIT_PATH_FUNC(git_path_rebase_am_dir, \"rebase-apply\");\n+\n+void rebase_am_init(struct rebase_am *state, const char *dir)\n+{\n+\tif (!dir)\n+\t\tdir = git_path_rebase_am_dir();\n+\trebase_options_init(&state->opts);\n+\tstate->dir = xstrdup(dir);\n+}\n+\n+void rebase_am_release(struct rebase_am *state)\n+{\n+\trebase_options_release(&state->opts);\n+\tfree(state->dir);\n+}\n+\n+int rebase_am_in_progress(const struct rebase_am *state)\n+{\n+\tconst char *dir = state ? state->dir : git_path_rebase_am_dir();\n+\tstruct stat st;\n+\n+\treturn !lstat(dir, &st) && S_ISDIR(st.st_mode);\n+}\n+\n+int rebase_am_load(struct rebase_am *state)\n+{\n+\tif (rebase_options_load(&state->opts, state->dir) < 0)\n+\t\treturn -1;\n+\n+\treturn 0;\n+}\n+\n+static int run_format_patch(const char *patches, const struct object_id *left,\n+\t\tconst struct object_id *right)\n+{\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tint ret;\n+\n+\tcp.git_cmd = 1;\n+\tcp.out = xopen(patches, O_WRONLY | O_CREAT, 0777);\n+\targv_array_push(&cp.args, \"format-patch\");\n+\targv_array_push(&cp.args, \"-k\");\n+\targv_array_push(&cp.args, \"--stdout\");\n+\targv_array_push(&cp.args, \"--full-index\");\n+\targv_array_push(&cp.args, \"--cherry-pick\");\n+\targv_array_push(&cp.args, \"--right-only\");\n+\targv_array_push(&cp.args, \"--src-prefix=a/\");\n+\targv_array_push(&cp.args, \"--dst-prefix=b/\");\n+\targv_array_push(&cp.args, \"--no-renames\");\n+\targv_array_push(&cp.args, \"--no-cover-letter\");\n+\targv_array_pushf(&cp.args, \"%s...%s\", oid_to_hex(left), oid_to_hex(right));\n+\n+\tret = run_command(&cp);\n+\tclose(cp.out);\n+\treturn ret;\n+}\n+\n+static int run_am(const struct rebase_am *state, const char *patches)\n+{\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tint ret;\n+\n+\tcp.git_cmd = 1;\n+\tcp.in = xopen(patches, O_RDONLY);\n+\targv_array_push(&cp.args, \"am\");\n+\targv_array_push(&cp.args, \"--rebasing\");\n+\tif (state->opts.resolvemsg)\n+\t\targv_array_pushf(&cp.args, \"--resolvemsg=%s\", state->opts.resolvemsg);\n+\n+\tret = run_command(&cp);\n+\tclose(cp.in);\n+\treturn ret;\n+}\n+\n+void rebase_am_run(struct rebase_am *state)\n+{\n+\tchar *patches;\n+\tint ret;\n+\n+\trebase_common_setup(&state->opts, state->dir);\n+\n+\tpatches = git_pathdup(\"rebased-patches\");\n+\tret = run_format_patch(patches, &state->opts.upstream, &state->opts.orig_head);\n+\tif (ret) {\n+\t\tunlink_or_warn(patches);\n+\t\tfprintf_ln(stderr, _(\"\\ngit encountered an error while preparing the patches to replay\\n\"\n+\t\t\t\"these revisions:\\n\"\n+\t\t\t\"\\n\"\n+\t\t\t\"    %s...%s\\n\"\n+\t\t\t\"\\n\"\n+\t\t\t\"As a result, git cannot rebase them.\"),\n+\t\t\t\toid_to_hex(&state->opts.upstream),\n+\t\t\t\toid_to_hex(&state->opts.orig_head));\n+\t\texit(ret);\n+\t}\n+\n+\tret = run_am(state, patches);\n+\tunlink_or_warn(patches);\n+\tif (ret) {\n+\t\trebase_options_save(&state->opts, state->dir);\n+\t\texit(ret);\n+\t}\n+\n+\tfree(patches);\n+\trebase_common_finish(&state->opts, state->dir);\n+}\ndiff --git a/rebase-am.h b/rebase-am.h\nnew file mode 100644\nindex 0000000..0b4348c\n--- /dev/null\n+++ b/rebase-am.h\n@@ -0,0 +1,22 @@\n+#ifndef REBASE_AM_H\n+#define REBASE_AM_H\n+#include \"rebase-common.h\"\n+\n+const char *git_path_rebase_am_dir(void);\n+\n+struct rebase_am {\n+\tstruct rebase_options opts;\n+\tchar *dir;\n+};\n+\n+void rebase_am_init(struct rebase_am *, const char *dir);\n+\n+void rebase_am_release(struct rebase_am *);\n+\n+int rebase_am_in_progress(const struct rebase_am *);\n+\n+int rebase_am_load(struct rebase_am *);\n+\n+void rebase_am_run(struct rebase_am *);\n+\n+#endif /* REBASE_AM_H */\ndiff --git a/rebase-common.c b/rebase-common.c\nindex 1835f08..8169fb6 100644\n--- a/rebase-common.c\n+++ b/rebase-common.c\n@@ -1,5 +1,8 @@\n #include \"cache.h\"\n #include \"rebase-common.h\"\n+#include \"dir.h\"\n+#include \"run-command.h\"\n+#include \"refs.h\"\n \n void rebase_options_init(struct rebase_options *opts)\n {\n@@ -95,3 +98,81 @@ void rebase_options_save(const struct rebase_options *opts, const char *dir)\n \twrite_state_text(dir, \"onto\", oid_to_hex(&opts->onto));\n \twrite_state_text(dir, \"orig-head\", oid_to_hex(&opts->orig_head));\n }\n+\n+static int detach_head(const struct object_id *commit, const char *onto_name)\n+{\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tint status;\n+\tconst char *reflog_action = getenv(\"GIT_REFLOG_ACTION\");\n+\tif (!reflog_action || !*reflog_action)\n+\t\treflog_action = \"rebase\";\n+\tcp.git_cmd = 1;\n+\targv_array_pushf(&cp.env_array, \"GIT_REFLOG_ACTION=%s: checkout %s\",\n+\t\t\treflog_action, onto_name ? onto_name : oid_to_hex(commit));\n+\targv_array_push(&cp.args, \"checkout\");\n+\targv_array_push(&cp.args, \"-q\");\n+\targv_array_push(&cp.args, \"--detach\");\n+\targv_array_push(&cp.args, oid_to_hex(commit));\n+\tstatus = run_command(&cp);\n+\n+\t/* reload cache as checkout will have modified it */\n+\tdiscard_cache();\n+\tread_cache();\n+\n+\treturn status;\n+}\n+\n+void rebase_common_setup(struct rebase_options *opts, const char *dir)\n+{\n+\t/* Detach HEAD and reset the tree */\n+\tprintf_ln(_(\"First, rewinding head to replay your work on top of it...\"));\n+\tif (detach_head(&opts->onto, opts->onto_name))\n+\t\tdie(_(\"could not detach HEAD\"));\n+\tupdate_ref(\"rebase\", \"ORIG_HEAD\", opts->orig_head.hash, NULL, 0,\n+\t\t\tUPDATE_REFS_DIE_ON_ERR);\n+}\n+\n+void rebase_common_destroy(struct rebase_options *opts, const char *dir)\n+{\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tstrbuf_addstr(&sb, dir);\n+\tremove_dir_recursively(&sb, 0);\n+\tstrbuf_release(&sb);\n+}\n+\n+static void move_to_original_branch(struct rebase_options *opts)\n+{\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tstruct object_id curr_head;\n+\n+\tif (!opts->orig_refname || !starts_with(opts->orig_refname, \"refs/\"))\n+\t\treturn;\n+\n+\tif (get_sha1(\"HEAD\", curr_head.hash) < 0)\n+\t\tdie(\"get_sha1() failed\");\n+\n+\tstrbuf_addf(&sb, \"rebase finished: %s onto %s\", opts->orig_refname, oid_to_hex(&opts->onto));\n+\tif (update_ref(sb.buf, opts->orig_refname, curr_head.hash, opts->orig_head.hash, 0, UPDATE_REFS_MSG_ON_ERR))\n+\t\tgoto fail;\n+\n+\tstrbuf_reset(&sb);\n+\tstrbuf_addf(&sb, \"rebase finished: returning to %s\", opts->orig_refname);\n+\tif (create_symref(\"HEAD\", opts->orig_refname, sb.buf))\n+\t\tgoto fail;\n+\n+\tstrbuf_release(&sb);\n+\n+\treturn;\n+fail:\n+\tdie(_(\"Could not move back to %s\"), opts->orig_refname);\n+}\n+\n+void rebase_common_finish(struct rebase_options *opts, const char *dir)\n+{\n+\tconst char *argv_gc_auto[] = {\"gc\", \"--auto\", NULL};\n+\n+\tmove_to_original_branch(opts);\n+\tclose_all_packs();\n+\trun_command_v_opt(argv_gc_auto, RUN_GIT_CMD);\n+\trebase_common_destroy(opts, dir);\n+}\ndiff --git a/rebase-common.h b/rebase-common.h\nindex 051c056..067ad0b 100644\n--- a/rebase-common.h\n+++ b/rebase-common.h\n@@ -24,4 +24,10 @@ int rebase_options_load(struct rebase_options *, const char *dir);\n \n void rebase_options_save(const struct rebase_options *, const char *dir);\n \n+void rebase_common_setup(struct rebase_options *, const char *dir);\n+\n+void rebase_common_destroy(struct rebase_options *, const char *dir);\n+\n+void rebase_common_finish(struct rebase_options *, const char *dir);\n+\n #endif /* REBASE_COMMON_H */\n-- \n2.7.0\n"},{"id":"280669","messageId":"1457779597-6918-8-git-send-email-pyokagan@gmail.com","threadId":"41678","inReplyTo":"1457779597-6918-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH/RFC/GSoC 07/17] rebase-common: implement refresh_and_write_cache()","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2016-03-12T10:46:27Z","receivedAt":"2016-03-12T10:46:27Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"In the upcoming git-rebase to C rewrite, it is a common operation to\nrefresh the index and write the resulting index.\n\nbuiltin/am.c already implements refresh_and_write_cache(), which is what\nwe want. Move it to rebase-common.c, so that it can be shared with all\nthe rebase backends, including git-am.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n builtin/am.c    | 14 +-------------\n rebase-common.c | 11 +++++++++++\n rebase-common.h |  5 +++++\n 3 files changed, 17 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/am.c b/builtin/am.c\nindex d003939..504b604 100644\n--- a/builtin/am.c\n+++ b/builtin/am.c\n@@ -28,6 +28,7 @@\n #include \"rerere.h\"\n #include \"prompt.h\"\n #include \"mailinfo.h\"\n+#include \"rebase-common.h\"\n \n /**\n  * Returns 1 if the file is empty or does not exist, 0 otherwise.\n@@ -1125,19 +1126,6 @@ static const char *msgnum(const struct am_state *state)\n }\n \n /**\n- * Refresh and write index.\n- */\n-static void refresh_and_write_cache(void)\n-{\n-\tstruct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));\n-\n-\thold_locked_index(lock_file, 1);\n-\trefresh_cache(REFRESH_QUIET);\n-\tif (write_locked_index(&the_index, lock_file, COMMIT_LOCK))\n-\t\tdie(_(\"unable to write index file\"));\n-}\n-\n-/**\n  * Returns 1 if the index differs from HEAD, 0 otherwise. When on an unborn\n  * branch, returns 1 if there are entries in the index, 0 otherwise. If an\n  * strbuf is provided, the space-separated list of files that differ will be\ndiff --git a/rebase-common.c b/rebase-common.c\nindex 8169fb6..b07e1f1 100644\n--- a/rebase-common.c\n+++ b/rebase-common.c\n@@ -3,6 +3,17 @@\n #include \"dir.h\"\n #include \"run-command.h\"\n #include \"refs.h\"\n+#include \"lockfile.h\"\n+\n+void refresh_and_write_cache(void)\n+{\n+\tstruct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));\n+\n+\thold_locked_index(lock_file, 1);\n+\trefresh_cache(REFRESH_QUIET);\n+\tif (write_locked_index(&the_index, lock_file, COMMIT_LOCK))\n+\t\tdie(_(\"unable to write index file\"));\n+}\n \n void rebase_options_init(struct rebase_options *opts)\n {\ndiff --git a/rebase-common.h b/rebase-common.h\nindex 067ad0b..8620e8c 100644\n--- a/rebase-common.h\n+++ b/rebase-common.h\n@@ -1,6 +1,11 @@\n #ifndef REBASE_COMMON_H\n #define REBASE_COMMON_H\n \n+/**\n+ * Refresh and write index.\n+ */\n+void refresh_and_write_cache(void);\n+\n /* common rebase backend options */\n struct rebase_options {\n \tstruct object_id onto;\n-- \n2.7.0\n"},{"id":"280671","messageId":"1457779597-6918-9-git-send-email-pyokagan@gmail.com","threadId":"41678","inReplyTo":"1457779597-6918-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH/RFC/GSoC 08/17] rebase-common: let refresh_and_write_cache() take a flags argument","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2016-03-12T10:46:28Z","receivedAt":"2016-03-12T10:46:28Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"refresh_and_write_cache() is a handy function for refreshing the index\nand writing the resulting index back to the filesystem. However, it\nalways calls refresh_cache() with REFRESH_QUIET. Allow callers to modify\nthe behavior of refresh_cache() by allowing callers to pass a flags\nargument to refresh_cache().\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n builtin/am.c    | 2 +-\n rebase-common.c | 4 ++--\n rebase-common.h | 2 +-\n 3 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/am.c b/builtin/am.c\nindex 504b604..5185719 100644\n--- a/builtin/am.c\n+++ b/builtin/am.c\n@@ -1815,7 +1815,7 @@ static void am_run(struct am_state *state, int resume)\n \n \tunlink(am_path(state, \"dirtyindex\"));\n \n-\trefresh_and_write_cache();\n+\trefresh_and_write_cache(REFRESH_QUIET);\n \n \tif (index_has_changes(&sb)) {\n \t\twrite_state_bool(state, \"dirtyindex\", 1);\ndiff --git a/rebase-common.c b/rebase-common.c\nindex b07e1f1..97b0687 100644\n--- a/rebase-common.c\n+++ b/rebase-common.c\n@@ -5,12 +5,12 @@\n #include \"refs.h\"\n #include \"lockfile.h\"\n \n-void refresh_and_write_cache(void)\n+void refresh_and_write_cache(unsigned int flags)\n {\n \tstruct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));\n \n \thold_locked_index(lock_file, 1);\n-\trefresh_cache(REFRESH_QUIET);\n+\trefresh_cache(flags);\n \tif (write_locked_index(&the_index, lock_file, COMMIT_LOCK))\n \t\tdie(_(\"unable to write index file\"));\n }\ndiff --git a/rebase-common.h b/rebase-common.h\nindex 8620e8c..4586f03 100644\n--- a/rebase-common.h\n+++ b/rebase-common.h\n@@ -4,7 +4,7 @@\n /**\n  * Refresh and write index.\n  */\n-void refresh_and_write_cache(void);\n+void refresh_and_write_cache(unsigned int);\n \n /* common rebase backend options */\n struct rebase_options {\n-- \n2.7.0\n"},{"id":"280670","messageId":"1457779597-6918-10-git-send-email-pyokagan@gmail.com","threadId":"41678","inReplyTo":"1457779597-6918-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH/RFC/GSoC 09/17] rebase-common: implement cache_has_unstaged_changes()","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2016-03-12T10:46:29Z","receivedAt":"2016-03-12T10:46:29Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"In the upcoming git-rebase-to-C rewrite, it is a common operation to\ncheck if the worktree has unstaged changes, so that it can complain that\nthe worktree is dirty.\n\nbuiltin/pull.c already implements this function. Move it to\nrebase-common.c so that it can be shared between all rebase backends and\ngit-pull.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n builtin/pull.c  | 19 ++-----------------\n rebase-common.c | 14 ++++++++++++++\n rebase-common.h |  5 +++++\n 3 files changed, 21 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 10eff03..9e65dc9 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -17,6 +17,7 @@\n #include \"revision.h\"\n #include \"tempfile.h\"\n #include \"lockfile.h\"\n+#include \"rebase-common.h\"\n \n enum rebase_type {\n \tREBASE_INVALID = -1,\n@@ -306,22 +307,6 @@ static enum rebase_type config_get_rebase(void)\n }\n \n /**\n- * Returns 1 if there are unstaged changes, 0 otherwise.\n- */\n-static int has_unstaged_changes(const char *prefix)\n-{\n-\tstruct rev_info rev_info;\n-\tint result;\n-\n-\tinit_revisions(&rev_info, prefix);\n-\tDIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES);\n-\tDIFF_OPT_SET(&rev_info.diffopt, QUICK);\n-\tdiff_setup_done(&rev_info.diffopt);\n-\tresult = run_diff_files(&rev_info, 0);\n-\treturn diff_result_code(&rev_info.diffopt, result);\n-}\n-\n-/**\n  * Returns 1 if there are uncommitted changes, 0 otherwise.\n  */\n static int has_uncommitted_changes(const char *prefix)\n@@ -355,7 +340,7 @@ static void die_on_unclean_work_tree(const char *prefix)\n \tupdate_index_if_able(&the_index, lock_file);\n \trollback_lock_file(lock_file);\n \n-\tif (has_unstaged_changes(prefix)) {\n+\tif (cache_has_unstaged_changes()) {\n \t\terror(_(\"Cannot pull with rebase: You have unstaged changes.\"));\n \t\tdo_die = 1;\n \t}\ndiff --git a/rebase-common.c b/rebase-common.c\nindex 97b0687..61be8f1 100644\n--- a/rebase-common.c\n+++ b/rebase-common.c\n@@ -4,6 +4,7 @@\n #include \"run-command.h\"\n #include \"refs.h\"\n #include \"lockfile.h\"\n+#include \"revision.h\"\n \n void refresh_and_write_cache(unsigned int flags)\n {\n@@ -15,6 +16,19 @@ void refresh_and_write_cache(unsigned int flags)\n \t\tdie(_(\"unable to write index file\"));\n }\n \n+int cache_has_unstaged_changes(void)\n+{\n+\tstruct rev_info rev_info;\n+\tint result;\n+\n+\tinit_revisions(&rev_info, NULL);\n+\tDIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES);\n+\tDIFF_OPT_SET(&rev_info.diffopt, QUICK);\n+\tdiff_setup_done(&rev_info.diffopt);\n+\tresult = run_diff_files(&rev_info, 0);\n+\treturn diff_result_code(&rev_info.diffopt, result);\n+}\n+\n void rebase_options_init(struct rebase_options *opts)\n {\n \toidclr(&opts->onto);\ndiff --git a/rebase-common.h b/rebase-common.h\nindex 4586f03..9d14e25 100644\n--- a/rebase-common.h\n+++ b/rebase-common.h\n@@ -6,6 +6,11 @@\n  */\n void refresh_and_write_cache(unsigned int);\n \n+/**\n+ * Returns 1 if there are unstaged changes, 0 otherwise.\n+ */\n+int cache_has_unstaged_changes(void);\n+\n /* common rebase backend options */\n struct rebase_options {\n \tstruct object_id onto;\n-- \n2.7.0\n"},{"id":"280673","messageId":"1457779597-6918-11-git-send-email-pyokagan@gmail.com","threadId":"41678","inReplyTo":"1457779597-6918-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH/RFC/GSoC 10/17] rebase-common: implement cache_has_uncommitted_changes()","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2016-03-12T10:46:30Z","receivedAt":"2016-03-12T10:46:30Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"In the upcoming git-rebase-to-C rewrite, it is a common opertation to\ncheck if the index has uncommitted changes, so that rebase can complain\nthat the index is dirty, or commit the uncommitted changes in the index.\n\nbuiltin/pull.c already implements the function we want. Move it to\nrebase-common.c so that it can be shared between all rebase backends and\ngit-pull.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n builtin/pull.c  | 22 +---------------------\n rebase-common.c | 17 +++++++++++++++++\n rebase-common.h |  5 +++++\n 3 files changed, 23 insertions(+), 21 deletions(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 9e65dc9..6be4213 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -307,26 +307,6 @@ static enum rebase_type config_get_rebase(void)\n }\n \n /**\n- * Returns 1 if there are uncommitted changes, 0 otherwise.\n- */\n-static int has_uncommitted_changes(const char *prefix)\n-{\n-\tstruct rev_info rev_info;\n-\tint result;\n-\n-\tif (is_cache_unborn())\n-\t\treturn 0;\n-\n-\tinit_revisions(&rev_info, prefix);\n-\tDIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES);\n-\tDIFF_OPT_SET(&rev_info.diffopt, QUICK);\n-\tadd_head_to_pending(&rev_info);\n-\tdiff_setup_done(&rev_info.diffopt);\n-\tresult = run_diff_index(&rev_info, 1);\n-\treturn diff_result_code(&rev_info.diffopt, result);\n-}\n-\n-/**\n  * If the work tree has unstaged or uncommitted changes, dies with the\n  * appropriate message.\n  */\n@@ -345,7 +325,7 @@ static void die_on_unclean_work_tree(const char *prefix)\n \t\tdo_die = 1;\n \t}\n \n-\tif (has_uncommitted_changes(prefix)) {\n+\tif (cache_has_uncommitted_changes()) {\n \t\tif (do_die)\n \t\t\terror(_(\"Additionally, your index contains uncommitted changes.\"));\n \t\telse\ndiff --git a/rebase-common.c b/rebase-common.c\nindex 61be8f1..94783a9 100644\n--- a/rebase-common.c\n+++ b/rebase-common.c\n@@ -29,6 +29,23 @@ int cache_has_unstaged_changes(void)\n \treturn diff_result_code(&rev_info.diffopt, result);\n }\n \n+int cache_has_uncommitted_changes(void)\n+{\n+\tstruct rev_info rev_info;\n+\tint result;\n+\n+\tif (is_cache_unborn())\n+\t\treturn 0;\n+\n+\tinit_revisions(&rev_info, NULL);\n+\tDIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES);\n+\tDIFF_OPT_SET(&rev_info.diffopt, QUICK);\n+\tadd_head_to_pending(&rev_info);\n+\tdiff_setup_done(&rev_info.diffopt);\n+\tresult = run_diff_index(&rev_info, 1);\n+\treturn diff_result_code(&rev_info.diffopt, result);\n+}\n+\n void rebase_options_init(struct rebase_options *opts)\n {\n \toidclr(&opts->onto);\ndiff --git a/rebase-common.h b/rebase-common.h\nindex 9d14e25..97d9a5b 100644\n--- a/rebase-common.h\n+++ b/rebase-common.h\n@@ -11,6 +11,11 @@ void refresh_and_write_cache(unsigned int);\n  */\n int cache_has_unstaged_changes(void);\n \n+/**\n+ * Returns 1 if there are uncommitted changes, 0 otherwise.\n+ */\n+int cache_has_uncommitted_changes(void);\n+\n /* common rebase backend options */\n struct rebase_options {\n \tstruct object_id onto;\n-- \n2.7.0\n"},{"id":"280668","messageId":"1457779597-6918-12-git-send-email-pyokagan@gmail.com","threadId":"41678","inReplyTo":"1457779597-6918-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH/RFC/GSoC 11/17] rebase-merge: introduce merge backend for builtin rebase","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2016-03-12T10:46:31Z","receivedAt":"2016-03-12T10:46:31Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Since 58634db (rebase: Allow merge strategies to be used when rebasing,\n2006-06-21), git-rebase supported rebasing with a merge strategy when\nthe -m switch is used.\n\nRe-implement a skeletal version of the above method of rebasing in a new\nrebase-merge backend for our builtin-rebase. This skeletal version is\nonly able to re-apply commits using the merge-recursive strategy, and is\nunable to resume from a conflict. Subsequent patches will re-implement\nall the missing features.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n Makefile         |   1 +\n builtin/rebase.c |  17 +++-\n rebase-merge.c   | 256 +++++++++++++++++++++++++++++++++++++++++++++++++++++++\n rebase-merge.h   |  28 ++++++\n 4 files changed, 300 insertions(+), 2 deletions(-)\n create mode 100644 rebase-merge.c\n create mode 100644 rebase-merge.h\n\ndiff --git a/Makefile b/Makefile\nindex a2618ea..d43e068 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -781,6 +781,7 @@ LIB_OBJS += reachable.o\n LIB_OBJS += read-cache.o\n LIB_OBJS += rebase-am.o\n LIB_OBJS += rebase-common.o\n+LIB_OBJS += rebase-merge.o\n LIB_OBJS += reflog-walk.o\n LIB_OBJS += refs.o\n LIB_OBJS += refs/files-backend.o\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex ec63d3b..6d42115 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -9,10 +9,12 @@\n #include \"branch.h\"\n #include \"refs.h\"\n #include \"rebase-am.h\"\n+#include \"rebase-merge.h\"\n \n enum rebase_type {\n \tREBASE_TYPE_NONE = 0,\n-\tREBASE_TYPE_AM\n+\tREBASE_TYPE_AM,\n+\tREBASE_TYPE_MERGE\n };\n \n static const char *rebase_dir(enum rebase_type type)\n@@ -20,6 +22,8 @@ static const char *rebase_dir(enum rebase_type type)\n \tswitch (type) {\n \tcase REBASE_TYPE_AM:\n \t\treturn git_path_rebase_am_dir();\n+\tcase REBASE_TYPE_MERGE:\n+\t\treturn git_path_rebase_merge_dir();\n \tdefault:\n \t\tdie(\"BUG: invalid rebase_type %d\", type);\n \t}\n@@ -137,6 +141,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \tstruct rebase_options rebase_opts;\n \tconst char *onto_name = NULL;\n \tconst char *branch_name;\n+\tint do_merge = 0;\n \n \tconst char * const usage[] = {\n \t\tN_(\"git rebase [options] [--onto <newbase>] [<upstream>] [<branch>]\"),\n@@ -146,6 +151,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t\tOPT_GROUP(N_(\"Available options are\")),\n \t\tOPT_STRING(0, \"onto\", &onto_name, NULL,\n \t\t\tN_(\"rebase onto given branch instead of upstream\")),\n+\t\tOPT_BOOL('m', \"merge\", &do_merge,\n+\t\t\tN_(\"use merging strategies to rebase\")),\n \t\tOPT_END()\n \t};\n \n@@ -225,7 +232,13 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t}\n \n \t/* Run the appropriate rebase backend */\n-\t{\n+\tif (do_merge) {\n+\t\tstruct rebase_merge state;\n+\t\trebase_merge_init(&state, rebase_dir(REBASE_TYPE_MERGE));\n+\t\trebase_options_swap(&state.opts, &rebase_opts);\n+\t\trebase_merge_run(&state);\n+\t\trebase_merge_release(&state);\n+\t} else {\n \t\tstruct rebase_am state;\n \t\trebase_am_init(&state, rebase_dir(REBASE_TYPE_AM));\n \t\trebase_options_swap(&state.opts, &rebase_opts);\ndiff --git a/rebase-merge.c b/rebase-merge.c\nnew file mode 100644\nindex 0000000..dc96faf\n--- /dev/null\n+++ b/rebase-merge.c\n@@ -0,0 +1,256 @@\n+#include \"cache.h\"\n+#include \"rebase-merge.h\"\n+#include \"run-command.h\"\n+#include \"dir.h\"\n+#include \"revision.h\"\n+\n+GIT_PATH_FUNC(git_path_rebase_merge_dir, \"rebase-merge\");\n+\n+void rebase_merge_init(struct rebase_merge *state, const char *dir)\n+{\n+\tif (!dir)\n+\t\tdir = git_path_rebase_merge_dir();\n+\trebase_options_init(&state->opts);\n+\tstate->dir = xstrdup(dir);\n+\tstate->msgnum = 0;\n+\tstate->end = 0;\n+\tstate->prec = 4;\n+}\n+\n+void rebase_merge_release(struct rebase_merge *state)\n+{\n+\trebase_options_release(&state->opts);\n+\tfree(state->dir);\n+}\n+\n+int rebase_merge_in_progress(const struct rebase_merge *state)\n+{\n+\tconst char *dir = state ? state->dir : git_path_rebase_merge_dir();\n+\tstruct stat st;\n+\n+\tif (lstat(dir, &st) || !S_ISDIR(st.st_mode))\n+\t\treturn 0;\n+\n+\tif (file_exists(mkpath(\"%s/interactive\", dir)))\n+\t\treturn 0;\n+\n+\treturn 1;\n+}\n+\n+static const char *state_path(const struct rebase_merge *state, const char *filename)\n+{\n+\treturn mkpath(\"%s/%s\", state->dir, filename);\n+}\n+\n+static int read_state_file(const struct rebase_merge *state, const char *filename, struct strbuf *sb)\n+{\n+\tconst char *path = state_path(state, filename);\n+\tif (strbuf_read_file(sb, path, 0) < 0)\n+\t\treturn error(_(\"could not read file %s\"), path);\n+\tstrbuf_trim(sb);\n+\treturn 0;\n+}\n+\n+static int read_state_ui(const struct rebase_merge *state, const char *filename, unsigned int *ui)\n+{\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tif (read_state_file(state, filename, &sb) < 0) {\n+\t\tstrbuf_release(&sb);\n+\t\treturn -1;\n+\t}\n+\tif (strtoul_ui(sb.buf, 10, ui) < 0) {\n+\t\tstrbuf_release(&sb);\n+\t\treturn error(_(\"could not parse %s\"), state_path(state, filename));\n+\t}\n+\tstrbuf_release(&sb);\n+\treturn 0;\n+}\n+\n+static int read_state_oid(const struct rebase_merge *state, const char *filename, struct object_id *oid)\n+{\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tif (read_state_file(state, filename, &sb) < 0) {\n+\t\tstrbuf_release(&sb);\n+\t\treturn -1;\n+\t}\n+\tif (sb.len != GIT_SHA1_HEXSZ || get_oid_hex(sb.buf, oid)) {\n+\t\tstrbuf_release(&sb);\n+\t\treturn error(_(\"could not parse %s\"), state_path(state, filename));\n+\t}\n+\tstrbuf_release(&sb);\n+\treturn 0;\n+}\n+\n+static int read_state_msgnum(const struct rebase_merge *state, unsigned int msgnum, struct object_id *oid)\n+{\n+\tchar *filename = xstrfmt(\"cmt.%u\", msgnum);\n+\tint ret = read_state_oid(state, filename, oid);\n+\tfree(filename);\n+\treturn ret;\n+}\n+\n+int rebase_merge_load(struct rebase_merge *state)\n+{\n+\tstruct strbuf sb = STRBUF_INIT;\n+\n+\tif (rebase_options_load(&state->opts, state->dir) < 0)\n+\t\treturn -1;\n+\n+\tif (read_state_file(state, \"onto_name\", &sb) < 0)\n+\t\treturn -1;\n+\tfree(state->opts.onto_name);\n+\tstate->opts.onto_name = strbuf_detach(&sb, NULL);\n+\n+\tif (read_state_ui(state, \"msgnum\", &state->msgnum) < 0)\n+\t\treturn -1;\n+\n+\tif (read_state_ui(state, \"end\", &state->end) < 0)\n+\t\treturn -1;\n+\n+\tstrbuf_release(&sb);\n+\treturn 0;\n+}\n+\n+static void write_state_text(const struct rebase_merge *state, const char *filename, const char *text)\n+{\n+\twrite_file(state_path(state, filename), \"%s\", text);\n+}\n+\n+static void write_state_ui(const struct rebase_merge *state, const char *filename, unsigned int ui)\n+{\n+\twrite_file(state_path(state, filename), \"%u\", ui);\n+}\n+\n+static void write_state_oid(const struct rebase_merge *state, const char *filename, const struct object_id *oid)\n+{\n+\twrite_file(state_path(state, filename), \"%s\", oid_to_hex(oid));\n+}\n+\n+static void rebase_merge_finish(struct rebase_merge *state)\n+{\n+\trebase_common_finish(&state->opts, state->dir);\n+\tprintf_ln(_(\"All done.\"));\n+}\n+\n+/**\n+ * Setup commits to be rebased.\n+ */\n+static unsigned int setup_commits(const char *dir, const struct object_id *upstream, const struct object_id *head)\n+{\n+\tstruct rev_info revs;\n+\tstruct argv_array args = ARGV_ARRAY_INIT;\n+\tstruct commit *commit;\n+\tunsigned int msgnum = 0;\n+\n+\tinit_revisions(&revs, NULL);\n+\targv_array_pushl(&args, \"rev-list\", \"--reverse\", \"--no-merges\", NULL);\n+\targv_array_pushf(&args, \"%s..%s\", oid_to_hex(upstream), oid_to_hex(head));\n+\tsetup_revisions(args.argc, args.argv, &revs, NULL);\n+\tif (prepare_revision_walk(&revs))\n+\t\tdie(\"revision walk setup failed\");\n+\twhile ((commit = get_revision(&revs)))\n+\t\twrite_file(mkpath(\"%s/cmt.%u\", dir, ++msgnum), \"%s\", oid_to_hex(&commit->object.oid));\n+\treset_revision_walk();\n+\targv_array_clear(&args);\n+\treturn msgnum;\n+}\n+\n+/**\n+ * Merge HEAD with oid\n+ */\n+static void do_merge(struct rebase_merge *state, unsigned int msgnum, const struct object_id *oid)\n+{\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tint ret;\n+\n+\tcp.git_cmd = 1;\n+\targv_array_pushf(&cp.env_array, \"GITHEAD_%s=HEAD~%u\", oid_to_hex(oid), state->end - msgnum);\n+\targv_array_pushf(&cp.env_array, \"GITHEAD_HEAD=%s\", state->opts.onto_name ? state->opts.onto_name : oid_to_hex(&state->opts.onto));\n+\targv_array_push(&cp.args, \"merge-recursive\");\n+\targv_array_pushf(&cp.args, \"%s^\", oid_to_hex(oid));\n+\targv_array_push(&cp.args, \"--\");\n+\targv_array_push(&cp.args, \"HEAD\");\n+\targv_array_push(&cp.args, oid_to_hex(oid));\n+\tret = run_command(&cp);\n+\tswitch (ret) {\n+\tcase 0:\n+\t\tbreak;\n+\tcase 1:\n+\t\tif (state->opts.resolvemsg)\n+\t\t\tfprintf_ln(stderr, \"%s\", state->opts.resolvemsg);\n+\t\texit(1);\n+\tcase 2:\n+\t\tfprintf_ln(stderr, _(\"Strategy: recursive failed, try another\"));\n+\t\tif (state->opts.resolvemsg)\n+\t\t\tfprintf_ln(stderr, \"%s\", state->opts.resolvemsg);\n+\t\texit(1);\n+\tdefault:\n+\t\tfprintf_ln(stderr, _(\"Unknown exit code (%d) from command\"), ret);\n+\t\texit(1);\n+\t}\n+\n+\tdiscard_cache();\n+\tread_cache();\n+}\n+\n+/**\n+ * Commit index\n+ */\n+static void do_commit(struct rebase_merge *state, unsigned int msgnum, const struct object_id *oid)\n+{\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\n+\tif (!cache_has_uncommitted_changes()) {\n+\t\tprintf_ln(_(\"Already applied: %0*d\"), state->prec, msgnum);\n+\t\treturn;\n+\t}\n+\n+\tcp.git_cmd = 1;\n+\targv_array_push(&cp.args, \"commit\");\n+\targv_array_push(&cp.args, \"--no-verify\");\n+\targv_array_pushl(&cp.args, \"-C\", oid_to_hex(oid), NULL);\n+\tif (run_command(&cp)) {\n+\n+\t\tfprintf_ln(stderr, _(\"Commit failed, please do not call \\\"git commit\\\"\\n\"\n+\t\t\t\t\t\"directly, but instead do one of the following:\"));\n+\t\tif (state->opts.resolvemsg)\n+\t\t\tfprintf_ln(stderr, \"%s\", state->opts.resolvemsg);\n+\t\texit(1);\n+\t}\n+\tprintf_ln(_(\"Committed: %0*d\"), state->prec, msgnum);\n+}\n+\n+static void do_rest(struct rebase_merge *state)\n+{\n+\twhile (state->msgnum <= state->end) {\n+\t\tstruct object_id oid;\n+\n+\t\tif (read_state_msgnum(state, state->msgnum, &oid) < 0)\n+\t\t\tdie(\"could not read msgnum commit\");\n+\t\twrite_state_oid(state, \"current\", &oid);\n+\t\tdo_merge(state, state->msgnum, &oid);\n+\t\tdo_commit(state, state->msgnum, &oid);\n+\t\twrite_state_ui(state, \"msgnum\", ++state->msgnum);\n+\t}\n+\n+\trebase_merge_finish(state);\n+}\n+\n+void rebase_merge_run(struct rebase_merge *state)\n+{\n+\trebase_common_setup(&state->opts, state->dir);\n+\n+\tif (mkdir(state->dir, 0777) < 0 && errno != EEXIST)\n+\t\tdie_errno(_(\"failed to create directory '%s'\"), state->dir);\n+\n+\trebase_options_save(&state->opts, state->dir);\n+\twrite_state_text(state, \"onto_name\", state->opts.onto_name ? state->opts.onto_name : oid_to_hex(&state->opts.onto));\n+\n+\tstate->msgnum = 1;\n+\twrite_state_ui(state, \"msgnum\", state->msgnum);\n+\n+\tstate->end = setup_commits(state->dir, &state->opts.upstream, &state->opts.orig_head);\n+\twrite_state_ui(state, \"end\", state->end);\n+\n+\tdo_rest(state);\n+}\ndiff --git a/rebase-merge.h b/rebase-merge.h\nnew file mode 100644\nindex 0000000..f0b54ef\n--- /dev/null\n+++ b/rebase-merge.h\n@@ -0,0 +1,28 @@\n+#ifndef REBASE_MERGE_H\n+#define REBASE_MERGE_H\n+#include \"rebase-common.h\"\n+\n+const char *git_path_rebase_merge_dir(void);\n+\n+/*\n+ * The rebase_merge backend is a merge-based non-interactive mode that copes\n+ * well with renamed files.\n+ */\n+struct rebase_merge {\n+\tstruct rebase_options opts;\n+\tchar *dir;\n+\tunsigned int msgnum, end;\n+\tint prec;\n+};\n+\n+void rebase_merge_init(struct rebase_merge *, const char *dir);\n+\n+void rebase_merge_release(struct rebase_merge *);\n+\n+int rebase_merge_in_progress(const struct rebase_merge *);\n+\n+int rebase_merge_load(struct rebase_merge *);\n+\n+void rebase_merge_run(struct rebase_merge *);\n+\n+#endif /* REBASE_MERGE_H */\n-- \n2.7.0\n"},{"id":"280672","messageId":"1457779597-6918-13-git-send-email-pyokagan@gmail.com","threadId":"41678","inReplyTo":"1457779597-6918-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH/RFC/GSoC 12/17] rebase-todo: introduce rebase_todo_item","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2016-03-12T10:46:32Z","receivedAt":"2016-03-12T10:46:32Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"In an interactive rebase, commands are read and executed from a todo\nlist (.git/rebase-merge/git-rebase-todo) to perform the rebase.\n\nIn the upcoming re-implementation of git-rebase -i in C, it is useful to\nbe able to parse each command into a data structure which can then be\noperated on. Implement rebase_todo_item for this.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n Makefile      |   1 +\n rebase-todo.c | 144 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n rebase-todo.h |  28 ++++++++++++\n 3 files changed, 173 insertions(+)\n create mode 100644 rebase-todo.c\n create mode 100644 rebase-todo.h\n\ndiff --git a/Makefile b/Makefile\nindex d43e068..8b928e4 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -782,6 +782,7 @@ LIB_OBJS += read-cache.o\n LIB_OBJS += rebase-am.o\n LIB_OBJS += rebase-common.o\n LIB_OBJS += rebase-merge.o\n+LIB_OBJS += rebase-todo.o\n LIB_OBJS += reflog-walk.o\n LIB_OBJS += refs.o\n LIB_OBJS += refs/files-backend.o\ndiff --git a/rebase-todo.c b/rebase-todo.c\nnew file mode 100644\nindex 0000000..ac6b222\n--- /dev/null\n+++ b/rebase-todo.c\n@@ -0,0 +1,144 @@\n+#include \"cache.h\"\n+#include \"rebase-todo.h\"\n+\n+/*\n+ * Used as the default `rest` value, so that users can always assume `rest` is\n+ * non NULL and `rest` is NUL terminated even for a freshly initialized\n+ * rebase_todo_item.\n+ */\n+static char rebase_todo_item_slopbuf[1];\n+\n+void rebase_todo_item_init(struct rebase_todo_item *item)\n+{\n+\titem->action = REBASE_TODO_NONE;\n+\toidclr(&item->oid);\n+\titem->rest = rebase_todo_item_slopbuf;\n+}\n+\n+void rebase_todo_item_release(struct rebase_todo_item *item)\n+{\n+\tif (item->rest != rebase_todo_item_slopbuf)\n+\t\tfree(item->rest);\n+\trebase_todo_item_init(item);\n+}\n+\n+void rebase_todo_item_copy(struct rebase_todo_item *dst, const struct rebase_todo_item *src)\n+{\n+\tif (dst->rest != rebase_todo_item_slopbuf)\n+\t\tfree(dst->rest);\n+\t*dst = *src;\n+\tdst->rest = xstrdup(src->rest);\n+}\n+\n+static const char *next_word(struct strbuf *sb, const char *str)\n+{\n+\tconst char *end;\n+\n+\twhile (*str && isspace(*str))\n+\t\tstr++;\n+\n+\tend = str;\n+\twhile (*end && !isspace(*end))\n+\t\tend++;\n+\n+\tstrbuf_reset(sb);\n+\tstrbuf_add(sb, str, end - str);\n+\treturn end;\n+}\n+\n+int rebase_todo_item_parse(struct rebase_todo_item *item, const char *line, int abbrev)\n+{\n+\tstruct strbuf word = STRBUF_INIT;\n+\tconst char *str = line;\n+\tint has_oid = 1, ret = 0;\n+\n+\twhile (*str && isspace(*str))\n+\t\tstr++;\n+\n+\tif (!*str || *str == comment_line_char) {\n+\t\titem->action = REBASE_TODO_NONE;\n+\t\toidclr(&item->oid);\n+\t\tif (item->rest != rebase_todo_item_slopbuf)\n+\t\t\tfree(item->rest);\n+\t\titem->rest = *str ? xstrdup(str) : rebase_todo_item_slopbuf;\n+\t\treturn 0;\n+\t}\n+\n+\tstr = next_word(&word, str);\n+\tif (!strcmp(word.buf, \"noop\")) {\n+\t\titem->action = REBASE_TODO_NOOP;\n+\t\thas_oid = 0;\n+\t} else if (!strcmp(word.buf, \"pick\") || !strcmp(word.buf, \"p\")) {\n+\t\titem->action = REBASE_TODO_PICK;\n+\t} else {\n+\t\tret = error(_(\"Unknown command: %s\"), word.buf);\n+\t\tgoto finish;\n+\t}\n+\n+\tif (has_oid) {\n+\t\tstr = next_word(&word, str);\n+\t\tif (abbrev) {\n+\t\t\t/* accept abbreviated object ids */\n+\t\t\tif (get_oid_commit(word.buf, &item->oid)) {\n+\t\t\t\tret = error(_(\"Not a commit: %s\"), word.buf);\n+\t\t\t\tgoto finish;\n+\t\t\t}\n+\t\t} else {\n+\t\t\tif (word.len != GIT_SHA1_HEXSZ || get_oid_hex(word.buf, &item->oid)) {\n+\t\t\t\tret = error(_(\"Invalid line: %s\"), line);\n+\t\t\t\tgoto finish;\n+\t\t\t}\n+\t\t}\n+\t} else {\n+\t\toidclr(&item->oid);\n+\t}\n+\n+\tif (*str && isspace(*str))\n+\t\tstr++;\n+\tif (*str) {\n+\t\tif (item->rest != rebase_todo_item_slopbuf)\n+\t\t\tfree(item->rest);\n+\t\titem->rest = xstrdup(str);\n+\t}\n+\n+finish:\n+\tstrbuf_release(&word);\n+\treturn ret;\n+}\n+\n+void strbuf_add_rebase_todo_item(struct strbuf *sb,\n+\t\t\t\t const struct rebase_todo_item *item, int abbrev)\n+{\n+\tint has_oid = 1;\n+\n+\tswitch (item->action) {\n+\tcase REBASE_TODO_NONE:\n+\t\thas_oid = 0;\n+\t\tbreak;\n+\tcase REBASE_TODO_NOOP:\n+\t\tstrbuf_addstr(sb, \"noop\");\n+\t\thas_oid = 0;\n+\t\tbreak;\n+\tcase REBASE_TODO_PICK:\n+\t\tstrbuf_addstr(sb, \"pick\");\n+\t\tbreak;\n+\tdefault:\n+\t\tdie(\"BUG: invalid rebase_todo_item action %d\", item->action);\n+\t}\n+\n+\tif (has_oid) {\n+\t\tstrbuf_addch(sb, ' ');\n+\t\tif (abbrev)\n+\t\t\tstrbuf_addstr(sb, find_unique_abbrev((unsigned char *)&item->oid.hash, DEFAULT_ABBREV));\n+\t\telse\n+\t\t\tstrbuf_addstr(sb, oid_to_hex(&item->oid));\n+\t}\n+\n+\tif (*item->rest) {\n+\t\tif (item->action != REBASE_TODO_NONE)\n+\t\t\tstrbuf_addch(sb, ' ');\n+\t\tstrbuf_addstr(sb, item->rest);\n+\t}\n+\n+\tstrbuf_addch(sb, '\\n');\n+}\ndiff --git a/rebase-todo.h b/rebase-todo.h\nnew file mode 100644\nindex 0000000..2eedbb0\n--- /dev/null\n+++ b/rebase-todo.h\n@@ -0,0 +1,28 @@\n+#ifndef REBASE_TODO_H\n+#define REBASE_TODO_H\n+\n+struct strbuf;\n+\n+enum rebase_todo_action {\n+\tREBASE_TODO_NONE = 0,\n+\tREBASE_TODO_NOOP,\n+\tREBASE_TODO_PICK\n+};\n+\n+struct rebase_todo_item {\n+\tenum rebase_todo_action action;\n+\tstruct object_id oid;\n+\tchar *rest;\n+};\n+\n+void rebase_todo_item_init(struct rebase_todo_item *);\n+\n+void rebase_todo_item_release(struct rebase_todo_item *);\n+\n+void rebase_todo_item_copy(struct rebase_todo_item *, const struct rebase_todo_item *);\n+\n+int rebase_todo_item_parse(struct rebase_todo_item *, const char *line, int abbrev);\n+\n+void strbuf_add_rebase_todo_item(struct strbuf *, const struct rebase_todo_item *, int abbrev);\n+\n+#endif /* REBASE_TODO_H */\n-- \n2.7.0\n"},{"id":"280676","messageId":"1457779597-6918-14-git-send-email-pyokagan@gmail.com","threadId":"41678","inReplyTo":"1457779597-6918-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH/RFC/GSoC 13/17] rebase-todo: introduce rebase_todo_list","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2016-03-12T10:46:33Z","receivedAt":"2016-03-12T10:46:33Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Implement rebase_todo_list, which is a resizable array of\nrebase_todo_items.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n rebase-todo.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n rebase-todo.h |  27 +++++++++++++++\n 2 files changed, 134 insertions(+)\n\ndiff --git a/rebase-todo.c b/rebase-todo.c\nindex ac6b222..4f14638 100644\n--- a/rebase-todo.c\n+++ b/rebase-todo.c\n@@ -142,3 +142,110 @@ void strbuf_add_rebase_todo_item(struct strbuf *sb,\n \n \tstrbuf_addch(sb, '\\n');\n }\n+\n+void rebase_todo_list_init(struct rebase_todo_list *list)\n+{\n+\tlist->items = NULL;\n+\tlist->nr = 0;\n+\tlist->alloc = 0;\n+}\n+\n+void rebase_todo_list_clear(struct rebase_todo_list *list)\n+{\n+\tunsigned int i;\n+\n+\tfor (i = 0; i < list->nr; i++)\n+\t\trebase_todo_item_release(&list->items[i]);\n+\tfree(list->items);\n+\trebase_todo_list_init(list);\n+}\n+\n+void rebase_todo_list_swap(struct rebase_todo_list *dst,\n+\t\t\t   struct rebase_todo_list *src)\n+{\n+\tstruct rebase_todo_list tmp = *dst;\n+\n+\t*dst = *src;\n+\t*src = tmp;\n+}\n+\n+unsigned int rebase_todo_list_count(const struct rebase_todo_list *list)\n+{\n+\tunsigned int i, count = 0;\n+\n+\tfor (i = 0; i < list->nr; i++)\n+\t\tif (list->items[i].action != REBASE_TODO_NONE)\n+\t\t\tcount++;\n+\treturn count;\n+}\n+\n+struct rebase_todo_item *rebase_todo_list_push(struct rebase_todo_list *list, const struct rebase_todo_item *src_item)\n+{\n+\tstruct rebase_todo_item *item = rebase_todo_list_push_empty(list);\n+\n+\trebase_todo_item_copy(item, src_item);\n+\treturn item;\n+}\n+\n+struct rebase_todo_item *rebase_todo_list_push_empty(struct rebase_todo_list *list)\n+{\n+\tstruct rebase_todo_item *item;\n+\n+\tALLOC_GROW(list->items, list->nr + 1, list->alloc);\n+\titem = &list->items[list->nr++];\n+\trebase_todo_item_init(item);\n+\treturn item;\n+}\n+\n+struct rebase_todo_item *rebase_todo_list_push_noop(struct rebase_todo_list *list)\n+{\n+\tstruct rebase_todo_item *item = rebase_todo_list_push_empty(list);\n+\n+\titem->action = REBASE_TODO_NOOP;\n+\treturn item;\n+}\n+\n+int rebase_todo_list_load(struct rebase_todo_list *list, const char *path, int abbrev)\n+{\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tFILE *fp;\n+\n+\tfp = fopen(path, \"r\");\n+\tif (!fp)\n+\t\treturn error(_(\"could not open %s for reading\"), path);\n+\n+\twhile (strbuf_getline(&sb, fp) != EOF) {\n+\t\tstruct rebase_todo_item *item = rebase_todo_list_push_empty(list);\n+\t\tif (rebase_todo_item_parse(item, sb.buf, abbrev) < 0) {\n+\t\t\trebase_todo_item_release(item);\n+\t\t\tlist->nr--;\n+\t\t\tstrbuf_release(&sb);\n+\t\t\tfclose(fp);\n+\t\t\treturn -1;\n+\t\t}\n+\t}\n+\tstrbuf_release(&sb);\n+\tfclose(fp);\n+\treturn 0;\n+}\n+\n+void rebase_todo_list_save(const struct rebase_todo_list *list, const char *filename, unsigned int offset, int abbrev)\n+{\n+\tchar *tmpfile = mkpathdup(\"%s.new\", filename);\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tint fd;\n+\n+\tfor (; offset < list->nr; offset++)\n+\t\tstrbuf_add_rebase_todo_item(&sb, &list->items[offset], abbrev);\n+\n+\tfd = xopen(tmpfile, O_WRONLY | O_CREAT | O_TRUNC, 0666);\n+\tif (write_in_full(fd, sb.buf, sb.len) != sb.len)\n+\t\tdie_errno(_(\"could not write to %s\"), tmpfile);\n+\tclose(fd);\n+\tstrbuf_release(&sb);\n+\n+\tif (rename(tmpfile, filename))\n+\t\tdie_errno(_(\"rename failed\"));\n+\n+\tfree(tmpfile);\n+}\ndiff --git a/rebase-todo.h b/rebase-todo.h\nindex 2eedbb0..f602fd2 100644\n--- a/rebase-todo.h\n+++ b/rebase-todo.h\n@@ -25,4 +25,31 @@ int rebase_todo_item_parse(struct rebase_todo_item *, const char *line, int abbr\n \n void strbuf_add_rebase_todo_item(struct strbuf *, const struct rebase_todo_item *, int abbrev);\n \n+struct rebase_todo_list {\n+\tstruct rebase_todo_item *items;\n+\tunsigned int nr, alloc;\n+};\n+\n+#define REBASE_TODO_LIST_INIT { NULL, 0, 0 }\n+\n+void rebase_todo_list_init(struct rebase_todo_list *);\n+\n+void rebase_todo_list_clear(struct rebase_todo_list *);\n+\n+void rebase_todo_list_swap(struct rebase_todo_list *dst, struct rebase_todo_list *src);\n+\n+unsigned int rebase_todo_list_count(const struct rebase_todo_list *);\n+\n+struct rebase_todo_item *rebase_todo_list_push(struct rebase_todo_list *,\n+\t\t\t\t\t       const struct rebase_todo_item *);\n+\n+struct rebase_todo_item *rebase_todo_list_push_empty(struct rebase_todo_list *);\n+\n+struct rebase_todo_item *rebase_todo_list_push_noop(struct rebase_todo_list *);\n+\n+int rebase_todo_list_load(struct rebase_todo_list *, const char *path, int abbrev);\n+\n+void rebase_todo_list_save(const struct rebase_todo_list *, const char *path,\n+\t\t\t   unsigned int offset, int abbrev);\n+\n #endif /* REBASE_TODO_H */\n-- \n2.7.0\n"},{"id":"280677","messageId":"1457779597-6918-15-git-send-email-pyokagan@gmail.com","threadId":"41678","inReplyTo":"1457779597-6918-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH/RFC/GSoC 14/17] status: use rebase_todo_list","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2016-03-12T10:46:34Z","receivedAt":"2016-03-12T10:46:34Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Since 84e6fb9 (status: give more information during rebase -i,\n2015-07-06), git status during an interactive rebase will show the list\nof commands that are done and yet to be done. It implemented its own\nhand-rolled parser in order to achieve this.\n\nNow that we are able to fully parse interactive rebase's todo lists with\nrebase_todo_list_parse(), use it in wt-status.c to reduce the amount of\ncode needed to implement this feature.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n\nThis patch is just an illustration, and is not quite right as it does not strip\ncomments and blank lines like the original did.\n\n wt-status.c | 100 +++++++++++++++---------------------------------------------\n 1 file changed, 25 insertions(+), 75 deletions(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex ab4f80d..96b82ef 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -15,6 +15,7 @@\n #include \"column.h\"\n #include \"strbuf.h\"\n #include \"utf8.h\"\n+#include \"rebase-todo.h\"\n \n static const char cut_line[] =\n \"------------------------ >8 ------------------------\\n\";\n@@ -1026,94 +1027,39 @@ static int split_commit_in_progress(struct wt_status *s)\n \treturn split_in_progress;\n }\n \n-/*\n- * Turn\n- * \"pick d6a2f0303e897ec257dd0e0a39a5ccb709bc2047 some message\"\n- * into\n- * \"pick d6a2f03 some message\"\n- *\n- * The function assumes that the line does not contain useless spaces\n- * before or after the command.\n- */\n-static void abbrev_sha1_in_line(struct strbuf *line)\n-{\n-\tstruct strbuf **split;\n-\tint i;\n-\n-\tif (starts_with(line->buf, \"exec \") ||\n-\t    starts_with(line->buf, \"x \"))\n-\t\treturn;\n-\n-\tsplit = strbuf_split_max(line, ' ', 3);\n-\tif (split[0] && split[1]) {\n-\t\tunsigned char sha1[20];\n-\t\tconst char *abbrev;\n-\n-\t\t/*\n-\t\t * strbuf_split_max left a space. Trim it and re-add\n-\t\t * it after abbreviation.\n-\t\t */\n-\t\tstrbuf_trim(split[1]);\n-\t\tif (!get_sha1(split[1]->buf, sha1)) {\n-\t\t\tabbrev = find_unique_abbrev(sha1, DEFAULT_ABBREV);\n-\t\t\tstrbuf_reset(split[1]);\n-\t\t\tstrbuf_addf(split[1], \"%s \", abbrev);\n-\t\t\tstrbuf_reset(line);\n-\t\t\tfor (i = 0; split[i]; i++)\n-\t\t\t\tstrbuf_addf(line, \"%s\", split[i]->buf);\n-\t\t}\n-\t}\n-\tfor (i = 0; split[i]; i++)\n-\t\tstrbuf_release(split[i]);\n-\n-}\n-\n-static void read_rebase_todolist(const char *fname, struct string_list *lines)\n-{\n-\tstruct strbuf line = STRBUF_INIT;\n-\tFILE *f = fopen(git_path(\"%s\", fname), \"r\");\n-\n-\tif (!f)\n-\t\tdie_errno(\"Could not open file %s for reading\",\n-\t\t\t  git_path(\"%s\", fname));\n-\twhile (!strbuf_getline_lf(&line, f)) {\n-\t\tif (line.len && line.buf[0] == comment_line_char)\n-\t\t\tcontinue;\n-\t\tstrbuf_trim(&line);\n-\t\tif (!line.len)\n-\t\t\tcontinue;\n-\t\tabbrev_sha1_in_line(&line);\n-\t\tstring_list_append(lines, line.buf);\n-\t}\n-}\n-\n static void show_rebase_information(struct wt_status *s,\n \t\t\t\t\tstruct wt_status_state *state,\n \t\t\t\t\tconst char *color)\n {\n \tif (state->rebase_interactive_in_progress) {\n-\t\tint i;\n-\t\tint nr_lines_to_show = 2;\n+\t\tunsigned int i;\n+\t\tunsigned int nr_lines_to_show = 2;\n+\t\tstruct strbuf sb = STRBUF_INIT;\n \n-\t\tstruct string_list have_done = STRING_LIST_INIT_DUP;\n-\t\tstruct string_list yet_to_do = STRING_LIST_INIT_DUP;\n+\t\tstruct rebase_todo_list have_done = REBASE_TODO_LIST_INIT;\n+\t\tstruct rebase_todo_list yet_to_do = REBASE_TODO_LIST_INIT;\n \n-\t\tread_rebase_todolist(\"rebase-merge/done\", &have_done);\n-\t\tread_rebase_todolist(\"rebase-merge/git-rebase-todo\", &yet_to_do);\n+\t\tif (rebase_todo_list_load(&have_done, git_path(\"rebase-merge/done\"), 1) < 0)\n+\t\t\treturn;\n+\t\tif (rebase_todo_list_load(&yet_to_do, git_path(\"rebase-merge/git-rebase-todo\"), 1) < 0)\n+\t\t\treturn;\n \n \t\tif (have_done.nr == 0)\n \t\t\tstatus_printf_ln(s, color, _(\"No commands done.\"));\n \t\telse {\n \t\t\tstatus_printf_ln(s, color,\n-\t\t\t\tQ_(\"Last command done (%d command done):\",\n-\t\t\t\t\t\"Last commands done (%d commands done):\",\n+\t\t\t\tQ_(\"Last command done (%u command done):\",\n+\t\t\t\t\t\"Last commands done (%u commands done):\",\n \t\t\t\t\thave_done.nr),\n \t\t\t\thave_done.nr);\n \t\t\tfor (i = (have_done.nr > nr_lines_to_show)\n \t\t\t\t? have_done.nr - nr_lines_to_show : 0;\n \t\t\t\ti < have_done.nr;\n-\t\t\t\ti++)\n-\t\t\t\tstatus_printf_ln(s, color, \"   %s\", have_done.items[i].string);\n+\t\t\t\ti++) {\n+\t\t\t\tstrbuf_reset(&sb);\n+\t\t\t\tstrbuf_add_rebase_todo_item(&sb, &have_done.items[i], 1);\n+\t\t\t\tstatus_printf(s, color, \"   %s\", sb.buf);\n+\t\t\t}\n \t\t\tif (have_done.nr > nr_lines_to_show && s->hints)\n \t\t\t\tstatus_printf_ln(s, color,\n \t\t\t\t\t_(\"  (see more in file %s)\"), git_path(\"rebase-merge/done\"));\n@@ -1128,14 +1074,18 @@ static void show_rebase_information(struct wt_status *s,\n \t\t\t\t\t\"Next commands to do (%d remaining commands):\",\n \t\t\t\t\tyet_to_do.nr),\n \t\t\t\tyet_to_do.nr);\n-\t\t\tfor (i = 0; i < nr_lines_to_show && i < yet_to_do.nr; i++)\n-\t\t\t\tstatus_printf_ln(s, color, \"   %s\", yet_to_do.items[i].string);\n+\t\t\tfor (i = 0; i < nr_lines_to_show && i < yet_to_do.nr; i++) {\n+\t\t\t\tstrbuf_reset(&sb);\n+\t\t\t\tstrbuf_add_rebase_todo_item(&sb, &yet_to_do.items[i], 1);\n+\t\t\t\tstatus_printf(s, color, \"   %s\", sb.buf);\n+\t\t\t}\n \t\t\tif (s->hints)\n \t\t\t\tstatus_printf_ln(s, color,\n \t\t\t\t\t_(\"  (use \\\"git rebase --edit-todo\\\" to view and edit)\"));\n \t\t}\n-\t\tstring_list_clear(&yet_to_do, 0);\n-\t\tstring_list_clear(&have_done, 0);\n+\t\trebase_todo_list_clear(&yet_to_do);\n+\t\trebase_todo_list_clear(&have_done);\n+\t\tstrbuf_release(&sb);\n \t}\n }\n \n-- \n2.7.0\n"},{"id":"280674","messageId":"1457779597-6918-16-git-send-email-pyokagan@gmail.com","threadId":"41678","inReplyTo":"1457779597-6918-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH/RFC/GSoC 15/17] wrapper: implement append_file()","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2016-03-12T10:46:35Z","receivedAt":"2016-03-12T10:46:35Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Signed-off-by: Paul Tan <pyokagan@gmail.com>\n---\n cache.h   |  1 +\n wrapper.c | 23 +++++++++++++++++++++++\n 2 files changed, 24 insertions(+)\n\ndiff --git a/cache.h b/cache.h\nindex 55d443e..aa5e97c 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1700,6 +1700,7 @@ static inline ssize_t write_str_in_full(int fd, const char *str)\n \n extern int write_file(const char *path, const char *fmt, ...);\n extern int write_file_gently(const char *path, const char *fmt, ...);\n+extern void append_file(const char *path, const char *fmt, ...);\n \n /* pager.c */\n extern void setup_pager(void);\ndiff --git a/wrapper.c b/wrapper.c\nindex 9afc1a0..cd77e94 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -709,6 +709,29 @@ int write_file_gently(const char *path, const char *fmt, ...)\n \treturn status;\n }\n \n+void append_file(const char *path, const char *fmt, ...)\n+{\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tint fd = open(path, O_WRONLY | O_CREAT | O_APPEND, 0666);\n+\tva_list params;\n+\tif (fd < 0)\n+\t\tdie_errno(_(\"could not open %s for appending\"), path);\n+\tva_start(params, fmt);\n+\tstrbuf_vaddf(&sb, fmt, params);\n+\tva_end(params);\n+\tstrbuf_complete_line(&sb);\n+\tif (write_in_full(fd, sb.buf, sb.len) != sb.len) {\n+\t\tint err = errno;\n+\t\tclose(fd);\n+\t\tstrbuf_release(&sb);\n+\t\terrno = err;\n+\t\tdie_errno(_(\"could not write to %s\"), path);\n+\t}\n+\tstrbuf_release(&sb);\n+\tif (close(fd))\n+\t\tdie_errno(_(\"could not close %s\"), path);\n+}\n+\n void sleep_millisec(int millisec)\n {\n \tpoll(NULL, 0, millisec);\n-- \n2.7.0\n"},{"id":"280675","messageId":"1457779597-6918-17-git-send-email-pyokagan@gmail.com","threadId":"41678","inReplyTo":"1457779597-6918-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH/RFC/GSoC 16/17] editor: implement git_sequence_editor() and launch_sequence_editor()","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2016-03-12T10:46:36Z","receivedAt":"2016-03-12T10:46:36Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Signed-off-by: Paul Tan <pyokagan@gmail.com>\n---\n cache.h  |  1 +\n editor.c | 27 +++++++++++++++++++++++++--\n strbuf.h |  1 +\n 3 files changed, 27 insertions(+), 2 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex aa5e97c..d7a6fc6 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1222,6 +1222,7 @@ extern const char *fmt_name(const char *name, const char *email);\n extern const char *ident_default_name(void);\n extern const char *ident_default_email(void);\n extern const char *git_editor(void);\n+extern const char *git_sequence_editor(void);\n extern const char *git_pager(int stdout_is_tty);\n extern int git_ident_config(const char *, const char *, void *);\n \ndiff --git a/editor.c b/editor.c\nindex 01c644c..4c5874b 100644\n--- a/editor.c\n+++ b/editor.c\n@@ -29,10 +29,22 @@ const char *git_editor(void)\n \treturn editor;\n }\n \n-int launch_editor(const char *path, struct strbuf *buffer, const char *const *env)\n+const char *git_sequence_editor(void)\n {\n-\tconst char *editor = git_editor();\n+\tconst char *sequence_editor = getenv(\"GIT_SEQUENCE_EDITOR\");\n+\n+\tif (sequence_editor && *sequence_editor)\n+\t\treturn sequence_editor;\n \n+\tgit_config_get_string_const(\"sequence.editor\", &sequence_editor);\n+\tif (sequence_editor && *sequence_editor)\n+\t\treturn sequence_editor;\n+\n+\treturn git_editor();\n+}\n+\n+static int launch_specific_editor(const char *editor, const char *path, struct strbuf *buffer, const char *const *env)\n+{\n \tif (!editor)\n \t\treturn error(\"Terminal is dumb, but EDITOR unset\");\n \n@@ -65,5 +77,16 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en\n \tif (strbuf_read_file(buffer, path, 0) < 0)\n \t\treturn error(\"could not read file '%s': %s\",\n \t\t\t\tpath, strerror(errno));\n+\n \treturn 0;\n }\n+\n+int launch_editor(const char *path, struct strbuf *buffer, const char *const *env)\n+{\n+\treturn launch_specific_editor(git_editor(), path, buffer, env);\n+}\n+\n+int launch_sequence_editor(const char *path, struct strbuf *buffer, const char *const *env)\n+{\n+\treturn launch_specific_editor(git_sequence_editor(), path, buffer, env);\n+}\ndiff --git a/strbuf.h b/strbuf.h\nindex f72fd14..aebdcd7 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -524,6 +524,7 @@ extern void strbuf_add_unique_abbrev(struct strbuf *sb,\n  * file's contents are not read into the buffer upon completion.\n  */\n extern int launch_editor(const char *path, struct strbuf *buffer, const char *const *env);\n+extern int launch_sequence_editor(const char *path, struct strbuf *buffer, const char *const *env);\n \n extern void strbuf_add_lines(struct strbuf *sb, const char *prefix, const char *buf, size_t size);\n \n-- \n2.7.0\n"},{"id":"280678","messageId":"1457779597-6918-18-git-send-email-pyokagan@gmail.com","threadId":"41678","inReplyTo":"1457779597-6918-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH/RFC/GSoC 17/17] rebase-interactive: introduce interactive backend for builtin rebase","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2016-03-12T10:46:37Z","receivedAt":"2016-03-12T10:46:37Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Since 1b1dce4 (Teach rebase an interactive mode, 2007-06-25), git-rebase\nsupports an interactive mode when passed the -i switch.\n\nIn interactive mode, git-rebase allows users to edit the list of patches\n(using the user's GIT_SEQUENCE_EDITOR), so that the user can reorder,\nedit and delete patches.\n\nRe-implement a skeletal version of the above feature by introducing a\nrebase-interactive backend for our builtin-rebase. This skeletal\nimplementation is only able to pick and re-order commits.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n Makefile             |   1 +\n builtin/rebase.c     |  17 ++-\n rebase-interactive.c | 375 +++++++++++++++++++++++++++++++++++++++++++++++++++\n rebase-interactive.h |  33 +++++\n 4 files changed, 424 insertions(+), 2 deletions(-)\n create mode 100644 rebase-interactive.c\n create mode 100644 rebase-interactive.h\n\ndiff --git a/Makefile b/Makefile\nindex 8b928e4..3bd3127 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -781,6 +781,7 @@ LIB_OBJS += reachable.o\n LIB_OBJS += read-cache.o\n LIB_OBJS += rebase-am.o\n LIB_OBJS += rebase-common.o\n+LIB_OBJS += rebase-interactive.o\n LIB_OBJS += rebase-merge.o\n LIB_OBJS += rebase-todo.o\n LIB_OBJS += reflog-walk.o\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 6d42115..d811a44 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -10,11 +10,13 @@\n #include \"refs.h\"\n #include \"rebase-am.h\"\n #include \"rebase-merge.h\"\n+#include \"rebase-interactive.h\"\n \n enum rebase_type {\n \tREBASE_TYPE_NONE = 0,\n \tREBASE_TYPE_AM,\n-\tREBASE_TYPE_MERGE\n+\tREBASE_TYPE_MERGE,\n+\tREBASE_TYPE_INTERACTIVE\n };\n \n static const char *rebase_dir(enum rebase_type type)\n@@ -24,6 +26,8 @@ static const char *rebase_dir(enum rebase_type type)\n \t\treturn git_path_rebase_am_dir();\n \tcase REBASE_TYPE_MERGE:\n \t\treturn git_path_rebase_merge_dir();\n+\tcase REBASE_TYPE_INTERACTIVE:\n+\t\treturn git_path_rebase_interactive_dir();\n \tdefault:\n \t\tdie(\"BUG: invalid rebase_type %d\", type);\n \t}\n@@ -142,6 +146,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \tconst char *onto_name = NULL;\n \tconst char *branch_name;\n \tint do_merge = 0;\n+\tint interactive = 0;\n \n \tconst char * const usage[] = {\n \t\tN_(\"git rebase [options] [--onto <newbase>] [<upstream>] [<branch>]\"),\n@@ -153,6 +158,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t\t\tN_(\"rebase onto given branch instead of upstream\")),\n \t\tOPT_BOOL('m', \"merge\", &do_merge,\n \t\t\tN_(\"use merging strategies to rebase\")),\n+\t\tOPT_BOOL('i', \"interactive\", &interactive,\n+\t\t\tN_(\"let the user edit the list of commits to rebase\")),\n \t\tOPT_END()\n \t};\n \n@@ -232,7 +239,13 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t}\n \n \t/* Run the appropriate rebase backend */\n-\tif (do_merge) {\n+\tif (interactive) {\n+\t\tstruct rebase_interactive state;\n+\t\trebase_interactive_init(&state, rebase_dir(REBASE_TYPE_INTERACTIVE));\n+\t\trebase_options_swap(&state.opts, &rebase_opts);\n+\t\trebase_interactive_run(&state);\n+\t\trebase_interactive_release(&state);\n+\t} else if (do_merge) {\n \t\tstruct rebase_merge state;\n \t\trebase_merge_init(&state, rebase_dir(REBASE_TYPE_MERGE));\n \t\trebase_options_swap(&state.opts, &rebase_opts);\ndiff --git a/rebase-interactive.c b/rebase-interactive.c\nnew file mode 100644\nindex 0000000..342a6fe\n--- /dev/null\n+++ b/rebase-interactive.c\n@@ -0,0 +1,375 @@\n+#include \"cache.h\"\n+#include \"rebase-interactive.h\"\n+#include \"argv-array.h\"\n+#include \"revision.h\"\n+#include \"dir.h\"\n+#include \"run-command.h\"\n+\n+static int is_empty_commit(struct commit *commit)\n+{\n+\tif (commit->parents)\n+\t\treturn !oidcmp(&commit->object.oid, &commit->parents->item->object.oid);\n+\telse\n+\t\treturn !hashcmp(commit->object.oid.hash, EMPTY_TREE_SHA1_BIN);\n+}\n+\n+GIT_PATH_FUNC(git_path_rebase_interactive_dir, \"rebase-merge\")\n+\n+void rebase_interactive_init(struct rebase_interactive *state, const char *dir)\n+{\n+\trebase_options_init(&state->opts);\n+\tif (!dir)\n+\t\tdir = git_path_rebase_interactive_dir();\n+\tstate->dir = xstrdup(dir);\n+\n+\tstate->todo_file = mkpathdup(\"%s/git-rebase-todo\", state->dir);\n+\trebase_todo_list_init(&state->todo);\n+\tstate->todo_offset = 0;\n+\tstate->todo_count = 0;\n+\n+\tstate->done_file = mkpathdup(\"%s/done\", state->dir);\n+\tstate->done_count = 0;\n+\n+\tstate->instruction_format = NULL;\n+\tgit_config_get_value(\"rebase.instructionFormat\", &state->instruction_format);\n+}\n+\n+void rebase_interactive_release(struct rebase_interactive *state)\n+{\n+\trebase_options_release(&state->opts);\n+\tfree(state->dir);\n+\n+\tfree(state->todo_file);\n+\trebase_todo_list_clear(&state->todo);\n+\n+\tfree(state->done_file);\n+}\n+\n+int rebase_interactive_in_progress(const struct rebase_interactive *state)\n+{\n+\tconst char *dir = state ? state->dir : git_path_rebase_interactive_dir();\n+\tstruct stat st;\n+\n+\tif (lstat(dir, &st) || !S_ISDIR(st.st_mode))\n+\t\treturn 0;\n+\n+\tif (lstat(mkpath(\"%s/interactive\", dir), &st) || !S_ISREG(st.st_mode))\n+\t\treturn 0;\n+\n+\treturn 1;\n+}\n+\n+int rebase_interactive_load(struct rebase_interactive *state)\n+{\n+\tstruct rebase_todo_list done;\n+\n+\t/* common rebase options */\n+\tif (rebase_options_load(&state->opts, state->dir) < 0)\n+\t\treturn -1;\n+\n+\t/* todo list */\n+\trebase_todo_list_clear(&state->todo);\n+\tif (rebase_todo_list_load(&state->todo, state->todo_file, 0) < 0)\n+\t\treturn -1;\n+\tstate->todo_offset = 0;\n+\tstate->todo_count = rebase_todo_list_count(&state->todo);\n+\n+\t/* done list */\n+\trebase_todo_list_init(&done);\n+\tif (file_exists(state->done_file) && rebase_todo_list_load(&done, state->done_file, 0) < 0)\n+\t\treturn -1;\n+\tstate->done_count = rebase_todo_list_count(&done);\n+\trebase_todo_list_clear(&done);\n+\n+\treturn 0;\n+}\n+\n+static int run_command_without_output(const struct rebase_interactive *state,\n+\t\t\t\t      struct child_process *cp)\n+{\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tint status;\n+\n+\tcp->stdout_to_stderr = 1;\n+\tcp->err = -1;\n+\tif (start_command(cp) < 0)\n+\t\treturn -1;\n+\n+\tif (strbuf_read(&sb, cp->err, 0) < 0) {\n+\t\tstrbuf_release(&sb);\n+\t\tclose(cp->err);\n+\t\tfinish_command(cp);\n+\t\treturn -1;\n+\t}\n+\n+\tclose(cp->err);\n+\tstatus = finish_command(cp);\n+\tif (status)\n+\t\tfputs(sb.buf, stderr);\n+\tstrbuf_release(&sb);\n+\treturn status;\n+}\n+\n+static int detach_head(const struct rebase_interactive *state, const struct object_id *onto, const char *onto_name)\n+{\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tconst char *reflog_action = getenv(\"GIT_REFLOG_ACTION\");\n+\n+\tif (!reflog_action)\n+\t\treflog_action = \"\";\n+\tif (!onto_name)\n+\t\tonto_name = oid_to_hex(onto);\n+\tcp.git_cmd = 1;\n+\targv_array_pushf(&cp.env_array, \"GIT_REFLOG_ACTION=%s: checkout %s\",\n+\t\t\treflog_action, onto_name);\n+\targv_array_push(&cp.args, \"checkout\");\n+\targv_array_push(&cp.args, oid_to_hex(onto));\n+\n+\tif (run_command_without_output(state, &cp))\n+\t\treturn -1;\n+\n+\tdiscard_cache();\n+\tread_cache();\n+\n+\treturn 0;\n+}\n+\n+static int gen_todo_list(struct rebase_interactive *state,\n+\t\t\t const struct object_id *left,\n+\t\t\t const struct object_id *right)\n+{\n+\tstruct rev_info revs;\n+\tstruct argv_array args = ARGV_ARRAY_INIT;\n+\tstruct pretty_print_context pretty_ctx = {};\n+\tstruct commit *commit;\n+\tconst char *instruction_format;\n+\n+\tinit_revisions(&revs, NULL);\n+\targv_array_push(&args, \"rev-list\");\n+\targv_array_pushl(&args, \"--no-merges\", \"--cherry-pick\", NULL);\n+\targv_array_pushl(&args, \"--reverse\", \"--right-only\", \"--topo-order\", NULL);\n+\targv_array_pushf(&args, \"%s...%s\", oid_to_hex(left), oid_to_hex(right));\n+\tsetup_revisions(args.argc, args.argv, &revs, NULL);\n+\n+\tif (prepare_revision_walk(&revs))\n+\t\tdie(\"revision walk setup failed\");\n+\n+\tpretty_ctx.fmt = CMIT_FMT_USERFORMAT;\n+\tpretty_ctx.abbrev = revs.abbrev;\n+\tpretty_ctx.output_encoding = get_commit_output_encoding();\n+\tpretty_ctx.color = 0;\n+\tinstruction_format = state->instruction_format;\n+\tif (!instruction_format)\n+\t\tinstruction_format = \"%s\";\n+\n+\twhile ((commit = get_revision(&revs))) {\n+\t\tstruct rebase_todo_item *item;\n+\t\tstruct strbuf sb = STRBUF_INIT;\n+\n+\t\titem = rebase_todo_list_push_empty(&state->todo);\n+\t\titem->action = REBASE_TODO_PICK;\n+\t\toidcpy(&item->oid, &commit->object.oid);\n+\n+\t\tif (is_empty_commit(commit) && single_parent(commit))\n+\t\t\titem->action = REBASE_TODO_NONE;\n+\n+\t\tformat_commit_message(commit, instruction_format, &sb, &pretty_ctx);\n+\t\tstrbuf_setlen(&sb, strcspn(sb.buf, \"\\n\"));\n+\t\tif (item->action == REBASE_TODO_PICK)\n+\t\t\titem->rest = strbuf_detach(&sb, NULL);\n+\t\telse\n+\t\t\titem->rest = xstrfmt(\"%c pick %s %s\", comment_line_char,\n+\t\t\t\t\t     oid_to_hex(&item->oid), sb.buf);\n+\t\tstrbuf_release(&sb);\n+\t}\n+\n+\tif (!state->todo.nr)\n+\t\trebase_todo_list_push_noop(&state->todo);\n+\n+\treset_revision_walk();\n+\targv_array_clear(&args);\n+\treturn 0;\n+}\n+\n+/**\n+ * Mark the current action as done.\n+ */\n+static void mark_action_done(struct rebase_interactive *state)\n+{\n+\tconst struct rebase_todo_item *done_item = &state->todo.items[state->todo_offset++];\n+\tstruct strbuf sb = STRBUF_INIT;\n+\n+\t/* update todo file */\n+\trebase_todo_list_save(&state->todo, state->todo_file, state->todo_offset, 0);\n+\n+\t/* update done file */\n+\tstrbuf_add_rebase_todo_item(&sb, done_item, 0);\n+\tappend_file(state->done_file, \"%s\", sb.buf);\n+\tstrbuf_release(&sb);\n+\n+\t/* update todo and done counts if item is not none */\n+\tif (done_item->action != REBASE_TODO_NONE) {\n+\t\tunsigned int total = state->todo_count + state->done_count;\n+\n+\t\tstate->todo_count--;\n+\t\tstate->done_count++;\n+\n+\t\tprintf(_(\"Rebasing (%u/%u)\\r\"), state->done_count, total);\n+\t}\n+}\n+\n+/**\n+ * Put the last action marked done at the beginning of the todo list again. If\n+ * there has not been an action marked done yet, leave the list of items on the\n+ * todo list unchanged.\n+ */\n+static void reschedule_last_action(struct rebase_interactive *state)\n+{\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tconst char *last_line;\n+\n+\tif (!state->todo_offset)\n+\t\treturn; /* no action marked done yet */\n+\n+\t/* update todo file */\n+\trebase_todo_list_save(&state->todo, state->todo_file, --state->todo_offset, 0);\n+\n+\t/* remove the last line from the done file */\n+\tif (strbuf_read_file(&sb, state->done_file, 0) < 0)\n+\t\tdie_errno(_(\"failed to read %s\"), state->done_file);\n+\tlast_line = sb.buf + sb.len;\n+\tif (*last_line == '\\n')\n+\t\tlast_line--;\n+\tlast_line = strrchr(last_line, '\\n');\n+\tif (last_line)\n+\t\tstrbuf_setlen(&sb, last_line - sb.buf);\n+\telse\n+\t\tstrbuf_reset(&sb);\n+\twrite_file(state->done_file, \"%s\", sb.buf);\n+\tstrbuf_release(&sb);\n+}\n+\n+/**\n+ * Pick a non-merge commit.\n+ */\n+static int pick_one_non_merge(struct rebase_interactive *state,\n+\t\t\t      const struct object_id *oid, int no_commit)\n+{\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tint status;\n+\n+\tcp.git_cmd = 1;\n+\tif (state->opts.resolvemsg)\n+\t\targv_array_pushf(&cp.env_array, \"GIT_CHERRY_PICK_HELP=%s\", state->opts.resolvemsg);\n+\targv_array_push(&cp.args, \"cherry-pick\");\n+\targv_array_push(&cp.args, \"--allow-empty\");\n+\tif (no_commit)\n+\t\targv_array_push(&cp.args, \"-n\");\n+\telse\n+\t\targv_array_push(&cp.args, \"--ff\");\n+\targv_array_push(&cp.args, oid_to_hex(oid));\n+\tstatus = run_command_without_output(state, &cp);\n+\n+\t/* Reload index as cherry-pick will have modified it */\n+\tdiscard_cache();\n+\tread_cache();\n+\n+\treturn status;\n+}\n+\n+/**\n+ * Pick a commit.\n+ */\n+static int pick_one(struct rebase_interactive *state, const struct object_id *oid,\n+\t\t    int no_commit)\n+{\n+\treturn pick_one_non_merge(state, oid, no_commit);\n+}\n+\n+static void do_pick(struct rebase_interactive *state,\n+\t\t    const struct rebase_todo_item *item)\n+{\n+\tint ret;\n+\tstruct object_id head;\n+\n+\tif (get_oid(\"HEAD\", &head))\n+\t\tdie(\"invalid head\");\n+\n+\tmark_action_done(state);\n+\tret = pick_one(state, &item->oid, 0);\n+\tif (ret != 0 && ret != 1)\n+\t\treschedule_last_action(state);\n+\tif (ret)\n+\t\tdie(_(\"Could not apply %s... %s\"), oid_to_hex(&item->oid), item->rest);\n+}\n+\n+static void do_item(struct rebase_interactive *state)\n+{\n+\tconst struct rebase_todo_item *item = &state->todo.items[state->todo_offset];\n+\n+\tswitch (item->action) {\n+\tcase REBASE_TODO_NONE:\n+\tcase REBASE_TODO_NOOP:\n+\t\tmark_action_done(state);\n+\t\tbreak;\n+\tcase REBASE_TODO_PICK:\n+\t\tdo_pick(state, item);\n+\t\tbreak;\n+\tdefault:\n+\t\tdie(\"BUG: invalid action %d\", item->action);\n+\t}\n+}\n+\n+static void do_rest(struct rebase_interactive *state)\n+{\n+\twhile (state->todo_offset < state->todo.nr)\n+\t\tdo_item(state);\n+\trebase_common_finish(&state->opts, state->dir);\n+}\n+\n+void rebase_interactive_run(struct rebase_interactive *state)\n+{\n+\tif (mkdir(state->dir, 0777) < 0 && errno != EEXIST)\n+\t\tdie_errno(_(\"failed to create directory '%s'\"), state->dir);\n+\n+\twrite_file(mkpath(\"%s/interactive\", state->dir), \"%s\", \"\");\n+\trebase_options_save(&state->opts, state->dir);\n+\n+\t/* generate initial todo list contents */\n+\tif (gen_todo_list(state, &state->opts.upstream, &state->opts.orig_head) < 0) {\n+\t\trebase_common_destroy(&state->opts, state->dir);\n+\t\tdie(\"could not generate todo list\");\n+\t}\n+\n+\t/* open editor on todo list */\n+\trebase_todo_list_save(&state->todo, state->todo_file, 0, 1);\n+\tif (launch_sequence_editor(state->todo_file, NULL, NULL) < 0) {\n+\t\trebase_common_destroy(&state->opts, state->dir);\n+\t\tdie(\"Could not execute editor\");\n+\t}\n+\n+\t/* re-read todo list (which will check the todo list format) */\n+\trebase_todo_list_clear(&state->todo);\n+\tif (rebase_todo_list_load(&state->todo, state->todo_file, 1) < 0)\n+\t\tdie(_(\"You can fix this with 'git rebase --edit-todo'\"));\n+\n+\t/* count the number of actions in todo list; exit if there are none */\n+\tstate->todo_count = rebase_todo_list_count(&state->todo);\n+\tif (!state->todo_count) {\n+\t\tfprintf_ln(stderr, _(\"Nothing to do\"));\n+\t\trebase_common_destroy(&state->opts, state->dir);\n+\t\texit(2);\n+\t}\n+\n+\t/* expand todo ids */\n+\tstate->todo_count = rebase_todo_list_count(&state->todo);\n+\trebase_todo_list_save(&state->todo, state->todo_file, 0, 0);\n+\n+\t/* checkout onto */\n+\tif (detach_head(state, &state->opts.onto, state->opts.onto_name) < 0) {\n+\t\trebase_common_destroy(&state->opts, state->dir);\n+\t\tdie(_(\"could not detach HEAD\"));\n+\t}\n+\n+\tdo_rest(state);\n+}\ndiff --git a/rebase-interactive.h b/rebase-interactive.h\nnew file mode 100644\nindex 0000000..bb64203\n--- /dev/null\n+++ b/rebase-interactive.h\n@@ -0,0 +1,33 @@\n+#ifndef REBASE_INTERACTIVE_H\n+#define REBASE_INTERACTIVE_H\n+#include \"rebase-common.h\"\n+#include \"rebase-todo.h\"\n+\n+const char *git_path_rebase_interactive_dir(void);\n+\n+struct rebase_interactive {\n+    struct rebase_options opts;\n+    char *dir;\n+\n+    char *todo_file;\n+    struct rebase_todo_list todo;\n+    unsigned int todo_offset;\n+    unsigned int todo_count;\n+\n+    char *done_file;\n+    unsigned int done_count;\n+\n+    const char *instruction_format;\n+};\n+\n+void rebase_interactive_init(struct rebase_interactive *, const char *);\n+\n+void rebase_interactive_release(struct rebase_interactive *);\n+\n+int rebase_interactive_in_progress(const struct rebase_interactive *);\n+\n+int rebase_interactive_load(struct rebase_interactive *);\n+\n+void rebase_interactive_run(struct rebase_interactive *);\n+\n+#endif /* REBASE_INTERACTIVE_H */\n-- \n2.7.0\n"},{"id":"280706","messageId":"CACsJy8BmiqFJ1tN6-uAWqXMUyvGRdWP2DVfgwE56Y1K9KHCsfQ@mail.gmail.com","threadId":"41678","inReplyTo":"1457779597-6918-1-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH/RFC/GSoC 00/17] A barebones git-rebase in C","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-03-14T12:15:45Z","receivedAt":"2016-03-14T12:15:45Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Mar 12, 2016 at 5:46 PM, Paul Tan <pyokagan@gmail.com> wrote:\n> So, we have around a 1.4x-1.8x speedup for Linux users, and a 1.7x-13x speedup\n> for Windows users. The annoying long delay before the interactive editor is\n> launched on Windows is gotten rid of, which I'm very happy about :-)\n\nNice numbers :-) Sorry I can't look at your patches yet. Just a very\nminor comment from diffstat..\n\n>  rebase-am.c                        | 110 +++++++++++\n>  rebase-am.h                        |  22 +++\n>  rebase-common.c                    | 220 ++++++++++++++++++++++\n>  rebase-common.h                    |  48 +++++\n>  rebase-interactive.c               | 375 +++++++++++++++++++++++++++++++++++++\n>  rebase-interactive.h               |  33 ++++\n>  rebase-merge.c                     | 256 +++++++++++++++++++++++++\n>  rebase-merge.h                     |  28 +++\n>  rebase-todo.c                      | 251 +++++++++++++++++++++++++\n>  rebase-todo.h                      |  55 ++++++\n\ntopdir is already very crowded. Maybe you could move all these files\nto \"rebase\" subdir.\n-- \nDuy\n"},{"id":"280708","messageId":"CAP8UFD0Fw1ZOQzPfF=bbEsCOhkoHfV5B5ayprxR6kWr6vApT5Q@mail.gmail.com","threadId":"41678","inReplyTo":"1457779597-6918-13-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH/RFC/GSoC 12/17] rebase-todo: introduce rebase_todo_item","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-03-14T13:43:59Z","receivedAt":"2016-03-14T13:43:59Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Sat, Mar 12, 2016 at 11:46 AM, Paul Tan <pyokagan@gmail.com> wrote:\n> In an interactive rebase, commands are read and executed from a todo\n> list (.git/rebase-merge/git-rebase-todo) to perform the rebase.\n>\n> In the upcoming re-implementation of git-rebase -i in C, it is useful to\n> be able to parse each command into a data structure which can then be\n> operated on. Implement rebase_todo_item for this.\n\nsequencer.{c,h} already has some code to parse and create todo lists\nfor cherry-picking or reverting multiple commits, so I am wondering if\nit would be possible to share some code?\n\nThanks!\n"},{"id":"280721","messageId":"CAGZ79kZw+y6G_Y+ZkRLK6a4CG5qycM_MJFRQoQccyq596L56kw@mail.gmail.com","threadId":"41678","inReplyTo":"CACsJy8BmiqFJ1tN6-uAWqXMUyvGRdWP2DVfgwE56Y1K9KHCsfQ@mail.gmail.com","subject":"Re: [PATCH/RFC/GSoC 00/17] A barebones git-rebase in C","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-03-14T17:32:47Z","receivedAt":"2016-03-14T17:32:47Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Mar 14, 2016 at 5:15 AM, Duy Nguyen <pclouds@gmail.com> wrote:\n> On Sat, Mar 12, 2016 at 5:46 PM, Paul Tan <pyokagan@gmail.com> wrote:\n>> So, we have around a 1.4x-1.8x speedup for Linux users, and a 1.7x-13x speedup\n>> for Windows users. The annoying long delay before the interactive editor is\n>> launched on Windows is gotten rid of, which I'm very happy about :-)\n>\n> Nice numbers :-) Sorry I can't look at your patches yet. Just a very\n> minor comment from diffstat..\n>\n>>  rebase-am.c                        | 110 +++++++++++\n>>  rebase-am.h                        |  22 +++\n>>  rebase-common.c                    | 220 ++++++++++++++++++++++\n>>  rebase-common.h                    |  48 +++++\n>>  rebase-interactive.c               | 375 +++++++++++++++++++++++++++++++++++++\n>>  rebase-interactive.h               |  33 ++++\n>>  rebase-merge.c                     | 256 +++++++++++++++++++++++++\n>>  rebase-merge.h                     |  28 +++\n>>  rebase-todo.c                      | 251 +++++++++++++++++++++++++\n>>  rebase-todo.h                      |  55 ++++++\n>\n> topdir is already very crowded. Maybe you could move all these files\n> to \"rebase\" subdir.\n\nor builtin/ (or even builtin/rebase) ?\n\nI thought the toplevel being crowded is by design such that it doesn't\nfeel like a java project where each file has 5 directories for itself.\n\nI'll look at the series later today.\n\nStefan\n\n> --\n> Duy\n"},{"id":"280727","messageId":"CAGZ79ka64xBABfsWtX6GmK+sdy=VziZeGBKz9A3V=jX9ZcyfyA@mail.gmail.com","threadId":"41678","inReplyTo":"1457779597-6918-4-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH/RFC/GSoC 03/17] builtin-rebase: implement skeletal builtin rebase","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-03-14T18:31:27Z","receivedAt":"2016-03-14T18:31:27Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sat, Mar 12, 2016 at 2:46 AM, Paul Tan <pyokagan@gmail.com> wrote:\n\n   \"Unlike when doing git-am, \"git am\" kept working during the whole series\n    and being switched from the shell to the C implementation in the\nlast commit,\n    this time rebase is broken in the series, and built up from here again\" ?\n\n> @@ -505,9 +504,6 @@ SCRIPT_SH += git-web--browse.sh\n>\n>  SCRIPT_LIB += git-mergetool--lib\n>  SCRIPT_LIB += git-parse-remote\n> -SCRIPT_LIB += git-rebase--am\n> -SCRIPT_LIB += git-rebase--interactive\n> -SCRIPT_LIB += git-rebase--merge\n\nYou would also want to retire these files to the contrib directory in\nthe same patch as you unlink them from the build process?\n\n>         { \"read-tree\", cmd_read_tree, RUN_SETUP },\n> +       { \"rebase\", cmd_rebase, RUN_SETUP | NEED_WORK_TREE },\n\n#TIL you cannot run rebase in a bare repository(, yet). I would have\nassumed you could.\n"},{"id":"280730","messageId":"xmqq8u1kaoj8.fsf@gitster.mtv.corp.google.com","threadId":"41678","inReplyTo":"CACsJy8BmiqFJ1tN6-uAWqXMUyvGRdWP2DVfgwE56Y1K9KHCsfQ@mail.gmail.com","subject":"Re: [PATCH/RFC/GSoC 00/17] A barebones git-rebase in C","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-14T18:43:23Z","receivedAt":"2016-03-14T18:43:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen <pclouds@gmail.com> writes:\n\n> On Sat, Mar 12, 2016 at 5:46 PM, Paul Tan <pyokagan@gmail.com> wrote:\n>> So, we have around a 1.4x-1.8x speedup for Linux users, and a 1.7x-13x speedup\n>> for Windows users. The annoying long delay before the interactive editor is\n>> launched on Windows is gotten rid of, which I'm very happy about :-)\n>\n> Nice numbers :-) Sorry I can't look at your patches yet. Just a very\n> minor comment from diffstat..\n>\n>>  rebase-am.c                        | 110 +++++++++++\n>>  rebase-am.h                        |  22 +++\n>>  rebase-common.c                    | 220 ++++++++++++++++++++++\n>>  rebase-common.h                    |  48 +++++\n>>  rebase-interactive.c               | 375 +++++++++++++++++++++++++++++++++++++\n>>  rebase-interactive.h               |  33 ++++\n>>  rebase-merge.c                     | 256 +++++++++++++++++++++++++\n>>  rebase-merge.h                     |  28 +++\n>>  rebase-todo.c                      | 251 +++++++++++++++++++++++++\n>>  rebase-todo.h                      |  55 ++++++\n>\n> topdir is already very crowded. Maybe you could move all these files\n> to \"rebase\" subdir.\n\nI think that makes sense.  I do not expect people depending on being\nable to say \"git rebase--am\" and have it do something useful, so\nthey won't belong to builtin/, but rebase/{am,common,...}.[ch] makes\nsense.\n\nWhile this series is still in the rough-outline phase, we can review\nthe patches without such movement, though ;-)\n"},{"id":"280733","messageId":"CAGZ79kZ-XvYni0eFeYCk1HoTndOrABCqmz59-2ciz0Sv1W1r+Q@mail.gmail.com","threadId":"41678","inReplyTo":"1457779597-6918-5-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH/RFC/GSoC 04/17] builtin-rebase: parse rebase arguments into a common rebase_options struct","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-03-14T20:05:08Z","receivedAt":"2016-03-14T20:05:08Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sat, Mar 12, 2016 at 2:46 AM, Paul Tan <pyokagan@gmail.com> wrote:\n> A non-root rebase takes 3 arguments:\n>\n> * branch_name -- the branch or commit that will be rebased. If it is not\n>   specified, the current branch is used.\n>\n> * upstream -- The upstream commit to compare against. If it is not\n>   specified, the configured upstream for the current branch is used.\n>\n> * onto (or newbase) -- The commit to be used as the starting point to\n>   re-apply the commits. If it is not specified, `upstream` is used.\n>\n> Since these parameters are used by all 3 rebase backends, introduce a\n> common rebase_options struct to hold all these options. Teach\n> builtin/rebase.c to handle the above arguments and store them in a\n> rebase_options struct. In later patches we will pass the rebase_options\n> struct to the appropriate backend to perform the rebase.\n>\n> Signed-off-by: Paul Tan <pyokagan@gmail.com>\n> ---\n>  Makefile         |   1 +\n>  builtin/rebase.c | 184 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n>  rebase-common.c  |  28 +++++++++\n>  rebase-common.h  |  23 +++++++\n>  4 files changed, 235 insertions(+), 1 deletion(-)\n>  create mode 100644 rebase-common.c\n>  create mode 100644 rebase-common.h\n>\n> diff --git a/Makefile b/Makefile\n> index ad98714..b29c672 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -779,6 +779,7 @@ LIB_OBJS += prompt.o\n>  LIB_OBJS += quote.o\n>  LIB_OBJS += reachable.o\n>  LIB_OBJS += read-cache.o\n> +LIB_OBJS += rebase-common.o\n>  LIB_OBJS += reflog-walk.o\n>  LIB_OBJS += refs.o\n>  LIB_OBJS += refs/files-backend.o\n> diff --git a/builtin/rebase.c b/builtin/rebase.c\n> index 04cc1bd..40176ca 100644\n> --- a/builtin/rebase.c\n> +++ b/builtin/rebase.c\n> @@ -4,6 +4,112 @@\n>  #include \"cache.h\"\n>  #include \"builtin.h\"\n>  #include \"parse-options.h\"\n> +#include \"rebase-common.h\"\n> +#include \"remote.h\"\n> +#include \"branch.h\"\n> +#include \"refs.h\"\n> +\n> +/**\n> + * Used by get_curr_branch_upstream_name() as a for_each_remote() callback to\n> + * retrieve the name of the remote if the repository only has one remote.\n> + */\n> +static int get_only_remote(struct remote *remote, void *cb_data)\n> +{\n> +       const char **remote_name = cb_data;\n> +\n> +       if (*remote_name)\n> +               return -1;\n> +\n> +       *remote_name = remote->name;\n> +       return 0;\n> +}\n> +\n> +const char *get_curr_branch_upstream_name(void)\n> +{\n> +       const char *upstream_name;\n> +       struct branch *curr_branch;\n> +\n> +       curr_branch = branch_get(\"HEAD\");\n> +       if (!curr_branch) {\n> +               fprintf_ln(stderr, _(\"You are not currently on a branch.\"));\n> +               fprintf_ln(stderr, _(\"Please specify which branch you want to rebase against.\"));\n> +               fprintf_ln(stderr, _(\"See git-rebase(1) for details.\"));\n> +               fprintf(stderr, \"\\n\");\n> +               fprintf_ln(stderr, \"    git rebase <branch>\");\n> +               fprintf(stderr, \"\\n\");\n> +               exit(1);\n> +       }\n> +\n> +       upstream_name = branch_get_upstream(curr_branch, NULL);\n> +       if (!upstream_name) {\n> +               const char *remote_name = NULL;\n> +\n> +               if (for_each_remote(get_only_remote, &remote_name) || !remote_name)\n> +                       remote_name = \"<remote>\";\n> +\n> +               fprintf_ln(stderr, _(\"There is no tracking information for the current branch.\"));\n> +               fprintf_ln(stderr, _(\"Please specify which branch you want to rebase against.\"));\n> +               fprintf_ln(stderr, _(\"See git-rebase(1) for details.\"));\n> +               fprintf(stderr, \"\\n\");\n> +               fprintf_ln(stderr, \"    git rebase <branch>\");\n> +               fprintf(stderr, \"\\n\");\n> +               fprintf_ln(stderr, _(\"If you wish to set tracking information for this branch you can do so with:\"));\n> +               fprintf(stderr, \"\\n\");\n> +               fprintf_ln(stderr, _(\"If you wish to set tracking information for this branch you can do so with:\\n\"\n> +               \"\\n\"\n> +               \"    git branch --set-upstream-to=%s/<branch> %s\\n\"),\n> +               remote_name, curr_branch->name);\n> +               exit(1);\n> +       }\n> +\n> +       return upstream_name;\n> +}\n> +\n> +/**\n> + * Given the --onto <name>, return the onto hash\n> + */\n> +static void get_onto_oid(const char *_onto_name, struct object_id *onto)\n> +{\n> +       char *onto_name = xstrdup(_onto_name);\n> +       struct commit *onto_commit;\n> +       char *dotdot;\n> +\n> +       dotdot = strstr(onto_name, \"...\");\n\nSo either the variable should be \"dotdotdot\" or \"threedots\",\nor there is a dot too much in the strstr.\n\n\"dotdot\" was is used in some directory handling code, which\nthis reminded me of. So I first wondered why we need to take\ncare of parent directories.\n\n> +       if (dotdot) {\n> +               const char *left = onto_name;\n> +               const char *right = dotdot + 3;\n> +               struct commit *left_commit, *right_commit;\n> +               struct commit_list *merge_bases;\n> +\n> +               *dotdot = 0;\n> +               if (!*left)\n> +                       left = \"HEAD\";\n> +               if (!*right)\n> +                       right = \"HEAD\";\n> +\n> +               /* git merge-base --all $left $right */\n> +               left_commit = lookup_commit_reference_by_name(left);\n> +               right_commit = lookup_commit_reference_by_name(right);\n> +               if (!left_commit || !right_commit)\n> +                       die(_(\"%s: there is no merge base\"), _onto_name);\n\n_onto_name seems to be different than what was passed in,\nbecause of *dotdot = 0 before, is that intentional?\n\n> +\n> +               merge_bases = get_merge_bases(left_commit, right_commit);\n> +               if (merge_bases && merge_bases->next)\n> +                       die(_(\"%s: there are more than one merge bases\"), _onto_name);\n> +               else if (!merge_bases)\n> +                       die(_(\"%s: there is no merge base\"), _onto_name);\n> +\n> +               onto_commit = merge_bases->item;\n> +               free_commit_list(merge_bases);\n> +       } else {\n> +               onto_commit = lookup_commit_reference_by_name(onto_name);\n> +               if (!onto_commit)\n> +                       die(_(\"invalid upstream %s\"), onto_name);\n> +       }\n> +\n> +       free(onto_name);\n> +       oidcpy(onto, &onto_commit->object.oid);\n> +}\n>\n>  static int git_rebase_config(const char *k, const char *v, void *cb)\n>  {\n> @@ -12,20 +118,96 @@ static int git_rebase_config(const char *k, const char *v, void *cb)\n>\n>  int cmd_rebase(int argc, const char **argv, const char *prefix)\n>  {\n> +       struct rebase_options rebase_opts;\n> +       const char *onto_name = NULL;\n> +       const char *branch_name;\n> +\n>         const char * const usage[] = {\n> -               N_(\"git rebase [options]\"),\n> +               N_(\"git rebase [options] [--onto <newbase>] [<upstream>] [<branch>]\"),\n>                 NULL\n>         };\n>         struct option options[] = {\n> +               OPT_GROUP(N_(\"Available options are\")),\n> +               OPT_STRING(0, \"onto\", &onto_name, NULL,\n> +                       N_(\"rebase onto given branch instead of upstream\")),\n>                 OPT_END()\n>         };\n>\n>         git_config(git_rebase_config, NULL);\n> +       rebase_options_init(&rebase_opts);\n> +       rebase_opts.resolvemsg = _(\"\\nWhen you have resolved this problem, run \\\"git rebase --continue\\\".\\n\"\n> +                       \"If you prefer to skip this patch, run \\\"git rebase --skip\\\" instead.\\n\"\n> +                       \"To check out the original branch and stop rebasing, run \\\"git rebase --abort\\\".\");\n>\n>         argc = parse_options(argc, argv, prefix, options, usage, 0);\n>\n>         if (read_cache_preload(NULL) < 0)\n>                 die(_(\"failed to read the index\"));\n>\n> +       /*\n> +        * Parse command-line arguments:\n> +        *    rebase [<options>] [<upstream_name>] [<branch_name>]\n> +        */\n> +\n> +       /* Parse <upstream_name> into rebase_opts.upstream */\n> +       {\n> +               const char *upstream_name;\n> +               if (argc > 2)\n> +                       usage_with_options(usage, options);\n> +               if (!argc) {\n> +                       upstream_name = get_curr_branch_upstream_name();\n> +               } else {\n> +                       upstream_name = argv[0];\n> +                       argv++, argc--;\n> +                       if (!strcmp(upstream_name, \"-\"))\n> +                               upstream_name = \"@{-1}\";\n> +               }\n\nSo first we extract the upstream_name then branch_name,\nso the parsing is rather\n\n     rebase [<options>] [<upstream_name> [<branch_name>]]\n\ni.e. you cannot give branch name only?\n\n> +               if (get_oid_commit(upstream_name, &rebase_opts.upstream))\n> +                       die(_(\"invalid upstream %s\"), upstream_name);\n> +               if (!onto_name)\n> +                       onto_name = upstream_name;\n> +       }\n> +\n> +       /*\n> +        * Parse --onto <onto_name> into rebase_opts.onto and\n> +        * rebase_opts.onto_name\n> +        */\n> +       get_onto_oid(onto_name, &rebase_opts.onto);\n> +       rebase_opts.onto_name = xstrdup(onto_name);\n> +\n> +       /*\n> +        * Parse <branch_name> into rebase_opts.orig_head and\n> +        * rebase_opts.orig_refname\n> +        */\n> +       branch_name = argv[0];\n> +       if (branch_name) {\n> +               /* Is branch_name a branch or commit? */\n> +               char *ref_name = xstrfmt(\"refs/heads/%s\", branch_name);\n> +               struct object_id orig_head_id;\n> +\n> +               if (!read_ref(ref_name, orig_head_id.hash)) {\n> +                       rebase_opts.orig_refname = ref_name;\n> +                       if (get_oid_commit(ref_name, &rebase_opts.orig_head))\n> +                               die(\"get_sha1_commit failed\");\n> +               } else if (!get_oid_commit(branch_name, &rebase_opts.orig_head)) {\n> +                       rebase_opts.orig_refname = NULL;\n> +                       free(ref_name);\n> +               } else {\n> +                       die(_(\"no such branch: %s\"), branch_name);\n> +               }\n> +       } else {\n> +               /* Do not need to switch branches, we are already on it */\n> +               struct branch *curr_branch = branch_get(\"HEAD\");\n> +\n> +               if (curr_branch)\n> +                       rebase_opts.orig_refname = xstrdup(curr_branch->refname);\n> +               else\n> +                       rebase_opts.orig_refname = NULL;\n> +\n> +               if (get_oid_commit(\"HEAD\", &rebase_opts.orig_head))\n> +                       die(_(\"Failed to resolve '%s' as a valid revision.\"), \"HEAD\");\n> +       }\n> +\n> +       rebase_options_release(&rebase_opts);\n>         return 0;\n>  }\n> diff --git a/rebase-common.c b/rebase-common.c\n> new file mode 100644\n> index 0000000..5a49ac4\n> --- /dev/null\n> +++ b/rebase-common.c\n> @@ -0,0 +1,28 @@\n> +#include \"cache.h\"\n> +#include \"rebase-common.h\"\n> +\n> +void rebase_options_init(struct rebase_options *opts)\n> +{\n> +       oidclr(&opts->onto);\n> +       opts->onto_name = NULL;\n> +\n> +       oidclr(&opts->upstream);\n> +\n> +       oidclr(&opts->orig_head);\n> +       opts->orig_refname = NULL;\n> +\n> +       opts->resolvemsg = NULL;\n> +}\n> +\n> +void rebase_options_release(struct rebase_options *opts)\n> +{\n> +       free(opts->onto_name);\n> +       free(opts->orig_refname);\n> +}\n> +\n> +void rebase_options_swap(struct rebase_options *dst, struct rebase_options *src)\n> +{\n> +       struct rebase_options tmp = *dst;\n> +       *dst = *src;\n> +       *src = tmp;\n> +}\n> diff --git a/rebase-common.h b/rebase-common.h\n> new file mode 100644\n> index 0000000..db5146a\n> --- /dev/null\n> +++ b/rebase-common.h\n> @@ -0,0 +1,23 @@\n> +#ifndef REBASE_COMMON_H\n> +#define REBASE_COMMON_H\n> +\n> +/* common rebase backend options */\n> +struct rebase_options {\n> +       struct object_id onto;\n> +       char *onto_name;\n> +\n> +       struct object_id upstream;\n> +\n> +       struct object_id orig_head;\n> +       char *orig_refname;\n> +\n> +       const char *resolvemsg;\n> +};\n> +\n> +void rebase_options_init(struct rebase_options *);\n> +\n> +void rebase_options_release(struct rebase_options *);\n> +\n> +void rebase_options_swap(struct rebase_options *dst, struct rebase_options *src);\n> +\n> +#endif /* REBASE_COMMON_H */\n> --\n> 2.7.0\n>\n"},{"id":"280735","messageId":"CAGZ79kYeYzi=J=dY27FqXp72BRe-Vmm4MR5Q6dFTMUP9CxYZcg@mail.gmail.com","threadId":"41678","inReplyTo":"1457779597-6918-6-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH/RFC/GSoC 05/17] rebase-options: implement rebase_options_load() and rebase_options_save()","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-03-14T20:30:41Z","receivedAt":"2016-03-14T20:30:41Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sat, Mar 12, 2016 at 2:46 AM, Paul Tan <pyokagan@gmail.com> wrote:\n> These functions can be used for loading and saving common rebase options\n> into a state directory.\n>\n> Signed-off-by: Paul Tan <pyokagan@gmail.com>\n> ---\n>  rebase-common.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n>  rebase-common.h |  4 ++++\n>  2 files changed, 73 insertions(+)\n>\n> diff --git a/rebase-common.c b/rebase-common.c\n> index 5a49ac4..1835f08 100644\n> --- a/rebase-common.c\n> +++ b/rebase-common.c\n> @@ -26,3 +26,72 @@ void rebase_options_swap(struct rebase_options *dst, struct rebase_options *src)\n>         *dst = *src;\n>         *src = tmp;\n>  }\n> +\n> +static int state_file_exists(const char *dir, const char *file)\n> +{\n> +       return file_exists(mkpath(\"%s/%s\", dir, file));\n> +}\n\nHow is this specific to the state file? All it does is create the\nleading directory\nif it doesn't exist? (So I'd expect file_exists(concat(dir, file)) to\nhave the same\nresult without actually creating the directory if it doesn't exist as\na side effect?\n\nIf the dir doesn't exist it can be created in rebase_options_load explicitly?\n\n\n> +\n> +static int read_state_file(struct strbuf *sb, const char *dir, const char *file)\n> +{\n> +       const char *path = mkpath(\"%s/%s\", dir, file);\n> +       strbuf_reset(sb);\n> +       if (strbuf_read_file(sb, path, 0) >= 0)\n> +               return sb->len;\n> +       else\n> +               return error(_(\"could not read '%s'\"), path);\n> +}\n> +\n> +int rebase_options_load(struct rebase_options *opts, const char *dir)\n> +{\n> +       struct strbuf sb = STRBUF_INIT;\n> +       const char *filename;\n> +\n> +       /* opts->orig_refname */\n> +       if (read_state_file(&sb, dir, \"head-name\") < 0)\n> +               return -1;\n> +       strbuf_trim(&sb);\n> +       if (starts_with(sb.buf, \"refs/heads/\"))\n> +               opts->orig_refname = strbuf_detach(&sb, NULL);\n> +       else if (!strcmp(sb.buf, \"detached HEAD\"))\n> +               opts->orig_refname = NULL;\n> +       else\n> +               return error(_(\"could not parse %s\"), mkpath(\"%s/%s\", dir, \"head-name\"));\n> +\n> +       /* opts->onto */\n> +       if (read_state_file(&sb, dir, \"onto\") < 0)\n> +               return -1;\n> +       strbuf_trim(&sb);\n> +       if (get_oid_hex(sb.buf, &opts->onto) < 0)\n> +               return error(_(\"could not parse %s\"), mkpath(\"%s/%s\", dir, \"onto\"));\n> +\n> +       /*\n> +        * We always write to orig-head, but interactive rebase used to write\n> +        * to head. Fall back to reading from head to cover for the case that\n> +        * the user upgraded git with an ongoing interactive rebase.\n> +        */\n> +       filename = state_file_exists(dir, \"orig-head\") ? \"orig-head\" : \"head\";\n> +       if (read_state_file(&sb, dir, filename) < 0)\n> +               return -1;\n\nSo from here on we always use \"orig-head\" instead of \"head\" for\ninteractive rebase.\nWould people ever rely on the (internal) file name and have e.g.\nscripts which operate\non the \"head\" file ?\n\n\n> +       strbuf_trim(&sb);\n> +       if (get_oid_hex(sb.buf, &opts->orig_head) < 0)\n> +               return error(_(\"could not parse %s\"), mkpath(\"%s/%s\", dir, filename));\n> +\n> +       strbuf_release(&sb);\n> +       return 0;\n> +}\n> +\n> +static int write_state_text(const char *dir, const char *file, const char *string)\n> +{\n> +       return write_file(mkpath(\"%s/%s\", dir, file), \"%s\", string);\n> +}\n\nSame comment as on checking the state files existence. I'm not sure if the side\neffect of creating the dir is better done explicitly where it is used.\nThe concat of dir and\nfile name can still be done in the helper though? (If the helper is\nneeded at all then)\n\n> +\n> +void rebase_options_save(const struct rebase_options *opts, const char *dir)\n> +{\n> +       const char *head_name = opts->orig_refname;\n> +       if (!head_name)\n> +               head_name = \"detached HEAD\";\n> +       write_state_text(dir, \"head-name\", head_name);\n> +       write_state_text(dir, \"onto\", oid_to_hex(&opts->onto));\n> +       write_state_text(dir, \"orig-head\", oid_to_hex(&opts->orig_head));\n> +}\n> diff --git a/rebase-common.h b/rebase-common.h\n> index db5146a..051c056 100644\n> --- a/rebase-common.h\n> +++ b/rebase-common.h\n> @@ -20,4 +20,8 @@ void rebase_options_release(struct rebase_options *);\n>\n>  void rebase_options_swap(struct rebase_options *dst, struct rebase_options *src);\n>\n> +int rebase_options_load(struct rebase_options *, const char *dir);\n> +\n> +void rebase_options_save(const struct rebase_options *, const char *dir);\n> +\n>  #endif /* REBASE_COMMON_H */\n> --\n> 2.7.0\n>\n"},{"id":"280736","messageId":"alpine.DEB.2.20.1603142131120.4690@virtualbox","threadId":"41678","inReplyTo":"CAP8UFD0Fw1ZOQzPfF=bbEsCOhkoHfV5B5ayprxR6kWr6vApT5Q@mail.gmail.com","subject":"Re: [PATCH/RFC/GSoC 12/17] rebase-todo: introduce rebase_todo_item","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-03-14T20:33:03Z","receivedAt":"2016-03-14T20:33:03Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Christian,\n\nOn Mon, 14 Mar 2016, Christian Couder wrote:\n\n> On Sat, Mar 12, 2016 at 11:46 AM, Paul Tan <pyokagan@gmail.com> wrote:\n> > In an interactive rebase, commands are read and executed from a todo\n> > list (.git/rebase-merge/git-rebase-todo) to perform the rebase.\n> >\n> > In the upcoming re-implementation of git-rebase -i in C, it is useful to\n> > be able to parse each command into a data structure which can then be\n> > operated on. Implement rebase_todo_item for this.\n> \n> sequencer.{c,h} already has some code to parse and create todo lists\n> for cherry-picking or reverting multiple commits, so I am wondering if\n> it would be possible to share some code?\n\nI did exactly that and plan to work full steam on that because I need it\nway before GSoC starts:\n\nhttps://github.com/git/git/compare/master...dscho:interactive-rebase\n\nNote: there are still a couple of things to be done. I will take care of\nthem.\n\nCiao,\nDscho\n"},{"id":"280738","messageId":"alpine.DEB.2.20.1603142142260.4690@virtualbox","threadId":"41678","inReplyTo":"CACsJy8BmiqFJ1tN6-uAWqXMUyvGRdWP2DVfgwE56Y1K9KHCsfQ@mail.gmail.com","subject":"Re: [PATCH/RFC/GSoC 00/17] A barebones git-rebase in C","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-03-14T20:44:44Z","receivedAt":"2016-03-14T20:44:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Duy,\n\nOn Mon, 14 Mar 2016, Duy Nguyen wrote:\n\n> On Sat, Mar 12, 2016 at 5:46 PM, Paul Tan <pyokagan@gmail.com> wrote:\n> \n> >  rebase-am.c                        | 110 +++++++++++\n> >  rebase-am.h                        |  22 +++\n> >  rebase-common.c                    | 220 ++++++++++++++++++++++\n> >  rebase-common.h                    |  48 +++++\n> >  rebase-interactive.c               | 375 +++++++++++++++++++++++++++++++++++++\n> >  rebase-interactive.h               |  33 ++++\n> >  rebase-merge.c                     | 256 +++++++++++++++++++++++++\n> >  rebase-merge.h                     |  28 +++\n> >  rebase-todo.c                      | 251 +++++++++++++++++++++++++\n> >  rebase-todo.h                      |  55 ++++++\n> \n> topdir is already very crowded. Maybe you could move all these files\n> to \"rebase\" subdir.\n\nYes, I had mentioned a couple times that my preference would be to\nintroduce a rebase--helper builtin and move functionality into it one by\none, which is incidentally exactly what I already did in my\n'interactive-rebase' branch. It still has a couple of very rough edges,\nbut I could not work on it today (because v2.7.3 surprised me and shuffled\nmy tasks around, and then I hunted two bugs the entire day and have to\ncontinue tomorrow).\n\nPaul, may I ask you to concentrate on the parts that are *not* the\ninteractive rebase?\n\nCiao,\nJohannes\n"},{"id":"280739","messageId":"alpine.DEB.2.20.1603142151230.4690@virtualbox","threadId":"41678","inReplyTo":"1457779597-6918-10-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH/RFC/GSoC 09/17] rebase-common: implement cache_has_unstaged_changes()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-03-14T20:54:19Z","receivedAt":"2016-03-14T20:54:19Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Paul,\n\nOn Sat, 12 Mar 2016, Paul Tan wrote:\n\n> In the upcoming git-rebase-to-C rewrite, it is a common operation to\n> check if the worktree has unstaged changes, so that it can complain that\n> the worktree is dirty.\n> \n> builtin/pull.c already implements this function. Move it to\n> rebase-common.c so that it can be shared between all rebase backends and\n> git-pull.\n\nThis function is not specific to rebases, even if you only use it for\nthose purposes for the moment.\n\nIn my 'interactive-rebase' branch, I moved it to wt-status (which is a\nmore logical place, methinks).\n\nAlso, you might want to join my discussion with Junio about the sense or\nnonsense of keeping the prefix parameter instead of silently removing it\nwhile moving the functions.\n\nFurthermore, it is not really the cache (which I thought we settled on\ncalling \"index\" these days) that has unstaged changes, but the working\ndirectory.\n\nFor simplicity's sake, I therefore kept the has_unstaged_changes() name\n(it is not like there is a lot of confusion *what* can have unstaged\nchanges).\n\nCiao,\nJohannes\n"},{"id":"280742","messageId":"xmqqshzs9369.fsf@gitster.mtv.corp.google.com","threadId":"41678","inReplyTo":"1457779597-6918-8-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH/RFC/GSoC 07/17] rebase-common: implement refresh_and_write_cache()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-14T21:10:06Z","receivedAt":"2016-03-14T21:10:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan <pyokagan@gmail.com> writes:\n\n> In the upcoming git-rebase to C rewrite, it is a common operation to\n> refresh the index and write the resulting index.\n>\n> builtin/am.c already implements refresh_and_write_cache(), which is what\n> we want. Move it to rebase-common.c, so that it can be shared with all\n> the rebase backends, including git-am.\n\nYour rebase-am might be one of the rebase backends, but git-am is\nnot, so it is misleading to count it among \"all the rebase\nbackends\".\n\nI would think that a better home for refresh_and_write_index() is\nright next to write_locked_index(), with #define in cache.h for\nrefresh_and_write_cache(), just like others.\n"},{"id":"280750","messageId":"xmqqoaag9177.fsf@gitster.mtv.corp.google.com","threadId":"41678","inReplyTo":"alpine.DEB.2.20.1603142151230.4690@virtualbox","subject":"Re: [PATCH/RFC/GSoC 09/17] rebase-common: implement cache_has_unstaged_changes()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-14T21:52:44Z","receivedAt":"2016-03-14T21:52:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Also, you might want to join my discussion with Junio about the sense or\n> nonsense of keeping the prefix parameter instead of silently removing it\n> while moving the functions.\n\nThis is a tangent to Paul's topic, but it is an important tangent in\nthe other thread, in that you didn't mention one thing there that I\nneeded to make an accurate assessment.  I wasn't aware of your plan\nof moving it and use it in a context unrelated to \"git pull\", hence\nkeeping the prefix would made perfect sense, as the enhanced error\nreporting (i.e. \"not only I am saying that you have modified files\nand hence you cannot proceed, I can tell you which paths have been\nmodified\") would happen inside the function if done in that context.\n\nIf the function will be made a public helper that may be called by\nanybody, a possible error reporting mechanism would be to give a\nlist of modified paths to the caller that asks them, and have the\ncaller apply its own \"prefix\" processing to make them relative.  The\npublic helper function will not even be a position to say \"you have\nmodified files and hence you cannot proceed\"--it will not be in a\nposition to even issue an error message.  The only thing it should\ndo is to communicate to the caller if there are modified files or\nnot (and leaving the decision on what to do to the caller--after\nall, the caller may want to do something only when there are\nmodified files, e.g. \"add .\" may decide not to do anything when\nthere is no change--so \"hence you cannot proceed\" is not its\nbusiness), and if the caller wants to see them, which paths are\ndirty.\n\nIncidentally, that is how wt_shortstatus_status() reports the list\nof modified paths, using s->prefix.\n"},{"id":"280763","messageId":"alpine.DEB.2.20.1603150755450.4690@virtualbox","threadId":"41678","inReplyTo":"1457779597-6918-17-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH/RFC/GSoC 16/17] editor: implement git_sequence_editor() and launch_sequence_editor()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-03-15T07:00:00Z","receivedAt":"2016-03-15T07:00:00Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Paul,\n\nOn Sat, 12 Mar 2016, Paul Tan wrote:\n\n> Signed-off-by: Paul Tan <pyokagan@gmail.com>\n\nThis commit message is very short.\n\n> ---\n>  cache.h  |  1 +\n\nNo need to clutter cache.h with a function that is only to be used by the\nsequencer. IOW let's make this static in sequencer.c.\n\nI would also prefer pairing this short function with the change that\nactually uses it (in my topic branches, I like to compile with -Werror,\nwhich would result in a failure due to an unused function), in the same\npatch.\n\nCiao,\nDscho\n"},{"id":"280767","messageId":"alpine.DEB.2.20.1603150800420.4690@virtualbox","threadId":"41678","inReplyTo":"1457779597-6918-18-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH/RFC/GSoC 17/17] rebase-interactive: introduce interactive backend for builtin rebase","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-03-15T07:57:55Z","receivedAt":"2016-03-15T07:57:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Paul,\n\nOn Sat, 12 Mar 2016, Paul Tan wrote:\n\n> Since 1b1dce4 (Teach rebase an interactive mode, 2007-06-25), git-rebase\n> supports an interactive mode when passed the -i switch.\n> \n> In interactive mode, git-rebase allows users to edit the list of patches\n> (using the user's GIT_SEQUENCE_EDITOR), so that the user can reorder,\n> edit and delete patches.\n> \n> Re-implement a skeletal version of the above feature by introducing a\n> rebase-interactive backend for our builtin-rebase. This skeletal\n> implementation is only able to pick and re-order commits.\n> \n> Signed-off-by: Paul Tan <pyokagan@gmail.com>\n\nIt is a pity that both of us worked on overlapping projects in stealth\nmode. Inevitably, some of the work is now wasted :-(\n\nNot all is lost, though.\n\nMuch of the code can be salvaged, although I really want to reiterate\nthat an all-or-nothing conversion of the rebase command is not going to\nfly.\n\nFor several reasons: it would be rather disruptive, huge and hard to\nreview. It would not let anybody else work on that huge task. And you're\nprone to fall behind due to Git's source code being in constant flux\n(including the rebase bits).\n\nThere is another, really important reason: if you package the conversion\ninto small, neat bundles, it is much easier to avoid too narrow a focus\nthat would tuck perfectly useful functions away in a location where it\ncannot be reused and where it is likely to be missed by other developers\nwho need the same, or similar functionality (point in case:\nhas_uncommitted_changes()). And we know that this happened in the past,\nand sometimes resulted in near-duplicated code, hence Karthik's Herculean,\nstill ongoing work.\n\nLastly, I need to point out that the conversion of rebase into a builtin\nis not the end game, it is the game's opening.\n\nI could imagine that other Git oldtimers are perfectly happy with the\nstate of the rebase family of commands. I am not. The user interface is\nklunky, some parts are designed wrong (--preserve-merges, I am looking at\nyou!), the *name* is completely unintuitive, for crying out loud! Just to\nname a *few* things, there is much more.\n\nI worked around the limitations of the --preserve-merges feature (yeah,\nblame me...) by inventing the \"Garden Shears\" [*1*] that can re-plant an\nentire thicket of topic branches on top of a moving upstream branch. It is\nsimilar in spirit to Junio's custom tools that recreate his 'pu' branch\nover and over again, but uses the interactive rebase as work horse.\n\nThe shears can also be used to fix up commits in the middle of a thicket\nof branches, which is why I wrote the shears script in the first place.\n\nThe fact that the interactive rebase does not do the job with which pretty\nmuch every project maintainer is faced points out two things: interactive\nrebase needs to learn new tricks, and we need plumbing to do interactive\nrebase's bidding (so that we/others can build better UIs, hopefully with\nmuch better names, too).\n\nI already know pretty well how I want to implement the shears as a new\nmode of the interactive rebase, once I finished teaching the sequencer how\nto process rebase -i's edit scripts (which somebody decided to name\n\"instruction sheets\" in the sequencer, to add confusion to poor naming).\n\nAll of this let's me think that there is just too much to do for a single\ndeveloper, and therefore whatever needs to be done must be done in a way\nthat allows more than a single person to complete the whole shebang, or\nat least their part of it.\n\nSo you see, there was a much larger master plan behind my recommendation\nto go the rebase--helper route.\n\nAs to my current state: Junio put me into quite a fix (without knowing it)\nby releasing 2.7.3 just after I took off for an extended offline weekend,\nand now I am scrambling because a change in MSYS2's runtime (actually,\nprobably more like: an update of the GCC that is used to compile the\nruntime, that now causes a regression) is keeping me away from my work on\nthe interactive rebase. Even so, I am pretty far along; There are only\nthree major things left to do: 1) fix fixups/squashes with fast-forwarding\npicks, 2) implement 'reword', 3) display the progress.  And of course 4)\nclean up the fallout. ;-)\n\nAt this point, I'd rather finish this myself than falling prey to Brooks'\nLaw.\n\nI also have to admit that I would love to give you a project over the\nsummer whose logical children are exciting enough to dabble with even\nduring the winter. And somehow I do not see that excitement in the boring\nconversion from shell to C (even if its outcome is well-needed).\n\nCiao,\nDscho\n\nFootnote *1*:\nhttps://github.com/git-for-windows/build-extra/blob/master/shears.sh\n"},{"id":"280768","messageId":"alpine.DEB.2.20.1603150900440.4690@virtualbox","threadId":"41678","inReplyTo":"CAGZ79ka64xBABfsWtX6GmK+sdy=VziZeGBKz9A3V=jX9ZcyfyA@mail.gmail.com","subject":"Re: [PATCH/RFC/GSoC 03/17] builtin-rebase: implement skeletal builtin rebase","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-03-15T08:01:46Z","receivedAt":"2016-03-15T08:01:46Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stefan,\n\nOn Mon, 14 Mar 2016, Stefan Beller wrote:\n\n> #TIL you cannot run rebase in a bare repository(, yet). I would have\n> assumed you could.\n\nEvery rebase bears the chance of merge conflicts. You need a working\ndirectory to resolve those.\n\nCiao,\nDscho\n"},{"id":"280772","messageId":"alpine.DEB.2.20.1603151134230.4690@virtualbox","threadId":"41678","inReplyTo":"1457779597-6918-5-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH/RFC/GSoC 04/17] builtin-rebase: parse rebase arguments into a common rebase_options struct","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-03-15T10:54:58Z","receivedAt":"2016-03-15T10:54:58Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Paul,\n\nOn Sat, 12 Mar 2016, Paul Tan wrote:\n\n> diff --git a/builtin/rebase.c b/builtin/rebase.c\n> index 04cc1bd..40176ca 100644\n> --- a/builtin/rebase.c\n> +++ b/builtin/rebase.c\n> @@ -4,6 +4,112 @@\n>  #include \"cache.h\"\n>  #include \"builtin.h\"\n>  #include \"parse-options.h\"\n> +#include \"rebase-common.h\"\n> +#include \"remote.h\"\n> +#include \"branch.h\"\n> +#include \"refs.h\"\n> +\n> +/**\n> + * Used by get_curr_branch_upstream_name() as a for_each_remote() callback to\n> + * retrieve the name of the remote if the repository only has one remote.\n> + */\n> +static int get_only_remote(struct remote *remote, void *cb_data)\n> +{\n> +\tconst char **remote_name = cb_data;\n> +\n> +\tif (*remote_name)\n> +\t\treturn -1;\n> +\n> +\t*remote_name = remote->name;\n> +\treturn 0;\n> +}\n\nThis function gets only the remote's name, not only the remote. And this\nis not really a functionality specific to rebase, is it?\n\n> +const char *get_curr_branch_upstream_name(void)\n> +{\n> +\tconst char *upstream_name;\n> +\tstruct branch *curr_branch;\n> +\n> +\tcurr_branch = branch_get(\"HEAD\");\n> +\tif (!curr_branch) {\n> +\t\tfprintf_ln(stderr, _(\"You are not currently on a branch.\"));\n> +\t\tfprintf_ln(stderr, _(\"Please specify which branch you want to rebase against.\"));\n> +\t\tfprintf_ln(stderr, _(\"See git-rebase(1) for details.\"));\n> +\t\tfprintf(stderr, \"\\n\");\n> +\t\tfprintf_ln(stderr, \"    git rebase <branch>\");\n> +\t\tfprintf(stderr, \"\\n\");\n> +\t\texit(1);\n> +\t}\n\nUrgh. Elswhere we have _(\"Blabla\\nBlublub\\n\") constructs, which is already\na little bit ugly, but this mix of fprintf_ln() and fprintf() together\nwith adding a whopping 3 strings (for the price of 1) for the translators\n(and missing one...) is too ugly for my taste.\n\nAlso, there is a horrible, horrible, horrible exit(1) there. I know, you\nput this into builtin/ and so we assume it is okay to just exit() left and\nright, but *why*? Is this not a function we might want to reuse elsewhere?\nAs such, it should live in remote.[ch], take a \"hint\" parameter in case\nthere is no current branch (and BTW \"HEAD\" should not be hard-coded to\nbegin with, but instead be another parameter), and it should return -1 on\nerror, not exit.\n\n> +\n> +\tupstream_name = branch_get_upstream(curr_branch, NULL);\n> +\tif (!upstream_name) {\n> +\t\tconst char *remote_name = NULL;\n> +\n> +\t\tif (for_each_remote(get_only_remote, &remote_name) || !remote_name)\n> +\t\t\tremote_name = \"<remote>\";\n> +\n> +\t\tfprintf_ln(stderr, _(\"There is no tracking information for the current branch.\"));\n> +\t\tfprintf_ln(stderr, _(\"Please specify which branch you want to rebase against.\"));\n> +\t\tfprintf_ln(stderr, _(\"See git-rebase(1) for details.\"));\n> +\t\tfprintf(stderr, \"\\n\");\n> +\t\tfprintf_ln(stderr, \"    git rebase <branch>\");\n> +\t\tfprintf(stderr, \"\\n\");\n> +\t\tfprintf_ln(stderr, _(\"If you wish to set tracking information for this branch you can do so with:\"));\n> +\t\tfprintf(stderr, \"\\n\");\n> +\t\tfprintf_ln(stderr, _(\"If you wish to set tracking information for this branch you can do so with:\\n\"\n> +\t\t\"\\n\"\n> +\t\t\"    git branch --set-upstream-to=%s/<branch> %s\\n\"),\n> +\t\tremote_name, curr_branch->name);\n> +\t\texit(1);\n> +\t}\n\nSame here. The rebase-specific part of the hint should be a parameter, the\nthing should not die at all, and it really wants to live in remote.[ch].\n\n> +/**\n> + * Given the --onto <name>, return the onto hash\n> + */\n> +static void get_onto_oid(const char *_onto_name, struct object_id *onto)\n> +{\n> +\tchar *onto_name = xstrdup(_onto_name);\n\nBy convention, variable names starting with an underscore are reserved for\nuse by the standard library.\n\n> +\tstruct commit *onto_commit;\n> +\tchar *dotdot;\n> +\n> +\tdotdot = strstr(onto_name, \"...\");\n> +\tif (dotdot) {\n> +\t\tconst char *left = onto_name;\n> +\t\tconst char *right = dotdot + 3;\n> +\t\tstruct commit *left_commit, *right_commit;\n> +\t\tstruct commit_list *merge_bases;\n> +\n> +\t\t*dotdot = 0;\n> +\t\tif (!*left)\n> +\t\t\tleft = \"HEAD\";\n> +\t\tif (!*right)\n> +\t\t\tright = \"HEAD\";\n> +\n> +\t\t/* git merge-base --all $left $right */\n> +\t\tleft_commit = lookup_commit_reference_by_name(left);\n> +\t\tright_commit = lookup_commit_reference_by_name(right);\n> +\t\tif (!left_commit || !right_commit)\n> +\t\t\tdie(_(\"%s: there is no merge base\"), _onto_name);\n> +\n> +\t\tmerge_bases = get_merge_bases(left_commit, right_commit);\n> +\t\tif (merge_bases && merge_bases->next)\n> +\t\t\tdie(_(\"%s: there are more than one merge bases\"), _onto_name);\n> +\t\telse if (!merge_bases)\n> +\t\t\tdie(_(\"%s: there is no merge base\"), _onto_name);\n> +\n> +\t\tonto_commit = merge_bases->item;\n> +\t\tfree_commit_list(merge_bases);\n> +\t} else {\n> +\t\tonto_commit = lookup_commit_reference_by_name(onto_name);\n> +\t\tif (!onto_commit)\n> +\t\t\tdie(_(\"invalid upstream %s\"), onto_name);\n> +\t}\n> +\n> +\tfree(onto_name);\n> +\toidcpy(onto, &onto_commit->object.oid);\n> +}\n\nA lot of this looks *awfully* like the parameters we throw at rev-list (or\nfor that matter, log). Why can't we reuse that machinery?\n\n> @@ -12,20 +118,96 @@ static int git_rebase_config(const char *k, const char *v, void *cb)\n>  \n>  int cmd_rebase(int argc, const char **argv, const char *prefix)\n>  {\n> +\tstruct rebase_options rebase_opts;\n> +\tconst char *onto_name = NULL;\n> +\tconst char *branch_name;\n> +\n>  \tconst char * const usage[] = {\n> -\t\tN_(\"git rebase [options]\"),\n> +\t\tN_(\"git rebase [options] [--onto <newbase>] [<upstream>] [<branch>]\"),\n>  \t\tNULL\n>  \t};\n>  \tstruct option options[] = {\n> +\t\tOPT_GROUP(N_(\"Available options are\")),\n> +\t\tOPT_STRING(0, \"onto\", &onto_name, NULL,\n> +\t\t\tN_(\"rebase onto given branch instead of upstream\")),\n>  \t\tOPT_END()\n>  \t};\n>  \n>  \tgit_config(git_rebase_config, NULL);\n> +\trebase_options_init(&rebase_opts);\n> +\trebase_opts.resolvemsg = _(\"\\nWhen you have resolved this problem, run \\\"git rebase --continue\\\".\\n\"\n> +\t\t\t\"If you prefer to skip this patch, run \\\"git rebase --skip\\\" instead.\\n\"\n> +\t\t\t\"To check out the original branch and stop rebasing, run \\\"git rebase --abort\\\".\");\n>  \n>  \targc = parse_options(argc, argv, prefix, options, usage, 0);\n>  \n>  \tif (read_cache_preload(NULL) < 0)\n>  \t\tdie(_(\"failed to read the index\"));\n>  \n> +\t/*\n> +\t * Parse command-line arguments:\n> +\t *    rebase [<options>] [<upstream_name>] [<branch_name>]\n> +\t */\n> +\n> +\t/* Parse <upstream_name> into rebase_opts.upstream */\n> +\t{\n\nIn Git, unless there are very compelling reasons, we avoid non-conditional\nblocks. Probably you did that to have this local declaration:\n\n> +\t\tconst char *upstream_name;\n\nBut that declaration can easily live in the cmd_rebase() scope,\nsimplifying the code and being easier on the reader's eyes.\n\n> +\t/*\n> +\t * Parse --onto <onto_name> into rebase_opts.onto and\n> +\t * rebase_opts.onto_name\n> +\t */\n> +\tget_onto_oid(onto_name, &rebase_opts.onto);\n> +\trebase_opts.onto_name = xstrdup(onto_name);\n\nMy, this onto_name() sure gets strdup()ed a lot... Maybe we can avoid\nthat?\n\n> +\t/*\n> +\t * Parse <branch_name> into rebase_opts.orig_head and\n> +\t * rebase_opts.orig_refname\n> +\t */\n> +\tbranch_name = argv[0];\n> +\tif (branch_name) {\n\nIn Git's source code, we appear to rely on argc instead on argv[argc]\nbeing NULL.\n\n> +\t\t/* Is branch_name a branch or commit? */\n> +\t\tchar *ref_name = xstrfmt(\"refs/heads/%s\", branch_name);\n> +\t\tstruct object_id orig_head_id;\n> +\n> +\t\tif (!read_ref(ref_name, orig_head_id.hash)) {\n> +\t\t\trebase_opts.orig_refname = ref_name;\n> +\t\t\tif (get_oid_commit(ref_name, &rebase_opts.orig_head))\n> +\t\t\t\tdie(\"get_sha1_commit failed\");\n> +\t\t} else if (!get_oid_commit(branch_name, &rebase_opts.orig_head)) {\n> +\t\t\trebase_opts.orig_refname = NULL;\n> +\t\t\tfree(ref_name);\n> +\t\t} else {\n> +\t\t\tdie(_(\"no such branch: %s\"), branch_name);\n> +\t\t}\n\nHere, ref_name does not get free()d. It lives on as\nrebase_opts.orig_refname but it gets increasingly fiddly to reason about\nthe correctness of the code.\n\nA better idea would be to leave the responsibility of keeping track\ncompletely with the caller, i.e. have the fields of the options struct as\nconst char *. Then you can make the values strbufs as needed and in the\ncase of a builtin that exits anyway, you do not even need to release in\nthe end.\n\n> diff --git a/rebase-common.c b/rebase-common.c\n\nAs pointed out elsewhere, it is not a good idea to put stuff used by the\nrebase into rebase-common.c. Either it is so specific to rebase that it\ncan go into rebase.c, or it is so not specific to rebase that it can go\ninto path.c, wt-status.c, diff.c etc\n\nCiao,\nJohannes\n"},{"id":"280773","messageId":"CACsJy8BOZsPcEgOLiBo4u4SAEzDVmFd_XgU3yq4P+rBGRyJx8w@mail.gmail.com","threadId":"41678","inReplyTo":"alpine.DEB.2.20.1603142151230.4690@virtualbox","subject":"Re: [PATCH/RFC/GSoC 09/17] rebase-common: implement cache_has_unstaged_changes()","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-03-15T11:07:35Z","receivedAt":"2016-03-15T11:07:35Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Mar 15, 2016 at 3:54 AM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> In my 'interactive-rebase' branch...\n\n64 commits! Maybe next time we should announce wip branches like this\nwhen we start doing stuff so people don't overlap. Of course these\nbranches do not have to be perfect (and can be force pushed from time\nto time, even).\n-- \nDuy\n"},{"id":"280775","messageId":"alpine.DEB.2.20.1603151249570.4690@virtualbox","threadId":"41678","inReplyTo":"xmqqoaag9177.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH/RFC/GSoC 09/17] rebase-common: implement cache_has_unstaged_changes()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-03-15T11:51:29Z","receivedAt":"2016-03-15T11:51:29Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Mon, 14 Mar 2016, Junio C Hamano wrote:\n\n> If the [has_uncommitted_changes()] function will be made a public helper\n> that may be called by anybody, a possible error reporting mechanism\n> would be to give a list of modified paths to the caller that asks them,\n> and have the caller apply its own \"prefix\" processing to make them\n> relative.\n\nBut the point of the has_uncommitted_changes() is to figure out as fast as\npossible whether there are uncommitted changes, not the exact list. In\nfact, we want to return with a \"yes\" as soon as we encounter the first\nuncommitted change.\n\nCiao,\nDscho\n"},{"id":"280782","messageId":"alpine.DEB.2.20.1603151515030.4690@virtualbox","threadId":"41678","inReplyTo":"CACsJy8BOZsPcEgOLiBo4u4SAEzDVmFd_XgU3yq4P+rBGRyJx8w@mail.gmail.com","subject":"Re: [PATCH/RFC/GSoC 09/17] rebase-common: implement cache_has_unstaged_changes()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-03-15T14:15:46Z","receivedAt":"2016-03-15T14:15:46Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Duy,\n\nOn Tue, 15 Mar 2016, Duy Nguyen wrote:\n\n> On Tue, Mar 15, 2016 at 3:54 AM, Johannes Schindelin\n> <Johannes.Schindelin@gmx.de> wrote:\n> > In my 'interactive-rebase' branch...\n> \n> 64 commits! Maybe next time we should announce wip branches like this\n> when we start doing stuff so people don't overlap. Of course these\n> branches do not have to be perfect (and can be force pushed from time\n> to time, even).\n\nMuch of this is cleanup in the beginning. And I tried to split out a patch\nI thought was uncontentious, and promptly ran into a philosophical\ndiscussion I did not seek ;-)\n\nCiao,\nDscho\n"},{"id":"280806","messageId":"CACRoPnRhhMM0e3S23KVnEANwNRDPq0P3hSqn5Zs1ksZxeaAoiA@mail.gmail.com","threadId":"41678","inReplyTo":"alpine.DEB.2.20.1603150800420.4690@virtualbox","subject":"Re: [PATCH/RFC/GSoC 17/17] rebase-interactive: introduce interactive backend for builtin rebase","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2016-03-15T16:48:27Z","receivedAt":"2016-03-15T16:48:27Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi Dscho,\n\nOn Tue, Mar 15, 2016 at 3:57 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> On Sat, 12 Mar 2016, Paul Tan wrote:\n>\n>> Since 1b1dce4 (Teach rebase an interactive mode, 2007-06-25), git-rebase\n>> supports an interactive mode when passed the -i switch.\n>>\n>> In interactive mode, git-rebase allows users to edit the list of patches\n>> (using the user's GIT_SEQUENCE_EDITOR), so that the user can reorder,\n>> edit and delete patches.\n>>\n>> Re-implement a skeletal version of the above feature by introducing a\n>> rebase-interactive backend for our builtin-rebase. This skeletal\n>> implementation is only able to pick and re-order commits.\n>>\n>> Signed-off-by: Paul Tan <pyokagan@gmail.com>\n>\n> It is a pity that both of us worked on overlapping projects in stealth\n> mode. Inevitably, some of the work is now wasted :-(\n\nNo worries, I did this series for my own interest, especially to get a\ngauge of the speedup between rebase in shell and C. GSoC applications\nhave opened and will close in 10 days time, so I wanted to get some\ndata before the deadline at least :-).\n\n> Not all is lost, though.\n>\n> Much of the code can be salvaged, although I really want to reiterate\n> that an all-or-nothing conversion of the rebase command is not going to\n> fly.\n\nSure. I admit that I concentrated more on how the \"final code\" would\nlook like, and not so much how the rewrite would be built upon in\npieces.\n\n> For several reasons: it would be rather disruptive, huge and hard to\n> review. It would not let anybody else work on that huge task. And you're\n> prone to fall behind due to Git's source code being in constant flux\n> (including the rebase bits).\n>\n> There is another, really important reason: if you package the conversion\n> into small, neat bundles, it is much easier to avoid too narrow a focus\n> that would tuck perfectly useful functions away in a location where it\n> cannot be reused and where it is likely to be missed by other developers\n> who need the same, or similar functionality (point in case:\n> has_uncommitted_changes()). And we know that this happened in the past,\n> and sometimes resulted in near-duplicated code, hence Karthik's Herculean,\n> still ongoing work.\n>\n> Lastly, I need to point out that the conversion of rebase into a builtin\n> is not the end game, it is the game's opening.\n>\n> [...]\n>\n> So you see, there was a much larger master plan behind my recommendation\n> to go the rebase--helper route.\n\nAh I see, thanks for publishing your branch and sharing your plans.\n\nOriginally I was thinking smaller -- rewrite git-rebase first,\nfollowing its shell script closely, and then doing the libification\nand optimization after that. However, I see now that you have grander\nplans than that :-).\n\n>\n> As to my current state: Junio put me into quite a fix (without knowing it)\n> by releasing 2.7.3 just after I took off for an extended offline weekend,\n> and now I am scrambling because a change in MSYS2's runtime (actually,\n> probably more like: an update of the GCC that is used to compile the\n> runtime, that now causes a regression) is keeping me away from my work on\n> the interactive rebase. Even so, I am pretty far along; There are only\n> three major things left to do: 1) fix fixups/squashes with fast-forwarding\n> picks, 2) implement 'reword', 3) display the progress.  And of course 4)\n> clean up the fallout. ;-)\n>\n> At this point, I'd rather finish this myself than falling prey to Brooks'\n> Law.\n\nOkay, I won't touch interactive rebase then.\n\n> I also have to admit that I would love to give you a project over the\n> summer whose logical children are exciting enough to dabble with even\n> during the winter. And somehow I do not see that excitement in the boring\n> conversion from shell to C (even if its outcome is well-needed).\n\nWell, that is subjective ;-).\n\nEven with interactive rebase out-of-bounds, I don't think it's a dead\nend though:\n\n1. git-rebase--am.sh, git-rebase--merge.sh and git-rebase.sh can be\nrewritten to C, and call git-rebase--interactive.sh externally, like\nwhat Duy demonstrated in his patch series. The timings show that am\nand merge rebase still benefit, and that way we will be closer to a\ngit-rebase in full C.\n\n2. git-commit can be libified, so that we can access its functionality\ndirectly. (sequencer.c runs it once per commit, rebase-interactive\nuses it for squashes etc.)\n\nOr would that be stepping on your toes?\n\n> Ciao,\n> Dscho\n>\n> Footnote *1*:\n> https://github.com/git-for-windows/build-extra/blob/master/shears.sh\n\nRegards,\nPaul\n"},{"id":"280829","messageId":"alpine.DEB.2.20.1603152041090.4690@virtualbox","threadId":"41678","inReplyTo":"CACRoPnRhhMM0e3S23KVnEANwNRDPq0P3hSqn5Zs1ksZxeaAoiA@mail.gmail.com","subject":"Re: [PATCH/RFC/GSoC 17/17] rebase-interactive: introduce interactive backend for builtin rebase","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-03-15T19:45:11Z","receivedAt":"2016-03-15T19:45:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Paul,\n\nOn Wed, 16 Mar 2016, Paul Tan wrote:\n\n> Even with interactive rebase out-of-bounds,\n\nIt is not really \"out-of-bounds\". It's more like: hold off integrating it\nuntil I'm done with v1 of the rebase--helper that does interactive\nrebase's heavy lifting (which should happen pretty soon).\n\n> I don't think it's a dead end though:\n> \n> 1. git-rebase--am.sh, git-rebase--merge.sh and git-rebase.sh can be\n> rewritten to C, and call git-rebase--interactive.sh externally, like\n> what Duy demonstrated in his patch series. The timings show that am\n> and merge rebase still benefit, and that way we will be closer to a\n> git-rebase in full C.\n> \n> 2. git-commit can be libified, so that we can access its functionality\n> directly. (sequencer.c runs it once per commit, rebase-interactive\n> uses it for squashes etc.)\n\nBoth look sound. With 1) I would also suggest a rebase--helper approach,\nthough. That way, you do not need to break the test suite at all but can\nflip the switch at the end of the patch series.\n\n> Or would that be stepping on your toes?\n\nNope. 2) is related to what I do, but I modified sequencer.c's\nrun_git_commit() (which uses run_command()). Your plan would make that\neven better (although the edit phase is of course not the\nperformance-critical part that we want to enhance).\n\nCiao,\nDscho\n"},{"id":"280877","messageId":"alpine.DEB.2.20.1603160855390.4690@virtualbox","threadId":"41678","inReplyTo":"1457779597-6918-2-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH/RFC/GSoC 01/17] perf: introduce performance tests for git-rebase","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-03-16T07:58:28Z","receivedAt":"2016-03-16T07:58:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Paul,\n\nOn Sat, 12 Mar 2016, Paul Tan wrote:\n\n> diff --git a/t/perf/p3404-rebase-interactive.sh b/t/perf/p3404-rebase-interactive.sh\n> new file mode 100755\n> index 0000000..aaca105\n> --- /dev/null\n> +++ b/t/perf/p3404-rebase-interactive.sh\n> @@ -0,0 +1,26 @@\n>\n> [...]\n>\n> +test_perf 'rebase -i --onto master^' '\n> +\tgit checkout perf-topic-branch &&\n> +\tgit reset --hard perf-topic-branch-initial &&\n> +\tGIT_SEQUENCE_EDITOR=: git rebase -i --onto master^ master\n> +'\n\nThis measures the performance of checkout && reset && rebase -i. Maybe we\nshould only test rebase -i?\n\nAlso, I would strongly recommend an extra test_commit after reset;\nOtherwise you would only test the logic that verifies that it can simply\nfast-forward instead of cherry-picking.\n\nCiao,\nDscho\n"},{"id":"280878","messageId":"alpine.DEB.2.20.1603160901520.4690@virtualbox","threadId":"41678","inReplyTo":"CAGZ79kYeYzi=J=dY27FqXp72BRe-Vmm4MR5Q6dFTMUP9CxYZcg@mail.gmail.com","subject":"Re: [PATCH/RFC/GSoC 05/17] rebase-options: implement rebase_options_load() and rebase_options_save()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-03-16T08:04:21Z","receivedAt":"2016-03-16T08:04:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Paul,\n\nOn Mon, 14 Mar 2016, Stefan Beller wrote:\n\n> On Sat, Mar 12, 2016 at 2:46 AM, Paul Tan <pyokagan@gmail.com> wrote:\n> > These functions can be used for loading and saving common rebase options\n> > into a state directory.\n> >\n> > Signed-off-by: Paul Tan <pyokagan@gmail.com>\n> > ---\n> >  rebase-common.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n> >  rebase-common.h |  4 ++++\n> >  2 files changed, 73 insertions(+)\n> >\n> > diff --git a/rebase-common.c b/rebase-common.c\n> > index 5a49ac4..1835f08 100644\n> > --- a/rebase-common.c\n> > +++ b/rebase-common.c\n> > @@ -26,3 +26,72 @@ void rebase_options_swap(struct rebase_options *dst, struct rebase_options *src)\n> >         *dst = *src;\n> >         *src = tmp;\n> >  }\n> > +\n> > +static int state_file_exists(const char *dir, const char *file)\n> > +{\n> > +       return file_exists(mkpath(\"%s/%s\", dir, file));\n> > +}\n> \n> How is this specific to the state file? All it does is create the\n> leading directory\n> if it doesn't exist? (So I'd expect file_exists(concat(dir, file)) to\n> have the same\n> result without actually creating the directory if it doesn't exist as\n> a side effect?\n> \n> If the dir doesn't exist it can be created in rebase_options_load explicitly?\n\nIn addition I want to point out that sequencer's replay_opts seem to be at\nleast related, but the patch shares none of its code with the sequencer.\nLet's avoid that.\n\nIn other words, let's try to add as little code as possible when we can\nenhance existing code.\n\nCiao,\nDscho\n"},{"id":"280888","messageId":"CACRoPnS=qg=a3xYKHyk-7E2HN5HhTimGirZcwL8hMa0xLY6KdA@mail.gmail.com","threadId":"41678","inReplyTo":"alpine.DEB.2.20.1603160855390.4690@virtualbox","subject":"Re: [PATCH/RFC/GSoC 01/17] perf: introduce performance tests for git-rebase","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2016-03-16T11:51:53Z","receivedAt":"2016-03-16T11:51:53Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi Dscho,\n\nOn Wed, Mar 16, 2016 at 3:58 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi Paul,\n>\n> On Sat, 12 Mar 2016, Paul Tan wrote:\n>\n>> diff --git a/t/perf/p3404-rebase-interactive.sh b/t/perf/p3404-rebase-interactive.sh\n>> new file mode 100755\n>> index 0000000..aaca105\n>> --- /dev/null\n>> +++ b/t/perf/p3404-rebase-interactive.sh\n>> @@ -0,0 +1,26 @@\n>>\n>> [...]\n>>\n>> +test_perf 'rebase -i --onto master^' '\n>> +     git checkout perf-topic-branch &&\n>> +     git reset --hard perf-topic-branch-initial &&\n>> +     GIT_SEQUENCE_EDITOR=: git rebase -i --onto master^ master\n>> +'\n>\n> This measures the performance of checkout && reset && rebase -i. Maybe we\n> should only test rebase -i?\n\ntest_perf runs the same script multiple times, so we need to reset\n--hard at least to undo the changes of the rebase.\n\nI think we can remove the reset if we use rebase -f and rebase onto\nthe same base, but -f was not implemented in this patch series.\n\n> Also, I would strongly recommend an extra test_commit after reset;\n> Otherwise you would only test the logic that verifies that it can simply\n> fast-forward instead of cherry-picking.\n\nOr, we could use the -f flag, I think.\n\nThanks,\nPaul\n"},{"id":"280890","messageId":"CACRoPnS7WWWVay9hAjXYgyeB=1A1gfARerKJe25APa-6u5cGaA@mail.gmail.com","threadId":"41678","inReplyTo":"CAGZ79kYeYzi=J=dY27FqXp72BRe-Vmm4MR5Q6dFTMUP9CxYZcg@mail.gmail.com","subject":"Re: [PATCH/RFC/GSoC 05/17] rebase-options: implement rebase_options_load() and rebase_options_save()","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2016-03-16T12:04:07Z","receivedAt":"2016-03-16T12:04:07Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi Stefan,\n\nOn Tue, Mar 15, 2016 at 4:30 AM, Stefan Beller <sbeller@google.com> wrote:\n> On Sat, Mar 12, 2016 at 2:46 AM, Paul Tan <pyokagan@gmail.com> wrote:\n>> These functions can be used for loading and saving common rebase options\n>> into a state directory.\n>>\n>> Signed-off-by: Paul Tan <pyokagan@gmail.com>\n>> ---\n>>  rebase-common.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n>>  rebase-common.h |  4 ++++\n>>  2 files changed, 73 insertions(+)\n>>\n>> diff --git a/rebase-common.c b/rebase-common.c\n>> index 5a49ac4..1835f08 100644\n>> --- a/rebase-common.c\n>> +++ b/rebase-common.c\n>> @@ -26,3 +26,72 @@ void rebase_options_swap(struct rebase_options *dst, struct rebase_options *src)\n>>         *dst = *src;\n>>         *src = tmp;\n>>  }\n>> +\n>> +static int state_file_exists(const char *dir, const char *file)\n>> +{\n>> +       return file_exists(mkpath(\"%s/%s\", dir, file));\n>> +}\n>\n> How is this specific to the state file? All it does is create the\n> leading directory\n> if it doesn't exist? (So I'd expect file_exists(concat(dir, file)) to\n> have the same\n> result without actually creating the directory if it doesn't exist as\n> a side effect?\n\nI don't quite understand, AFAIK mkpath() does not create any\ndirectories as a side-effect. And yes, I just wanted a short way to\nsay file_exists(concat(dir, file)) or file_exists(mkpath(\"%s/%s\", dir,\nfile)) without cluttering up the code.\n\n> If the dir doesn't exist it can be created in rebase_options_load explicitly?\n\nI don't intend to create any directories if they do not exist.\n\n>> +\n>> +static int read_state_file(struct strbuf *sb, const char *dir, const char *file)\n>> +{\n>> +       const char *path = mkpath(\"%s/%s\", dir, file);\n>> +       strbuf_reset(sb);\n>> +       if (strbuf_read_file(sb, path, 0) >= 0)\n>> +               return sb->len;\n>> +       else\n>> +               return error(_(\"could not read '%s'\"), path);\n>> +}\n>> +\n>> +int rebase_options_load(struct rebase_options *opts, const char *dir)\n>> +{\n>> +       struct strbuf sb = STRBUF_INIT;\n>> +       const char *filename;\n>> +\n>> +       /* opts->orig_refname */\n>> +       if (read_state_file(&sb, dir, \"head-name\") < 0)\n>> +               return -1;\n>> +       strbuf_trim(&sb);\n>> +       if (starts_with(sb.buf, \"refs/heads/\"))\n>> +               opts->orig_refname = strbuf_detach(&sb, NULL);\n>> +       else if (!strcmp(sb.buf, \"detached HEAD\"))\n>> +               opts->orig_refname = NULL;\n>> +       else\n>> +               return error(_(\"could not parse %s\"), mkpath(\"%s/%s\", dir, \"head-name\"));\n>> +\n>> +       /* opts->onto */\n>> +       if (read_state_file(&sb, dir, \"onto\") < 0)\n>> +               return -1;\n>> +       strbuf_trim(&sb);\n>> +       if (get_oid_hex(sb.buf, &opts->onto) < 0)\n>> +               return error(_(\"could not parse %s\"), mkpath(\"%s/%s\", dir, \"onto\"));\n>> +\n>> +       /*\n>> +        * We always write to orig-head, but interactive rebase used to write\n>> +        * to head. Fall back to reading from head to cover for the case that\n>> +        * the user upgraded git with an ongoing interactive rebase.\n>> +        */\n>> +       filename = state_file_exists(dir, \"orig-head\") ? \"orig-head\" : \"head\";\n>> +       if (read_state_file(&sb, dir, filename) < 0)\n>> +               return -1;\n>\n> So from here on we always use \"orig-head\" instead of \"head\" for\n> interactive rebase.\n> Would people ever rely on the (internal) file name and have e.g.\n> scripts which operate\n> on the \"head\" file ?\n\nThis backwards-compatibility code is just a straight port from the\ncode in git-rebase.sh.\n\nThe usage of orig-head has been around since 2011 with 84df456\n(rebase: extract code for writing basic state, 2011-02-06), so I guess\nif people had issues with it, it would have been reported.\n\n>\n>\n>> +       strbuf_trim(&sb);\n>> +       if (get_oid_hex(sb.buf, &opts->orig_head) < 0)\n>> +               return error(_(\"could not parse %s\"), mkpath(\"%s/%s\", dir, filename));\n>> +\n>> +       strbuf_release(&sb);\n>> +       return 0;\n>> +}\n>> +\n>> +static int write_state_text(const char *dir, const char *file, const char *string)\n>> +{\n>> +       return write_file(mkpath(\"%s/%s\", dir, file), \"%s\", string);\n>> +}\n>\n> Same comment as on checking the state files existence. I'm not sure if the side\n> effect of creating the dir is better done explicitly where it is used.\n> The concat of dir and\n> file name can still be done in the helper though? (If the helper is\n> needed at all then)\n\nSame as above -- AFAIK I don't think mkpath() creates any directories\nas a side-effect.\n\n>\n>> +\n>> +void rebase_options_save(const struct rebase_options *opts, const char *dir)\n>> +{\n>> +       const char *head_name = opts->orig_refname;\n>> +       if (!head_name)\n>> +               head_name = \"detached HEAD\";\n>> +       write_state_text(dir, \"head-name\", head_name);\n>> +       write_state_text(dir, \"onto\", oid_to_hex(&opts->onto));\n>> +       write_state_text(dir, \"orig-head\", oid_to_hex(&opts->orig_head));\n>> +}\n>> diff --git a/rebase-common.h b/rebase-common.h\n>> index db5146a..051c056 100644\n>> --- a/rebase-common.h\n>> +++ b/rebase-common.h\n>> @@ -20,4 +20,8 @@ void rebase_options_release(struct rebase_options *);\n>>\n>>  void rebase_options_swap(struct rebase_options *dst, struct rebase_options *src);\n>>\n>> +int rebase_options_load(struct rebase_options *, const char *dir);\n>> +\n>> +void rebase_options_save(const struct rebase_options *, const char *dir);\n>> +\n>>  #endif /* REBASE_COMMON_H */\n>> --\n>> 2.7.0\n\nThanks,\nPaul\n"},{"id":"280891","messageId":"CACRoPnS4JpNNACz4T0F0vFs3ogG+nzk-y1=zc1UrtAZKaEnggg@mail.gmail.com","threadId":"41678","inReplyTo":"alpine.DEB.2.20.1603160901520.4690@virtualbox","subject":"Re: [PATCH/RFC/GSoC 05/17] rebase-options: implement rebase_options_load() and rebase_options_save()","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2016-03-16T12:28:56Z","receivedAt":"2016-03-16T12:28:56Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi Dscho,\n\nOn Wed, Mar 16, 2016 at 4:04 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> In addition I want to point out that sequencer's replay_opts seem to be at\n> least related, but the patch shares none of its code with the sequencer.\n> Let's avoid that.\n>\n> In other words, let's try to add as little code as possible when we can\n> enhance existing code.\n\nWell, both git-rebase--am.sh and git-rebase--merge.sh do not use the\nsequencer functionality at all, and we don't see git-am for example\nneeding to be aware of onto, orig-head, head-name etc.\n\nBesides, I don't see why the sequencer needs to be aware of these\nrebase-specific options. For simplicity[1], I would think the\nsequencer would only need to be aware of what the todo list is, since\nthat is common to cherry-pick/revert and rebase-i, and all the other\nnon-sequencer related stuff like checking out the --onto <newbase>,\nupdating refs can be done at the rebase-interactive.c or\ngit-rebase--interactive.sh layer.\n\n[1] Of course, it's kind of unfortunate that sequencer.c has to be\naware of whether it is being called as cherry-pick or revert, but I\ndon't see why implementing interactive rebase functionality needs to\nmake the same mistake.\n\nThanks,\nPaul\n"},{"id":"280892","messageId":"CACRoPnTpHR7Bx9TVAK-dTFgSOj2XVk3F8ApBEcywxESDQUS8VA@mail.gmail.com","threadId":"41678","inReplyTo":"xmqq8u1kaoj8.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH/RFC/GSoC 00/17] A barebones git-rebase in C","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2016-03-16T12:46:04Z","receivedAt":"2016-03-16T12:46:04Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"On Tue, Mar 15, 2016 at 2:43 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Duy Nguyen <pclouds@gmail.com> writes:\n>\n>> On Sat, Mar 12, 2016 at 5:46 PM, Paul Tan <pyokagan@gmail.com> wrote:\n>>> So, we have around a 1.4x-1.8x speedup for Linux users, and a 1.7x-13x speedup\n>>> for Windows users. The annoying long delay before the interactive editor is\n>>> launched on Windows is gotten rid of, which I'm very happy about :-)\n>>\n>> Nice numbers :-) Sorry I can't look at your patches yet. Just a very\n>> minor comment from diffstat..\n>>\n>>>  rebase-am.c                        | 110 +++++++++++\n>>>  rebase-am.h                        |  22 +++\n>>>  rebase-common.c                    | 220 ++++++++++++++++++++++\n>>>  rebase-common.h                    |  48 +++++\n>>>  rebase-interactive.c               | 375 +++++++++++++++++++++++++++++++++++++\n>>>  rebase-interactive.h               |  33 ++++\n>>>  rebase-merge.c                     | 256 +++++++++++++++++++++++++\n>>>  rebase-merge.h                     |  28 +++\n>>>  rebase-todo.c                      | 251 +++++++++++++++++++++++++\n>>>  rebase-todo.h                      |  55 ++++++\n>>\n>> topdir is already very crowded. Maybe you could move all these files\n>> to \"rebase\" subdir.\n>\n> I think that makes sense.  I do not expect people depending on being\n> able to say \"git rebase--am\" and have it do something useful, so\n> they won't belong to builtin/, but rebase/{am,common,...}.[ch] makes\n> sense.\n\nSure, I'll do that.\n\nRegards,\nPaul\n"},{"id":"280894","messageId":"CACRoPnRH1D=8k5uuUahh1MJOAXsWkhY0fWev2AQhJm5+WWk5rQ@mail.gmail.com","threadId":"41678","inReplyTo":"CAP8UFD0Fw1ZOQzPfF=bbEsCOhkoHfV5B5ayprxR6kWr6vApT5Q@mail.gmail.com","subject":"Re: [PATCH/RFC/GSoC 12/17] rebase-todo: introduce rebase_todo_item","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2016-03-16T12:54:55Z","receivedAt":"2016-03-16T12:54:55Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi Christian,\n\nOn Mon, Mar 14, 2016 at 9:43 PM, Christian Couder\n<christian.couder@gmail.com> wrote:\n> On Sat, Mar 12, 2016 at 11:46 AM, Paul Tan <pyokagan@gmail.com> wrote:\n>> In an interactive rebase, commands are read and executed from a todo\n>> list (.git/rebase-merge/git-rebase-todo) to perform the rebase.\n>>\n>> In the upcoming re-implementation of git-rebase -i in C, it is useful to\n>> be able to parse each command into a data structure which can then be\n>> operated on. Implement rebase_todo_item for this.\n>\n> sequencer.{c,h} already has some code to parse and create todo lists\n> for cherry-picking or reverting multiple commits, so I am wondering if\n> it would be possible to share some code?\n\nAFAIK, sequencer.c as it is in master parses the todo list\ndestructively and does not keep the associated action for each commit\nand the \"rest\" string. interactive rebase does keep those, so I needed\na different data structure rather than the one currently being used in\nsequencer.c.\n\nAs I said in another thread, originally I wanted to keep the scope\nsimple, and just do the rewrite of rebase from C to shell, and let any\nfurther libifications and optimizations come after, so I didn't want\nto touch sequencer for now. However, it turns out that Dscho has grand\nplans for the unification of sequencer and interactive rebase, so I'll\ngo with that :-)\n\nRegards,\nPaul\n"},{"id":"280895","messageId":"CACRoPnS1VikcT3qutXTT5SMLLHeo87M_wzqVKvYG02X7HUCZcw@mail.gmail.com","threadId":"41678","inReplyTo":"xmqqshzs9369.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH/RFC/GSoC 07/17] rebase-common: implement refresh_and_write_cache()","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2016-03-16T12:56:09Z","receivedAt":"2016-03-16T12:56:09Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"On Tue, Mar 15, 2016 at 5:10 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Paul Tan <pyokagan@gmail.com> writes:\n>\n>> In the upcoming git-rebase to C rewrite, it is a common operation to\n>> refresh the index and write the resulting index.\n>>\n>> builtin/am.c already implements refresh_and_write_cache(), which is what\n>> we want. Move it to rebase-common.c, so that it can be shared with all\n>> the rebase backends, including git-am.\n>\n> Your rebase-am might be one of the rebase backends, but git-am is\n> not, so it is misleading to count it among \"all the rebase\n> backends\".\n>\n> I would think that a better home for refresh_and_write_index() is\n> right next to write_locked_index(), with #define in cache.h for\n> refresh_and_write_cache(), just like others.\n\nOkay, thanks for suggesting a better location.\n\nRegards,\nPaul\n"},{"id":"280896","messageId":"CACRoPnRMOp38vfkQZjmkUqr+urN8NYcNN_oNzHtJqfyTorr1ug@mail.gmail.com","threadId":"41678","inReplyTo":"alpine.DEB.2.20.1603150755450.4690@virtualbox","subject":"Re: [PATCH/RFC/GSoC 16/17] editor: implement git_sequence_editor() and launch_sequence_editor()","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2016-03-16T13:06:09Z","receivedAt":"2016-03-16T13:06:09Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi Dscho,\n\nOn Tue, Mar 15, 2016 at 3:00 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> On Sat, 12 Mar 2016, Paul Tan wrote:\n>> ---\n>>  cache.h  |  1 +\n>\n> No need to clutter cache.h with a function that is only to be used by the\n> sequencer. IOW let's make this static in sequencer.c.\n\nThe function needs to be implemented in editor.c because it would be\nbetter for it to share the same code that launch_editor() uses\n(implemented in this patch by splitting the logic into a static\nfunction launch_specific_editor())\n\nWe could move the declaration to sequencer.h though.\n\n> I would also prefer pairing this short function with the change that\n> actually uses it (in my topic branches, I like to compile with -Werror,\n> which would result in a failure due to an unused function), in the same\n> patch.\n\nSure.\n\nRegards,\nPaul\n"},{"id":"280897","messageId":"alpine.DEB.2.20.1603160905230.4690@virtualbox","threadId":"41678","inReplyTo":"1457779597-6918-7-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH/RFC/GSoC 06/17] rebase-am: introduce am backend for builtin rebase","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-03-16T13:21:16Z","receivedAt":"2016-03-16T13:21:16Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Paul,\n\nOn Sat, 12 Mar 2016, Paul Tan wrote:\n\n> diff --git a/builtin/rebase.c b/builtin/rebase.c\n> index 40176ca..ec63d3b 100644\n> --- a/builtin/rebase.c\n> +++ b/builtin/rebase.c\n> @@ -8,6 +8,22 @@\n>  #include \"remote.h\"\n>  #include \"branch.h\"\n>  #include \"refs.h\"\n> +#include \"rebase-am.h\"\n> +\n> +enum rebase_type {\n> +\tREBASE_TYPE_NONE = 0,\n> +\tREBASE_TYPE_AM\n> +};\n> +\n> +static const char *rebase_dir(enum rebase_type type)\n> +{\n> +\tswitch (type) {\n> +\tcase REBASE_TYPE_AM:\n> +\t\treturn git_path_rebase_am_dir();\n> +\tdefault:\n> +\t\tdie(\"BUG: invalid rebase_type %d\", type);\n> +\t}\n> +}\n\nThis is awfully close to what the sequencer sports. So close that I would\ncall it technical debt.\n\nIt would most likely result in easier-to-maintain source code if the\nsequencer and the rebase code shared as much as possible (in\nsequencer.[ch], for historical reasons).\n\n\n> diff --git a/rebase-am.c b/rebase-am.c\n> new file mode 100644\n> index 0000000..53e8798\n> --- /dev/null\n> +++ b/rebase-am.c\n> @@ -0,0 +1,110 @@\n> +#include \"cache.h\"\n> +#include \"rebase-am.h\"\n> +#include \"run-command.h\"\n> +\n> +GIT_PATH_FUNC(git_path_rebase_am_dir, \"rebase-apply\");\n> +\n> +void rebase_am_init(struct rebase_am *state, const char *dir)\n> +{\n> +\tif (!dir)\n> +\t\tdir = git_path_rebase_am_dir();\n> +\trebase_options_init(&state->opts);\n> +\tstate->dir = xstrdup(dir);\n> +}\n\nDoes it really make sense to have completely separate structs for the\ndifferent rebase types? I think not. It would not hurt IMO to have a\ncouple of fields that are only used for certain rebase types but not\nothers. The benefit of being able to reuse, code would far outweigh that\nminimal cost.\n\nIt all comes back to my favorite paradigm: DRY. Don't Repeat Yourself.\n\nAnother important paradigm is: avoid feautures that you do not use. In\nthis instance, I have to ask why the init function accepts the \"dir\"\nparameter? Do we ever need it? And if yes, would it make more sense to\nintroduce the parameter with the patch that actually uses it?\n\n> +\n> +void rebase_am_release(struct rebase_am *state)\n> +{\n> +\trebase_options_release(&state->opts);\n> +\tfree(state->dir);\n\nUrgh. The only reason for this free() and the corresponding xstrdup() is\nso that the caller *may* release the directory before finishing the rebase\n*if* it overrides the directory. That's not very elegant.\n\nWhy not simply state (by declaring the field as const char *) that it is\n*not* the rebase machinery's duty to take care of the memory management of\nthis string?\n\nThis would simplify the common code flow, especially if it was done to\n*all* strings in the state struct.\n\n> +int rebase_am_in_progress(const struct rebase_am *state)\n> +{\n> +\tconst char *dir = state ? state->dir : git_path_rebase_am_dir();\n> +\tstruct stat st;\n> +\n> +\treturn !lstat(dir, &st) && S_ISDIR(st.st_mode);\n> +}\n\nThis function is sobbing inconsolably for being stuck into the rebase-am\npart of the code, with a name that ensures that it will never grow up and\nbecome more useful. Between its miserable life, it dreams of being named\ndir_exists() and living the high life next to its buddy, file_exists().\n\n> +int rebase_am_load(struct rebase_am *state)\n> +{\n> +\tif (rebase_options_load(&state->opts, state->dir) < 0)\n> +\t\treturn -1;\n> +\n> +\treturn 0;\n> +}\n\n:-(\n\nThis looks like adding code for adding code's sake. Not only does it craft\nits own return value instead of reusing rebase_options_load()'s, it is\njust wrapping a single, simple statement, therefore its only use is to add\none layer of indirection.\n\n> +static int run_format_patch(const char *patches, const struct object_id *left,\n> +\t\tconst struct object_id *right)\n> +{\n> +\tstruct child_process cp = CHILD_PROCESS_INIT;\n> +\tint ret;\n> +\n> +\tcp.git_cmd = 1;\n> +\tcp.out = xopen(patches, O_WRONLY | O_CREAT, 0777);\n> +\targv_array_push(&cp.args, \"format-patch\");\n> +\targv_array_push(&cp.args, \"-k\");\n> +\targv_array_push(&cp.args, \"--stdout\");\n> +\targv_array_push(&cp.args, \"--full-index\");\n> +\targv_array_push(&cp.args, \"--cherry-pick\");\n> +\targv_array_push(&cp.args, \"--right-only\");\n> +\targv_array_push(&cp.args, \"--src-prefix=a/\");\n> +\targv_array_push(&cp.args, \"--dst-prefix=b/\");\n> +\targv_array_push(&cp.args, \"--no-renames\");\n> +\targv_array_push(&cp.args, \"--no-cover-letter\");\n> +\targv_array_pushf(&cp.args, \"%s...%s\", oid_to_hex(left), oid_to_hex(right));\n> +\n> +\tret = run_command(&cp);\n> +\tclose(cp.out);\n> +\treturn ret;\n> +}\n> +\n> +static int run_am(const struct rebase_am *state, const char *patches)\n> +{\n> +\tstruct child_process cp = CHILD_PROCESS_INIT;\n> +\tint ret;\n> +\n> +\tcp.git_cmd = 1;\n> +\tcp.in = xopen(patches, O_RDONLY);\n> +\targv_array_push(&cp.args, \"am\");\n> +\targv_array_push(&cp.args, \"--rebasing\");\n> +\tif (state->opts.resolvemsg)\n> +\t\targv_array_pushf(&cp.args, \"--resolvemsg=%s\", state->opts.resolvemsg);\n> +\n> +\tret = run_command(&cp);\n> +\tclose(cp.in);\n> +\treturn ret;\n> +}\n\nYeah, these functions really want to use libification for the full, raw\nspeed improvement that we are going for.\n\nAnd rather than following the shell script slavishly, we should *really*\nconsider doing better: the shell script cannot access Git's data\nstructures directly, therefore it has to use this roundabout way: format\npatches as a mailbox only to parse it back into the individual patches\nthat libgit.a already had available when it formatted the mailbox.\n\nThis consideration is pretty important: I do not think that the current\nfunction signatures are correct with that end goal in mind.\n\n> +void rebase_am_run(struct rebase_am *state)\n> +{\n> +\tchar *patches;\n> +\tint ret;\n> +\n> +\trebase_common_setup(&state->opts, state->dir);\n> +\n> +\tpatches = git_pathdup(\"rebased-patches\");\n> +\tret = run_format_patch(patches, &state->opts.upstream, &state->opts.orig_head);\n\nLet's wrap the lines at 80 columns/row.\n\n> +\tif (ret) {\n> +\t\tunlink_or_warn(patches);\n> +\t\tfprintf_ln(stderr, _(\"\\ngit encountered an error while preparing the patches to replay\\n\"\n> +\t\t\t\"these revisions:\\n\"\n\nAlso here, the common way to do this is:\n\n\t\tfprintf_ln(stderr, _(\"\\ngit encountered an error while \"\n\t\t\t\t\"preparing the patches to replay\\n\"\n\t\t\t\"these revisions:\\n\"\n\t\t\t[...]\n\n> diff --git a/rebase-common.c b/rebase-common.c\n> index 1835f08..8169fb6 100644\n> --- a/rebase-common.c\n> +++ b/rebase-common.c\n> @@ -1,5 +1,8 @@\n>  #include \"cache.h\"\n>  #include \"rebase-common.h\"\n> +#include \"dir.h\"\n> +#include \"run-command.h\"\n> +#include \"refs.h\"\n>  \n>  void rebase_options_init(struct rebase_options *opts)\n>  {\n> @@ -95,3 +98,81 @@ void rebase_options_save(const struct rebase_options *opts, const char *dir)\n>  \twrite_state_text(dir, \"onto\", oid_to_hex(&opts->onto));\n>  \twrite_state_text(dir, \"orig-head\", oid_to_hex(&opts->orig_head));\n>  }\n> +\n> +static int detach_head(const struct object_id *commit, const char *onto_name)\n> +{\n\nAgain, this is a very sad function. It would like to work for so many\ncommands, but it is stuck into the rebase-common.c file where nobody ever\ncares about it.\n\nA better place might be branch.[ch], maybe even under a better name\n(although this is a matter of contention, as many Git old-timers have an\nintuitive understanding of what \"detached HEAD\" means that is not at all\nshared by new Git users).\n\n> +\tconst char *reflog_action = getenv(\"GIT_REFLOG_ACTION\");\n> +\tif (!reflog_action || !*reflog_action)\n> +\t\treflog_action = \"rebase\";\n> +\tcp.git_cmd = 1;\n> +\targv_array_pushf(&cp.env_array, \"GIT_REFLOG_ACTION=%s: checkout %s\",\n> +\t\t\treflog_action, onto_name ? onto_name : oid_to_hex(commit));\n\nThe REFLOG_ACTION code seems to be a prime candidate for simplification\nthrough a simple, small function owning a static strbuf.\n\n> +void rebase_common_setup(struct rebase_options *opts, const char *dir)\n> +{\n> +\t/* Detach HEAD and reset the tree */\n> +\tprintf_ln(_(\"First, rewinding head to replay your work on top of it...\"));\n> +\tif (detach_head(&opts->onto, opts->onto_name))\n> +\t\tdie(_(\"could not detach HEAD\"));\n> +\tupdate_ref(\"rebase\", \"ORIG_HEAD\", opts->orig_head.hash, NULL, 0,\n> +\t\t\tUPDATE_REFS_DIE_ON_ERR);\n> +}\n\nIf we were to truly reuse the setup between rebase types (and really\npreferably extend sequencer's already existing code), we could imitate the\nexisting \"rebase (am)\" reflog message.\n\n> +void rebase_common_destroy(struct rebase_options *opts, const char *dir)\n> +{\n> +\tstruct strbuf sb = STRBUF_INIT;\n> +\tstrbuf_addstr(&sb, dir);\n> +\tremove_dir_recursively(&sb, 0);\n> +\tstrbuf_release(&sb);\n> +}\n\nIt is *really* fragile to separate the directory from the rebase options.\nThat makes it *way* too easy to (attempt to) remove the wrong directory\n(that might not even exist, but we'll never get notified about that\nerror).\n\n> +static void move_to_original_branch(struct rebase_options *opts)\n> +{\n\nThis function does not really *move* to the original branch. Instead, it\n*updates* the original branch to the current HEAD and then \"un-detaches\"\nthe HEAD.\n\nIn addition, if the function states that it wants to work with an original\nbranch, it should accept the name of the original branch, not some\nrebase_options.\n\n> +\tstruct strbuf sb = STRBUF_INIT;\n> +\tstruct object_id curr_head;\n> +\n> +\tif (!opts->orig_refname || !starts_with(opts->orig_refname, \"refs/\"))\n> +\t\treturn;\n\nThe caller should take care of this. Alternatively, this function should\nreturn with an error.\n\n> +\tif (get_sha1(\"HEAD\", curr_head.hash) < 0)\n> +\t\tdie(\"get_sha1() failed\");\n\nNope. No die() in library functions, please.\n\n> +\tstrbuf_addf(&sb, \"rebase finished: %s onto %s\", opts->orig_refname, oid_to_hex(&opts->onto));\n> +\tif (update_ref(sb.buf, opts->orig_refname, curr_head.hash, opts->orig_head.hash, 0, UPDATE_REFS_MSG_ON_ERR))\n> +\t\tgoto fail;\n\nOverly long lines.\n\nAnd here you see how beautiful a simple\n\n\tstatic const char *reflog_message(const char *action,\n\t\tconst char *fmt, ...)\n\t{\n\t\tstatic strbuf buf = STRBUF_INIT;\n\n\t\tva_list ap;\n\t\tva_start(ap, fmt);\n\n\t\tstrbuf_reset(&buf);\n\t\tstrbuf_addf(_(\"rebase %s\"), action);\n\t\tstrbuf_vaddf(sb, fmt, ap);\n\n\t\tva_end(ap);\n\t\treturn &buf.buf;\n\t}\n\nwould make this code and pretty much all of the other places where a\nreflog message is needed.\n\nFWIW I think this could be of even more general use, outside of rebase.\n\n> +\tstrbuf_reset(&sb);\n> +\tstrbuf_addf(&sb, \"rebase finished: returning to %s\", opts->orig_refname);\n> +\tif (create_symref(\"HEAD\", opts->orig_refname, sb.buf))\n> +\t\tgoto fail;\n> +\n> +\tstrbuf_release(&sb);\n> +\n> +\treturn;\n> +fail:\n> +\tdie(_(\"Could not move back to %s\"), opts->orig_refname);\n\nAgain, no die(), please. Besides, we should tell the user what went wrong:\nthere are two possibilities here, after all (updating the branch or\nupdating HEAD).\n\n> +void rebase_common_finish(struct rebase_options *opts, const char *dir)\n> +{\n> +\tconst char *argv_gc_auto[] = {\"gc\", \"--auto\", NULL};\n> +\n> +\tmove_to_original_branch(opts);\n> +\tclose_all_packs();\n> +\trun_command_v_opt(argv_gc_auto, RUN_GIT_CMD);\n> +\trebase_common_destroy(opts, dir);\n> +}\n\nSo now we have *two* functions to clean up afterwards. Better to have a\nsingle one.\n\nBesides, this would be a perfect opportunity to refactor out gc_auto()\n(that closes all packs and then runs the command) and to update all code\nlocations that can make use of this function, opening the door for a\nsingle point of libification of auto-gc'ing.\n\n> diff --git a/rebase-common.h b/rebase-common.h\n> index 051c056..067ad0b 100644\n> --- a/rebase-common.h\n> +++ b/rebase-common.h\n> @@ -24,4 +24,10 @@ int rebase_options_load(struct rebase_options *, const char *dir);\n>  \n>  void rebase_options_save(const struct rebase_options *, const char *dir);\n>  \n> +void rebase_common_setup(struct rebase_options *, const char *dir);\n> +\n> +void rebase_common_destroy(struct rebase_options *, const char *dir);\n> +\n> +void rebase_common_finish(struct rebase_options *, const char *dir);\n\nAgain, this is not very DRY. Does it really have to repeat that it is the\n*common* rebase functionality? Why not simply say init_rebase()? And why\npass that dir all the time? This looks really more like copy-edited code\nthan like a carefully designed API...\n\nFor one, the rebase_options should actually be the replay_options (the\nentire original idea of the sequencer was to be the plumbing behind the\nrebase, after all, not that that idea was implemented very well).\n\nI would also much prefer to extend the sequencer functionality through\ncallbacks (e.g. for updating, and switching back to, the original branch\nat the end, skipping the updating step in case the user wants to abort)\nthan repeat essentially the same calls in all rebase varieties. Just think\nabout it: *all* of them have to call rebase_common_setup() in *their own*\nsetup() functions, same for destroy() and for finish().\n\nIf there was a set of callbacks, depending on the replay type, all of this\ncould be neatly tucked away behind a common interface.\n\nCiao,\nDscho\n"},{"id":"280909","messageId":"alpine.DEB.2.20.1603161647560.4690@virtualbox","threadId":"41678","inReplyTo":"CACRoPnRH1D=8k5uuUahh1MJOAXsWkhY0fWev2AQhJm5+WWk5rQ@mail.gmail.com","subject":"Re: [PATCH/RFC/GSoC 12/17] rebase-todo: introduce rebase_todo_item","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-03-16T15:55:01Z","receivedAt":"2016-03-16T15:55:01Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Paul,\n\nOn Wed, 16 Mar 2016, Paul Tan wrote:\n\n> On Mon, Mar 14, 2016 at 9:43 PM, Christian Couder\n> <christian.couder@gmail.com> wrote:\n> > On Sat, Mar 12, 2016 at 11:46 AM, Paul Tan <pyokagan@gmail.com> wrote:\n> >> In an interactive rebase, commands are read and executed from a todo\n> >> list (.git/rebase-merge/git-rebase-todo) to perform the rebase.\n> >>\n> >> In the upcoming re-implementation of git-rebase -i in C, it is useful\n> >> to be able to parse each command into a data structure which can then\n> >> be operated on. Implement rebase_todo_item for this.\n> >\n> > sequencer.{c,h} already has some code to parse and create todo lists\n> > for cherry-picking or reverting multiple commits, so I am wondering if\n> > it would be possible to share some code?\n> \n> AFAIK, sequencer.c as it is in master parses the todo list\n> destructively and does not keep the associated action for each commit\n> and the \"rest\" string.\n\nThis is a *serious* mistake in the implementation of the sequencer, I\nagree.\n\nTherefore it is a good idea to fix that mistake instead of leaving it in\nplace.\n\nAnd that is exactly what I did:\n\n\thttps://github.com/dscho/git/commit/b9b5b7351\n\nPlease note that the commit is marked as a \"TODO\" because it has to\nreintroduce a stupidly strict behavior (the sequencer expects the commands\nin the todo script to agree with the overall action, i.e. if the action is\nREPLAY_REVERT, the todo script can only contain \"revert\" commands, if the\naction is REPLAY_PICK, the todo list can only contain \"pick\" commands). Of\ncourse this restriction is totally arbitrary and even unwanted, so I will\nlift it after this commit. Or maybe I'll just lift it before this commit.\nYeah, I'll do that instead.\n\n> As I said in another thread, originally I wanted to keep the scope\n> simple, and just do the rewrite of rebase from C to shell, and let any\n> further libifications and optimizations come after, so I didn't want\n> to touch sequencer for now.\n\nWe know, however, how leaving technical debt for later will just make sure\nthat technical debt accumulates...\n\nAnd while I have a *tremendous* respect for what Karthik did (and\ncontinues to do, even long after his GSoC project ended!), I do not want\nto ask any future GSoC student to clean up the mess that we produce right\nnow... So let's just not add more technical debt than we absolutely have\nto.\n\nCiao,\nDscho\n"},{"id":"280910","messageId":"alpine.DEB.2.20.1603161656130.4690@virtualbox","threadId":"41678","inReplyTo":"CACRoPnS=qg=a3xYKHyk-7E2HN5HhTimGirZcwL8hMa0xLY6KdA@mail.gmail.com","subject":"Re: [PATCH/RFC/GSoC 01/17] perf: introduce performance tests for git-rebase","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-03-16T15:59:09Z","receivedAt":"2016-03-16T15:59:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Paul,\n\nOn Wed, 16 Mar 2016, Paul Tan wrote:\n\n> On Wed, Mar 16, 2016 at 3:58 PM, Johannes Schindelin\n> <Johannes.Schindelin@gmx.de> wrote:\n> >\n> > On Sat, 12 Mar 2016, Paul Tan wrote:\n> >\n> >> diff --git a/t/perf/p3404-rebase-interactive.sh b/t/perf/p3404-rebase-interactive.sh\n> >> new file mode 100755\n> >> index 0000000..aaca105\n> >> --- /dev/null\n> >> +++ b/t/perf/p3404-rebase-interactive.sh\n> >> @@ -0,0 +1,26 @@\n> >>\n> >> [...]\n> >>\n> >> +test_perf 'rebase -i --onto master^' '\n> >> +     git checkout perf-topic-branch &&\n> >> +     git reset --hard perf-topic-branch-initial &&\n> >> +     GIT_SEQUENCE_EDITOR=: git rebase -i --onto master^ master\n> >> +'\n> >\n> > This measures the performance of checkout && reset && rebase -i. Maybe we\n> > should only test rebase -i?\n> \n> test_perf runs the same script multiple times, so we need to reset\n> --hard at least to undo the changes of the rebase.\n> \n> I think we can remove the reset if we use rebase -f and rebase onto\n> the same base, but -f was not implemented in this patch series.\n\nHrm. rebase -f just makes the reset an implicit part of the rebase, so it\nseems we cannot perf *just* the rebase. We are stuck with perf'ing also\nthe reset. Sad.\n\n> > Also, I would strongly recommend an extra test_commit after reset;\n> > Otherwise you would only test the logic that verifies that it can simply\n> > fast-forward instead of cherry-picking.\n> \n> Or, we could use the -f flag, I think.\n\nYeah, we can do that, too.\n\nCiao,\nDscho\n"},{"id":"280922","messageId":"CAGZ79kZYOkeeujSQ16LOSQukDOBEhuqOq_C9qeXMaT9vn+RqHA@mail.gmail.com","threadId":"41678","inReplyTo":"CACRoPnS7WWWVay9hAjXYgyeB=1A1gfARerKJe25APa-6u5cGaA@mail.gmail.com","subject":"Re: [PATCH/RFC/GSoC 05/17] rebase-options: implement rebase_options_load() and rebase_options_save()","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-03-16T17:10:15Z","receivedAt":"2016-03-16T17:10:15Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Mar 16, 2016 at 5:04 AM, Paul Tan <pyokagan@gmail.com> wrote:\n>>\n>> How is this specific to the state file? All it does is create the\n>> leading directory\n>> if it doesn't exist? (So I'd expect file_exists(concat(dir, file)) to\n>> have the same\n>> result without actually creating the directory if it doesn't exist as\n>> a side effect?\n>\n> I don't quite understand, AFAIK mkpath() does not create any\n> directories as a side-effect. And yes, I just wanted a short way to\n> say file_exists(concat(dir, file)) or file_exists(mkpath(\"%s/%s\", dir,\n> file)) without cluttering up the code.\n\nMy bad. I should not assume functions doing stuff as their name might suggest.\n(It \"makes the path\" but only in terms of creating the right string, not on the\nfile system, where you'd use functions like safe_create_leading_directories.\nI thought all that was implied in mkpath).\n\n>\n>> If the dir doesn't exist it can be created in rebase_options_load explicitly?\n>\n> I don't intend to create any directories if they do not exist.\n>\n>>> +\n>>> +static int read_state_file(struct strbuf *sb, const char *dir, const char *file)\n>>> +{\n>>> +       const char *path = mkpath(\"%s/%s\", dir, file);\n>>> +       strbuf_reset(sb);\n>>> +       if (strbuf_read_file(sb, path, 0) >= 0)\n>>> +               return sb->len;\n>>> +       else\n>>> +               return error(_(\"could not read '%s'\"), path);\n>>> +}\n>>> +\n>>> +int rebase_options_load(struct rebase_options *opts, const char *dir)\n>>> +{\n>>> +       struct strbuf sb = STRBUF_INIT;\n>>> +       const char *filename;\n>>> +\n>>> +       /* opts->orig_refname */\n>>> +       if (read_state_file(&sb, dir, \"head-name\") < 0)\n>>> +               return -1;\n>>> +       strbuf_trim(&sb);\n>>> +       if (starts_with(sb.buf, \"refs/heads/\"))\n>>> +               opts->orig_refname = strbuf_detach(&sb, NULL);\n>>> +       else if (!strcmp(sb.buf, \"detached HEAD\"))\n>>> +               opts->orig_refname = NULL;\n>>> +       else\n>>> +               return error(_(\"could not parse %s\"), mkpath(\"%s/%s\", dir, \"head-name\"));\n>>> +\n>>> +       /* opts->onto */\n>>> +       if (read_state_file(&sb, dir, \"onto\") < 0)\n>>> +               return -1;\n>>> +       strbuf_trim(&sb);\n>>> +       if (get_oid_hex(sb.buf, &opts->onto) < 0)\n>>> +               return error(_(\"could not parse %s\"), mkpath(\"%s/%s\", dir, \"onto\"));\n>>> +\n>>> +       /*\n>>> +        * We always write to orig-head, but interactive rebase used to write\n>>> +        * to head. Fall back to reading from head to cover for the case that\n>>> +        * the user upgraded git with an ongoing interactive rebase.\n>>> +        */\n>>> +       filename = state_file_exists(dir, \"orig-head\") ? \"orig-head\" : \"head\";\n>>> +       if (read_state_file(&sb, dir, filename) < 0)\n>>> +               return -1;\n>>\n>> So from here on we always use \"orig-head\" instead of \"head\" for\n>> interactive rebase.\n>> Would people ever rely on the (internal) file name and have e.g.\n>> scripts which operate\n>> on the \"head\" file ?\n>\n> This backwards-compatibility code is just a straight port from the\n> code in git-rebase.sh.\n>\n> The usage of orig-head has been around since 2011 with 84df456\n> (rebase: extract code for writing basic state, 2011-02-06), so I guess\n> if people had issues with it, it would have been reported.\n\nI did not read the rebase shell code, but commented on the C code only.\nIf this is already in there, let's keep it.\nSorry for the confusion.\n\nThanks,\nStefan\n"},{"id":"280923","messageId":"alpine.DEB.2.20.1603161802080.4690@virtualbox","threadId":"41678","inReplyTo":"CACRoPnS4JpNNACz4T0F0vFs3ogG+nzk-y1=zc1UrtAZKaEnggg@mail.gmail.com","subject":"Re: [PATCH/RFC/GSoC 05/17] rebase-options: implement rebase_options_load() and rebase_options_save()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-03-16T17:11:10Z","receivedAt":"2016-03-16T17:11:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Paul,\n\nOn Wed, 16 Mar 2016, Paul Tan wrote:\n\n> On Wed, Mar 16, 2016 at 4:04 PM, Johannes Schindelin\n> <Johannes.Schindelin@gmx.de> wrote:\n> > In addition I want to point out that sequencer's replay_opts seem to be at\n> > least related, but the patch shares none of its code with the sequencer.\n> > Let's avoid that.\n> >\n> > In other words, let's try to add as little code as possible when we can\n> > enhance existing code.\n> \n> Well, both git-rebase--am.sh and git-rebase--merge.sh do not use the\n> sequencer functionality at all, and we don't see git-am for example\n> needing to be aware of onto, orig-head, head-name etc.\n\nThat is arguing that the implementation of --am and --merge is too far\naway from the sequencer and therefore should not be made closer.\n\nBy that token, has_unstaged_changes() should never be allowed to call\ninit_revisions(): it *never* looks at any revisions at all!\n\nAnd the idea of the sequencer is so much more related to --am and --merge\nthan unstaged changes are to revisions: the entire purpose of the\nsequencer (no matter the *current* implementation with all its\nlimitations) is to apply a bunch of patches, in sequence. That is pretty\nmuch precisely what *all* members of the rebase family are about.\n\nIn other words: please do not let current limitations dictate that we\nshould introduce diverging code for essentially the same workflow.\n\nCiao,\nDscho\n"},{"id":"280939","messageId":"alpine.DEB.2.20.1603161918410.4690@virtualbox","threadId":"41678","inReplyTo":"CACRoPnRMOp38vfkQZjmkUqr+urN8NYcNN_oNzHtJqfyTorr1ug@mail.gmail.com","subject":"Re: [PATCH/RFC/GSoC 16/17] editor: implement git_sequence_editor() and launch_sequence_editor()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-03-16T18:21:23Z","receivedAt":"2016-03-16T18:21:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Paul,\n\nOn Wed, 16 Mar 2016, Paul Tan wrote:\n\n> Hi Dscho,\n> \n> On Tue, Mar 15, 2016 at 3:00 PM, Johannes Schindelin\n> <Johannes.Schindelin@gmx.de> wrote:\n> > On Sat, 12 Mar 2016, Paul Tan wrote:\n> >> ---\n> >>  cache.h  |  1 +\n> >\n> > No need to clutter cache.h with a function that is only to be used by the\n> > sequencer. IOW let's make this static in sequencer.c.\n> \n> The function needs to be implemented in editor.c\n\nNo, *another* function needs to be implemented in editor.c: one that\naccepts the editor itself as parameter. You did that, but then you wrapped\nit as git_sequencer_editor() and left the *really* useful function *still*\nstatic to editor.c.\n\nOr maybe the best solution would be to simply extend git_editor() to\naccept the editor as an additional, first parameter, falling back to the\ncurrent behavior if NULL is passed (and then change all callers to pass\nNULL).\n\nI guess my preference would be with the latter, that would make for the\nmost elegant, minimally invasive and most reusable solution.\n\nCiao,\nDscho\n"},{"id":"281139","messageId":"20160318110134.GA16750@hank","threadId":"41678","inReplyTo":"alpine.DEB.2.20.1603161656130.4690@virtualbox","subject":"Re: [PATCH/RFC/GSoC 01/17] perf: introduce performance tests for git-rebase","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2016-03-18T11:01:34Z","receivedAt":"2016-03-18T11:01:34Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 03/16, Johannes Schindelin wrote:\n> Hi Paul,\n> \n> On Wed, 16 Mar 2016, Paul Tan wrote:\n> \n> > On Wed, Mar 16, 2016 at 3:58 PM, Johannes Schindelin\n> > <Johannes.Schindelin@gmx.de> wrote:\n> > >\n> > > On Sat, 12 Mar 2016, Paul Tan wrote:\n> > >\n> > >> diff --git a/t/perf/p3404-rebase-interactive.sh b/t/perf/p3404-rebase-interactive.sh\n> > >> new file mode 100755\n> > >> index 0000000..aaca105\n> > >> --- /dev/null\n> > >> +++ b/t/perf/p3404-rebase-interactive.sh\n> > >> @@ -0,0 +1,26 @@\n> > >>\n> > >> [...]\n> > >>\n> > >> +test_perf 'rebase -i --onto master^' '\n> > >> +     git checkout perf-topic-branch &&\n> > >> +     git reset --hard perf-topic-branch-initial &&\n> > >> +     GIT_SEQUENCE_EDITOR=: git rebase -i --onto master^ master\n> > >> +'\n> > >\n> > > This measures the performance of checkout && reset && rebase -i. Maybe we\n> > > should only test rebase -i?\n> > \n> > test_perf runs the same script multiple times, so we need to reset\n> > --hard at least to undo the changes of the rebase.\n> > \n> > I think we can remove the reset if we use rebase -f and rebase onto\n> > the same base, but -f was not implemented in this patch series.\n> \n> Hrm. rebase -f just makes the reset an implicit part of the rebase, so it\n> seems we cannot perf *just* the rebase. We are stuck with perf'ing also\n> the reset. Sad.\n\nI had the same problem back when I was working on index-v5 and posted\na patch series.  The discussion about it is at [1].  Maybe it could be\nworth resurrecting?\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/1379419842-32627-1-git-send-email-t.gummerer@gmail.com\n\n-- \nThomas\n"},{"id":"281154","messageId":"alpine.DEB.2.20.1603181659200.4690@virtualbox","threadId":"41678","inReplyTo":"20160318110134.GA16750@hank","subject":"Re: [PATCH/RFC/GSoC 01/17] perf: introduce performance tests for git-rebase","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-03-18T16:00:44Z","receivedAt":"2016-03-18T16:00:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Thomas,\n\nOn Fri, 18 Mar 2016, Thomas Gummerer wrote:\n\n> On 03/16, Johannes Schindelin wrote:\n> > \n> > On Wed, 16 Mar 2016, Paul Tan wrote:\n> > \n> > > On Wed, Mar 16, 2016 at 3:58 PM, Johannes Schindelin\n> > > <Johannes.Schindelin@gmx.de> wrote:\n> > > >\n> > > > On Sat, 12 Mar 2016, Paul Tan wrote:\n> > > >\n> > > >> diff --git a/t/perf/p3404-rebase-interactive.sh b/t/perf/p3404-rebase-interactive.sh\n> > > >> new file mode 100755\n> > > >> index 0000000..aaca105\n> > > >> --- /dev/null\n> > > >> +++ b/t/perf/p3404-rebase-interactive.sh\n> > > >> @@ -0,0 +1,26 @@\n> > > >>\n> > > >> [...]\n> > > >>\n> > > >> +test_perf 'rebase -i --onto master^' '\n> > > >> +     git checkout perf-topic-branch &&\n> > > >> +     git reset --hard perf-topic-branch-initial &&\n> > > >> +     GIT_SEQUENCE_EDITOR=: git rebase -i --onto master^ master\n> > > >> +'\n> > > >\n> > > > This measures the performance of checkout && reset && rebase -i. Maybe we\n> > > > should only test rebase -i?\n> > > \n> > > test_perf runs the same script multiple times, so we need to reset\n> > > --hard at least to undo the changes of the rebase.\n> > > \n> > > I think we can remove the reset if we use rebase -f and rebase onto\n> > > the same base, but -f was not implemented in this patch series.\n> > \n> > Hrm. rebase -f just makes the reset an implicit part of the rebase, so it\n> > seems we cannot perf *just* the rebase. We are stuck with perf'ing also\n> > the reset. Sad.\n> \n> I had the same problem back when I was working on index-v5 and posted\n> a patch series.  The discussion about it is at [1].  Maybe it could be\n> worth resurrecting?\n> \n> [1] http://thread.gmane.org/gmane.comp.version-control.git/1379419842-32627-1-git-send-email-t.gummerer@gmail.com\n\nYes, I agree that something like that is needed. The proposed commit\nmessage suggests that things get simpler, though, while I would contend\nthat timings get more accurate.\n\nAnd I think you could simply move the test_start command, but that's just\nfrom a *very* cursory reading of the patch.\n\nCiao,\nDscho\n"},{"id":"281278","messageId":"20160320140000.GB32027@hank","threadId":"41678","inReplyTo":"alpine.DEB.2.20.1603181659200.4690@virtualbox","subject":"Re: [PATCH/RFC/GSoC 01/17] perf: introduce performance tests for git-rebase","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2016-03-20T14:00:02Z","receivedAt":"2016-03-20T14:00:02Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 03/18, Johannes Schindelin wrote:\n> Hi Thomas,\n> \n> On Fri, 18 Mar 2016, Thomas Gummerer wrote:\n> \n> > On 03/16, Johannes Schindelin wrote:\n> > > Hrm. rebase -f just makes the reset an implicit part of the rebase, so it\n> > > seems we cannot perf *just* the rebase. We are stuck with perf'ing also\n> > > the reset. Sad.\n> > \n> > I had the same problem back when I was working on index-v5 and posted\n> > a patch series.  The discussion about it is at [1].  Maybe it could be\n> > worth resurrecting?\n> > \n> > [1] http://thread.gmane.org/gmane.comp.version-control.git/1379419842-32627-1-git-send-email-t.gummerer@gmail.com\n> \n> Yes, I agree that something like that is needed. The proposed commit\n> message suggests that things get simpler, though, while I would contend\n> that timings get more accurate.\n> \n> And I think you could simply move the test_start command, but that's just\n> from a *very* cursory reading of the patch.\n\nIs it possible that you might have missed patch 2/2 [1]?  The first\npatch there is only the preparation for making the timings more\nacurate when some cleanup is involved.  Just moving the test_start\ncommand wouldn't do anything for the timings to get more acurate, as\nthe timings are measured in the test_run_perf_ function (it's outside\nof the diff in the patch series).\n\nI'm also not sure whether the test_perf_cleanup or if the other series\nin the thread that came out of a suggestion by Junio makes more sense\n[2]. (That is minus patch 3/3 in that series, which was added so we\ncould have a user of the new series, but if it's going to be used here\nthat's unnecessary).\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/234874/focus=234875\n[2] http://thread.gmane.org/gmane.comp.version-control.git/234874/focus=235241\n\n\n> Ciao,\n> Dscho\n\n-- \nThomas\n"},{"id":"281333","messageId":"alpine.DEB.2.20.1603210853570.4690@virtualbox","threadId":"41678","inReplyTo":"20160320140000.GB32027@hank","subject":"Re: [PATCH/RFC/GSoC 01/17] perf: introduce performance tests for git-rebase","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-03-21T07:54:35Z","receivedAt":"2016-03-21T07:54:35Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Thomas,\n\nOn Sun, 20 Mar 2016, Thomas Gummerer wrote:\n\n> On 03/18, Johannes Schindelin wrote:\n> > \n> > On Fri, 18 Mar 2016, Thomas Gummerer wrote:\n> > \n> > > [1] http://thread.gmane.org/gmane.comp.version-control.git/1379419842-32627-1-git-send-email-t.gummerer@gmail.com\n> > \n> > Yes, I agree that something like that is needed. The proposed commit\n> > message suggests that things get simpler, though, while I would\n> > contend that timings get more accurate.\n> > \n> > And I think you could simply move the test_start command, but that's\n> > just from a *very* cursory reading of the patch.\n> \n> Is it possible that you might have missed patch 2/2 [1]?\n\nYes, I did not read that patch. Sorry for the noise.\n\nCiao,\nDscho\n"},{"id":"281342","messageId":"CACRoPnTeb78JSN+bTM04u6E5E=fMWY1h6ef0GDDP0DoaADuTNQ@mail.gmail.com","threadId":"41678","inReplyTo":"alpine.DEB.2.20.1603161802080.4690@virtualbox","subject":"Re: [PATCH/RFC/GSoC 05/17] rebase-options: implement rebase_options_load() and rebase_options_save()","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2016-03-21T14:55:09Z","receivedAt":"2016-03-21T14:55:09Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi Dscho,\n\n(Sorry for the very late reply, I got caught up with some unexpected\nwork and am still clearing my inbox ><)\n\nOn Thu, Mar 17, 2016 at 1:11 AM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> On Wed, 16 Mar 2016, Paul Tan wrote:\n>> On Wed, Mar 16, 2016 at 4:04 PM, Johannes Schindelin\n>> <Johannes.Schindelin@gmx.de> wrote:\n>> > In addition I want to point out that sequencer's replay_opts seem to be at\n>> > least related, but the patch shares none of its code with the sequencer.\n>> > Let's avoid that.\n>> >\n>> > In other words, let's try to add as little code as possible when we can\n>> > enhance existing code.\n>>\n>> Well, both git-rebase--am.sh and git-rebase--merge.sh do not use the\n>> sequencer functionality at all, and we don't see git-am for example\n>> needing to be aware of onto, orig-head, head-name etc.\n>\n> That is arguing that the implementation of --am and --merge is too far\n> away from the sequencer and therefore should not be made closer.\n>\n> By that token, has_unstaged_changes() should never be allowed to call\n> init_revisions(): it *never* looks at any revisions at all!\n>\n> And the idea of the sequencer is so much more related to --am and --merge\n> than unstaged changes are to revisions: the entire purpose of the\n> sequencer (no matter the *current* implementation with all its\n> limitations) is to apply a bunch of patches, in sequence. That is pretty\n> much precisely what *all* members of the rebase family are about.\n>\n> In other words: please do not let current limitations dictate that we\n> should introduce diverging code for essentially the same workflow.\n\nAh, so you are thinking of replacing the --am and --merge scripts with\nsequencer? That sounds great :-)\n\nI'll wait for your sequencer patch series then.\n\nThanks!\nPaul\n"}]}