Re: [PATCH v2 1/4] rebase: start implementing it as a builtin
From: Junio C Hamano <hidden>
Date: 2018-07-03 21:09:07
Pratik Karki [off-list ref] writes:
This commit imitates the strategy that was used to convert the difftool to a builtin, see be8a90e (difftool: add a skeleton for the upcoming builtin, 2017-01-17) for details: This commit renames the shell script `git-rebase.sh` to `git-legacy-rebase.sh` and hands off to it by default.
I'd appreciate it if mentors can teach a bit more log message writing to students, thanks.
quoted hunk
diff --git a/builtin/rebase.c b/builtin/rebase.c new file mode 100644 index 000000000..c0c9d6cd2 --- /dev/null +++ b/builtin/rebase.c@@ -0,0 +1,55 @@ +/* + * "git rebase" builtin command
Nice ;-)
+ * Copyright (c) 2018 Pratik Karki
+ */
+
+#include "builtin.h"
+#include "run-command.h"
+#include "exec-cmd.h"
+#include "argv-array.h"
+#include "dir.h"
+
+static int use_builtin_rebase(void)
+{
+ struct child_process cp = CHILD_PROCESS_INIT;
+ struct strbuf out = STRBUF_INIT;
+ int ret;
+
+ argv_array_pushl(&cp.args,
+ "config", "--bool", "rebase.usebuiltin", NULL);
+ cp.git_cmd = 1;
+ if (capture_command(&cp, &out, 6))
+ return 0;
+
+ strbuf_trim(&out);
+ ret = !strcmp("true", out.buf);
+ strbuf_release(&out);
+ return ret;
+}OK. As we give "--bool", it is perfectly OK to check with "true" and no other forms of positive setting.
+int cmd_rebase(int argc, const char **argv, const char *prefix)
+{
+ /*
+ * NEEDSWORK: Once the builtin rebase has been tested enough
+ * and git-legacy-rebase.sh is retired to contrib/, this preambleAlign these asterisks, i.e. /* * NEEDSWORK: ... */
+ if (!use_builtin_rebase()) {
+ const char *path = mkpath("%s/git-legacy-rebase",
+ git_exec_path());
+
+ if (sane_execvp(path, (char **)argv) < 0)
+ die_errno("could not exec %s", path);
+
+ return 0;
Hmph, I know this was inherited from the old commit you modelled
this patch after, but can sane_execvp() ever give us control back
with non-negative value returned? IOW I am wondering if this should
be more like
if (sane_execvp(...) < 0)
die_errno(...);
else
die("sane_execvp() returned???");
Not worth a reroll, but something to think about.
quoted hunk
diff --git a/git-rebase.sh b/git-legacy-rebase.sh similarity index 100% rename from git-rebase.sh rename to git-legacy-rebase.sh diff --git a/git.c b/git.c index 9dbe6ffaa..bacfefb2d 100644 --- a/git.c +++ b/git.c@@ -518,6 +518,12 @@ static struct cmd_struct commands[] = { { "pull", cmd_pull, RUN_SETUP | NEED_WORK_TREE }, { "push", cmd_push, RUN_SETUP }, { "read-tree", cmd_read_tree, RUN_SETUP | SUPPORT_SUPER_PREFIX}, + /* + *NEEDSWORK: Until the rebase is independent and needs no redirection + to rebase shell script this is kept as is, then should be changed to + RUN_SETUP | NEED_WORK_TREE + */
Likewise.
+ { "rebase", cmd_rebase },
{ "rebase--helper", cmd_rebase__helper, RUN_SETUP | NEED_WORK_TREE },
{ "receive-pack", cmd_receive_pack },
{ "reflog", cmd_reflog, RUN_SETUP },