git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	phillip.wood@dunelm.org.uk
Cc: Warren He <pickydaemon@gmail.com>, git@vger.kernel.org, wh109@yahoo.com
Subject: Re: [PATCH v2] rebase: introduce --update-branches option
Date: Mon, 23 Sep 2019 10:40:30 +0100	[thread overview]
Message-ID: <6a440f3e-611e-c00d-9b4d-1f7e2fea853e@gmail.com> (raw)
In-Reply-To: <nycvar.QRO.7.76.6.1909091605540.5377@tvgsbejvaqbjf.bet>

Hi

On 09/09/2019 15:13, Johannes Schindelin wrote:
> Hi,
> 
> On Mon, 9 Sep 2019, Phillip Wood wrote:
> 
>> On 08/09/2019 00:44, Warren He wrote:
>>> Everyone in this thread, thanks for your support and encouragement.
>>> [...]
>>>> It should not really imply `--interactive`, but `--rebase-merges`.
>>>
>>> `imply_interactive` doesn't fully switch on `--interactive`, i.e., causing
>>> the
>>> editor to open. It only selects the backend, which I think we're saying is
>>> the
>>> right thing. I've dropped the `-i` from the test description.
>>>
>>> And we don't really have to imply --rebase-merges, in case someone would
>>> prefer
>>> to linearize things, which who knows? Running that non-rebase-merges command
>>> in
>>> the example scenario from my original post should give something like this:
>>
>> I think it would probably be less confusing to default to preserving merges
> 
> s/preserving/rebasing/?
> 
>> and having an option to turn that off - people are going to be surprised if
>> their history is linearized.
> 
> I don't think it makes any sense to linearize the history while updating
> branches, as the commits will be all jumbled up. Imagine this history:
> 
> 	- A - B - C - D -
> 	    \       /
> 	      E - F
> 
> Typically, it does not elicit any "bug" reports, but this can easily be
> linearized to
> 
> 	- A' - B' - E' - C' - F' -
> 
> In my mind, it makes no sense to update any local branches that pointed
> to C and F to point to C' and F', respectively.
> 

I agree

>> [...]
>>> * * *
>>>
>>> And then there's the discussion about using `exec git branch -f`. To
>>> summarize
>>> the issues collected from the entire thread:
>>>
>>> 1. the changes aren't atomically applied at the end of the rebase
>>> 2. it fails when the branch is checked out in a worktree
>>> 3. it clobbers the branch if anything else updates it during the rebase
>>> 4. the way we prepare the unprefixed branch doesn't work right some exotic
>>> cases
>>> 5. the reflog message it leaves is uninformative
>>>
>>> For #4, I think we've lucked out actually. The `load_ref_decorations`
>>> routine we
>>> use determines that a ref is `DECORATION_REF_LOCAL` under the condition
>>> `starts_with(refname, "refs/heads/")` (log-tree.c:114, add_ref_decoration),
>>> so
>>> `prettify_refname` will find the prefix and skip it. But that's an invariant
>>> maintained by two pieces of code pretty far away from each other.
>>>
>>> For #5, for the convenience of readers, the reflog entry it leaves looks
>>> like this:
>>>
>>> ```
>>> 00873f2 feat-e@{0}: branch: Reset to HEAD
>>> ```
>>>
>>> Not great.
>>>
>>> I haven't made any changes to this yet, but I've thought about what I want.
>>> My
>>> favorite so far is to add a new todo command that just does everything
>>> right. It
>>> would make a temparary ref `refs/rewritten-heads/xxx` (or something), and
>>> update
>>> `refs/heads/xxx` at the end.
>>
>> I think that's the best way to do it. If we had a command like 'branch
>> <branch-name>' that creates a ref to remember the current HEAD and saves the
>> current branch head. Then at the end rebase can update the branches to point
>> to the saved commits if the branch is unchanged. If the rebase is aborted then
>> we don't end up with some branches updated and others not.
> 
> I'd avoid cluttering the space with more commands. For `branch`, for
> example, the natural short command would be `b`, but that already means
> `break`.

We could just not have a short name, after all --update-branch is hardly 
a short alternative

> 
> In contrast, I would think that
> 
> 	label --update-branch my-side-track
> 
> would make for a nicer read than
> 
> 	label my-side-track
> 	branch my-side-track

I agree it would be nice to do both on a single line, my argument was 
mainly against using 'exec branch ...' so that we can defer the branch 
updates until the very end of the rebase. The branch command could set a 
label as well or we could add an option to label I'm not that bothered 
either way at the moment. Another possibility which we probably don't 
want is to have labels starting refs/ imply --update-branch

> Of course, it would be a lot harder to bring back the safety of `git
> update-ref`'s `<old-revision>` safe-guard, in either forms.

Is it that difficult to write the current branch HEAD to a file when we 
label it and then read those back at the end when we update the refs? or 
are you thinking of calling 'git branch' instead of 
ref_transaction_update()? I'm not sure what the advantage of 'git 
branch' is though.

Best Wishes

Phillip

> 
> And of course, the first form would _require_ the logic to be moved to
> `make_script_with_merges()` because we could not otherwise guarantee
> that the labels corresponding to local branch names aren't already in
> use, for different commits.
> 
>> Side Note
>>    I'd avoid creating another worktree local ref refs/rewritten-heads/.
>>    Either store them under refs/rewritten/ or refs/worktree/
> 
> Yep.
> 
> Ciao,
> Dscho
> 

  parent reply	other threads:[~2019-09-23  9:40 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-02 23:41 [PATCH] rebase: introduce --update-branches option Warren He
2019-09-02 23:41 ` Warren He
2019-09-03 13:26   ` Johannes Schindelin
2019-09-07 23:44     ` [PATCH v2] " Warren He
2019-09-07 23:44       ` Warren He
2019-09-09 10:44       ` Phillip Wood
2019-09-09 14:13         ` Johannes Schindelin
2019-09-09 18:11           ` Junio C Hamano
2019-09-23  9:40           ` Phillip Wood [this message]
2019-09-03  0:50 ` [PATCH] " brian m. carlson
2019-09-03  1:21   ` Junio C Hamano
2019-09-03  1:39     ` Taylor Blau
2019-09-07 23:41       ` Warren He
2019-09-08 18:04         ` brian m. carlson
2019-09-03 12:19 ` Phillip Wood
2019-09-07 23:43   ` Warren He

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=6a440f3e-611e-c00d-9b4d-1f7e2fea853e@gmail.com \
    --to=phillip.wood123@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=phillip.wood@dunelm.org.uk \
    --cc=pickydaemon@gmail.com \
    --cc=wh109@yahoo.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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).