[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Fix issue #3686 - executable bit not set during merge

From: Daniel Becroft <djcbecroft_at_gmail.com>
Date: Thu, 10 Feb 2011 07:21:30 +1000

>
> > Index: subversion/libsvn_wc/merge.c
>> > ===================================================================
>> > --- subversion/libsvn_wc/merge.c (revision 1068136)
>> > +++ subversion/libsvn_wc/merge.c (working copy)
>> > @@ -1094,6 +1094,7 @@
>> > const char *left_label,
>> > const char *right_label,
>> > const char *target_label,
>> > + svn_boolean_t dry_run,
>> > const svn_wc_conflict_version_t *left_version,
>> > const svn_wc_conflict_version_t *right_version,
>> > const char *detranslated_target_abspath,
>> > @@ -1111,13 +1112,31 @@
>> > const char *merge_dirpath, *merge_filename;
>> > const char *conflict_wrk;
>> > svn_skel_t *work_item;
>> > + svn_boolean_t same_contents = FALSE;
>> >
>> > SVN_ERR_ASSERT(svn_dirent_is_absolute(target_abspath));
>> >
>> > *work_items = NULL;
>> >
>> > svn_dirent_split(&merge_dirpath, &merge_filename, target_abspath,
>> pool);
>> > +
>> > + /* 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. */
>> > + SVN_ERR(svn_io_files_contents_same_p(&same_contents,
>> > + left_abspath,
>> > + target_abspath,
>> > + scratch_pool));
>> >
>> > + if (same_contents)
>> > + {
>> > + if (!dry_run)
>> > + SVN_ERR(svn_io_file_move(right_abspath, target_abspath,
>> > + scratch_pool));
>>
>> Isn't calling svn_io_file_move() directly still bypassing the work queue?
>> Shouldn't this code queue a task instead?
>>
>
> Hmm ... I'll look into that. The test went from XFAIL to XPASS, and I was
> working off the "minimum required to get test to pass" mantra.
>

Hey Stefan,

It was only partially bypassing the work queue. The items to update the
executable bit (and read/write bits) is always done at the end of the
svn_wc_merge4() function, so that's why the test started passing. However,
you are correct - I should have been using the svn_wc__wq_build_file_install
workqueue task to copy the file across (this is what happens when a conflict
is resolved with "theirs-full").

An updated patch is attached.

[[[
Fix issue #3686 - executable bit not set during merge.

The cause was the special case in libsvn_client, which bypassed the use of
the
workqueue. This logic has now been moved into libsvn_wc.

Additionally, this change allows the status of binary files (during a
dry-run
merge) to be reported correctly (previously, all binary files were reported
as
conflicted).

* subversion/libsvn_client/merge.c
 (merge_file_changed): Removed binary-merge special case (now in libsvn_wc).
   Removed merge_required variable (resulting in indentation changes).

* subversion/libsvn_wc/merge.c
 (merge_binary_file): Added dry_run parameter. Add the special case merging
  of binary files.
 (svn_wc__internal_merge): Removed dry_run check for binary files, and pass
to
  merge_binary_file instead.

* subversion/tests/cmdline/merge_tests.py
 (merge_change_to_file_with_executable): Remove @XFail decorator.
]]]

Cheers,
Daniel B
(who is not a C programmer by trade ...)

Received on 2011-02-09 22:22:30 CET

This is an archived mail posted to the Subversion Dev mailing list.