On Mon, Dec 08, 2008 at 11:50:56AM +0000, Julian Foad wrote:
> Neels Janosch Hofmeyr wrote:
> > I've just found out that we're not actually raising a tree-conflict upon an
> > added-added conflict involving only a directory. It *is* reported on files
> > when similarly named files are in the two added dirs.
> >
> > See attached reproduction script (which was aiming at testing whether such
> > an add-add conflict on a dir was blocked when stepping into it).
>
> It's tested by tree_conflict_tests.py 8 "up/sw dir: add onto add", which
> is XFAIL.
>
> This code is in update_editor.c:add_directory():
> [[[
> else /* Obstructing dir *is* versioned or scheduled for addition. */
> {
> const svn_wc_entry_t *entry;
>
> SVN_ERR(svn_wc_entry(&entry, db->path, adm_access, FALSE, pool));
>
> /* Anything other than a dir scheduled for addition without
> history is an error. */
> /* ### what's this "add_existed" clause for? */
> if (entry
> && entry->schedule == svn_wc_schedule_add
> && ! entry->copied)
> {
> db->add_existed = TRUE;
> }
> else
> {
> (Raise a tree conflict)
> }
> ]]]
>
> The case of a directory already scheduled for addition WITHOUT HISTORY
> is treated as a special case. I couldn't understand why, and wrote that
> "###" comment. One of us can take another look at it some time and see
> if we can figure it out.
Well, it looks like this flag is only used to mold the locally
added directory into the update, as if nothing had happened.
The assumption probably being that two independently added directories
with the same path do not conflict, and that their content will be
merged later during the editor drive (note the "change rev from 0
to the target revision" comment in the code quoted below).
In a text-conflict-only world, this makes sense, so the code using
the add_existed flag also makes sense from that point of view:
/* Comment added by me for this mail:
The following resets the schedule from 'add' to 'normal',
so that local_add + remote_add == normal
*/
if (db->add_existed)
{
tmp_entry.schedule = svn_wc_schedule_normal;
modify_flags |= SVN_WC__ENTRY_MODIFY_SCHEDULE |
SVN_WC__ENTRY_MODIFY_FORCE;
}
SVN_ERR(svn_wc__entry_modify(adm_access, db->name, &tmp_entry,
modify_flags,
TRUE /* immediate write */, pool));
if (db->add_existed)
{
/* Immediately tweak the schedule for "this dir" so it too
is no longer scheduled for addition. Change rev from 0
to the target revision allowing prep_directory() to do
its thing without error. */
modify_flags = SVN_WC__ENTRY_MODIFY_SCHEDULE
| SVN_WC__ENTRY_MODIFY_FORCE | SVN_WC__ENTRY_MODIFY_REVISION;
SVN_ERR(svn_wc_adm_retrieve(&adm_access,
db->edit_baton->adm_access,
db->path, pool));
tmp_entry.revision = *(eb->target_revision);
if (eb->switch_url)
{
tmp_entry.url = svn_path_url_add_component(eb->switch_url,
db->name, pool);
modify_flags |= SVN_WC__ENTRY_MODIFY_URL;
}
SVN_ERR(svn_wc__entry_modify(adm_access, NULL, &tmp_entry,
modify_flags,
TRUE /* immediate write */, pool));
}
}
I'd say be bold and remove all this, flag a tree conflict like we
ought to, and see how many tests break >:]
Stefan
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=981120
Received on 2008-12-08 13:24:33 CET