Philip Martin <philip_at_codematters.co.uk> writes:
>> Log:
>> Fix issue #3198: don't commit an add of a missing item.
>>
>> * subversion/libsvn_client/commit_util.c
>> (harvest_committables): Check for existence before adding.
>>
>> [...]
>
> Adding svn_io_check_path calls often introduces races; in this case if
> the file is deleted afer the check then the original problem is still
> present. Now modifying the wc during a commit is not really expected
> to work, but it would be good if the client failed without breaking
> the working copy. A more robust solution would be better error
> handling when attempting to open the missing file (I'm assuming that's
> possible but I haven't looked at the code).
Thanks for the review.
I agree with you, of course, though in practice r31196 will prevent the
situation from coming up in almost all cases.
Doing the Truly Right Thing is surprisingly hard here, in part because
svn_wc_text_modified_p() is documented to claim "not modified" if the
file is simply missing. I don't know why we do this, but presumably it
was on purpose, since we took the trouble to document it. This means
that by the time we get to commit_util.c:do_item_commit(), the file is
already not thought to be modified, that is
item->state_flags & SVN_CLIENT_COMMIT_ITEM_TEXT_MODS
is false. This is because earlier, in harvest_committables(), we asked
svn_wc_text_modified_p(), and the answer was "not modified".
Now, this is not insoluble: we could ask there if the file is missing,
set the SVN_CLIENT_COMMIT_ITEM_TEXT_MODS bit if it *is* missing, so that
later we'd try to open it to send the textdelta and error at that point.
But although that would be correct, it would be counterintuitive and
IMHO make the code less comprehensible.
What's really called for is some code reorganization, and/or figuring
out why svn_wc_text_modified_p() treats missing as unmodified and
(maybe) changing that.
But I didn't want to go down that road right now. r31196 solves most of
the problem, and life is short. Thoughts?
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-05-15 23:40:51 CEST