Re: [PATCH v2 1/1] builtin rebase: prepare for builtin rebase -i

2018-08-31 Thread Jeff King
On Fri, Aug 31, 2018 at 10:38:44PM +0200, Johannes Schindelin wrote:

> > > Is there any reason why you avoid using `git rebase -ir` here? This should
> > > be so much easier via
> > > 
> > >   git checkout pk/rebase-i-in-c-6-final
> > >   git rebase -ir js/rebase-in-c-5.5-work-with-rebase-i-in-c^
> > > 
> > > and then inserting this at the appropriate position, followed by the `git
> > > range-diff @{-1}...`:
> > > 
> > >   git am -s mbox
> > >   git update-ref js/rebase-in-c-5.5-work-with-rebase-i-in-c HEAD
> > 
> > Related discussion, including a fantasy tangent by me (downthread):
> > 
> >   
> > https://public-inbox.org/git/20180727080807.ga11...@sigill.intra.peff.net/#t
> 
> I have no idea what you meant there...

I thought you were asking why Junio does not just use "git am" from
inside "git rebase". I asked the same thing recently, and the answer is
because he is afraid of how the two interact. I dug a little into it
(the fantasy part is that I laid out a dream for how operations like
this could safely stack).

-Peff


Re: [PATCH v2 1/1] builtin rebase: prepare for builtin rebase -i

2018-08-31 Thread Johannes Schindelin
Hi Peff,

On Thu, 30 Aug 2018, Jeff King wrote:

> On Thu, Aug 30, 2018 at 01:03:41PM +0200, Johannes Schindelin wrote:
> 
> > > Will replace by doing:
> > > 
> > > $ git checkout js/rebase-in-c-5.5-work-with-rebase-i-in-c
> > > $ git checkout HEAD^
> > > $ git am -s mbox
> > > $ git range-diff @{-1}...
> > > $ git checkout -B @{-1}
> > > 
> > > $ git checkout pk/rebase-i-in-c-6-final
> > > $ git rebase --onto js/rebase-in-c-5.5-work-with-rebase-i-in-c \
> > >   js/rebase-in-c-5.5-work-with-rebase-i-in-c@{1} HEAD^0
> > > $ git range-diff @{-1}...
> > > $ git checkout -B @{-1}
> > > 
> > > to update the two topics and then rebuilding the integration
> > > branches the usual way.  I also need to replace the "other" topic
> > > used in this topic, so the actual procedure would be a bit more
> > > involved than the above, though.
> > 
> > Is there any reason why you avoid using `git rebase -ir` here? This should
> > be so much easier via
> > 
> > git checkout pk/rebase-i-in-c-6-final
> > git rebase -ir js/rebase-in-c-5.5-work-with-rebase-i-in-c^
> > 
> > and then inserting this at the appropriate position, followed by the `git
> > range-diff @{-1}...`:
> > 
> > git am -s mbox
> > git update-ref js/rebase-in-c-5.5-work-with-rebase-i-in-c HEAD
> 
> Related discussion, including a fantasy tangent by me (downthread):
> 
>   https://public-inbox.org/git/20180727080807.ga11...@sigill.intra.peff.net/#t

I have no idea what you meant there...

Ciao,
Dscho


Re: [PATCH v2 1/1] builtin rebase: prepare for builtin rebase -i

2018-08-30 Thread Jeff King
On Thu, Aug 30, 2018 at 01:03:41PM +0200, Johannes Schindelin wrote:

> > Will replace by doing:
> > 
> > $ git checkout js/rebase-in-c-5.5-work-with-rebase-i-in-c
> > $ git checkout HEAD^
> > $ git am -s mbox
> > $ git range-diff @{-1}...
> > $ git checkout -B @{-1}
> > 
> > $ git checkout pk/rebase-i-in-c-6-final
> > $ git rebase --onto js/rebase-in-c-5.5-work-with-rebase-i-in-c \
> >   js/rebase-in-c-5.5-work-with-rebase-i-in-c@{1} HEAD^0
> > $ git range-diff @{-1}...
> > $ git checkout -B @{-1}
> > 
> > to update the two topics and then rebuilding the integration
> > branches the usual way.  I also need to replace the "other" topic
> > used in this topic, so the actual procedure would be a bit more
> > involved than the above, though.
> 
> Is there any reason why you avoid using `git rebase -ir` here? This should
> be so much easier via
> 
>   git checkout pk/rebase-i-in-c-6-final
>   git rebase -ir js/rebase-in-c-5.5-work-with-rebase-i-in-c^
> 
> and then inserting this at the appropriate position, followed by the `git
> range-diff @{-1}...`:
> 
>   git am -s mbox
>   git update-ref js/rebase-in-c-5.5-work-with-rebase-i-in-c HEAD

Related discussion, including a fantasy tangent by me (downthread):

  https://public-inbox.org/git/20180727080807.ga11...@sigill.intra.peff.net/#t

-Peff


Re: [PATCH v2 1/1] builtin rebase: prepare for builtin rebase -i

2018-08-30 Thread Johannes Schindelin
Hi Junio,

On Wed, 29 Aug 2018, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" 
> writes:
> 
> > From: Johannes Schindelin 
> >
> > The builtin rebase and the builtin interactive rebase have been
> > developed independently, on purpose: Google Summer of Code rules
> > specifically state that students have to work on independent projects,
> > they cannot collaborate on the same project.
> 
> A much better description, especially without the less relevant "the
> reason probably is..." omitted from here.  The author's personal
> guess, while adding it does not help understanding what is already
> in the above paragraph an iota, is still a fine reading material in
> the cover letter 0/1, though.

I addressed Jonathan's concern, though.

> > One fallout is that the rebase-in-c and rebase-i-in-c patches cause no
> > merge conflicts but a royal number of tests in the test suite to fail.
> >
> > It is easy to explain why: rebase-in-c was developed under the
> > assumption that all rebase backends are implemented in Unix shell script
> > and can be sourced via `. git-rebase--`, which is no longer
> > true with rebase-i-in-c, where git-rebase--interactive is a hard-linked
> > builtin.
> >
> > This patch fixes that.
> >
> > Signed-off-by: Johannes Schindelin 
> > ---
> >  builtin/rebase.c | 81 
> >  1 file changed, 81 insertions(+)
> 
> 
> Will replace by doing:
> 
> $ git checkout js/rebase-in-c-5.5-work-with-rebase-i-in-c
> $ git checkout HEAD^
> $ git am -s mbox
> $ git range-diff @{-1}...
> $ git checkout -B @{-1}
> 
> $ git checkout pk/rebase-i-in-c-6-final
> $ git rebase --onto js/rebase-in-c-5.5-work-with-rebase-i-in-c \
>   js/rebase-in-c-5.5-work-with-rebase-i-in-c@{1} HEAD^0
> $ git range-diff @{-1}...
> $ git checkout -B @{-1}
> 
> to update the two topics and then rebuilding the integration
> branches the usual way.  I also need to replace the "other" topic
> used in this topic, so the actual procedure would be a bit more
> involved than the above, though.

Is there any reason why you avoid using `git rebase -ir` here? This should
be so much easier via

git checkout pk/rebase-i-in-c-6-final
git rebase -ir js/rebase-in-c-5.5-work-with-rebase-i-in-c^

and then inserting this at the appropriate position, followed by the `git
range-diff @{-1}...`:

git am -s mbox
git update-ref js/rebase-in-c-5.5-work-with-rebase-i-in-c HEAD

Ciao,
Dscho



Re: [PATCH v2 1/1] builtin rebase: prepare for builtin rebase -i

2018-08-29 Thread Junio C Hamano
"Johannes Schindelin via GitGitGadget" 
writes:

> From: Johannes Schindelin 
>
> The builtin rebase and the builtin interactive rebase have been
> developed independently, on purpose: Google Summer of Code rules
> specifically state that students have to work on independent projects,
> they cannot collaborate on the same project.

A much better description, especially without the less relevant "the
reason probably is..." omitted from here.  The author's personal
guess, while adding it does not help understanding what is already
in the above paragraph an iota, is still a fine reading material in
the cover letter 0/1, though.

> One fallout is that the rebase-in-c and rebase-i-in-c patches cause no
> merge conflicts but a royal number of tests in the test suite to fail.
>
> It is easy to explain why: rebase-in-c was developed under the
> assumption that all rebase backends are implemented in Unix shell script
> and can be sourced via `. git-rebase--`, which is no longer
> true with rebase-i-in-c, where git-rebase--interactive is a hard-linked
> builtin.
>
> This patch fixes that.
>
> Signed-off-by: Johannes Schindelin 
> ---
>  builtin/rebase.c | 81 
>  1 file changed, 81 insertions(+)


Will replace by doing:

$ git checkout js/rebase-in-c-5.5-work-with-rebase-i-in-c
$ git checkout HEAD^
$ git am -s mbox
$ git range-diff @{-1}...
$ git checkout -B @{-1}

$ git checkout pk/rebase-i-in-c-6-final
$ git rebase --onto js/rebase-in-c-5.5-work-with-rebase-i-in-c \
  js/rebase-in-c-5.5-work-with-rebase-i-in-c@{1} HEAD^0
$ git range-diff @{-1}...
$ git checkout -B @{-1}

to update the two topics and then rebuilding the integration
branches the usual way.  I also need to replace the "other" topic
used in this topic, so the actual procedure would be a bit more
involved than the above, though.

Thanks.


[PATCH v2 1/1] builtin rebase: prepare for builtin rebase -i

2018-08-29 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

The builtin rebase and the builtin interactive rebase have been
developed independently, on purpose: Google Summer of Code rules
specifically state that students have to work on independent projects,
they cannot collaborate on the same project.

One fallout is that the rebase-in-c and rebase-i-in-c patches cause no
merge conflicts but a royal number of tests in the test suite to fail.

It is easy to explain why: rebase-in-c was developed under the
assumption that all rebase backends are implemented in Unix shell script
and can be sourced via `. git-rebase--`, which is no longer
true with rebase-i-in-c, where git-rebase--interactive is a hard-linked
builtin.

This patch fixes that.

Signed-off-by: Johannes Schindelin 
---
 builtin/rebase.c | 81 
 1 file changed, 81 insertions(+)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 4e69458161..99fd5d4017 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -326,6 +326,13 @@ static void add_var(struct strbuf *buf, const char *name, 
const char *value)
}
 }
 
+static const char *resolvemsg =
+N_("Resolve all conflicts manually, mark them as resolved with\n"
+"\"git add/rm \", then run \"git rebase --continue\".\n"
+"You can instead skip this commit: run \"git rebase --skip\".\n"
+"To abort and get back to the state before \"git rebase\", run "
+"\"git rebase --abort\".");
+
 static int run_specific_rebase(struct rebase_options *opts)
 {
const char *argv[] = { NULL, NULL };
@@ -333,6 +340,79 @@ static int run_specific_rebase(struct rebase_options *opts)
int status;
const char *backend, *backend_func;
 
+   if (opts->type == REBASE_INTERACTIVE) {
+   /* Run builtin interactive rebase */
+   struct child_process child = CHILD_PROCESS_INIT;
+
+   argv_array_pushf(_array, "GIT_CHERRY_PICK_HELP=%s",
+resolvemsg);
+   if (!(opts->flags & REBASE_INTERACTIVE_EXPLICIT)) {
+   argv_array_push(_array, "GIT_EDITOR=:");
+   opts->autosquash = 0;
+   }
+
+   child.git_cmd = 1;
+   argv_array_push(, "rebase--interactive");
+
+   if (opts->action)
+   argv_array_pushf(, "--%s", opts->action);
+   if (opts->keep_empty)
+   argv_array_push(, "--keep-empty");
+   if (opts->rebase_merges)
+   argv_array_push(, "--rebase-merges");
+   if (opts->rebase_cousins)
+   argv_array_push(, "--rebase-cousins");
+   if (opts->autosquash)
+   argv_array_push(, "--autosquash");
+   if (opts->flags & REBASE_VERBOSE)
+   argv_array_push(, "--verbose");
+   if (opts->flags & REBASE_FORCE)
+   argv_array_push(, "--no-ff");
+   if (opts->restrict_revision)
+   argv_array_pushf(,
+"--restrict-revision=^%s",
+
oid_to_hex(>restrict_revision->object.oid));
+   if (opts->upstream)
+   argv_array_pushf(, "--upstream=%s",
+
oid_to_hex(>upstream->object.oid));
+   if (opts->onto)
+   argv_array_pushf(, "--onto=%s",
+oid_to_hex(>onto->object.oid));
+   if (opts->squash_onto)
+   argv_array_pushf(, "--squash-onto=%s",
+oid_to_hex(opts->squash_onto));
+   if (opts->onto_name)
+   argv_array_pushf(, "--onto-name=%s",
+opts->onto_name);
+   argv_array_pushf(, "--head-name=%s",
+opts->head_name ?
+opts->head_name : "detached HEAD");
+   if (opts->strategy)
+   argv_array_pushf(, "--strategy=%s",
+opts->strategy);
+   if (opts->strategy_opts)
+   argv_array_pushf(, "--strategy-opts=%s",
+opts->strategy_opts);
+   if (opts->switch_to)
+   argv_array_pushf(, "--switch-to=%s",
+opts->switch_to);
+   if (opts->cmd)
+   argv_array_pushf(, "--cmd=%s", opts->cmd);
+   if (opts->allow_empty_message)
+   argv_array_push(, "--allow-empty-message");
+   if (opts->allow_rerere_autoupdate > 0)
+   argv_array_push(, "--rerere-autoupdate");
+   else if (opts->allow_rerere_autoupdate == 0)
+   argv_array_push(,