Re: svn commit: r1409883 - in /subversion/trunk/subversion: libsvn_wc/externals.c tests/cmdline/externals_tests.py

[Daniel Shahaf <d....@daniel.shahaf.name>]

neels@apache.org wrote on Thu, Nov 15, 2012 at 16:57:06 -0000:
> Author: neels
> Date: Thu Nov 15 16:57:05 2012
> New Revision: 1409883
> 
> URL: http://svn.apache.org/viewvc?rev=1409883&view=rev
> Log:
> Implement issue #4227: Externals: Multiple definitions for the same path; 
> limitation: only check for duplicates within the same svn:externals prop.
> 
> @@ -164,15 +164,16 @@ svn_wc_parse_externals_description3(apr_
>                                      apr_pool_t *pool)
>  {
>    int i;
> +  int j;
>    apr_array_header_t *externals = NULL;
>    apr_array_header_t *lines = svn_cstring_split(desc, "\n\r", TRUE, pool);
>    const char *parent_directory_display = svn_path_is_url(parent_directory) ?
>      parent_directory : svn_dirent_local_style(parent_directory, pool);
>  
>    /* If an error occurs halfway through parsing, *externals_p should stay
> -   * untouched. So, store the list in a local var first. */
> -  if (externals_p)
> -    externals = apr_array_make(pool, 1, sizeof(svn_wc_external_item2_t *));
> +   * untouched. So, store the list in a local var first. Since we are checking
> +   * for duplicate targets, always compose the list regardless. */
> +  externals = apr_array_make(pool, 1, sizeof(svn_wc_external_item2_t *));
>  
>    for (i = 0; i < lines->nelts; i++)
>      {
> @@ -330,8 +331,23 @@ svn_wc_parse_externals_description3(apr_
>              item->url = svn_dirent_canonicalize(item->url, pool);
>          }
>  
> -      if (externals)
> -        APR_ARRAY_PUSH(externals, svn_wc_external_item2_t *) = item;
> +      /* Has the same WC target path already been mentioned in this prop? */
> +      for (j = 0; j < externals->nelts; j++)
> +        {

This looks like an O(n²) algorithm, maybe implement something more
efficient?

e.g., you could construct a hash with all ->target_dir's as keys, and
then check whether (apr_hash_count() == externals->nelts).

> +          svn_wc_external_item2_t *other_item =
> +              APR_ARRAY_IDX(externals, j, svn_wc_external_item2_t *);
> +
> +          if (strcmp(item->target_dir, other_item->target_dir) == 0)
> +            return svn_error_createf
> +              (SVN_ERR_CLIENT_INVALID_EXTERNALS_DESCRIPTION, NULL,
> +               _("Invalid %s property on '%s': "
> +                 "target '%s' appears more than once"),
> +               SVN_PROP_EXTERNALS,
> +               parent_directory_display,
> +               item->target_dir);
> +        }
> +
> +      APR_ARRAY_PUSH(externals, svn_wc_external_item2_t *) = item;
>      }
>  
>    if (externals_p)
>