You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2015/03/17 13:05:11 UTC
svn commit: r1667280 - in /subversion/trunk/subversion: libsvn_wc/merge.c
tests/cmdline/merge_tests.py
Author: stsp
Date: Tue Mar 17 12:05:10 2015
New Revision: 1667280
URL: http://svn.apache.org/r1667280
Log:
Always install a .mine file for conflicted binary files, not just in
case the binary file was detranslated.
This makes the 'mine-full' option work again from the conflict prompt.
Before this change, an assertion in libsvn_wc failed when the 'mine-full'
option was used since no path for 'mine' was recorded in conflict storage.
* subversion/libsvn_wc/merge.c
(merge_binary_file): Always create .mine, if necessary as a copy
of the conflicted working file. Update docstring.
* subversion/tests/cmdline/merge_tests.py
(dry_run_merge_conflicting_binary): Update on-disk test expectations.
Modified:
subversion/trunk/subversion/libsvn_wc/merge.c
subversion/trunk/subversion/tests/cmdline/merge_tests.py
Modified: subversion/trunk/subversion/libsvn_wc/merge.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/merge.c?rev=1667280&r1=1667279&r2=1667280&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/merge.c (original)
+++ subversion/trunk/subversion/libsvn_wc/merge.c Tue Mar 17 12:05:10 2015
@@ -962,9 +962,10 @@ merge_text_file(svn_skel_t **work_items,
* Copy* the files at LEFT_ABSPATH and RIGHT_ABSPATH into the same directory
* as the target file, giving them unique names that start with the target
* file's name and end with LEFT_LABEL and RIGHT_LABEL respectively.
- * If the merge target has been 'detranslated' to repository normal form,
- * move the detranslated file similarly to a unique name ending with
- * TARGET_LABEL.
+ *
+ * Copy the working state of the merge target to a unique name ending with
+ * TARGET_LABEL. If the merge target has been 'detranslated' to repository
+ * normal form, use the detranslated form instead of the working state.
*
* ### * Why do we copy the left and right temp files when we could (maybe
* not always?) move them?
@@ -1035,26 +1036,31 @@ merge_binary_file(svn_skel_t **work_item
SVN_ERR(svn_io_copy_file(left_abspath, left_copy, TRUE, pool));
SVN_ERR(svn_io_copy_file(right_abspath, right_copy, TRUE, pool));
+ /* Create a .mine file too */
+ SVN_ERR(svn_io_open_uniquely_named(NULL,
+ &conflict_wrk,
+ merge_dirpath,
+ merge_filename,
+ target_label,
+ svn_io_file_del_none,
+ pool, pool));
+
/* Was the merge target detranslated? */
if (strcmp(mt->local_abspath, detranslated_target_abspath) != 0)
- {
- /* Create a .mine file too */
- SVN_ERR(svn_io_open_uniquely_named(NULL,
- &conflict_wrk,
- merge_dirpath,
- merge_filename,
- target_label,
- svn_io_file_del_none,
- pool, pool));
- SVN_ERR(svn_wc__wq_build_file_move(work_items, mt->db,
- mt->local_abspath,
- detranslated_target_abspath,
- conflict_wrk,
- pool, result_pool));
- }
+ SVN_ERR(svn_wc__wq_build_file_move(work_items, mt->db,
+ mt->local_abspath,
+ detranslated_target_abspath,
+ conflict_wrk,
+ pool, result_pool));
else
{
- conflict_wrk = NULL;
+ /* Ensure .mine file is created (translation is a no-op).
+ * ### add workq API to copy without translation? */
+ SVN_ERR(svn_wc__wq_build_file_copy_translated(work_items, mt->db,
+ mt->local_abspath,
+ mt->local_abspath,
+ conflict_wrk,
+ pool, result_pool));
}
/* Mark target_abspath's entry as "Conflicted", and start tracking
Modified: subversion/trunk/subversion/tests/cmdline/merge_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/merge_tests.py?rev=1667280&r1=1667279&r2=1667280&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/merge_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/merge_tests.py Tue Mar 17 12:05:10 2015
@@ -16009,6 +16009,11 @@ def dry_run_merge_conflicting_binary(sbo
'A/theta.merge-right.r3' :
Item(contents= theta_contents + "some extra junk")
})
+ # verify content of working backup ("mine") file
+ expected_disk.add({
+ 'A/theta.working' :
+ Item(contents= theta_contents + "some other junk")
+ })
expected_status = svntest.actions.get_virginal_state(other_wc, 1)
expected_status.add({
Re: svn commit: r1667280 - in /subversion/trunk/subversion: libsvn_wc/merge.c tests/cmdline/merge_tests.py
Posted by Bert Huijben <be...@qqmail.nl>.
We leave mine as NULL in svn_wc_entry_t since 1.0 in this case. libsvn_wc, svn, and all other users should just handle this properly. Where they don’t there is a bug, that should be fixed here instead of by redesigning the way we store binary file conflicts. (Note that there were more cases were some markers are missing... We call these tree conflicts today, but we hadn’t those in 1.5)
Storing the path itself in the skel will break other code. (Like the code that removes all the markerfiles in a dir. And the feature that a conflict can be resolved by removing the marker files. Not to forget svn_wc_entry_t) . Whatever we store in the transient/in memory only svn_wc_conflict_description2_t is less well defined. I’m guessing we just copied svn_wc_entry_t at first in cases like info (caused by missing tests and knowledge for this case), but pass the actual path in an interactive situation.
Bert
Sent from Windows Mail
From: Stefan Sperling
Sent: Thursday, March 19, 2015 1:02 AM
To: Bert Huijben
Cc: Branko Čibej, dev@subversion.apache.org
On Wed, Mar 18, 2015 at 10:53:11PM +0100, Bert Huijben wrote:
> (Extending my original answer)
>
> Mine-full should just be implemented as a simple resolve without changing the file. The right 'mine' file is already in the right place on a conflict of a binary file.
>
> Creating a copy of a file, to allow replacing the original file with an identical copy is not that useful.
>
>
> For text conflicts we create a file with conflict markers, and make the original file available as mine.
> For binary files we can't (and don't) create such a file, so the file is untouched.
>
> Where does creating a copy help?
>
> Bert
Implementing the "mine-full" option as a simple resolve sounds good.
Restoring the working file from a copy might be a bad idea indeed.
Perhaps the user did change the file after we copied it!
So we could store the file's working copy path as "mine".
However, this isn't really what I thought we were discussing. You said:
"I think it is better to make the interactive conflict resolver work the way we
always installed binary conflicts since 1.0, than to change the way we install
conflict markers for a bugfix of the conflict resolver."
The way I understand this statement is:
Don't change libsvn_wc, change the conflict resolver in 'svn' instead.
Am I misunderstanding?
I don't see a way of fixing this problem in 'svn' since we'll need to store
some path as 'mine' in conflict meta data, or we need to fix the parser in
libsvn_wc to never leave the 'mine' path as NULL. A NULL path will cause an
assertion in libsvn_wc if the client passes svn_wc_conflict_choose_mine_full.
Hence my question whether you want to stop using 'mine-full' altogether...
Re: svn commit: r1667280 - in /subversion/trunk/subversion:
libsvn_wc/merge.c tests/cmdline/merge_tests.py
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Stefan Sperling wrote on Thu, Mar 19, 2015 at 11:31:09 +0100:
> On Thu, Mar 19, 2015 at 10:19:18AM +0100, Stefan Sperling wrote:
> > I agree with Daniel's point that resolving to 'mine-full' is not the same as
> > resolving to 'working'. Else, why do we even have these two different options?
> >
> > Perhaps the answer is to stop offering 'mine-full' for binaries, since SVN
> > does not preserve the pre-update state which corresponds to 'mine-full',
> > and never did.
>
> See r1667691 and r1667692. Is this better?
I haven't reviewed the code, but making "mine-full" always mean "the
working file as it was just before the conflict" (and erroring out if
that option is selected when that file is unavailable) makes sense to me.
Incidentally, the error message added in r1667691 doesn't seem to get
triggered when a non-binary file's .mine file is missing:
% ls
iota iota.mine iota.r1 iota.r2
% rm iota.mine
% svn resolve --accept=mf iota
svn: warning: W155027: Unable to resolve conflicts on '/tmp/svn/wc/iota'
svn: E155027: Failure occurred resolving one or more conflicts
It would be nice to have the r1667691 error in this case too, since the
present error doesn't explain the reason resolution failed.
(In the above excerpt, iota is a non-binary file.)
Daniel
Re: svn commit: r1667280 - in /subversion/trunk/subversion:
libsvn_wc/merge.c tests/cmdline/merge_tests.py
Posted by Stefan Sperling <st...@elego.de>.
On Thu, Mar 19, 2015 at 10:19:18AM +0100, Stefan Sperling wrote:
> I agree with Daniel's point that resolving to 'mine-full' is not the same as
> resolving to 'working'. Else, why do we even have these two different options?
>
> Perhaps the answer is to stop offering 'mine-full' for binaries, since SVN
> does not preserve the pre-update state which corresponds to 'mine-full',
> and never did.
See r1667691 and r1667692. Is this better?
Re: svn commit: r1667280 - in /subversion/trunk/subversion:
libsvn_wc/merge.c tests/cmdline/merge_tests.py
Posted by Stefan Sperling <st...@elego.de>.
On Thu, Mar 19, 2015 at 07:27:54AM +0000, Bert Huijben wrote:
> If Subversion considers the file to be unmergeable, the .mine file isn't
> created, since it would be identical to the working file.)
I don't think that's a very good argument. There are two problems with this:
The book's argument hinges on the assumption that the working file is owned
by SVN. But it is owned by the user so it's not safe to assume its contents
match some known state.
And the pre-update state is not preserved for binary files, while it is for
text files. That's inconsistent, and perhaps inconvenient for the user.
What if they changed the binary in an attempt to resolve the conflict but
then decide they'd rather have the pre-update state back?
But I won't bother arguing about historical behaviour. I'll try to fix the
problem without changing what's stored in the conflict storage.
I agree with Daniel's point that resolving to 'mine-full' is not the same as
resolving to 'working'. Else, why do we even have these two different options?
Perhaps the answer is to stop offering 'mine-full' for binaries, since SVN
does not preserve the pre-update state which corresponds to 'mine-full',
and never did.
There is clearly a gaping disconnect between the resolver user interface and
the conflict storage implementation. There were too many cooks working during
different time periods and not talking to each other...
Re: svn commit: r1667280 - in /subversion/trunk/subversion:
libsvn_wc/merge.c tests/cmdline/merge_tests.py
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Why are you quoting the svnbook definition of .mine .rOLD .rNEW to me?
I know what they are.
If your point was the ".mine file isn't created because it would be
identical to the working file", then, again: I don't care about the
implementation right now. I don't care whether the contents is stored
in the .mine file, in the pristine store, or anywhere else. I care
about the semantics of 'svn resolve' from a user's perspective.
Daniel
Bert Huijben wrote on Thu, Mar 19, 2015 at 07:27:54 +0000:
> From the svn book (‘Postponing conflicts’ in the 1.7 version. Probably the main text before interactive resolution was added)
>
> [[
>
>
> For every conflicted file, Subversion places three extra unversioned files in your working copy:
>
> filename.mine
> This is the file as it existed in your working copy before you began the update process. It contains any local modifications you had made to the file up to that point. (If Subversion considers the file to be unmergeable, the .mine file isn't created, since it would be identical to the working file.)
> filename.rOLDREV
> This is the file as it existed in the BASE revision—that is, the unmodified revision of the file in your working copy before you began the update process—where OLDREV is that base revision number.
> filename.rNEWREV
> This is the file that your Subversion client just received from the server via the update of your working copy, where NEWREV corresponds to the revision number to which you were updating (HEAD, unless otherwise requested).
>
>
>
> ]]
>
>
> Bert
>
>
> Sent from Windows Mail
>
>
>
>
>
> From: Daniel Shahaf
> Sent: Thursday, March 19, 2015 8:04 AM
> To: Bert Huijben
> Cc: dev@subversion.apache.org
>
>
>
>
>
> Stefan Sperling wrote on Thu, Mar 19, 2015 at 01:02:12 +0100:
> > On Wed, Mar 18, 2015 at 10:53:11PM +0100, Bert Huijben wrote:
> > > (Extending my original answer)
> > >
> > > Mine-full should just be implemented as a simple resolve without changing the file. The right 'mine' file is already in the right place on a conflict of a binary file.
> > >
> > > Creating a copy of a file, to allow replacing the original file with an identical copy is not that useful.
> > >
> > >
> > > For text conflicts we create a file with conflict markers, and make the original file available as mine.
> > > For binary files we can't (and don't) create such a file, so the file is untouched.
> > >
> > > Where does creating a copy help?
> > >
> > > Bert
> >
> > Implementing the "mine-full" option as a simple resolve sounds good.
> > Restoring the working file from a copy might be a bad idea indeed.
> > Perhaps the user did change the file after we copied it!
> > So we could store the file's working copy path as "mine".
>
> Actually, as a user, if I do:
>
> % sha1sum file.bin
> 0xfoo
> % svn update --non-interactive file.bin
> C file.bin
> % edit file.bin
> % sha1sum file.bin
> 0xbar
> % svn resolve --accept=mine-full file.bin
>
> I'd expect the original version (0xfoo) to be installed. That's how it
> works for non-binary files. To accept the edited-post-conflict version
> (0xbar), I'd use accept=working.
>
> Note that mine-full irreversibly destroys the edited-post-conflict
> (0xbar) version of the file. Do people use mine-full and expect it to
> leave the 0xbar version untouched?
>
> Cheers,
>
> Daniel
Re: svn commit: r1667280 - in /subversion/trunk/subversion: libsvn_wc/merge.c tests/cmdline/merge_tests.py
Posted by Bert Huijben <be...@qqmail.nl>.
From the svn book (‘Postponing conflicts’ in the 1.7 version. Probably the main text before interactive resolution was added)
[[
For every conflicted file, Subversion places three extra unversioned files in your working copy:
filename.mine
This is the file as it existed in your working copy before you began the update process. It contains any local modifications you had made to the file up to that point. (If Subversion considers the file to be unmergeable, the .mine file isn't created, since it would be identical to the working file.)
filename.rOLDREV
This is the file as it existed in the BASE revision—that is, the unmodified revision of the file in your working copy before you began the update process—where OLDREV is that base revision number.
filename.rNEWREV
This is the file that your Subversion client just received from the server via the update of your working copy, where NEWREV corresponds to the revision number to which you were updating (HEAD, unless otherwise requested).
]]
Bert
Sent from Windows Mail
From: Daniel Shahaf
Sent: Thursday, March 19, 2015 8:04 AM
To: Bert Huijben
Cc: dev@subversion.apache.org
Stefan Sperling wrote on Thu, Mar 19, 2015 at 01:02:12 +0100:
> On Wed, Mar 18, 2015 at 10:53:11PM +0100, Bert Huijben wrote:
> > (Extending my original answer)
> >
> > Mine-full should just be implemented as a simple resolve without changing the file. The right 'mine' file is already in the right place on a conflict of a binary file.
> >
> > Creating a copy of a file, to allow replacing the original file with an identical copy is not that useful.
> >
> >
> > For text conflicts we create a file with conflict markers, and make the original file available as mine.
> > For binary files we can't (and don't) create such a file, so the file is untouched.
> >
> > Where does creating a copy help?
> >
> > Bert
>
> Implementing the "mine-full" option as a simple resolve sounds good.
> Restoring the working file from a copy might be a bad idea indeed.
> Perhaps the user did change the file after we copied it!
> So we could store the file's working copy path as "mine".
Actually, as a user, if I do:
% sha1sum file.bin
0xfoo
% svn update --non-interactive file.bin
C file.bin
% edit file.bin
% sha1sum file.bin
0xbar
% svn resolve --accept=mine-full file.bin
I'd expect the original version (0xfoo) to be installed. That's how it
works for non-binary files. To accept the edited-post-conflict version
(0xbar), I'd use accept=working.
Note that mine-full irreversibly destroys the edited-post-conflict
(0xbar) version of the file. Do people use mine-full and expect it to
leave the 0xbar version untouched?
Cheers,
Daniel
Re: svn commit: r1667280 - in /subversion/trunk/subversion: libsvn_wc/merge.c tests/cmdline/merge_tests.py
Posted by Bert Huijben <be...@qqmail.nl>.
This is not how Subversion worked until stsp changed this earlier this week. It worked this way for text conflicts, but not for binary conflicts.
What you ask is a bigger change, that in my opinion is better solved by storing the conflict details as checksums pointing into the binary blob/pristine store… as prepared wc.db was prepared for pre 1.7, but was never implemented yet. (Requires format dump… and checksum triggers in wc.db)
Bert
Sent from Windows Mail
From: Daniel Shahaf
Sent: Thursday, March 19, 2015 8:04 AM
To: Bert Huijben
Cc: dev@subversion.apache.org
Stefan Sperling wrote on Thu, Mar 19, 2015 at 01:02:12 +0100:
> On Wed, Mar 18, 2015 at 10:53:11PM +0100, Bert Huijben wrote:
> > (Extending my original answer)
> >
> > Mine-full should just be implemented as a simple resolve without changing the file. The right 'mine' file is already in the right place on a conflict of a binary file.
> >
> > Creating a copy of a file, to allow replacing the original file with an identical copy is not that useful.
> >
> >
> > For text conflicts we create a file with conflict markers, and make the original file available as mine.
> > For binary files we can't (and don't) create such a file, so the file is untouched.
> >
> > Where does creating a copy help?
> >
> > Bert
>
> Implementing the "mine-full" option as a simple resolve sounds good.
> Restoring the working file from a copy might be a bad idea indeed.
> Perhaps the user did change the file after we copied it!
> So we could store the file's working copy path as "mine".
Actually, as a user, if I do:
% sha1sum file.bin
0xfoo
% svn update --non-interactive file.bin
C file.bin
% edit file.bin
% sha1sum file.bin
0xbar
% svn resolve --accept=mine-full file.bin
I'd expect the original version (0xfoo) to be installed. That's how it
works for non-binary files. To accept the edited-post-conflict version
(0xbar), I'd use accept=working.
Note that mine-full irreversibly destroys the edited-post-conflict
(0xbar) version of the file. Do people use mine-full and expect it to
leave the 0xbar version untouched?
Cheers,
Daniel
Re: svn commit: r1667280 - in /subversion/trunk/subversion:
libsvn_wc/merge.c tests/cmdline/merge_tests.py
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Stefan Sperling wrote on Thu, Mar 19, 2015 at 01:02:12 +0100:
> On Wed, Mar 18, 2015 at 10:53:11PM +0100, Bert Huijben wrote:
> > (Extending my original answer)
> >
> > Mine-full should just be implemented as a simple resolve without changing the file. The right 'mine' file is already in the right place on a conflict of a binary file.
> >
> > Creating a copy of a file, to allow replacing the original file with an identical copy is not that useful.
> >
> >
> > For text conflicts we create a file with conflict markers, and make the original file available as mine.
> > For binary files we can't (and don't) create such a file, so the file is untouched.
> >
> > Where does creating a copy help?
> >
> > Bert
>
> Implementing the "mine-full" option as a simple resolve sounds good.
> Restoring the working file from a copy might be a bad idea indeed.
> Perhaps the user did change the file after we copied it!
> So we could store the file's working copy path as "mine".
Actually, as a user, if I do:
% sha1sum file.bin
0xfoo
% svn update --non-interactive file.bin
C file.bin
% edit file.bin
% sha1sum file.bin
0xbar
% svn resolve --accept=mine-full file.bin
I'd expect the original version (0xfoo) to be installed. That's how it
works for non-binary files. To accept the edited-post-conflict version
(0xbar), I'd use accept=working.
Note that mine-full irreversibly destroys the edited-post-conflict
(0xbar) version of the file. Do people use mine-full and expect it to
leave the 0xbar version untouched?
Cheers,
Daniel
Re: svn commit: r1667280 - in /subversion/trunk/subversion:
libsvn_wc/merge.c tests/cmdline/merge_tests.py
Posted by Stefan Sperling <st...@elego.de>.
On Wed, Mar 18, 2015 at 10:53:11PM +0100, Bert Huijben wrote:
> (Extending my original answer)
>
> Mine-full should just be implemented as a simple resolve without changing the file. The right 'mine' file is already in the right place on a conflict of a binary file.
>
> Creating a copy of a file, to allow replacing the original file with an identical copy is not that useful.
>
>
> For text conflicts we create a file with conflict markers, and make the original file available as mine.
> For binary files we can't (and don't) create such a file, so the file is untouched.
>
> Where does creating a copy help?
>
> Bert
Implementing the "mine-full" option as a simple resolve sounds good.
Restoring the working file from a copy might be a bad idea indeed.
Perhaps the user did change the file after we copied it!
So we could store the file's working copy path as "mine".
However, this isn't really what I thought we were discussing. You said:
"I think it is better to make the interactive conflict resolver work the way we
always installed binary conflicts since 1.0, than to change the way we install
conflict markers for a bugfix of the conflict resolver."
The way I understand this statement is:
Don't change libsvn_wc, change the conflict resolver in 'svn' instead.
Am I misunderstanding?
I don't see a way of fixing this problem in 'svn' since we'll need to store
some path as 'mine' in conflict meta data, or we need to fix the parser in
libsvn_wc to never leave the 'mine' path as NULL. A NULL path will cause an
assertion in libsvn_wc if the client passes svn_wc_conflict_choose_mine_full.
Hence my question whether you want to stop using 'mine-full' altogether...
RE: svn commit: r1667280 - in /subversion/trunk/subversion: libsvn_wc/merge.c tests/cmdline/merge_tests.py
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Branko Čibej [mailto:brane@wandisco.com]
> Sent: woensdag 18 maart 2015 02:00
> To: dev@subversion.apache.org
> Subject: Re: svn commit: r1667280 - in /subversion/trunk/subversion:
> libsvn_wc/merge.c tests/cmdline/merge_tests.py
>
> On 17.03.2015 22:53, Stefan Sperling wrote:
> > On Tue, Mar 17, 2015 at 10:11:28PM +0100, Bert Huijben wrote:
> >>
> >>> -----Original Message-----
> >>> From: stsp@apache.org [mailto:stsp@apache.org]
> >>> Sent: dinsdag 17 maart 2015 13:05
> >>> To: commits@subversion.apache.org
> >>> Subject: svn commit: r1667280 - in /subversion/trunk/subversion:
> >>> libsvn_wc/merge.c tests/cmdline/merge_tests.py
> >>>
> >>> Author: stsp
> >>> Date: Tue Mar 17 12:05:10 2015
> >>> New Revision: 1667280
> >>>
> >>> URL: http://svn.apache.org/r1667280
> >>> Log:
> >>> Always install a .mine file for conflicted binary files, not just in
> >>> case the binary file was detranslated.
> >>>
> >>> This makes the 'mine-full' option work again from the conflict prompt.
> >>> Before this change, an assertion in libsvn_wc failed when the 'mine-full'
> >>> option was used since no path for 'mine' was recorded in conflict storage.
> >> I think it is better to make the interactive conflict resolver work the way we
> always installed binary conflicts since 1.0, than to change the way we install
> conflict markers for a bugfix of the conflict resolver.
> >>
> >> In case of conflicts of a binary file there is no pre-merged file with conflict
> markers and therefore we just leave the 'mine' file just where it is... At the
> original location.
> >> (For text conflicts we move it to a different location, and then install the pre-
> merged file with conflict markers there)
> >>
> >>
> >> This patch introduces a second copy of the same file, while clients always
> had to handle this case for 1.0-1.8 anyway.
> >>
> >>
> >> If the interactive conflict resolver needs a patch, we should probably also
> backport this patch.
> >>
> >> Bert
> >>
> > The problem is that choosing 'mine-full' crashes in libsvn_wc.
> > Are you suggesting we shouldn't offer the 'mine-full' option for binary files?
>
> What does providing that option have to do with creating a duplicate
> file in the WC?
(Extending my original answer)
Mine-full should just be implemented as a simple resolve without changing the file. The right 'mine' file is already in the right place on a conflict of a binary file.
Creating a copy of a file, to allow replacing the original file with an identical copy is not that useful.
For text conflicts we create a file with conflict markers, and make the original file available as mine.
For binary files we can't (and don't) create such a file, so the file is untouched.
Where does creating a copy help?
Bert
Re: svn commit: r1667280 - in /subversion/trunk/subversion: libsvn_wc/merge.c
tests/cmdline/merge_tests.py
Posted by Branko Čibej <br...@wandisco.com>.
On 17.03.2015 22:53, Stefan Sperling wrote:
> On Tue, Mar 17, 2015 at 10:11:28PM +0100, Bert Huijben wrote:
>>
>>> -----Original Message-----
>>> From: stsp@apache.org [mailto:stsp@apache.org]
>>> Sent: dinsdag 17 maart 2015 13:05
>>> To: commits@subversion.apache.org
>>> Subject: svn commit: r1667280 - in /subversion/trunk/subversion:
>>> libsvn_wc/merge.c tests/cmdline/merge_tests.py
>>>
>>> Author: stsp
>>> Date: Tue Mar 17 12:05:10 2015
>>> New Revision: 1667280
>>>
>>> URL: http://svn.apache.org/r1667280
>>> Log:
>>> Always install a .mine file for conflicted binary files, not just in
>>> case the binary file was detranslated.
>>>
>>> This makes the 'mine-full' option work again from the conflict prompt.
>>> Before this change, an assertion in libsvn_wc failed when the 'mine-full'
>>> option was used since no path for 'mine' was recorded in conflict storage.
>> I think it is better to make the interactive conflict resolver work the way we always installed binary conflicts since 1.0, than to change the way we install conflict markers for a bugfix of the conflict resolver.
>>
>> In case of conflicts of a binary file there is no pre-merged file with conflict markers and therefore we just leave the 'mine' file just where it is... At the original location.
>> (For text conflicts we move it to a different location, and then install the pre-merged file with conflict markers there)
>>
>>
>> This patch introduces a second copy of the same file, while clients always had to handle this case for 1.0-1.8 anyway.
>>
>>
>> If the interactive conflict resolver needs a patch, we should probably also backport this patch.
>>
>> Bert
>>
> The problem is that choosing 'mine-full' crashes in libsvn_wc.
> Are you suggesting we shouldn't offer the 'mine-full' option for binary files?
What does providing that option have to do with creating a duplicate
file in the WC?
-- Brane
Re: svn commit: r1667280 - in /subversion/trunk/subversion:
libsvn_wc/merge.c tests/cmdline/merge_tests.py
Posted by Stefan Sperling <st...@elego.de>.
On Tue, Mar 17, 2015 at 10:11:28PM +0100, Bert Huijben wrote:
>
>
> > -----Original Message-----
> > From: stsp@apache.org [mailto:stsp@apache.org]
> > Sent: dinsdag 17 maart 2015 13:05
> > To: commits@subversion.apache.org
> > Subject: svn commit: r1667280 - in /subversion/trunk/subversion:
> > libsvn_wc/merge.c tests/cmdline/merge_tests.py
> >
> > Author: stsp
> > Date: Tue Mar 17 12:05:10 2015
> > New Revision: 1667280
> >
> > URL: http://svn.apache.org/r1667280
> > Log:
> > Always install a .mine file for conflicted binary files, not just in
> > case the binary file was detranslated.
> >
> > This makes the 'mine-full' option work again from the conflict prompt.
> > Before this change, an assertion in libsvn_wc failed when the 'mine-full'
> > option was used since no path for 'mine' was recorded in conflict storage.
>
> I think it is better to make the interactive conflict resolver work the way we always installed binary conflicts since 1.0, than to change the way we install conflict markers for a bugfix of the conflict resolver.
>
> In case of conflicts of a binary file there is no pre-merged file with conflict markers and therefore we just leave the 'mine' file just where it is... At the original location.
> (For text conflicts we move it to a different location, and then install the pre-merged file with conflict markers there)
>
>
> This patch introduces a second copy of the same file, while clients always had to handle this case for 1.0-1.8 anyway.
>
>
> If the interactive conflict resolver needs a patch, we should probably also backport this patch.
>
> Bert
>
The problem is that choosing 'mine-full' crashes in libsvn_wc.
Are you suggesting we shouldn't offer the 'mine-full' option for binary files?
RE: svn commit: r1667280 - in /subversion/trunk/subversion: libsvn_wc/merge.c tests/cmdline/merge_tests.py
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: stsp@apache.org [mailto:stsp@apache.org]
> Sent: dinsdag 17 maart 2015 13:05
> To: commits@subversion.apache.org
> Subject: svn commit: r1667280 - in /subversion/trunk/subversion:
> libsvn_wc/merge.c tests/cmdline/merge_tests.py
>
> Author: stsp
> Date: Tue Mar 17 12:05:10 2015
> New Revision: 1667280
>
> URL: http://svn.apache.org/r1667280
> Log:
> Always install a .mine file for conflicted binary files, not just in
> case the binary file was detranslated.
>
> This makes the 'mine-full' option work again from the conflict prompt.
> Before this change, an assertion in libsvn_wc failed when the 'mine-full'
> option was used since no path for 'mine' was recorded in conflict storage.
I think it is better to make the interactive conflict resolver work the way we always installed binary conflicts since 1.0, than to change the way we install conflict markers for a bugfix of the conflict resolver.
In case of conflicts of a binary file there is no pre-merged file with conflict markers and therefore we just leave the 'mine' file just where it is... At the original location.
(For text conflicts we move it to a different location, and then install the pre-merged file with conflict markers there)
This patch introduces a second copy of the same file, while clients always had to handle this case for 1.0-1.8 anyway.
If the interactive conflict resolver needs a patch, we should probably also backport this patch.
Bert