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

Re: [PATCH]svn_wc__find_wc_root

From: Stefan Sperling <stsp_at_elego.de>
Date: Fri, 26 Jun 2009 15:31:01 +0100

On Fri, Jun 26, 2009 at 08:31:15PM +0800, yellow.flying wrote:
> Hey,
> I tweak some some spot, but I do not know whether it is a little clear now.
> Several have mentioned wc-ng to us, but I do not think our work can base
> on wc-ng right now, because we all are not very clear what wc-ng is exactly
> like. If it is possible I will try to solve this problem base on wc-ng in the
> future.
> Someone also have mentioned performance, but I do not have better ideas
> when it is based on wc-1. So maybe we can solve this problem when wc-ng
> is ok.

> + while(1)
> + {
> + svn_pool_clear(iterpool);
> + SVN_ERR(svn_wc_adm_open3(&adm_access, NULL, absolute,
> + FALSE, /* Write lock */
> + 0, /* lock levels */
> + NULL, NULL, iterpool));

Hmmm.... wait...

I think we're trying to do things the wrong way around.

I don't think that opening an access baton like this will work while
a commit is in progress. It will work while no commit is in progress,
so in your testing the function has suceeded so far.

But it will fail during a commit because the root directory of the
working copy directory might already be locked for writing.
See libsvn_client/commit.c, line 1518:

  SVN_ERR(svn_wc_adm_open3(&base_dir_access, NULL, base_dir,
                           TRUE, /* Write lock */

So I think we'll need to change the logic.
Let's start again from the top.

Recall again what we want to achieve:

== Goal ==
We want to get a set of access batons, one for the root of each
working copy involved in a commit. As input, we get a list
of paths that are part of an arbitrary amount of working copies.
We also want to group those paths by their working copy root.

Then we want to make a commit from each working copy root.

Doing a single commit for all working copies at once is too hard.
See svn_client__do_commit and follow the use of its base_dir_access,
which is the root of the working copy the commit is made from.
There is so much code that assumes a single access baton for
the root that you would run out of time for your gsoc project
before you are anywhere near done changing it all.

== What's there now ==
Currently, the code "condenses" all target paths, which means it
tries to find a common root for them, and then it tries to lock that
common root. If the common root is not a working copy, locking the
common root fails and the commit is simply aborted ("svn: <common root>
is not a working copy").

== What we can do instead ==
So let's say we did the following instead of aborting the commit:

  If locking the common root failed:

    For each target path we got:

      For each working copy root we already know (initially we don't
        know any working copy roots):
          Check if the current target path is below the working copy root.
          If it is, put it into this root's group ("commit packet")
          and continue with the next target path.

      If we end up here, no suitable working copy root was found
      for the current target path.
      Walk the current target path downwards, starting from the common
      root (the root which we could not lock).
      Try to lock the current directory at each step.
      If locking succeeds, we have found a new WC root!
      Store its access baton in the set of known working copy roots.
      Put the current target path into the group of the root we just found.

  Now run a commit for each working copy root we found.
  This is done just like before when the code only knew about
  a single root, but it's done for each root in turn.

  Also, make svn_client_commit() return an array of svn_commit_info_t
  objects, one for each commit made, instead of just a single
  svn_commit_info_t object.

How does this plan sound to you?
All of this could be done directly in libsvn_client/commit.c.
You can add static helper functions to that file which do
the work described above.

If you agree to this plan, I'm sorry that your current
svn_wc__find_wc_root() patch might not be committed after all.
But at least for me, the patch helped me a lot to actually
understand the problem :)

Should we update the notes/ file with the new plan?

Received on 2009-06-26 16:31:23 CEST

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.