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

dumper/loader bugs revealed!

From: Ben Collins-Sussman <sussman_at_collab.net>
Date: 2002-06-12 18:24:12 CEST

Okay, there seem to be two problems with our repository dump/load
system, and now I have the fixes for both bugs at my fingertips.

  1. As Brane diagnosed yesterday, our svn_stream_from_stdio()
      routines are opening win32-specific streams that use CRLF and
      interpret Ctrl-Z within a .doc file as a premature EOF. Thus
      the loader fails to load the dumpfile on Win32.

      Brane has posted a patch that opens apr_file_t versions of stdin
      and stdout, and builds streams around them. I'll be tweaking
      that patch and applying it to svnadmin/main.c in both HEAD and
      the r2092 branch. It's a fine short-term fix. The long term
      fix revolves around deeper issues about opening files in
      'binary' mode on win32.

  2. I've been discussing another bug with Ronald Simmons; his
      dumpfile contains bogus instructions:

         r8: cp /foo /bar
        r13: replace /bar [i.e. delete, then re-add]
        r16: delete /bar/file.txt

      After playing with some repro recipes, I have *two* issues to
      discuss here:

A. 'svn cp' behavior

  Apparently libsvn_fs cheerfully allows destination overwrites when
  copying things. That is, if you use svn_fs_copy() to copy a file or
  directory, any entry which has the same name in the destination will
  be *removed*, and *overwritten* with the new item.

  According to cmpilato, this is an intentional fs design decision; I
  wasn't aware of it, but whatever. So be it.

  Because of this fs behavior, our client libraries do a number of
  "sanity checks" to prevent this behavior, when doing 'svn cp URL
  URL' in particular. The client libs will throw an error if the
  destination file already exists. However, the libs are *not*
  detecting the case when a destination directory already exists!
  Directories are just silently replaced.

  I don't like this at all. This is backwards from unix cp
  semantics. IIRC, unix cp allows you to overwrite files, but not
  dirs. Why are we doing it the other way around? I'm going to file
  an issue on this.

B. Two kinds of 'replace'.

  Ronald Simmons thought that it was totally normal for 'svn cp URL
  URL' to overwrite destination directories. He was issuing this
  command over and over as a way of promoting code from one branch to
  another, rather than using 'svn merge' to port changes.

  Thus he revealed a bug in the dumper code: in r13 above, dir_delta
  says, "delete /bar; add /bar with history"... which is correct,
  that's what actually happened in r13.

  Our dumper says: "oh! delete + add = replace!", so it just writes
  "replace /bar" in the dumpfile. But the dumper/loader use the
  "replace" verb to mean "delete and add", not "delete and add with
  history". Doh. So a simple bugfix is to make the dumper write out
  "delete; add with history" whenever history is present. Otherwise,
  it can write out a normal compressed "replace" command.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Jun 12 18:21:08 2002

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.