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

Re: [PATCH] Tree-conflicts: do_entry_deletion segfault

From: Karl Fogel <kfogel_at_red-bean.com>
Date: Mon, 08 Sep 2008 22:00:36 -0400

Neels Hofmeyr <neels_at_elego.de> writes:
> And the mail server swallows *.sh files. Great. Trying again.

I noticed several block messages from you (I'm one of the moderators),
with names like "root_conflict3.sh" and such. I have no idea why they
got blocked, unless the system actually bases it on the name of the
file, which would seem silly, but is, I suppose, possible.

If you can't attach them, putting them inline is the way to go, I
guess. Seems you did that. Sorry for the trouble.

-Karl

> Neels Hofmeyr wrote:
>>
>> Julian Foad wrote:
>>> On Fri, 2008-09-05 at 04:25 +0200, Neels Hofmeyr wrote:
>>>> Quoting:
>>>>
>>>> * subversion/libsvn_wc/update_editor.c
>>>> (bump_dir_info): Update a doc-string to allow for a directory to have tree
>>>> conflicts.
>>>> (entry_has_local_mods, check_tree_conflict): New functions.
>>>> (do_entry_deletion): Have the parent's admin access baton passed in by the
>>>> caller. Check for tree conflicts.
>>>> ### Broken when parent_adm_access is NULL.
>>>>
>>>>
>>>> I've already posted a fix for this one almost two weeks ago. It wasn't being
>>>> heard much.
>>> That's my fault, Neels. I read your message then, and the earlier ones,
>>> and started to reply, but lost my way. Sorry.
>>>
>>>> So I'm posting an update, using today's tree-conflicts branch, compacting my
>>>> explanations. You may refer to the following mail for more (confusing) detail:
>>> Excellent. Thanks.
>>>
>>>> http://subversion.tigris.org/servlets/ReadMsg?listName=dev&msgNo=142154
>>>> Date: Sat, 23 Aug 2008 22:43:11 +0200
>>>> From: Neels Hofmeyr <neels_at_elego.de>
>>>> Subject: Re: Tree-conflicts branch - log message / review
>>>>
>>>>
>>>>
>>>> Two problems coincide:
>>>>
>>>> 1. Tree conflict detection is skipped when examining the path that was
>>>> specifically given as target. E.g. `svn update A/D/G' means that while G's
>>>> contents are checked for tree conflicts, G itself will not be checked.
>>>>
>>>> 2. Tree conflict detection segfaults if run in that situation given in point
>>>> 1 above.
>>>>
>>>>
>>>> This patch fixes both, plus it tweaks two "unrelated" cmdline test to ignore
>>>> a 'C' marker caused by the fix. The fix itself consists of three parts:
>>>>
>>>> i) A change made in the tree-conflicts branch is reverted to trunk:
>>>> do_entry_deletion() does not need a PARENT_ADM_ACCESS parameter. It was
>>>> being used to indicate the specific case mentioned in point 1 above, so as
>>>> to be able to skip tree-conflicts detection in that case, oddly enough. Stsp
>>>> has pointed out that this change must have been committed by accident.
>>> Heh. It was me who added that. It wasn't by accident. I was trying to
>>> solve the problem of when the target is the root of the WC and therefore
>>> there is no parent WC directory. However, I didn't properly solve that
>>> problem, and I forgot to mention it in the log message, so we might very
>>> well call it an accident.
>>
>> lol, sorry and thanks :)
>>
>> But, about when the target is the working copy root: that got me thinking.
>> There is no parent directory to report conflicts in! And what will
>> adm_retrieve return in such a case?
>>
>> This is the exact case:
>> 1. Directory X has been issued on the svn commandline (e.g. `svn update X')
>> 2. This directory X is a tree-conflict victim, and
>> 3. X also happens to be the root of the local working copy.
>>
>> (It doesn't make sense to think of the repository root being a tree
>> conflicts victim. Here, though, X has been checked out explicitly, and is
>> the working copy root, as one would check out `trunk')
>>
>> There has been word that tree conflicts should be reported at the actual
>> victim nodes, not in their respective parent directories. That would
>> sufficiently prevent this problem altogether.
>>
>> But for now, I tried to create conflicts in a working copy's root dir. It
>> seems that at least update is waterproof, in a way. I've attached scripts
>> that run the cases listed here, numbers corresponding:
>>
>>
>> 1) Try to update the working copy root to a deleted revision (no conflicts):
>> "svn: Target path does not exist"
>>
>> 2) Prop conflicts are handled as they should.
>>
>> 3) Run `svn rm .' in the working copy root and try to commit:
>> States that the commit succeeded but warns about ugly problems.
>> Status complains about a "missing" file removed by svn itself.
>>
>> 4) First run `svn rm .' in the working copy root, then try to update it to a
>> deleted revision: same as 1): "svn: Target path does not exist".
>> Also, `svn rm .' fails to lock a subdir, whatever that means.
>>
>> 5) First run `svn rm .' in the working copy root, then try to update it to a
>> modified revision: same as 3): commit succeeded but warns about ugly
>> problems. Also, the update "restores" a file removed by svn itself.
>>
>>
>> It is pretty obvious that these cases are not considered valid. At least no
>> user out there will run into the problem I am poking for here, because it
>> doesn't even run that far, for other reasons.
>>
>> Have I missed a case? Otherwise everything is fair enough, at least from the
>> tree-conflicts point of view ... switch and merge remain to be checked, but
>> I expect the same stuff happening.
>>
>>
>>
>>> I made just one tweak: There was a point where you wrote "if
>>> (svn_io_check_path(...) != SVN_NO_ERROR ..." which would leak the error
>>> object if it threw an error. Instead, we have to call the function on
>>> its own, as "SVN_ERR(svn_io_check_path(...));". Any error that it might
>>> return is one that we don't know how to deal with, so it is OK to just
>>> return the error to our caller.
>>
>> Ah yes, true. (Hasn't this been said before somewhere, too?)
>> I checked and yours is better. I had assumed that the SVN_ERR indicates a
>> missing directory, but duh.
>>
>>
>>> Committed in r32932.
>> Thanks!
>>
>> ~Neels
>>
>>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-09-09 04:32:52 CEST

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