[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: David Glasser <glasser_at_davidglasser.net>
Date: 2007-10-19 01:56:40 CEST

... and guess what! This bug can even strike *without* causing an
error; it can give you a wc which is completely self-consistent but is
different from the repository. Specifically, removing the two perl
lines from the previous script does that. Here's another version
which actually ensures that the files end up with the right contents.

--dave

On 10/18/07, David Glasser <glasser@davidglasser.net> wrote:
> Grr, arg! I've been testing with the wrong version of my script (and
> the one I attached earlier was wrong too). You're right; the patch
> doesn't work.
>
> One thing to do might be to instrument calls to svn_wc_adm_open3.
>
> (The only difference between previous and current test is a filename.
> Damn bugs that depend on order of iteration...)
>
> Real test attached.
>
> --dave
>
> On 10/18/07, Ben Collins-Sussman <sussman@red-bean.com> wrote:
> > I get the same checksum error even with my patch.
> >
> >
> > On 10/18/07, David Glasser <glasser@davidglasser.net> wrote:
> > > On 10/18/07, Ben Collins-Sussman <sussman@red-bean.com> wrote:
> > > > Let me elaborate on this bug: it's something epg, glasser and I
> > > > discovered fooling around within Google today. It only exists using
> > > > the latest 1.5 server and client.
> > > >
> > > > The basic symptom is: you start a checkout, and at some point the 1.5
> > > > server says "add_file(foo, copyfrom=bar@10". The client searches
> > > > around for bar@10 in the working copy, and finds it. Or rather it
> > > > *thinks* it finds it, but it's actually the wrong version of the file.
> > > > So, rather than just doing a side svn_ra_get_file() fetch (as it
> > > > should), it installs the wrong version of the file with a quick wc-wc
> > > > copy. The server then tries to apply a txdelta to the file, and the
> > > > checksum fails... boom, failed checkout.
> > > >
> > > > The question is: why the wc-searching routine
> > > > (update_editor.c:locate_copyfrom()) think that it's found the correct
> > > > version of the file, when it's not? Answer: it seems to be getting
> > > > stale svn_wc_entry_t information. It's reading a couple of entries
> > > > files from disk, rather than from a more recent adm_access set still
> > > > held in memory, and getting an incorrect (stale) last-changed-rev
> > > > value. From poking around in gdb, it seems that the entry_t's which
> > > > are write-cached in memory are correct -- they have the proper
> > > > last-changed-rev values, but simply aren't flushed to disk yet.
> > > >
> > > > I attempted to solve this by passing in the edit_baton->access_set to
> > > > the searching routine, but it's not fixing the problem. I've attached
> > > > my patch-in-progress below. Can we get some more eyes on this
> > > > problem?
> > >
> > > Define "not fixing the problem". It makes my script pass for me...
> > >
> > > --dave
> > >
> > > >
> > > >
> > > >
> > > > Index: subversion/libsvn_wc/update_editor.c
> > > > ===================================================================
> > > > --- subversion/libsvn_wc/update_editor.c (revision 27277)
> > > > +++ subversion/libsvn_wc/update_editor.c (working copy)
> > > > @@ -2829,6 +2829,8 @@
> > > > copy for an pre-existing versioned file which is exactly equal to
> > > > COPYFROM_PATH@COPYFROM_REV.
> > > >
> > > > + Use existing ASSOCIATED access-sets to examine svn_wc_entry_t's.
> > > > +
> > > > If the file is found, return the absolute path to it in
> > > > *RETURN_PATH, as well as a (read-only) access_t for its parent in
> > > > *RETURN_ACCESS. If the file isn't found, set *RETURN_PATH to NULL.
> > > > @@ -2838,6 +2840,7 @@
> > > > svn_revnum_t copyfrom_rev,
> > > > const char *dest_dir,
> > > > const svn_wc_entry_t *dest_entry,
> > > > + svn_wc_adm_access_t *associated,
> > > > const char **return_path,
> > > > svn_wc_adm_access_t **return_access,
> > > > apr_pool_t *pool)
> > > > @@ -2897,10 +2900,10 @@
> > > > SVN_ERR(svn_io_check_path(cwd->data, &kind, subpool));
> > > > if (kind != svn_node_dir)
> > > > return SVN_NO_ERROR;
> > > > - err = svn_wc_adm_open3(&ancestor_access, NULL, cwd->data,
> > > > - FALSE, /* open read-only, please */
> > > > - 0, /* open only this directory */
> > > > - NULL, NULL, subpool);
> > > > + err = svn_wc_adm_probe_try3(&ancestor_access, associated, cwd->data,
> > > > + FALSE, /* open read-only, please */
> > > > + 0, /* open only this directory */
> > > > + NULL, NULL, subpool);
> > > > if (err && err->apr_err == SVN_ERR_WC_NOT_DIRECTORY)
> > > > {
> > > > /* The common ancestor directory isn't version-controlled. */
> > > > @@ -2941,10 +2944,10 @@
> > > > return SVN_NO_ERROR;
> > > >
> > > > /* Next: is the file's parent-dir under version control? */
> > > > - err = svn_wc_adm_open3(&ancestor_access, NULL, cwd_parent->data,
> > > > - FALSE, /* open read-only, please */
> > > > - 0, /* open only the parent dir */
> > > > - NULL, NULL, pool);
> > > > + err = svn_wc_adm_probe_try3(&ancestor_access, associated,
> > > cwd_parent->data,
> > > > + FALSE, /* open read-only, please */
> > > > + 0, /* open only the parent dir */
> > > > + NULL, NULL, pool);
> > > > if (err && err->apr_err == SVN_ERR_WC_NOT_DIRECTORY)
> > > > {
> > > > svn_error_clear(err);
> > > > @@ -3052,7 +3055,7 @@
> > > > /* Attempt to locate the copyfrom_path in the working copy first. */
> > > > SVN_ERR(svn_wc_entry(&path_entry, pb->path, eb->adm_access, FALSE,
> > > pool));
> > > > err = locate_copyfrom(copyfrom_path, copyfrom_rev,
> > > > - pb->path, path_entry,
> > > > + pb->path, path_entry, eb->adm_access,
> > > > &src_path, &src_access, pool);
> > > > if (err && err->apr_err == SVN_ERR_WC_COPYFROM_PATH_NOT_FOUND)
> > > > svn_error_clear(err);
> > > >
> > >
> > >
> > > --
> > > David Glasser | glasser_at_davidglasser.net | http://www.davidglasser.net/
> > >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> > For additional commands, e-mail: dev-help@subversion.tigris.org
> >
> >
>
>
> --
> David Glasser | glasser_at_davidglasser.net | http://www.davidglasser.net/
>
>

-- 
David Glasser | glasser_at_davidglasser.net | http://www.davidglasser.net/


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

  • application/x-sh attachment: doit.sh
Received on Fri Oct 19 01:56: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.