Git development
 help / color / mirror / Atom feed
From: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Francis Moreau <francis.moro@gmail.com>,
	Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>,
	Michael J Gruber <git@drmicha.warpmail.net>,
	git@vger.kernel.org
Subject: Re: git-rebase skips automatically no more needed commits
Date: Wed, 14 Sep 2011 22:19:39 -0400 (EDT)	[thread overview]
Message-ID: <alpine.DEB.2.00.1109122139110.12564@debian> (raw)
In-Reply-To: <alpine.DEB.2.00.1109090900301.12564@debian>

On Fri, 9 Sep 2011, Martin von Zweigbergk wrote:

> On Fri, 9 Sep 2011, Francis Moreau wrote:
> 
> > please let me know when you submitting your work, I'm interested to see it.
> 
> Will do. It's not really that much work [...]

It seems to be a little less straight-forward than I had first
expected.

We want to get all commits in $oldbase..$branch that do not share
patch-id with commits in $oldbase..$newbase. These are the commits
prefixed by '+' in 'git cherry $newbase $branch $oldbase'. However,
just getting the list of commits is not enough, because we currently
don't explicitly provide a list of commits to create patches from, but
instead run something like 'git format-patch --ignore-if-in-upstream
--stdout | git am --rebasing'.

I can see a few different solutions.

 1. Teach git-format-patch a --stdin option that makes it read the
    list of commits from the command line. git-format-patch currently
    itself walks the history, so I'm not sure how such an option
    should interact with current option. The options should probably
    be mutually exclusive.

 2. Somehow modify git-rebase--am.sh not to depend on format-patch. In
    [1], Junio mentions rewriting git-rebase--am.sh "to have
    format-patch avoid the cost of actually generating the patch text"
    and "when using "am" for rebasing we do not really care anything
    but the commit object names". If all we need is the commit name,
    why would we not use cherry-pick/sequencer instead of git-am?
    Sorry if this makes no sense; I'm not familiar with the git-am
    code.

 3. If I'm reading the code correctly, 'git cherry $upstream $branch
    $limit' does one walk to find patch-ids for "$upstream
    ^$branch". It then walks "$branch ^$upstream ^$limit" and removes
    the commits that have the same patch-id as a commit found in the
    first walk. Since format-patch uses the revision walking
    machinery, we could make it possible to ask git-rev-list directly
    what git-cherry does, i.e. to ask for "commits in a..b that don't
    share patch-id with commits in c..d". We could even make it more
    generic by allowing queries like "commits in
    $rev_list_expression_1 that don't share patch-id with commits in
    $rev_list_expression_2".

I'm not sure which option is better. Option 1 seems easiest to
implement. Option 3 seems a bit harder, but may bring more value. I'm
just not sure if teaching rev-list about patch-ids is generally useful
or if it would feel more like a hack for this specific case. Option 2
is less clear to me and I would probably need more input before
implementing, but I trust Junio that it would be a good long-term
solution.

I will be moving soon and I don't know how much time I will have to
implement any of this. Still, I would be happy to hear your
opinions. Maybe I'm just missing something and there is even a simple
solution to my problem.


Martin


[1] http://thread.gmane.org/gmane.comp.version-control.git/180976/focus=181085

  parent reply	other threads:[~2011-09-15  2:25 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-06 16:08 git-rebase skips automatically no more needed commits Francis Moreau
2011-09-06 17:09 ` Junio C Hamano
2011-09-06 19:28   ` Francis Moreau
2011-09-07 13:19     ` Michael J Gruber
     [not found]       ` <CAC9WiBi_bkLNJZckq2fr3eb6ie+KVfauE_PyA3GcaW5Ga-isFw@mail.gmail.com>
2011-09-07 16:29         ` Junio C Hamano
2011-09-08  7:10           ` Francis Moreau
2011-09-08 13:14             ` Martin von Zweigbergk
     [not found]               ` <CAC9WiBiMYUfaPtrXyW=W7qaZnJqLeFO-B3od7X4u8wUrb8hfUA@mail.gmail.com>
2011-09-09  2:23                 ` Martin von Zweigbergk
2011-09-09  6:43                   ` Francis Moreau
2011-09-09 13:06                     ` Martin von Zweigbergk
2011-09-09 13:25                       ` Francis Moreau
2011-09-15  2:19                       ` Martin von Zweigbergk [this message]
2011-09-15  3:41                         ` Junio C Hamano
2011-09-16  1:27                           ` Martin von Zweigbergk
2011-09-20 20:45                             ` Martin von Zweigbergk
2011-09-24  9:12                               ` Ramkumar Ramachandra
2011-09-25  3:25                                 ` Martin von Zweigbergk
2011-09-26  9:10                                   ` Thomas Rast
2011-09-28 14:49                                     ` Martin von Zweigbergk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.00.1109122139110.12564@debian \
    --to=martin.von.zweigbergk@gmail.com \
    --cc=francis.moro@gmail.com \
    --cc=git@drmicha.warpmail.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox