Re: [PATCH] merge_blobs: use strbuf instead of manually-sized mmfile_t

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

 



Thank you for working on this! Let me just address two things from an outsider's perspective:

On 2016-02-16 at 06:50, Jeff King wrote:
Yeah, maybe. There were two reasons I avoided adding a test.

One, I secretly hoped that by dragging my feet we could get consensus on
just ripping out merge-tree entirely. ;)

Since I cannot comment on the code quality or maturity of merge-tree, but would appreciate if this tool worked as expected, let me quote Chris' comment[1] on why he uses merge-tree instead of just "git merge", hoping that it has an impact upon your decision whether to keep the code or "ripping it out":

"One might ask, why not use the porcelain 'git merge' command? One
reason is, in the context of the two phase commit protocol, we'd rather
do pretty much everything we possibly can in the voting stage, leaving
ourselves with nothing to do in the finish phase except updating the
head to the commit we just created, and possibly updating the working
directory--operations that are guaranteed to work. Since 'git merge'
will update the head, we'd prefer to do it during the final phase of the
commit, but we can't guarantee it will work. There is not a convenient
way to do a merge dry run during the voting phase. Although I can
conceive of ways to do the merge during the voting phase and roll back
to the previous head if we need to, that feels a little riskier. Doing
the merge ourselves, here, also frees us from having to work with a
working directory, required by the porcelain 'git merge' command. This
means we can use bare repositories and/or have transactions that use
a head other than the repositories 'current' head."

Two, I'm not sure what the test output _should_ be. I think this case is
buggy. So we can check that we don't corrupt the heap, which is nice,

What do you mean exactly by "this case is buggy"? Doesn't make sense to me.

Stefan

[1] https://github.com/Pylons/acidfs/blob/master/acidfs/__init__.py
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[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]