You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Kevin Pilch-Bisson <ke...@pilch-bisson.net> on 2001/05/16 19:32:37 UTC

Re: CVS update: MODIFIED: client ...

What does this function offer that svn_path_condense doesn't, except for
preserving order?  Why does order need to be preserved? Maybe
svn_path_condense_targets should be modified instead?


On Wed, May 16, 2001 at 06:58:31PM -0000, cmpilato@tigris.org wrote:
>   User: cmpilato
>   Date: 01/05/16 11:58:31
> 
>   Modified:    subversion/client update-cmd.c
>   Log:
>   Resolution for IssueZilla issue #386
>   
>   * include/svn_path.h
>   * include/target.c
>   
>     (svn_path_remove_redundancies): New function.
>   
>   * client/update-cmd.c
>   
>     (svn_cl__update): Remove redundancies from the target list,
>     preserving the order of those targets, using new
>     svn_path_remove_redundancies function.
>   
>   Revision  Changes    Path
>   1.35      +12 -0     subversion/subversion/include/svn_path.h
>   
>   http://subversion.tigris.org/source/browse/subversion/subversion/include/svn_path.h.diff?r1=1.34&r2=1.35
>   
>   (In the diff below, changes in quantity of whitespace are not shown.)
>   
>   Index: svn_path.h
>   ===================================================================
>   RCS file: /usr/local/tigris/data/helm/cvs/repository/subversion/subversion/include/svn_path.h,v
>   retrieving revision 1.34
>   retrieving revision 1.35
>   diff -u -b -r1.34 -r1.35
>   --- svn_path.h	2001/05/11 02:59:26	1.34
>   +++ svn_path.h	2001/05/16 18:58:30	1.35
>   @@ -181,6 +181,18 @@
>                               const apr_array_header_t *targets,
>                               apr_pool_t *pool);
>    
>   +
>   +/* Copy a list of TARGETS, one at a time, into PCONDENSED_TARGETS,
>   +   omitting any targets that are found earlier in the list, or whose
>   +   ancestor is found earlier in the list.  Ordering of targets in the
>   +   original list is preserved in the condensed list of targets.  Use
>   +   POOL for any allocations.  */
>   +svn_error_t *
>   +svn_path_remove_redundancies (apr_array_header_t **pcondensed_targets,
>   +                              const apr_array_header_t *targets,
>   +                              apr_pool_t *pool);
>   +
>   +
>    /* Decompose PATH into an array of svn_string_t components, allocated
>       in POOL.  STYLE indicates the dir separator to split the string on.
>       If PATH is absolute, the first component will be a lone dir
>   
>   
>   
>   1.7       +93 -0     subversion/subversion/libsvn_subr/target.c
>   
>   http://subversion.tigris.org/source/browse/subversion/subversion/libsvn_subr/target.c.diff?r1=1.6&r2=1.7
>   
>   (In the diff below, changes in quantity of whitespace are not shown.)
>   
>   Index: target.c
>   ===================================================================
>   RCS file: /usr/local/tigris/data/helm/cvs/repository/subversion/subversion/libsvn_subr/target.c,v
>   retrieving revision 1.6
>   retrieving revision 1.7
>   diff -u -b -r1.6 -r1.7
>   --- target.c	2001/05/11 03:21:51	1.6
>   +++ target.c	2001/05/16 18:58:31	1.7
>   @@ -252,6 +252,99 @@
>      return SVN_NO_ERROR;
>    }
>    
>   +
>   +svn_error_t *
>   +svn_path_remove_redundancies (apr_array_header_t **pcondensed_targets,
>   +                              const apr_array_header_t *targets,
>   +                              apr_pool_t *pool)
>   +{
>   +  apr_pool_t *temp_pool;
>   +  apr_array_header_t *abs_targets;
>   +  apr_array_header_t *rel_targets;
>   +  int i;
>   +
>   +  if ((targets->nelts <= 0) || (! pcondensed_targets))
>   +    {
>   +      /* No targets or no place to store our work means this function
>   +         really has nothing to do. */
>   +      if (pcondensed_targets)
>   +        *pcondensed_targets = NULL;
>   +      return SVN_NO_ERROR;
>   +    }
>   +
>   +  /* Initialize our temporary pool. */
>   +  temp_pool = svn_pool_create (pool);
>   +
>   +  /* Create our list of absolute paths for our "keepers" */
>   +  abs_targets = apr_array_make (temp_pool, targets->nelts, 
>   +                                sizeof (svn_string_t *));
>   +
>   +  /* Create our list of untainted paths for our "keepers" */
>   +  rel_targets = apr_array_make (pool, targets->nelts,
>   +                                sizeof (svn_string_t *));
>   +
>   +  /* For each target in our list we do the following:
>   +
>   +     1.  Calculate its absolute path (ABS_PATH).
>   +     2.  See if any of the keepers in ABS_TARGETS is a parent of, or
>   +         is the same path as, ABS_PATH.  If so, we ignore this
>   +         target.  If not, however, add this target's absolute path to
>   +         ABS_TARGETS and its original path to REL_TARGETS.
>   +  */
>   +  for (i = 0; i < targets->nelts; i++)
>   +    {
>   +      svn_string_t *rel_path = ((svn_string_t **)targets->elts)[i];
>   +      svn_string_t *abs_path;
>   +      int j;
>   +      svn_boolean_t keep_me;
>   +
>   +      /* Get the absolute path for this target. */
>   +      SVN_ERR (svn_path_get_absolute (&abs_path, rel_path, temp_pool));
>   +
>   +      /* For each keeper in ABS_TARGETS, see if this target is the
>   +         same as or a child of that keeper. */
>   +      keep_me = TRUE;
>   +      for (j = 0; j < abs_targets->nelts; j++)
>   +        {
>   +          svn_string_t *keeper = ((svn_string_t **)abs_targets->elts)[j];
>   +          
>   +          /* Quit here if we find this path already in the keepers. */
>   +          if (svn_string_compare (keeper, abs_path))
>   +            {
>   +              keep_me = FALSE;
>   +              break;
>   +            }
>   +          
>   +          /* Quit here if this path is a child of one of the keepers. */
>   +          if (svn_path_is_child (keeper, abs_path, temp_pool))
>   +            { 
>   +              keep_me = FALSE;
>   +              break;
>   +            }
>   +        }
>   +
>   +      /* If this is a new keeper, add its absolute path to ABS_TARGETS
>   +         and its original path to REL_TARGETS. */
>   +      if (keep_me)
>   +        {
>   +          (* ((svn_string_t **) apr_array_push (abs_targets))) = abs_path;
>   +          (* ((svn_string_t **) apr_array_push (rel_targets))) = rel_path;
>   +        }
>   +    }
>   +  
>   +  /* Destroy our temporary pool. */
>   +  svn_pool_destroy (temp_pool);
>   +
>   +  /* Make sure we return the list of untainted keeper paths. */
>   +  *pcondensed_targets = rel_targets;
>   +  
>   +  return SVN_NO_ERROR;
>   +}
>   +      
>   +
>   +  
>   +
>   +
>    
>    /* 
>     * local variables:
>   
>   
>   
>   1.19      +8 -2      subversion/subversion/client/update-cmd.c
>   
>   http://subversion.tigris.org/source/browse/subversion/subversion/client/update-cmd.c.diff?r1=1.18&r2=1.19
>   
>   (In the diff below, changes in quantity of whitespace are not shown.)
>   
>   Index: update-cmd.c
>   ===================================================================
>   RCS file: /usr/local/tigris/data/helm/cvs/repository/subversion/subversion/client/update-cmd.c,v
>   retrieving revision 1.18
>   retrieving revision 1.19
>   diff -u -b -r1.18 -r1.19
>   --- update-cmd.c	2001/05/09 19:22:15	1.18
>   +++ update-cmd.c	2001/05/16 18:58:31	1.19
>   @@ -35,6 +35,7 @@
>                    apr_pool_t *pool)
>    {
>      apr_array_header_t *targets;
>   +  apr_array_header_t *condensed_targets;
>      int i;
>    
>      targets = svn_cl__args_to_target_array (os, pool);
>   @@ -42,9 +43,14 @@
>      /* Add "." if user passed 0 arguments */
>      svn_cl__push_implicit_dot_target(targets, pool);
>    
>   -  for (i = 0; i < targets->nelts; i++)
>   +  /* Remove redundancies from the target list while preserving order. */
>   +  SVN_ERR (svn_path_remove_redundancies (&condensed_targets,
>   +                                         targets,
>   +                                         pool));
>   +
>   +  for (i = 0; i < condensed_targets->nelts; i++)
>        {
>   -      svn_string_t *target = ((svn_string_t **) (targets->elts))[i];
>   +      svn_string_t *target = ((svn_string_t **) (condensed_targets->elts))[i];
>          const svn_delta_edit_fns_t *trace_editor;
>          void *trace_edit_baton;
>          svn_string_t *parent_dir, *entry;
>   
>   
>   
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: cvs-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: cvs-help@subversion.tigris.org
> 

-- 
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Kevin Pilch-Bisson                    http://www.pilch-bisson.net
     "Historically speaking, the presences of wheels in Unix
     has never precluded their reinvention." - Larry Wall
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Re: CVS update: MODIFIED: client ...

Posted by Kevin Pilch-Bisson <ke...@pilch-bisson.net>.
Understand 1.  Still unclear on 2.  (Probably lack of sleep, I have been
a little busy lately).  Feel free to add the docstring to the code for
now.  Maybe once I grok what you are doing I will modify it somehow.

Didn't mean to be terse before, just so you know, it was simply a matter
of wondering.


On Wed, May 16, 2001 at 02:59:49PM -0500, cmpilato@collab.net wrote:
> Kevin Pilch-Bisson <ke...@pilch-bisson.net> writes:
> 
> > What does this function offer that svn_path_condense doesn't, except for
> > preserving order?  Why does order need to be preserved? Maybe
> > svn_path_condense_targets should be modified instead?
> 
> Dammit, the very fact that you ask this reminds me that I forgot to
> write up the explanation.
> 
> Here's the short version:
> 
> 1.  Disclaimer: if you wish to debate the following, talk to Karl. :-)
>     Order matters for updates because a multi-arg update is not
>     atomic, and CVS users are used to, when doing 'cvs up targetA
>     targetB' seeing targetA get updated, then targetB.  I think the
>     idea is that if you're in a time-sensitive or flaky-network
>     situation, a user can say, "I really *need* to update
>     wc/A/D/G/tau, but I might as well update my whole working copy if
>     I can."  So that user will do 'svn up wc/A/D/G/tau wc', and if
>     something dies in the middles of the 'wc' update, at least the
>     user has 'tau' up-to-date.
> 
> 2.  Also, we have this notion of an anchor and a target for updates
>     (the anchor is where the update editor is rooted, the target is
>     the actual thing we want to update).  I needed a function that
>     would NOT screw with my input paths so that I could tell the
>     difference between someone being in A/D and saying 'svn up G' and
>     being in A/D/G and saying 'svn up .' -- believe it or not, these
>     two things don't mean the same thing.  svn_path_condense_targets
>     plays with absolute paths (which is fine, so does
>     svn_path_remove_redundancies), but the difference is that it
>     actually tweaks those targets to be relative to the "grandfather
>     path" common to all the targets.  Updates don't require a
>     "grandfather path" at all, and even if it did, the whole
>     conversion to an absolute path drops the crucial difference
>     between saying "i'm in foo, update bar" and "i'm in foo/bar,
>     update '.'"
> 
> Hope this helps.
> 
> By the way, there are likely to be some changes to the way commits
> work that may require similar requirements as the update case.  If
> this happens, I will most certainly be trying to merge your function
> and mine together!

-- 
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Kevin Pilch-Bisson                    http://www.pilch-bisson.net
     "Historically speaking, the presences of wheels in Unix
     has never precluded their reinvention." - Larry Wall
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Re: CVS update: MODIFIED: client ...

Posted by cm...@collab.net.
Karl Fogel <kf...@collab.net> writes:

> > Dammit, the very fact that you ask this reminds me that I forgot to
> > write up the explanation.
> 
> :-)
> 
> Good explanation; maybe it should live in the code, near where the
> tweak is done to the target list...?

Was already planning it, but wanted to wait until Kevin expressed
satisfaction with the explanation(s).  Once that occurs, docstrings
will be updated!

Kevin, did I explain the situation well enough?

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

Re: CVS update: MODIFIED: client ...

Posted by Karl Fogel <kf...@collab.net>.
cmpilato@collab.net writes:
> Dammit, the very fact that you ask this reminds me that I forgot to
> write up the explanation.

:-)

Good explanation; maybe it should live in the code, near where the
tweak is done to the target list...?

-K


> Here's the short version:
> 
> 1.  Disclaimer: if you wish to debate the following, talk to Karl. :-)
>     Order matters for updates because a multi-arg update is not
>     atomic, and CVS users are used to, when doing 'cvs up targetA
>     targetB' seeing targetA get updated, then targetB.  I think the
>     idea is that if you're in a time-sensitive or flaky-network
>     situation, a user can say, "I really *need* to update
>     wc/A/D/G/tau, but I might as well update my whole working copy if
>     I can."  So that user will do 'svn up wc/A/D/G/tau wc', and if
>     something dies in the middles of the 'wc' update, at least the
>     user has 'tau' up-to-date.
> 
> 2.  Also, we have this notion of an anchor and a target for updates
>     (the anchor is where the update editor is rooted, the target is
>     the actual thing we want to update).  I needed a function that
>     would NOT screw with my input paths so that I could tell the
>     difference between someone being in A/D and saying 'svn up G' and
>     being in A/D/G and saying 'svn up .' -- believe it or not, these
>     two things don't mean the same thing.  svn_path_condense_targets
>     plays with absolute paths (which is fine, so does
>     svn_path_remove_redundancies), but the difference is that it
>     actually tweaks those targets to be relative to the "grandfather
>     path" common to all the targets.  Updates don't require a
>     "grandfather path" at all, and even if it did, the whole
>     conversion to an absolute path drops the crucial difference
>     between saying "i'm in foo, update bar" and "i'm in foo/bar,
>     update '.'"
> 
> Hope this helps.
> 
> By the way, there are likely to be some changes to the way commits
> work that may require similar requirements as the update case.  If
> this happens, I will most certainly be trying to merge your function
> and mine together!
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org

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

