On Sun, 2008-09-07 at 22:40 -0400, Karl Fogel wrote:
> While working on issue #3282, I discovered that the propset below did
> not error:
>
> $ svn rm some_versioned_file
> $ svn propset pname pval some_versioned_file
>
> Instead, it just set 'pname' to 'pval' on 'some_versioned_file'.
>
> That seemed wrong! If a file is scheduled for delete, should we
> really allow further data-adding operations on it, like propset?
It's certainly wrong. I looked at this before (can't find any email I
may have written about it though), and I think I found "svn"
inconsistent: I think more than one of the property commands have this
bug (pretending to read or write properties of a schedule-delete item)
while the majority of commands correctly treat a schedule-delete state
as a state that does not and cannot have properties at all.
> (This is tangentially related to issue #3282, by the way, though I
> don't think it is necessary to change this behavior to fix #3282.)
>
> So I came up with a patch (see end of this mail). Unfortunately, my
> patch causes three merge tests to fail:
>
> FAIL: merge_tests.py 61: merge fails if subtree is deleted on src
> FAIL: merge_tests.py 93: dont merge revs into a subtree that predate it
> FAIL: merge_tests.py 98: subtree merge source might not exist
>
> Let's look at the first test (61) as an example. It fails because it
> gets a new error (the one I intended, though not one the test expects).
> Here's the relevant excerpt from running 'merge_tests.py -v 61':
>
> --- Merging r3 through r5 into \
> 'svn-test-work/working_copies/merge_tests-61/A_copy':
> D svn-test-work/working_copies/merge_tests-61/A_copy/D/gamma
> subversion/libsvn_wc/props.c:2497: (apr_err=155023)
> svn: 'svn-test-work/working_copies/merge_tests-61/A_copy/D/gamma' \
> is scheduled for deletion; cannot set property 'svn:mergeinfo' on it
>
> Hmmm. There are a couple of different ways we could go from here:
>
> 1) I could tweak my patch to move the schedule-delete check up to
> somewhere "higher" than libsvn_wc (maybe libsvn_client? I'm not
> sure what entry point the mergeinfo code is using...)
>
> 2) We could ask ourselves whether setting new mergeinfo on a
> schedule-delete target really makes sense. If it doesn't, we
> could trap and clear the error, or maybe detect that the entry
> is schedule-delete and not attempt to set the mergeinfo in the
> first place.
>
> 3) Something I haven't thought of.
>
> I lean toward (2), but I'm not knowledgeable enough to be confident
> that's the right way to go. Thoughts?
Yes, (2). I'm confident that any attempt to read or manipulate a
property on a schedule-delete item is an error, at both the WC and
client levels, regardless of what the current implementation does.
- Julian
---------------------------------------------------------------------
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-08 18:25:26 CEST