On Sat, Feb 12, 2011 at 7:31 AM, Daniel Shahaf <d.s_at_daniel.shahaf.name>wrote:
> Daniel Becroft wrote on Sat, Feb 12, 2011 at 06:27:31 +1000:
> > On Fri, Feb 11, 2011 at 11:26 PM, Daniel Shahaf <d.s_at_daniel.shahaf.name
> >wrote:
> > > Daniel Becroft wrote on Thu, Feb 10, 2011 at 07:21:30 +1000:
> > > > @@ -1118,6 +1120,33 @@ merge_binary_file(svn_skel_t **work_items,
> > > > + /* Attempt to merge the binary file. At the moment, we can only
> > > > + handle the special case: if the LEFT side of the merge is equal
> > > > + to WORKING, then we can copy RIGHT directly. */
> > >
> > > The comment in libsvn_client mentioned two special case, what happened
> > > to the other one? Does the existing wc code already handle it? (I'd be
> > > surprised)
> > >
> > > - Alternately, if the 'left' side of the merge doesn't exist
> in
> > > - the repository, and the 'right' side of the merge is
> > > - identical to the WC, pretend we did the merge (a no-op).
> > >
> >
> > I've been trying to think of a valid scenario for this to occur, but I
> can't
> > seem to think of one. There's a comment further up:
> >
>
> Concocted:
> % svn add foo.bin
> % svn ci -m r5
> % svn rm ^/foo.bin -m r6
> % svnmucc -m r7 put foo.bin ^/foo.bin
> % svn merge -c7 ^/ ./
>
Maybe it's early, and the coffee hasn't kicked in yet, but wouldn't (and
shouldn't) this give a tree-conflict? foo.bin_at_7 and foo.bin in the WC are
two different nodes (with the same name).
I just tried the above (without svnmucc, but just a svn rm and svn add) with
both 1.6.x, and trunk, and both raised a tree conflict:
Using 1.6.x:
--- Merging r3 into '.':
C foo.txt
Summary of conflicts:
Tree conflicts: 1
Using trunk:
--- Merging r3 into '.':
C foo.bin
--- Recording mergeinfo for merge of r3 into '.':
U .
Summary of conflicts:
Tree conflicts: 1
Replacing the -c with a -r in the 'svn merge' command gives a status of "R
foo.bin", which is correct.
> > /* Other easy outs: if the merge target isn't under version
> > control, or is just missing from disk, fogettaboutit. There's no
> > way svn_wc_merge4() can do the merge. */
> >
> > This should apply to all situations (binary and text files), so I think
> this
> > second "special case" is redundant.
> >
>
> Not sure. If the merge-left does not exist, and the local file doesn't
> exist either, then the situation is 'compatible' and can be merged...
>
Isn't this just an incoming add (which is handled by a different function)?
> (that depends on where exactly the local file is missing --- in BASE, in
> WORKING, or in ACTUAL)
>
I'll look into this a bit more. As I said, the behavior for binary files is
now the same as text files, especially in terms of missing files in
BASE/WORKING/ACTUAL. What that behavior is, I'm not 100% sure on.
<snipped />
> > > You only need HAS_LOCAL_MODS now when CONTENT_STATE is set. Shouldn't
> > > you skip this call when CONTENT_STATE is NULL then? (It may perform
> > > stat() or read().)
> > >
> >
> > Agreed, however this was always the case. The only change I made to this
> > section was indentation
> > due to removing the surrounding if (merge_required) { } clause.
>
> Oh, you're right. I was confused because the 'diff -w' patch showed
> these two lines as removed and later re-added; your original patch
> shows it correctly.
>
> Sorry for my confusion.
>
> > I didn't want to make other changes that would be clouded.
> >
>
> +1
>
> > I can submit a follow-up patch that fixes this as well, if necessary.
>
> That would be great, assuming that the FALSE *really* should be changed
> to TRUE. (I haven't investigated that myself.)
>
Not sure what is is in reference to. I was thinking of just putting a if
(content_state) before the local modifications check (again, coffee may not
have kicked in yet).
Cheers,
Daniel^2.
Received on 2011-02-11 23:38:13 CET