Re: CVS update: MODIFIED: client ...

Posted by Kevin Pilch-Bisson <ke...@pilch-bisson.net>.
On Thu, May 17, 2001 at 09:32:22AM -0500, cmpilato@collab.net wrote:
> Kevin Pilch-Bisson <ke...@pilch-bisson.net> writes:
> 
> > I am guessing that the difference has to do with whether or not we make
> > a change to the parent entries file?  We shouldn't have to anyway should
> > we?  I thought that the entry in the parent dir just contained the name,
> > and all the info was in the THIS_DIR entry within the child dir.
> 
> Perhaps reading the big ol' docstring for svn_wc_get_actual_update_target 
> in libsvn_wc/get_editor.c would help.  I don't feel like I'm likely to
> explain it much better than I did in that comment.
> 
That did the trick for me.  I hadn't considered the case where a dir had
been removed and replaced with a file.

Will try to think about how to go about doing that via the condense
targets route, but not sure how much bandwidth I can give to it for a
while.
-- 
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Kevin Pilch-Bisson                    http://www.pilch-bisson.net
     "Historically speaking, the presences of wheels in Unix
     has never precluded their reinvention." - Larry Wall
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Re: CVS update: MODIFIED: client ...

Posted by cm...@collab.net.
Kevin Pilch-Bisson <ke...@pilch-bisson.net> writes:

> I am guessing that the difference has to do with whether or not we make
> a change to the parent entries file?  We shouldn't have to anyway should
> we?  I thought that the entry in the parent dir just contained the name,
> and all the info was in the THIS_DIR entry within the child dir.

Perhaps reading the big ol' docstring for svn_wc_get_actual_update_target 
in libsvn_wc/get_editor.c would help.  I don't feel like I'm likely to
explain it much better than I did in that comment.





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

Re: CVS update: MODIFIED: client ...

Posted by Kevin Pilch-Bisson <ke...@pilch-bisson.net>.
On Wed, May 16, 2001 at 02:59:49PM -0500, cmpilato@collab.net wrote:
> 2.  Also, we have this notion of an anchor and a target for updates
>     (the anchor is where the update editor is rooted, the target is
>     the actual thing we want to update).  I needed a function that
>     would NOT screw with my input paths so that I could tell the
>     difference between someone being in A/D and saying 'svn up G' and
>     being in A/D/G and saying 'svn up .' -- believe it or not, these
>     two things don't mean the same thing.  svn_path_condense_targets
>     plays with absolute paths (which is fine, so does
>     svn_path_remove_redundancies), but the difference is that it
>     actually tweaks those targets to be relative to the "grandfather
>     path" common to all the targets.  Updates don't require a
>     "grandfather path" at all, and even if it did, the whole
>     conversion to an absolute path drops the crucial difference
>     between saying "i'm in foo, update bar" and "i'm in foo/bar,
>     update '.'"
> 
I am guessing that the difference has to do with whether or not we make
a change to the parent entries file?  We shouldn't have to anyway should
we?  I thought that the entry in the parent dir just contained the name,
and all the info was in the THIS_DIR entry within the child dir.

If we do, what do we do in the case of 'svn update foo/bar
../../baz'  What do we take as the place the editor is rooted for the
second case?

> Hope this helps.
> 
> By the way, there are likely to be some changes to the way commits
> work that may require similar requirements as the update case.  If
> this happens, I will most certainly be trying to merge your function
> and mine together!
> 
Okay, sounds good.

-- 
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Kevin Pilch-Bisson                    http://www.pilch-bisson.net
     "Historically speaking, the presences of wheels in Unix
     has never precluded their reinvention." - Larry Wall
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Re: CVS update: MODIFIED: client ...

Posted by cm...@collab.net.
Kevin Pilch-Bisson <ke...@pilch-bisson.net> writes:

> What does this function offer that svn_path_condense doesn't, except for
> preserving order?  Why does order need to be preserved? Maybe
> svn_path_condense_targets should be modified instead?

Dammit, the very fact that you ask this reminds me that I forgot to
write up the explanation.

Here's the short version:

1.  Disclaimer: if you wish to debate the following, talk to Karl. :-)
    Order matters for updates because a multi-arg update is not
    atomic, and CVS users are used to, when doing 'cvs up targetA
    targetB' seeing targetA get updated, then targetB.  I think the
    idea is that if you're in a time-sensitive or flaky-network
    situation, a user can say, "I really *need* to update
    wc/A/D/G/tau, but I might as well update my whole working copy if
    I can."  So that user will do 'svn up wc/A/D/G/tau wc', and if
    something dies in the middles of the 'wc' update, at least the
    user has 'tau' up-to-date.

2.  Also, we have this notion of an anchor and a target for updates
    (the anchor is where the update editor is rooted, the target is
    the actual thing we want to update).  I needed a function that
    would NOT screw with my input paths so that I could tell the
    difference between someone being in A/D and saying 'svn up G' and
    being in A/D/G and saying 'svn up .' -- believe it or not, these
    two things don't mean the same thing.  svn_path_condense_targets
    plays with absolute paths (which is fine, so does
    svn_path_remove_redundancies), but the difference is that it
    actually tweaks those targets to be relative to the "grandfather
    path" common to all the targets.  Updates don't require a
    "grandfather path" at all, and even if it did, the whole
    conversion to an absolute path drops the crucial difference
    between saying "i'm in foo, update bar" and "i'm in foo/bar,
    update '.'"

Hope this helps.

By the way, there are likely to be some changes to the way commits
work that may require similar requirements as the update case.  If
this happens, I will most certainly be trying to merge your function
and mine together!

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