[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: Philip Martin <philip_at_codematters.co.uk>
Date: 2007-10-19 17:06:54 CEST

"Ben Collins-Sussman" <sussman@red-bean.com> writes:

> 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.

If I quit out of gdb after running the first log, and then run cleanup
I get:

  $ svn st -uv wc
         * wc/aardvark
                  3 1 pm wc/alpha
  ! * 3 ? ? wc
  Status against revision: 3
  $ cat wc/alpha
  This is the file 'g123'.
  $ cat wc/.svn/text-base/alpha.svn-base
  This is the file 'g123'.

Notice that alpha is showen as up-to-date despite having
committed-rev=1 and the wrong working text and text-base. That's the
sort of thing the log files are supposed to prevent. I can't run
update, because of the bug, but even if I could how would the client
know to update alpha?

>> >> 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.

In this case alpha is created by doing svn_ra_get_file(gamma@1) and
copying committed-rev=1. The log file that creates alpha sets
committed-rev=1. I don't see how that can be anything other than a
bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Oct 19 17:07:12 2007

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