You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by kf...@collab.net on 2004/10/18 05:05:59 UTC

Re: svn commit: r11439 - trunk/subversion/libsvn_client

lundblad@tigris.org writes:
> --- trunk/subversion/libsvn_client/status.c	(original)
> +++ trunk/subversion/libsvn_client/status.c	Sat Oct 16 15:42:30 2004
> @@ -119,27 +124,52 @@
>      return svn_error_createf (SVN_ERR_UNVERSIONED_RESOURCE, NULL,
>                                _("'%s' is not under version control"), path);
>    
> -  /* Close up our ADM area.  We'll be re-opening soon. */
> -  SVN_ERR (svn_wc_adm_close (adm_access));
> +  /* Need to lock the tree.  We lock the anchor first, to not lock too much
> +     if we have a target.  We need to lock the target and immediate children
> +     if the status is non-recursive.  Else, we lock the whole hierarchy
> +     under target.  If we are contacting the repository, we need a recursive
> +     lock under anchor so that auth data can be stored. */

So "that auth data can be stored"?  What sort of auth data?

Btw, I looked at the rest of the commit, but wasn't able to fully
follow it (which is not a statement about your code, more about my not
having worked in libsvn_wc in too long).  I'll try to review it with
Ben Collins-Sussman tomorrow.

Best,
-Karl

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

Re: svn commit: r11439 - trunk/subversion/libsvn_client

Posted by Philip Martin <ph...@codematters.co.uk>.
"Peter N. Lundblad" <pe...@famlundblad.se> writes:

> If someone with deeper knowledge can confirm that locking the whole tree
> just for authentication isn't necessary anymore, then I could elminitate
> this.

It appears that the old comment came from r4311 so it refers to the
old auth system and is no longer relevant.

-- 
Philip Martin

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

Re: svn commit: r11439 - trunk/subversion/libsvn_client

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Thu, 28 Oct 2004, Julian Foad wrote:

> Peter N. Lundblad wrote:
> > ------------------------------------------------------------------------
> > r11439 | lundblad | 2004-10-16 21:42:30 +0100 (Sat, 16 Oct 2004) | 6 lines
> >
> > Fix slow "svn st" where the targets are files by not locking the whole anchor
> > tree.
> >
> > * libsvn_client/status.c (svn_client_status): Don't lock the whole directory
> >   hierarchy if not necessary.
>
> In the log message we give the path to each file relative to the root of the
> source tree - thus it should be "subversion/libsvn_client/status.c".
>
Hmmm, you're right. Interesting no one has complained before...

Also, this is what svn-log-message in Emacs svn-dev.el does for me. Anyone
knows why. I ahve my soruces in ~/src/svn/svn. Maybe others use another
convention?

 > > +  if (update)
> > +    {
> [...]
> > +    }
> > +  else
> > +    {
> [...]
> > +          err = svn_wc_adm_open2 (&target_access, anchor_access, path,
> > +                                  FALSE, (descend || update) ? -1 : 1, pool);
>
> The variable "update" is redundant in that test since it is known to be false.
>
True. I'll eliminate the whole special-case for the update case. It's not
necessary anymore according to earlier discussion.

Thanks for the review,
//Peter

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

Re: svn commit: r11439 - trunk/subversion/libsvn_client

Posted by Julian Foad <ju...@btopenworld.com>.
Peter N. Lundblad wrote:
> ------------------------------------------------------------------------
> r11439 | lundblad | 2004-10-16 21:42:30 +0100 (Sat, 16 Oct 2004) | 6 lines
> 
> Fix slow "svn st" where the targets are files by not locking the whole anchor
> tree.
> 
> * libsvn_client/status.c (svn_client_status): Don't lock the whole directory
>   hierarchy if not necessary.

In the log message we give the path to each file relative to the root of the 
source tree - thus it should be "subversion/libsvn_client/status.c".

> +  if (update)
> +    {
[...]
> +    }
> +  else
> +    {
[...]
> +          err = svn_wc_adm_open2 (&target_access, anchor_access, path,
> +                                  FALSE, (descend || update) ? -1 : 1, pool);

The variable "update" is redundant in that test since it is known to be false.

- Julian

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

Re: svn commit: r11439 - trunk/subversion/libsvn_client

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Mon, 18 Oct 2004, Tobias Ringström wrote:

> kfogel@collab.net wrote:
>
> >So "that auth data can be stored"?  What sort of auth data?
> >
> >
> It looks like a copied comment from the old code, and I think Peter just
> copied the functionality from the old code.
>
That's exactly the case:-)

> Peter, a long time ago, Suversion stored the authentication data in the
> .svn directories, but it was moved to e.g. ~/.subversion/auth before you
> got here. It's possible that the code no longer really requires that
> recursive lock because of that change.  I should warn you that I have
> not looked at the code in question, so I cannot tell you for certain.
>
So even more optimization is possible (and code decomplexification). This
is good, but I don't think the code is wrong as-is.

If someone with deeper knowledge can confirm that locking the whole tree
just for authentication isn't necessary anymore, then I could elminitate
this.

Thanks,
//Peter

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


Re: svn commit: r11439 - trunk/subversion/libsvn_client

Posted by Tobias Ringström <to...@ringstrom.mine.nu>.
kfogel@collab.net wrote:

>So "that auth data can be stored"?  What sort of auth data?
>  
>
It looks like a copied comment from the old code, and I think Peter just 
copied the functionality from the old code.

Peter, a long time ago, Suversion stored the authentication data in the 
.svn directories, but it was moved to e.g. ~/.subversion/auth before you 
got here. It's possible that the code no longer really requires that 
recursive lock because of that change.  I should warn you that I have 
not looked at the code in question, so I cannot tell you for certain.

/Tobias


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