Re: [PATCH 11/11] builtin rebase: support `git rebase `
Hi Duy, On Wed, 8 Aug 2018, Duy Nguyen wrote: > On Wed, Aug 8, 2018 at 3:55 PM Pratik Karki wrote: > > > > diff --git a/builtin/rebase.c b/builtin/rebase.c > > index 63634210c0..b2ddfa8dbf 100644 > > --- a/builtin/rebase.c > > +++ b/builtin/rebase.c > > @@ -585,7 +602,8 @@ int cmd_rebase(int argc, const char **argv, const char > > *prefix) > > } > > if (get_oid("HEAD", &options.orig_head)) > > die(_("Could not resolve HEAD to a revision")); > > - } > > + } else > > + BUG("unexpected number of arguments left to parse"); > > Does this mean "git base one two three" triggers this BUG? If so, this > should be a die() instead. I did not real the full source code, so > maybe this case is already caught higher up. As you can see from https://github.com/git/git/blob/v2.18.0/git-rebase.sh#L615 the original, Unix shell script version of `git rebase` also says "BUG" here. And if you care to look at https://github.com/git/git/blob/3358abdcb/builtin/rebase.c#L870-L872 you will see that there is a proper check for the correct amount of command-line parameters. So at this point, it would indeed indicate a bug if the `argc` had an unexpected value. Ciao, Dscho
Re: [PATCH 11/11] builtin rebase: support `git rebase `
(not really a review, this patch just happens to catch my eyes) On Wed, Aug 8, 2018 at 3:55 PM Pratik Karki wrote: > > This commit adds support for `switch-to` which is used to switch to the > target branch if needed. The equivalent codes found in shell script > `git-legacy-rebase.sh` is converted to builtin `rebase.c`. > > Signed-off-by: Pratik Karki > --- > builtin/rebase.c | 48 > 1 file changed, 44 insertions(+), 4 deletions(-) > > diff --git a/builtin/rebase.c b/builtin/rebase.c > index 63634210c0..b2ddfa8dbf 100644 > --- a/builtin/rebase.c > +++ b/builtin/rebase.c > @@ -79,6 +79,7 @@ struct rebase_options { > struct commit *onto; > const char *onto_name; > const char *revisions; > + const char *switch_to; > int root; > struct commit *restrict_revision; > int dont_finish_rebase; > @@ -186,6 +187,8 @@ static int run_specific_rebase(struct rebase_options > *opts) > opts->flags & REBASE_DIFFSTAT ? "t" : ""); > add_var(&script_snippet, "force_rebase", > opts->flags & REBASE_FORCE ? "t" : ""); > + if (opts->switch_to) > + add_var(&script_snippet, "switch_to", opts->switch_to); > > switch (opts->type) { > case REBASE_AM: > @@ -564,9 +567,23 @@ int cmd_rebase(int argc, const char **argv, const char > *prefix) > * orig_head -- commit object name of tip of the branch before > rebasing > * head_name -- refs/heads/ or NULL (detached HEAD) > */ > - if (argc > 0) > -die("TODO: handle switch_to"); > - else { > + if (argc == 1) { > + /* Is it "rebase other branchname" or "rebase other commit"? > */ > + branch_name = argv[0]; > + options.switch_to = argv[0]; > + > + /* Is it a local branch? */ > + strbuf_reset(&buf); > + strbuf_addf(&buf, "refs/heads/%s", branch_name); > + if (!read_ref(buf.buf, &options.orig_head)) > + options.head_name = xstrdup(buf.buf); > + /* If not is it a valid ref (branch or commit)? */ > + else if (!get_oid(branch_name, &options.orig_head)) > + options.head_name = NULL; > + else > + die(_("fatal: no such branch/commit '%s'"), die() automatically adds "fatal:" so you should not add it yourself here > + branch_name); > + } else if (argc == 0) { > /* Do not need to switch branches, we are already on it. */ > options.head_name = > xstrdup_or_null(resolve_ref_unsafe("HEAD", 0, NULL, > @@ -585,7 +602,8 @@ int cmd_rebase(int argc, const char **argv, const char > *prefix) > } > if (get_oid("HEAD", &options.orig_head)) > die(_("Could not resolve HEAD to a revision")); > - } > + } else > + BUG("unexpected number of arguments left to parse"); Does this mean "git base one two three" triggers this BUG? If so, this should be a die() instead. I did not real the full source code, so maybe this case is already caught higher up. -- Duy
[PATCH 11/11] builtin rebase: support `git rebase `
This commit adds support for `switch-to` which is used to switch to the target branch if needed. The equivalent codes found in shell script `git-legacy-rebase.sh` is converted to builtin `rebase.c`. Signed-off-by: Pratik Karki --- builtin/rebase.c | 48 1 file changed, 44 insertions(+), 4 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index 63634210c0..b2ddfa8dbf 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -79,6 +79,7 @@ struct rebase_options { struct commit *onto; const char *onto_name; const char *revisions; + const char *switch_to; int root; struct commit *restrict_revision; int dont_finish_rebase; @@ -186,6 +187,8 @@ static int run_specific_rebase(struct rebase_options *opts) opts->flags & REBASE_DIFFSTAT ? "t" : ""); add_var(&script_snippet, "force_rebase", opts->flags & REBASE_FORCE ? "t" : ""); + if (opts->switch_to) + add_var(&script_snippet, "switch_to", opts->switch_to); switch (opts->type) { case REBASE_AM: @@ -564,9 +567,23 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) * orig_head -- commit object name of tip of the branch before rebasing * head_name -- refs/heads/ or NULL (detached HEAD) */ - if (argc > 0) -die("TODO: handle switch_to"); - else { + if (argc == 1) { + /* Is it "rebase other branchname" or "rebase other commit"? */ + branch_name = argv[0]; + options.switch_to = argv[0]; + + /* Is it a local branch? */ + strbuf_reset(&buf); + strbuf_addf(&buf, "refs/heads/%s", branch_name); + if (!read_ref(buf.buf, &options.orig_head)) + options.head_name = xstrdup(buf.buf); + /* If not is it a valid ref (branch or commit)? */ + else if (!get_oid(branch_name, &options.orig_head)) + options.head_name = NULL; + else + die(_("fatal: no such branch/commit '%s'"), + branch_name); + } else if (argc == 0) { /* Do not need to switch branches, we are already on it. */ options.head_name = xstrdup_or_null(resolve_ref_unsafe("HEAD", 0, NULL, @@ -585,7 +602,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) } if (get_oid("HEAD", &options.orig_head)) die(_("Could not resolve HEAD to a revision")); - } + } else + BUG("unexpected number of arguments left to parse"); if (read_index(the_repository->index) < 0) die(_("could not read index")); @@ -612,6 +630,28 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) int flag; if (!(options.flags & REBASE_FORCE)) { + /* Lazily switch to the target branch if needed... */ + if (options.switch_to) { + struct object_id oid; + + if (get_oid(options.switch_to, &oid) < 0) { + ret = !!error(_("could not parse '%s'"), + options.switch_to); + goto cleanup; + } + + strbuf_reset(&buf); + strbuf_addf(&buf, "rebase: checkout %s", + options.switch_to); + if (reset_head(&oid, "checkout", + options.head_name, 0) < 0) { + ret = !!error(_("could not switch to " + "%s"), + options.switch_to); + goto cleanup; + } + } + if (!(options.flags & REBASE_NO_QUIET)) ; /* be quiet */ else if (!strcmp(branch_name, "HEAD") && -- 2.18.0