Re: [PATCH v8 06/16] sequencer: introduce the `merge` command

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 23/04/18 13:20, Johannes Schindelin wrote:
Hi Phillip,

On Sat, 21 Apr 2018, Phillip Wood wrote:

On 21/04/18 11:33, Johannes Schindelin wrote:
This patch is part of the effort to reimplement `--preserve-merges` with
a substantially improved design, a design that has been developed in the
Git for Windows project to maintain the dozens of Windows-specific patch
series on top of upstream Git.

The previous patch implemented the `label` and `reset` commands to label
commits and to reset to labeled commits. This patch adds the `merge`
command, with the following syntax:

The two patches seem to have been fused together in this series.

Indeed. I have yet to investigate further how that happened, it could be a
bug in my series after all.

If the reset command fails because it would overwrite untracked files it
says

error: Untracked working tree file 'b' would be overwritten by merge.

Followed by the hint to edit the todo file. Saying 'merge' rather 'reset' is
possibly confusing to users. Perhaps it could call
setup_unpack_trees_porcelain(), though that would need to be extended to
handle 'reset'.

Yes, and it changes global state :-(

Maybe we can leave it as-is for now? After all, it should be clear to the
user what is happening. The most important part is the "Untracked working
tree file"...

I'm fine with leaving it, I've might get round to doing a small series to clean things up slightly in a few weeks. At the moment setup_unpack_trees_porcelain() leaks memory as it is called for each merge and allocates new strings each time. It would also be nice if the error messages reflected the command, so it said 'cherry-pick', 'revert' or 'reset' rather than 'merge'

Also it currently refuses to overwrite ignored files which is either
annoying or safe depending on one's point of view.

Let me think about that. My gut feeling says: if it is easy to do, then
let's nuke ignored files, but keep untracked files. But I do not think
that the unpack-trees machinery was taught to know about .gitignore...

Seeing as `label` and `reset` really are mostly about revisions we see
along the lines, I think that the common case will *not* overwrite any
untracked files, ever. You would have to use `reset` on a
not-previously-seen commit and/or add `exec` commands designed to
interfere with the `reset`.

So I tend to want to not bother with discerning between untracked and
ignored files here.

I don't think it's a pressing concern. In the past I once had a patch series that removed some tracked files in favor of having the build system generate them and added them to .gitignore. Each time I rebased I had to manually remove them at some stage which was annoying but that is quite a rare occurrence.

Best Wishes

Phillip


Ciao,
Dscho





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux