You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "C. Michael Pilato" <cm...@collab.net> on 2011/03/03 19:40:55 UTC

Re: svn commit: r1076712 - in /subversion/trunk/subversion/libsvn_wc: revision_status.c wc-queries.sql wc_db.c wc_db.h

On 03/03/2011 01:15 PM, stsp@apache.org wrote:
> Author: stsp
> Date: Thu Mar  3 18:15:46 2011
> New Revision: 1076712
> 
> URL: http://svn.apache.org/viewvc?rev=1076712&view=rev
> Log:
> As a first step towards eliminating use of the node walker in
> svn_wc_revision_status2(), use a DB query to determine the min/max revisions
> of the working copy, instead of obtaining this information during the crawl.

I was *just* making changes in this space.  Why?  Because this code needs to
ignore file externals when determining the revision range and status and
such.  In fact, I was just getting ready to run the full test suite with the
attached patch, which appears to fix issue #3816[1].  Can we modify your
MIN_MAX sqlite statement to skip nodes which are file externals?

-- C-Mike

[1] http://subversion.tigris.org/issues/show_bug.cgi?id=3816

-----------------

Index: subversion/libsvn_wc/revision_status.c
===================================================================
--- subversion/libsvn_wc/revision_status.c	(revision 1076717)
+++ subversion/libsvn_wc/revision_status.c	(working copy)
@@ -61,6 +61,7 @@
   svn_revnum_t revision;
   svn_revnum_t item_rev;
   svn_depth_t depth;
