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

Re: Nasty double-replace copy-on-update bug

From: Ben Collins-Sussman <sussman_at_red-bean.com>
Date: 2007-10-19 16:28:41 CEST

On 10/19/07, Philip Martin <philip@codematters.co.uk> wrote:

> How the logs and cache should interact is a difficult question to
> answer. We have the current behaviour which is the result of
> evolution rather than any grand unified design.

Tell me about it. :-)

> Why does add_file_with_history flush and run the log, the one with the
> wrong committed-rev? I guess it's to enable a text-delta to be
> applied, but it does seem to be sort of in the middle of things. Is
> the working copy consistent if the process gets interrupted after the
> log has been run? If there is no log file cleanup will simply remove
> the locks and assume the working copy is OK.

We deliberately flush the parent-dir's log, otherwise the newly
"installed" file doesn't exist. As you realized, it needs to exist so
that the server can (possibly) send a subsequent apply_textdelta()
against it. I added this behavior after chatting with Erik Huelsmann,
who explained that files which are created/installed by
add_file()/close_file() pairs don't truly come into existence until
the parent directory is explicitly closed via close_directory(); our
best solution was to run the parent-dir's existing log "early" within
this routine, rather than waiting till later. It should be safe,
though. Notice that the log is reset properly, so that a new loggy
for the directory (and it's incoming children) can be created and
used. If the checkout is interrupted right after the parent's log is
run 'early', I see no reason why the wc would be inconsistent. The
directory should still be marked 'incomplete', thus allowing for a
restartable update.

> >> Perhaps properties like committed-rev should be filtered out of then
> >> above loop? Perhaps only regular props should be transferred?
> >
> > Maybe, but that seems like sidestepping the problem, which is that the
> > left hand of the library knows something (that the server has set
> > committed-rev) that the right hand doesn't...
> I think that transferring committed-rev from one file to another is a
> bug, that's not sidestepping anything.

I don't think this is actually happening.

Notice the two blocks above the loop where properties are applied to
the file-baton. If locate_copyfrom() finds a candidate, then it
copies the candidate's text-base into place and loads the candidate's
base-props into memory. In the alternative block (where no candidate
can be found), we call svn_ra_get_file() to fetch text to disk and
props into memory.

The props in memory are then applied to the file-baton. The
difference, though, is that the first blocks just loads up typical
user-created properties, while svn_ra_get_file() returns *all* props:
normal user props, but also "weird" live properties like the
svn:wc:entry:* props that get cached in .svn/entries. (This is
perfectly fine; close_file() knows how to filter these different
types of props.)

So in the case of an svn_ra_get_file() fetch, the
svn:wc:entry:last-committed-rev property is definitely being fetched
and pushed into the file-baton. But when we're copying an existing
file's properties over, we're not dup'ing the last-committed-rev prop
at all.

To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Oct 19 16:28:52 2007

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.