You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Näslund <da...@longitudo.com> on 2009/10/27 21:28:54 UTC
Issue 1493 - property merges should use libsvn_diff
I have some questions about the three way diffs of properties.
When digging into the merge code for issue 1493 I found that it does use
svn_diff_mem_string_diff3() and svn_diff_mem_string_output_unified() in
libsvn_wc/props.c (maybe_generate_propconflict). I suppose the issue
should be closed then?
On the other hand I haven't been able to view the resulting three way
diff! If I choose 'diff-full' in the interactive conflict resolutioner I
get this:
[[[
Conflict for property 'prop' discovered on '/tmp/wc2/foo'.
Select: (p) postpone,
(mf) mine-full, (tf) theirs-full,
(s) show all options: df
Invalid option; there's no merged version to diff.
]]]
For a property conflict I get theese files:
[[[
? foo.prej
C foo
]]]
Where foo.prej contains:
[[[
Trying to change property 'prop' from 'dog' to 'horse',
but the property has been locally changed from 'dog' to 'cat'.
]]]
Wouldn't it be better to have a three way diff text in foo.prej instead?
I'm attaching the script I used for testing the merge.
Daniel Näslund
Re: Issue 1493 - property merges should use libsvn_diff
Posted by "C. Michael Pilato" <cm...@collab.net>.
David Glasser wrote:
> On Thu, Oct 29, 2009 at 4:10 PM, C. Michael Pilato <cm...@collab.net> wrote:
>> Daniel Näslund wrote:
>>> On Wed, Oct 28, 2009 at 11:37:48PM +0100, C. Michael Pilato wrote:
>>>> Why not change the message to simply refer to files in the working copy
>>>> where the user can examine the various property values for themselves?
>>>>
>>>> Trying to the value of property 'NAME', but there appears to already
>>>> be a property 'NAME' with the same final value. See NAME.prej.mine
>>>> (or NAME.prej.theirs) for the property value.
>>>>
>>>> That kinda thing?
>> [...]
>>
>>> My only concern is for adding extra prej.{mine,theirs} to the directory.
>>> But if they would be automatically cleaned up when the conflict is
>>> resolved then there would be no harm done.
>> Hrm... yeah, I would, too.
>>
>>> I would feel even better if there was a way to use plain diff3-format
>>> in .prej-files but maybe there is too little to win? How often
>>> do we have long properties spanning multiple lines?
>> svn:mergeinfo and svn:ignores are the only of our own properties that do right?
>
> And svn:externals. (I can think of a few other tools that use
> multi-line properties.)
It might be overkill, but what if we attempted the diff and, if the result
would be a simple "Binary files differ" type result, fallback to the
mine/theirs file drops?
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2412862
Re: Issue 1493 - property merges should use libsvn_diff
Posted by David Glasser <gl...@davidglasser.net>.
On Thu, Oct 29, 2009 at 4:10 PM, C. Michael Pilato <cm...@collab.net> wrote:
> Daniel Näslund wrote:
>> On Wed, Oct 28, 2009 at 11:37:48PM +0100, C. Michael Pilato wrote:
>>> Why not change the message to simply refer to files in the working copy
>>> where the user can examine the various property values for themselves?
>>>
>>> Trying to the value of property 'NAME', but there appears to already
>>> be a property 'NAME' with the same final value. See NAME.prej.mine
>>> (or NAME.prej.theirs) for the property value.
>>>
>>> That kinda thing?
>
> [...]
>
>> My only concern is for adding extra prej.{mine,theirs} to the directory.
>> But if they would be automatically cleaned up when the conflict is
>> resolved then there would be no harm done.
>
> Hrm... yeah, I would, too.
>
>> I would feel even better if there was a way to use plain diff3-format
>> in .prej-files but maybe there is too little to win? How often
>> do we have long properties spanning multiple lines?
>
> svn:mergeinfo and svn:ignores are the only of our own properties that do right?
And svn:externals. (I can think of a few other tools that use
multi-line properties.)
--dave
--
glasser@davidglasser.net | langtonlabs.org | flickr.com/photos/glasser/
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2412861
Re: Issue 1493 - property merges should use libsvn_diff
Posted by "C. Michael Pilato" <cm...@collab.net>.
Daniel Näslund wrote:
> On Wed, Oct 28, 2009 at 11:37:48PM +0100, C. Michael Pilato wrote:
>> Why not change the message to simply refer to files in the working copy
>> where the user can examine the various property values for themselves?
>>
>> Trying to the value of property 'NAME', but there appears to already
>> be a property 'NAME' with the same final value. See NAME.prej.mine
>> (or NAME.prej.theirs) for the property value.
>>
>> That kinda thing?
[...]
> My only concern is for adding extra prej.{mine,theirs} to the directory.
> But if they would be automatically cleaned up when the conflict is
> resolved then there would be no harm done.
Hrm... yeah, I would, too.
> I would feel even better if there was a way to use plain diff3-format
> in .prej-files but maybe there is too little to win? How often
> do we have long properties spanning multiple lines?
svn:mergeinfo and svn:ignores are the only of our own properties that do right?
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2412856
Re: Issue 1493 - property merges should use libsvn_diff
Posted by Daniel Näslund <da...@longitudo.com>.
On Wed, Oct 28, 2009 at 11:37:48PM +0100, C. Michael Pilato wrote:
> > I could create a string from the svn_diff_mem_string_output_merge2()
> > created stream and pass that back to be used in the above code. But
> > would it be a problem to have a diff3 text for some conflicts and the
> > error message for others?
> >
> > Does the diff3 format handle deletes and adds? I first thought that it
> > was just a question of leaving one part of the diff blank but that seems
> > to easy.
> >
> >> Well we just had a good laugh here by modifying your script to set
> >> an SVG image as the property value instead of 'horse'.
> >> I'm not gonna try a PNG because that would probably screw the terminal :)
> >
> > I could write a parser for translating svg files into ascii art when
> > stored as a property. :-)
>
> Why not change the message to simply refer to files in the working copy
> where the user can examine the various property values for themselves?
>
> Trying to the value of property 'NAME', but there appears to already
> be a property 'NAME' with the same final value. See NAME.prej.mine
> (or NAME.prej.theirs) for the property value.
>
> That kinda thing?
That would be better than this output:
[[[
Trying to change property 'prop' from 'A
Whole
Lotta
Lines' to 'Some
Other
Randomly
Choosen
Words',
but the property has been locally changed from 'A
Whole
Lotta
Lines' to 'The
Brown
Fox
Jumped'.
]]]
My only concern is for adding extra prej.{mine,theirs} to the directory.
But if they would be automatically cleaned up when the conflict is
resolved then there would be no harm done.
I would feel even better if there was a way to use plain diff3-format
in .prej-files but maybe there is too little to win? How often
do we have long properties spanning multiple lines?
/Daniel
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2412854
Re: Issue 1493 - property merges should use libsvn_diff
Posted by "C. Michael Pilato" <cm...@collab.net>.
Daniel Näslund wrote:
> On Tue, Oct 27, 2009 at 11:09:40PM +0100, Stefan Sperling wrote:
>>> When digging into the merge code for issue 1493 I found that it does use
>>> svn_diff_mem_string_diff3() and svn_diff_mem_string_output_unified() in
>>> libsvn_wc/props.c (maybe_generate_propconflict). I suppose the issue
>>> should be closed then?
>>>
>>> On the other hand I haven't been able to view the resulting three way
>>> diff! If I choose 'diff-full' in the interactive conflict resolutioner I
>>> get this:
>>>
>>> [[[
>>> Conflict for property 'prop' discovered on '/tmp/wc2/foo'.
>>> Select: (p) postpone,
>>> (mf) mine-full, (tf) theirs-full,
>>> (s) show all options: df
>>> Invalid option; there's no merged version to diff.
>> No idea what's going wrong here yet, but it certainly doesn't look
>> right.
>
>>> Where foo.prej contains:
>>>
>>> [[[
>>> Trying to change property 'prop' from 'dog' to 'horse',
>>> but the property has been locally changed from 'dog' to 'cat'.
>>> ]]]
>> Isn't that the best 3-way diff format ever?
>
> Maybe... It is declared in this piece of code in libsvn_wc/props.c
> (apply_single_generic_prop_change):
> [[[
>
> if (got_conflict)
> {
> /* Describe the conflict, referring to base_val as well as
> working_val for the user's convenience. */
> if (working_val && base_val
> && svn_string_compare(working_val, base_val))
> *conflict = svn_string_createf
> (result_pool,
> _("Trying to change property '%s' from '%s' to '%s',\n"
> "but property already exists with value '%s'."),
> propname, old_val->data, new_val->data, working_val->data);
> else if (working_val && base_val)
> *conflict = svn_string_createf
> (result_pool,
> _("Trying to change property '%s' from '%s' to '%s',\n"
> "but the property has been locally changed from '%s' to "
> "'%s'."),
> propname, old_val->data, new_val->data,
> base_val->data, working_val->data);
> else if (working_val)
> *conflict = svn_string_createf
> (result_pool,
> _("Trying to change property '%s' from '%s' to '%s',\n"
> "but property has been locally added with value "
> "'%s'."),
> propname, old_val->data, new_val->data, working_val->data);
> else if (base_val)
> *conflict = svn_string_createf
> (result_pool,
> _("Trying to change property '%s' from '%s' to '%s',\n"
> "but it has been locally deleted."),
> propname, old_val->data, new_val->data);
> else
> *conflict = svn_string_createf
> (result_pool,
> _("Trying to change property '%s' from '%s' to '%s',\n"
> "but the property does not exist."),
> propname, old_val->data, new_val->data);
> }
> }
> ]]]
>
> I could create a string from the svn_diff_mem_string_output_merge2()
> created stream and pass that back to be used in the above code. But
> would it be a problem to have a diff3 text for some conflicts and the
> error message for others?
>
> Does the diff3 format handle deletes and adds? I first thought that it
> was just a question of leaving one part of the diff blank but that seems
> to easy.
>
>> Well we just had a good laugh here by modifying your script to set
>> an SVG image as the property value instead of 'horse'.
>> I'm not gonna try a PNG because that would probably screw the terminal :)
>
> I could write a parser for translating svg files into ascii art when
> stored as a property. :-)
Why not change the message to simply refer to files in the working copy
where the user can examine the various property values for themselves?
Trying to the value of property 'NAME', but there appears to already
be a property 'NAME' with the same final value. See NAME.prej.mine
(or NAME.prej.theirs) for the property value.
That kinda thing?
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2412290
Re: Issue 1493 - property merges should use libsvn_diff
Posted by Daniel Näslund <da...@longitudo.com>.
On Tue, Oct 27, 2009 at 11:09:40PM +0100, Stefan Sperling wrote:
> > When digging into the merge code for issue 1493 I found that it does use
> > svn_diff_mem_string_diff3() and svn_diff_mem_string_output_unified() in
> > libsvn_wc/props.c (maybe_generate_propconflict). I suppose the issue
> > should be closed then?
> >
> > On the other hand I haven't been able to view the resulting three way
> > diff! If I choose 'diff-full' in the interactive conflict resolutioner I
> > get this:
> >
> > [[[
> > Conflict for property 'prop' discovered on '/tmp/wc2/foo'.
> > Select: (p) postpone,
> > (mf) mine-full, (tf) theirs-full,
> > (s) show all options: df
> > Invalid option; there's no merged version to diff.
>
> No idea what's going wrong here yet, but it certainly doesn't look
> right.
> > Where foo.prej contains:
> >
> > [[[
> > Trying to change property 'prop' from 'dog' to 'horse',
> > but the property has been locally changed from 'dog' to 'cat'.
> > ]]]
>
> Isn't that the best 3-way diff format ever?
Maybe... It is declared in this piece of code in libsvn_wc/props.c
(apply_single_generic_prop_change):
[[[
if (got_conflict)
{
/* Describe the conflict, referring to base_val as well as
working_val for the user's convenience. */
if (working_val && base_val
&& svn_string_compare(working_val, base_val))
*conflict = svn_string_createf
(result_pool,
_("Trying to change property '%s' from '%s' to '%s',\n"
"but property already exists with value '%s'."),
propname, old_val->data, new_val->data, working_val->data);
else if (working_val && base_val)
*conflict = svn_string_createf
(result_pool,
_("Trying to change property '%s' from '%s' to '%s',\n"
"but the property has been locally changed from '%s' to "
"'%s'."),
propname, old_val->data, new_val->data,
base_val->data, working_val->data);
else if (working_val)
*conflict = svn_string_createf
(result_pool,
_("Trying to change property '%s' from '%s' to '%s',\n"
"but property has been locally added with value "
"'%s'."),
propname, old_val->data, new_val->data, working_val->data);
else if (base_val)
*conflict = svn_string_createf
(result_pool,
_("Trying to change property '%s' from '%s' to '%s',\n"
"but it has been locally deleted."),
propname, old_val->data, new_val->data);
else
*conflict = svn_string_createf
(result_pool,
_("Trying to change property '%s' from '%s' to '%s',\n"
"but the property does not exist."),
propname, old_val->data, new_val->data);
}
}
]]]
I could create a string from the svn_diff_mem_string_output_merge2()
created stream and pass that back to be used in the above code. But
would it be a problem to have a diff3 text for some conflicts and the
error message for others?
Does the diff3 format handle deletes and adds? I first thought that it
was just a question of leaving one part of the diff blank but that seems
to easy.
> Well we just had a good laugh here by modifying your script to set
> an SVG image as the property value instead of 'horse'.
> I'm not gonna try a PNG because that would probably screw the terminal :)
I could write a parser for translating svg files into ascii art when
stored as a property. :-)
/Daniel
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2412268
Re: Issue 1493 - property merges should use libsvn_diff
Posted by Stefan Sperling <st...@elego.de>.
On Tue, Oct 27, 2009 at 10:28:54PM +0100, Daniel Näslund wrote:
> I have some questions about the three way diffs of properties.
>
> When digging into the merge code for issue 1493 I found that it does use
> svn_diff_mem_string_diff3() and svn_diff_mem_string_output_unified() in
> libsvn_wc/props.c (maybe_generate_propconflict). I suppose the issue
> should be closed then?
>
> On the other hand I haven't been able to view the resulting three way
> diff! If I choose 'diff-full' in the interactive conflict resolutioner I
> get this:
>
> [[[
> Conflict for property 'prop' discovered on '/tmp/wc2/foo'.
> Select: (p) postpone,
> (mf) mine-full, (tf) theirs-full,
> (s) show all options: df
> Invalid option; there's no merged version to diff.
No idea what's going wrong here yet, but it certainly doesn't look
right.
> ]]]
>
> For a property conflict I get theese files:
>
> [[[
> ? foo.prej
> C foo
> ]]]
>
> Where foo.prej contains:
>
> [[[
> Trying to change property 'prop' from 'dog' to 'horse',
> but the property has been locally changed from 'dog' to 'cat'.
> ]]]
Isn't that the best 3-way diff format ever?
Well we just had a good laugh here by modifying your script to set
an SVG image as the property value instead of 'horse'.
I'm not gonna try a PNG because that would probably screw the terminal :)
> Wouldn't it be better to have a three way diff text in foo.prej instead?
Definitely. We might want to make sure the content isn't binary
before printing a diff.
Stefan
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2411845