[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 14:02:06 CEST

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

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

I think there are two bugs here. I believe it is wrong for
locate_copyfrom to use a read-only snapshot of the working copy, your
patch to pass an access baton looks like the right thing to do,
although I think that the _try calls should take write locks. However
that's not what is causing the current failure.

The reason that locate_copyfrom thinks it has the right file is that
when alpha first gets added it has committed-rev=1 and that meets the
conditions required by locate_copyfrom. This happens because alpha
itself has copyfrom info and update_editor.c:add_file_with_history
calls eb->fetch_func to get gamma@1 from the repository and then there
is a bit of code that does:

  /* Loop over whatever base-props we have in memory, faking change_file_prop()
     calls against the file baton. */
  for (hi = apr_hash_first(pool, base_props); hi; hi = apr_hash_next(hi))
    {
      const void *key;
      void *val;
      const char *propname;
      svn_string_t *propval;
      apr_hash_this(hi, &key, NULL, &val);
      propname = key;
      propval = val;
      SVN_ERR(change_file_prop(tfb, propname, propval, pool));
    }

which transfers the gamma@1 props to the new alpha, including
committed-rev=1. Later on alpha will get corrected to have
committed-rev=3 but before that something else happens to look for
alpha@1.

Perhaps properties like committed-rev should be filtered out of then
above loop? Perhaps only regular props should be transferred?

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Oct 19 14:02:25 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.