+  svn_boolean_t is_file_external;
   svn_wc__db_status_t status;

   SVN_ERR(svn_wc__db_read_info(&status, NULL, &revision, NULL,
@@ -89,6 +90,12 @@
       wb->result->modified = TRUE;
     }

+  /* Ignore file externals. */
+  SVN_ERR(svn_wc__internal_is_file_external(&is_file_external, wb->db,
+                                            local_abspath, scratch_pool));
+  if (is_file_external)
+    return SVN_NO_ERROR;
+
   if (! wb->result->switched)
     {
       svn_boolean_t wc_root;

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: svn commit: r1076712 - in /subversion/trunk/subversion/libsvn_wc: revision_status.c wc-queries.sql wc_db.c wc_db.h

Posted by "C. Michael Pilato" <cm...@collab.net>.
On 03/03/2011 01:48 PM, Stefan Sperling wrote:
> On Thu, Mar 03, 2011 at 01:40:55PM -0500, C. Michael Pilato wrote:
>> On 03/03/2011 01:15 PM, stsp@apache.org wrote:
>>> Author: stsp
>>> Date: Thu Mar  3 18:15:46 2011
>>> New Revision: 1076712
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1076712&view=rev
>>> Log:
>>> As a first step towards eliminating use of the node walker in
>>> svn_wc_revision_status2(), use a DB query to determine the min/max revisions
>>> of the working copy, instead of obtaining this information during the crawl.
>>
>> I was *just* making changes in this space.  Why?  Because this code needs to
>> ignore file externals when determining the revision range and status and
>> such.  In fact, I was just getting ready to run the full test suite with the
>> attached patch, which appears to fix issue #3816[1].  Can we modify your
>> MIN_MAX sqlite statement to skip nodes which are file externals?
> 
> Off-hand I have no better idea than selecting properties, parsing them,
> and determining if any of them configures a file external.
> That could be made to work but it's quite hairy.
> 
> I think that, going forward, we'll need a better representation
> of externals in the DB in order to be able to make queries consider
> them in a more straightforward way.
> 
> What do you think about trying to tackle that instead of adding
> little hacks here and there to fix file external bugs?
> Hacks that ultimately limit progress in wc-ng?

Well, this should be easier than parsing properties, really.  I'm not trying
to ignore the directory which holds the svn:externals property, just the
file which is created as the result of that property.  I'm pretty sure those
are already demarcated in the WC-NG schema in a easy-to-spot fashion.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: svn commit: r1076712 - in /subversion/trunk/subversion/libsvn_wc: revision_status.c wc-queries.sql wc_db.c wc_db.h

Posted by "C. Michael Pilato" <cm...@collab.net>.
On 03/03/2011 01:50 PM, Stefan Sperling wrote:
>> I think that, going forward, we'll need a better representation
>> of externals in the DB in order to be able to make queries consider
>> them in a more straightforward way.
> 
> Hmmm... until then, we could probably use this hack in the NODES table:
> 
>   /* The serialized file external information. */
>   /* ### hack.  hack.  hack.
>      ### This information is already stored in properties, but because the
>      ### current working copy implementation is such a pain, we can't
>      ### readily retrieve it, hence this temporary cache column.
>      ### When it is removed, be sure to remove the extra column from
>      ### the db-tests.
> 
>      ### Note: This is only here as a hack, and should *NOT* be added
>      ### to any wc_db APIs.  */
>   file_external  TEXT,

Yes, yes, this was the easy-to-spot fashion I was thinking of!

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: svn commit: r1076712 - in /subversion/trunk/subversion/libsvn_wc: revision_status.c wc-queries.sql wc_db.c wc_db.h

Posted by Stefan Sperling <st...@elego.de>.
On Thu, Mar 03, 2011 at 07:48:22PM +0100, Stefan Sperling wrote:
> On Thu, Mar 03, 2011 at 01:40:55PM -0500, C. Michael Pilato wrote:
> > On 03/03/2011 01:15 PM, stsp@apache.org wrote:
> > > Author: stsp
> > > Date: Thu Mar  3 18:15:46 2011
> > > New Revision: 1076712
> > > 
> > > URL: http://svn.apache.org/viewvc?rev=1076712&view=rev
> > > Log:
> > > As a first step towards eliminating use of the node walker in
> > > svn_wc_revision_status2(), use a DB query to determine the min/max revisions
> > > of the working copy, instead of obtaining this information during the crawl.
> > 
> > I was *just* making changes in this space.  Why?  Because this code needs to
> > ignore file externals when determining the revision range and status and
> > such.  In fact, I was just getting ready to run the full test suite with the
> > attached patch, which appears to fix issue #3816[1].  Can we modify your
> > MIN_MAX sqlite statement to skip nodes which are file externals?
> 
> Off-hand I have no better idea than selecting properties, parsing them,
> and determining if any of them configures a file external.
> That could be made to work but it's quite hairy.
> 
> I think that, going forward, we'll need a better representation
> of externals in the DB in order to be able to make queries consider
> them in a more straightforward way.

Hmmm... until then, we could probably use this hack in the NODES table:

  /* The serialized file external information. */
  /* ### hack.  hack.  hack.
     ### This information is already stored in properties, but because the
     ### current working copy implementation is such a pain, we can't
     ### readily retrieve it, hence this temporary cache column.
     ### When it is removed, be sure to remove the extra column from
     ### the db-tests.

     ### Note: This is only here as a hack, and should *NOT* be added
     ### to any wc_db APIs.  */
  file_external  TEXT,


Re: svn commit: r1076712 - in /subversion/trunk/subversion/libsvn_wc: revision_status.c wc-queries.sql wc_db.c wc_db.h

Posted by Stefan Sperling <st...@elego.de>.
On Thu, Mar 03, 2011 at 01:40:55PM -0500, C. Michael Pilato wrote:
> On 03/03/2011 01:15 PM, stsp@apache.org wrote:
> > Author: stsp
> > Date: Thu Mar  3 18:15:46 2011
> > New Revision: 1076712
> > 
> > URL: http://svn.apache.org/viewvc?rev=1076712&view=rev
> > Log:
> > As a first step towards eliminating use of the node walker in
> > svn_wc_revision_status2(), use a DB query to determine the min/max revisions
> > of the working copy, instead of obtaining this information during the crawl.
> 
> I was *just* making changes in this space.  Why?  Because this code needs to
> ignore file externals when determining the revision range and status and
> such.  In fact, I was just getting ready to run the full test suite with the
> attached patch, which appears to fix issue #3816[1].  Can we modify your
> MIN_MAX sqlite statement to skip nodes which are file externals?

Off-hand I have no better idea than selecting properties, parsing them,
and determining if any of them configures a file external.
That could be made to work but it's quite hairy.

I think that, going forward, we'll need a better representation
of externals in the DB in order to be able to make queries consider
them in a more straightforward way.

What do you think about trying to tackle that instead of adding
little hacks here and there to fix file external bugs?
Hacks that ultimately limit progress in wc-ng?

> 
> -- C-Mike
> 
> [1] http://subversion.tigris.org/issues/show_bug.cgi?id=3816
> 
> -----------------
> 
> Index: subversion/libsvn_wc/revision_status.c
> ===================================================================
> --- subversion/libsvn_wc/revision_status.c	(revision 1076717)
> +++ subversion/libsvn_wc/revision_status.c	(working copy)
> @@ -61,6 +61,7 @@
>    svn_revnum_t revision;
>    svn_revnum_t item_rev;
>    svn_depth_t depth;
> +  svn_boolean_t is_file_external;
>    svn_wc__db_status_t status;
> 
>    SVN_ERR(svn_wc__db_read_info(&status, NULL, &revision, NULL,
> @@ -89,6 +90,12 @@
>        wb->result->modified = TRUE;
>      }
> 
> +  /* Ignore file externals. */
> +  SVN_ERR(svn_wc__internal_is_file_external(&is_file_external, wb->db,
> +                                            local_abspath, scratch_pool));
> +  if (is_file_external)
> +    return SVN_NO_ERROR;
> +
>    if (! wb->result->switched)
>      {
>        svn_boolean_t wc_root;
> 
> -- 
> C. Michael Pilato <cm...@collab.net>
> CollabNet   <>   www.collab.net   <>   Distributed Development On Demand
>