On 2/6/07, Peter Lundblad <plundblad@google.com> wrote:
> Erik Huelsmann writes:
> > On 2/6/07, Daniel Rall <dlr@collab.net> wrote:
> > > On Tue, 06 Feb 2007, Malcolm Rowe wrote:
> > >
> > > > On Tue, Feb 06, 2007 at 03:50:17PM +0100, Henner Zeller wrote:
> > > > > >I'm not that familiar with the code myself, but I'm surprised that we
> > > > > >unconditionally remove the contents without checking them first
> > > > >
> > > > > Well its good to do so because the tmp/ area might have some old files
> > > > > being lying around that we don't want to keep (hence the name tmp/).
> > > > > Instead of looking for these files, the whole tmp/ is just removed
> > > > > completely and rebuild.
> > > > >
> > > >
> > > > Okay, that does make sense. I see that we only recreate the tmp/ area
> > > > after we run the log for the wc, which will presumably fail if there are
> > > > files referenced in the log that don't exist in tmp/ (because tmp/
> > > > itself doesn't exist).
> >
> > Exactly. This is the problem I was referring to. The problem is that
> > successful execution of the log file doesn't guarantee we did the
> > right thing if the tmp area doesn't exist: we ignore ENOENT, so, we
> > can't assume successful completion was really successful, can we?
> >
> Then, that is a problem that would need fixing in general.
> In general, we assume that nothing changes in the admin area between a failing
> operation and the rerun of the logs.
> If that's not the case (say a file inside .svn/tmp disapears), all sorts of
> things can go wrong, but this has nothing to do with this patch.
Well, but the fact that the tmp area is missing doesn't tell us
whether it was between a failed operation and a rerun of logs and
whether it was empty or not, or am I misunderstanding what the patch
is supposed to fix?
> > If those files referenced in the logs don't exist *and* we execute the
> > log, is the end result guaranteed to be a consistent (known) state? Or
> > can we end up with a modified working copy admin area which is missing
> > the files in the working copy?
> >
> > > I'm in favor of this patch. It passes the test suite; I'll be
> > > committing it later today unless any other issues with it are raised.
> >
> > Do we know for certain this is a change for the better? Our working
> > copy isn't known for its stability as it is, so, I'm all in favor of
> > changing it, but I'd like it to be a sure change on the side of
> > stability...
> >
> I'd say this is a *slight* usability improvement which doesn't
> make the situation with the current log-based system worse
> than it already is. Improving the robustness of rerunning logs
> (say by making sure that the destination of the move
> is the expected one or something) would be another change
> that might add more complexity than is worth.
Right.
bye,
Erik.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Feb 6 22:33:38 2007