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

This is Issue #2977. It is a release blocker.

--dave

On 10/18/07, David Glasser <glasser@davidglasser.net> wrote:
> See update_test 40 as of r27286.
>
> --dave
>
> On 10/18/07, David Glasser <glasser@davidglasser.net> wrote:
> > ... 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/
> >
> >
>
>
> --
> 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