Re: svn commit: r1424469 - in /subversion/trunk/subversion: libsvn_client/repos_diff.c tests/cmdline/merge_reintegrate_tests.py tests/cmdline/merge_tests.py tests/cmdline/merge_tree_conflict_tests.py

2012-12-21 Thread Paul Burba
On Thu, Dec 20, 2012 at 9:01 AM,   wrote:
> Author: rhuijben
> Date: Thu Dec 20 14:01:37 2012
> New Revision: 1424469
>
> URL: http://svn.apache.org/viewvc?rev=1424469&view=rev
> Log:
> In the repository-repository diff handler: Don't retrieve pristine properties
> when we already know that there are no differences to report on them.
>
> By checking whether we really need the properties we avoid about 1000 ra calls
> over running our test suite (ra local). This also resolves many spurious merge
> changes that just change entry props.
>
> On top of that stop reporting entry prop only changes as a file/directory
> change to avoid doing unneeded work in the merge and diff handling.
>
> This fixes some issues identified when the merge code was updated to do a
> better obstruction detection, as originally we just skipped these non-changes
> there in the merge code.
>
> It is possible that this obscures some tree conflicts which were identified by
> entry prop changes that should have been detected by the real change down the
> tree (which might have been skipped).
>
> * subversion/libsvn_client/repos_diff.c
>   (dir_baton,
>file_baton): Add has_propchange boolean.
>   (close_file,
>close_directory): Only retrieve the pristine properties if we may be going
>  to use them in a callback.
>
>   (change_file_prop,
>change_dir_prop): Detect if we have a real property change and if we have
>  store that information.
>
> * subversion/tests/cmdline/merge_reintegrate_tests.py
>   (reintegrate_on_shallow_wc): Don't assume to get spurious change event.
>
> * subversion/tests/cmdline/merge_tests.py
>   (merge_to_sparse_directories): Don't expect entry prop change only skips.
>
> * subversion/tests/cmdline/merge_tree_conflict_tests.py
>   (tree_conflicts_merge_edit_onto_missing,
>tree_conflicts_merge_del_onto_missing):
> Remove some unexpected change notifications.
>
> Modified:
> subversion/trunk/subversion/libsvn_client/repos_diff.c
> subversion/trunk/subversion/tests/cmdline/merge_reintegrate_tests.py
> subversion/trunk/subversion/tests/cmdline/merge_tests.py
> subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py
>
> Modified: subversion/trunk/subversion/libsvn_client/repos_diff.c
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/repos_diff.c?rev=1424469&r1=1424468&r2=1424469&view=diff
> ==
> --- subversion/trunk/subversion/libsvn_client/repos_diff.c (original)
> +++ subversion/trunk/subversion/libsvn_client/repos_diff.c Thu Dec 20 
> 14:01:37 2012
> @@ -143,6 +143,9 @@ struct dir_baton {
>/* A cache of any property changes (svn_prop_t) received for this dir. */
>apr_array_header_t *propchanges;
>
> +  /* Boolean indicating whether a node property was changed */
> +  svn_boolean_t has_propchange;
> +
>/* The pool passed in by add_dir, open_dir, or open_root.
>   Also, the pool this dir baton is allocated in. */
>apr_pool_t *pool;
> @@ -199,6 +202,9 @@ struct file_baton {
>/* A cache of any property changes (svn_prop_t) received for this file. */
>apr_array_header_t *propchanges;
>
> +  /* Boolean indicating whether a node property was changed */
> +  svn_boolean_t has_propchange;
> +
>/* The pool passed in by add_file or open_file.
>   Also, the pool this file_baton is allocated in. */
>apr_pool_t *pool;
> @@ -971,9 +977,11 @@ close_file(void *file_baton,
>fb->path));
>  }
>
> -  if (!fb->added && fb->propchanges->nelts > 0)
> +  if (fb->path_end_revision || fb->has_propchange)
>  {
> -  if (!fb->pristine_props)
> +  const char *mimetype1, *mimetype2;
> +
> +  if (!fb->added && !fb->pristine_props)
>  {
>/* We didn't receive a text change, so we have no pristine props.
>   Retrieve just the props now. */
> @@ -981,11 +989,7 @@ close_file(void *file_baton,
>  }
>
>remove_non_prop_changes(fb->pristine_props, fb->propchanges);
> -}
>
> -  if (fb->path_end_revision || fb->propchanges->nelts > 0)
> -{
> -  const char *mimetype1, *mimetype2;
>get_file_mime_types(&mimetype1, &mimetype2, fb);
>
>
> @@ -1100,37 +1104,40 @@ close_directory(void *dir_baton,
>
>scratch_pool = db->pool;
>
> -  if (db->added)
> +  if (db->has_propchange)
>  {
> -  pristine_props = eb->empty_hash;
> -}
> -  else
> -{
> -  SVN_ERR(svn_ra_get_dir2(eb->ra_session, NULL, NULL, &pristine_props,
> -  db->path, db->base_revision, 0, scratch_pool));
> -}
> -
> -  if (db->propchanges->nelts > 0)
> -{
> -  remove_non_prop_changes(pristine_props, db->propchanges);
> -}
> +  if (db->added)
> +{
> +  pristine_props = eb->empty_hash;
> +}
> +  else
> +{
> +  SVN_ERR(svn_ra_get_dir2(eb->ra_session, NULL, NULL, 
> &pristine_

Re: svn commit: r1381800 - /subversion/trunk/subversion/libsvn_subr/io.c

2012-09-07 Thread Paul Burba
On Thu, Sep 6, 2012 at 7:32 PM,   wrote:
> Author: stefan2
> Date: Thu Sep  6 23:32:11 2012
> New Revision: 1381800
>
> URL: http://svn.apache.org/viewvc?rev=1381800&view=rev
> Log:
> Re-implement svn_io_read_length_line as this is one of the
> most-called functions in SVN. Instead of reading data a byte
> at a time, read 128 byte chunks and scan those.
>
> * subversion/libsvn_subr/io.c
>   (svn_io_read_length_line): reimplement
>
> Modified:
> subversion/trunk/subversion/libsvn_subr/io.c
>
> Modified: subversion/trunk/subversion/libsvn_subr/io.c
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1381800&r1=1381799&r2=1381800&view=diff
> ==
> --- subversion/trunk/subversion/libsvn_subr/io.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/io.c Thu Sep  6 23:32:11 2012
> @@ -3427,30 +3427,60 @@ svn_error_t *
>  svn_io_read_length_line(apr_file_t *file, char *buf, apr_size_t *limit,
>  apr_pool_t *pool)
>  {
> +  /* variables */
> +  apr_size_t total_read = 0;
> +  svn_boolean_t eof = FALSE;
>const char *name;
>svn_error_t *err;
> -  apr_size_t i;
> -  char c;
> +  apr_size_t buf_size = *limit;
>
> -  for (i = 0; i < *limit; i++)
> +  while (buf_size > 0)
>  {
> -  SVN_ERR(svn_io_file_getc(&c, file, pool));
> -  /* Note: this error could be APR_EOF, which
> - is totally fine.  The caller should be aware of
> - this. */
> -
> -  if (c == '\n')
> +  /* read a fair chunk of data at once. But don't get too ambitious
> +   * as that would result in too much waste. Also make sure we can
> +   * put a NUL after the last byte read.
> +   */
> +  apr_size_t to_read = buf_size < 129 ? buf_size - 1 : 128;
> +  apr_size_t bytes_read = 0;
> +  char *eol;
> +
> +  /* read data block (or just a part of it) */
> +  SVN_ERR(svn_io_file_read_full2(file, buf, to_read,
> + &bytes_read, &eof, pool));
> +
> +  /* look or a newline char */
> +  buf[bytes_read] = 0;
> +  eol = strchr(buf, '\n');
> +  if (eol)
>  {
> -  buf[i] = '\0';
> -  *limit = i;
> +  apr_off_t offset = (eol + 1 - buf) - bytes_read;
> +
> +  *eol = 0;
> +  *limit = total_read + (eol - buf);
> +
> +  /* correct the file pointer:
> +   * appear as though we just had read the newline char
> +   */
> +  SVN_ERR(svn_io_file_seek(file, APR_CUR, &offset, pool));
> +
>return SVN_NO_ERROR;
>  }
> -  else
> +  else if (eof)
>  {
> -  buf[i] = c;
> +  /* no EOL found but we hit the end of the file.
> +   * Generate a nice EOF error object and return it.
> +   */
> +  char dummy;
> +  SVN_ERR(svn_io_file_getc(&dummy, file, pool));
>  }
> +
> +  /* next data chunk */
> +  buf_size -= bytes_read;
> +  buf += bytes_read;
> +  total_read += bytes_read;
>  }
>
> +  /* buffer limit has been exceeded without finding the EOL */
>err = svn_io_file_name_get(&name, file, pool);
>if (err)
>  name = NULL;
>
>

Anybody else seeing this?

I haven't figured out why yet, but r1381800 is causing failures on my
Windows box, all similar to this:

C:\SVN\src-trunk>win-tests.py -d -c --test=basic#1 --log-to-stdout
--log-level=DEBUG
Testing Debug configuration on local repository.
START: basic_tests.py
I: CMD: svnadmin.exe create svn-test-work\local_tmp\repos
--bdb-txn-nosync --fs-type=fsfs
I: 
I: CMD: svn.exe import -m "Log message for revision 1."
svn-test-work\local_tmp\greekfiles
file:///C:/SVN/src-trunk/Debug/subversion/tests/cmdline/svn-test-work
/local_tmp/repos --config-dir
C:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\config
--password rayjandom --no-auth-cache --username jra
ndom
I: CMD: C:\SVN\src-trunk\Debug\subversion\svn\svn.exe import -m "Log
message for revision 1." svn-test-work\local_tmp\greekfiles
file:///C:/SVN/src-trunk/Debug/
subversion/tests/cmdline/svn-test-work/local_tmp/repos --config-dir
C:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\config
--password ra
yjandom --no-auth-cache --username jrandom exited with 1
I: 
I: ..\..\..\subversion\svn\import-cmd.c:128: (apr_err=70014)
I: ..\..\..\subversion\libsvn_client\commit.c:976: (apr_err=70014)
I: ..\..\..\subversion\libsvn_client\commit.c:712: (apr_err=70014)
I: ..\..\..\subversion\libsvn_client\commit.c:442: (apr_err=70014)
I: ..\..\..\subversion\libsvn_client\commit.c:535: (apr_err=70014)
I: ..\..\..\subversion\libsvn_repos\commit.c:348: (apr_err=70014)
I: ..\..\..\subversion\libsvn_fs\fs-loader.c:1123: (apr_err=70014)
I: ..\..\..\subversion\libsvn_fs_fs\tree.c:1827: (apr_err=70014)
I: ..\..\..\subversion\libsvn_fs_fs\tree.c:675: (apr_err=70014)
I: ..\..\..\subversion\libsvn_fs_fs\dag.c:1147: (apr_e

Re: svn commit: r1357313 - in /subversion/trunk/subversion/svn: cl.h notify.c update-cmd.c

2012-09-04 Thread Paul Burba
On Wed, Jul 4, 2012 at 11:40 AM,   wrote:
> Author: stsp
> Date: Wed Jul  4 15:40:59 2012
> New Revision: 1357313
>
> URL: http://svn.apache.org/viewvc?rev=1357313&view=rev
> Log:
> When running the conflict resolver at the end of an 'svn update' operation,
> resolve conflicts only on paths which got new conflicts flagged during the
> update operation, rather than also resolving conflicts which were left behind
> within the update targets by some other operation.
>
> * subversion/svn/cl.h
>   (svn_cl__notifier_get_conflicted_paths): Declare.
>
> * subversion/svn/notify.c
>   (notify_baton): Add conflicted_paths hash.
>   (add_conflicted_path): New helper to add a conflicted path to above hash.
>   (notify): Add any confliced paths to above hash.
>   (svn_cl__get_notifier): Initialise the conflicted_paths hash.
>   (svn_cl__notifier_get_conflicted_paths): Return a path-wise sorted array
>of confliced paths added during notification.
>
> * subversion/svn/update-cmd.c
>   (svn_cl__update): Pass the list of newly conflicted paths to the resolver,
>rather than passing the entire update target list.
>
> Modified:
> subversion/trunk/subversion/svn/cl.h
> subversion/trunk/subversion/svn/notify.c
> subversion/trunk/subversion/svn/update-cmd.c
>
> Modified: subversion/trunk/subversion/svn/cl.h
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/cl.h?rev=1357313&r1=1357312&r2=1357313&view=diff
> ==
> --- subversion/trunk/subversion/svn/cl.h (original)
> +++ subversion/trunk/subversion/svn/cl.h Wed Jul  4 15:40:59 2012
> @@ -607,6 +607,10 @@ svn_cl__notifier_mark_wc_to_repos_copy(v
>  svn_boolean_t
>  svn_cl__notifier_check_conflicts(void *baton);
>
> +/* Return a sorted array of conflicted paths detected during notification. */
> +apr_array_header_t *
> +svn_cl__notifier_get_conflicted_paths(void *baton, apr_pool_t *result_pool);
> +
>  /* Baton for use with svn_cl__check_externals_failed_notify_wrapper(). */
>  struct svn_cl__check_externals_failed_notify_baton
>  {
>
> Modified: subversion/trunk/subversion/svn/notify.c
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/notify.c?rev=1357313&r1=1357312&r2=1357313&view=diff
> ==
> --- subversion/trunk/subversion/svn/notify.c (original)
> +++ subversion/trunk/subversion/svn/notify.c Wed Jul  4 15:40:59 2012
> @@ -35,6 +35,7 @@
>  #include "svn_pools.h"
>  #include "svn_dirent_uri.h"
>  #include "svn_path.h"
> +#include "svn_sorts.h"
>  #include "cl.h"
>
>  #include "svn_private_config.h"
> @@ -57,6 +58,7 @@ struct notify_baton
>unsigned int prop_conflicts;
>unsigned int tree_conflicts;
>unsigned int skipped_paths;
> +  apr_hash_t *conflicted_paths;
>
>/* The cwd, for use in decomposing absolute paths. */
>const char *path_prefix;
> @@ -100,6 +102,16 @@ svn_cl__print_conflict_stats(void *notif
>return SVN_NO_ERROR;
>  }
>
> +/* Add a conflicted path to the list of conflicted paths stored
> + * in the notify baton. */
> +static void
> +add_conflicted_path(struct notify_baton *nb, const char *path)
> +{
> +  apr_hash_set(nb->conflicted_paths,
> +   apr_pstrdup(apr_hash_pool_get(nb->conflicted_paths), path),
> +   APR_HASH_KEY_STRING, "");
> +}
> +
>  /* This implements `svn_wc_notify_func2_t'.
>   * NOTE: This function can't fail, so we just ignore any print errors. */
>  static void
> @@ -213,6 +225,7 @@ notify(void *baton, const svn_wc_notify_
>if (n->content_state == svn_wc_notify_state_conflicted)
>  {
>nb->text_conflicts++;
> +  add_conflicted_path(nb, n->path);
>if ((err = svn_cmdline_printf(pool, "C%s\n", path_local)))
>  goto print_error;
>  }
> @@ -228,6 +241,7 @@ notify(void *baton, const svn_wc_notify_
>if (n->content_state == svn_wc_notify_state_conflicted)
>  {
>nb->text_conflicts++;
> +  add_conflicted_path(nb, n->path);
>statchar_buf[0] = 'C';
>  }
>else
> @@ -236,6 +250,7 @@ notify(void *baton, const svn_wc_notify_
>if (n->prop_state == svn_wc_notify_state_conflicted)
>  {
>nb->prop_conflicts++;
> +  add_conflicted_path(nb, n->path);
>statchar_buf[1] = 'C';
>  }
>else if (n->prop_state == svn_wc_notify_state_merged)
> @@ -302,6 +317,7 @@ notify(void *baton, const svn_wc_notify_
>  if (n->content_state == svn_wc_notify_state_conflicted)
>{
>  nb->text_conflicts++;
> +add_conflicted_path(nb, n->path);
>  statchar_buf[0] = 'C';
>}
>  else if (n->kind == svn_node_file)
> @@ -315,6 +331,7 @@ notify(void *baton, const svn_wc_notify_
>  if (n->prop_state == svn_wc_notify_state_conflicted)
>{
>  nb->prop_con

Re: svn commit: r1341848 - in /subversion/trunk/subversion: libsvn_wc/wc-queries.sql tests/libsvn_wc/wc-queries-test.c

2012-05-23 Thread Paul Burba
On Wed, May 23, 2012 at 8:53 AM,   wrote:
> Author: rhuijben
> Date: Wed May 23 12:53:10 2012
> New Revision: 1341848
>
> URL: http://svn.apache.org/viewvc?rev=1341848&view=rev
> Log:
> Help the Sqlite query planner a bit by rewriting two queries in a way that
> makes it use indexes, where it didn't before.
>
> * subversion/libsvn_wc/wc-queries.sql
>  (STMT_RECURSIVE_UPDATE_NODE_REPO,
>   STMT_SELECT_EXTERNALS_DEFINED): Make the OR operation the outer operation
>     by duplicating some cheap tests.

Hi Bert,

Could you explain in a bit more detail how/why this optimization works?

-- 
Paul T. Burba
CollabNet, Inc. -- www.collab.net -- Enterprise Cloud Development
Skype: ptburba

> * subversion/tests/libsvn_wc/wc-queries-test.c
>  (slow_statements): Remove two slow statements.
>
> Modified:
>    subversion/trunk/subversion/libsvn_wc/wc-queries.sql
>    subversion/trunk/subversion/tests/libsvn_wc/wc-queries-test.c
>
> Modified: subversion/trunk/subversion/libsvn_wc/wc-queries.sql
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc-queries.sql?rev=1341848&r1=1341847&r2=1341848&view=diff
> ==
> --- subversion/trunk/subversion/libsvn_wc/wc-queries.sql (original)
> +++ subversion/trunk/subversion/libsvn_wc/wc-queries.sql Wed May 23 12:53:10 
> 2012
> @@ -328,10 +328,15 @@ WHERE dav_cache IS NOT NULL AND wc_id =
>
>  -- STMT_RECURSIVE_UPDATE_NODE_REPO
>  UPDATE nodes SET repos_id = ?4, dav_cache = NULL
> -WHERE wc_id = ?1
> -  AND repos_id = ?3
> -  AND (local_relpath = ?2
> -       OR IS_STRICT_DESCENDANT_OF(local_relpath, ?2))
> +/* ### The Sqlite optimizer needs help here ###
> + * WHERE wc_id = ?1
> + *   AND repos_id = ?3
> + *   AND (local_relpath = ?2
> + *        OR IS_STRICT_DESCENDANT_OF(local_relpath, ?2))*/
> +WHERE (wc_id = ?1 AND local_relpath = ?2 AND repos_id = ?3)
> +   OR (wc_id = ?1 AND IS_STRICT_DESCENDANT_OF(local_relpath, ?2)
> +       AND repos_id = ?3)
> +
>
>  -- STMT_UPDATE_LOCK_REPOS_ID
>  UPDATE lock SET repos_id = ?2
> @@ -995,6 +1000,7 @@ SELECT local_relpath, kind, repos_id, de
>  FROM externals
>  LEFT OUTER JOIN repository ON repository.id = externals.repos_id
>  WHERE wc_id = ?1
> +  AND IS_STRICT_DESCENDANT_OF(local_relpath, ?2)
>   AND def_revision IS NULL
>   AND repos_id = (SELECT repos_id FROM nodes
>                   WHERE nodes.local_relpath = ?2)
> @@ -1014,9 +1020,12 @@ WHERE wc_id = ?1
>  -- STMT_SELECT_EXTERNALS_DEFINED
>  SELECT local_relpath, def_local_relpath
>  FROM externals
> -WHERE wc_id = ?1
> -  AND (def_local_relpath = ?2
> -       OR IS_STRICT_DESCENDANT_OF(def_local_relpath, ?2))
> +/* ### The Sqlite optimizer needs help here ###
> + * WHERE wc_id = ?1
> + *   AND (def_local_relpath = ?2
> + *        OR IS_STRICT_DESCENDANT_OF(def_local_relpath, ?2)) */
> +WHERE (wc_id = ?1 AND def_local_relpath = ?2)
> +   OR (wc_id = ?1 AND IS_STRICT_DESCENDANT_OF(def_local_relpath, ?2))
>
>  -- STMT_DELETE_EXTERNAL
>  DELETE FROM externals
>
> Modified: subversion/trunk/subversion/tests/libsvn_wc/wc-queries-test.c
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_wc/wc-queries-test.c?rev=1341848&r1=1341847&r2=1341848&view=diff
> ==
> --- subversion/trunk/subversion/tests/libsvn_wc/wc-queries-test.c (original)
> +++ subversion/trunk/subversion/tests/libsvn_wc/wc-queries-test.c Wed May 23 
> 12:53:10 2012
> @@ -79,7 +79,6 @@ static const int schema_statements[] =
>  static const int slow_statements[] =
>  {
>   /* Operate on the entire WC */
> -  STMT_RECURSIVE_UPDATE_NODE_REPO,
>   STMT_HAS_SWITCHED_WCROOT,
>   STMT_HAS_SWITCHED_WCROOT_REPOS_ROOT,
>   STMT_SELECT_ALL_NODES,
> @@ -93,7 +92,6 @@ static const int slow_statements[] =
>
>   /* Need review: */
>   STMT_SELECT_COMMITTABLE_EXTERNALS_BELOW,
> -  STMT_SELECT_EXTERNALS_DEFINED,
>   STMT_SELECT_EXTERNAL_PROPERTIES,
>   STMT_DELETE_ACTUAL_EMPTIES,


Re: svn commit: r1229980 - /subversion/trunk/subversion/libsvn_wc/externals.c

2012-01-11 Thread Paul Burba
On Wed, Jan 11, 2012 at 10:21 AM, Bert Huijben  wrote:
>> -Original Message-
>> From: Paul Burba [mailto:ptbu...@gmail.com]
>> Sent: woensdag 11 januari 2012 16:14
>> To: d...@subversion.apache.org; Bert Huijben
>> Cc: commits@subversion.apache.org
>> Subject: Re: svn commit: r1229980 -
>> /subversion/trunk/subversion/libsvn_wc/externals.c
>>
>> On Wed, Jan 11, 2012 at 7:29 AM,   wrote:
>> > Author: rhuijben
>> > Date: Wed Jan 11 12:29:49 2012
>> > New Revision: 1229980
>> >
>> > URL: http://svn.apache.org/viewvc?rev=1229980&view=rev
>> > Log:
>> > * subversion/libsvn_wc/externals.c
>> >  (close_file): Following up on r1229975, read the right set of properties 
>> > in
>> >    each variable. Use had_props to avoid an unneeded database transaction
>> in
>> >    some cases.
>> >
>> > Modified:
>> >    subversion/trunk/subversion/libsvn_wc/externals.c
>> >
>> > Modified: subversion/trunk/subversion/libsvn_wc/externals.c
>> > URL:
>> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/exte
>> rnals.c?rev=1229980&r1=1229979&r2=1229980&view=diff
>> >
>> ==
>> 
>> > --- subversion/trunk/subversion/libsvn_wc/externals.c (original)
>> > +++ subversion/trunk/subversion/libsvn_wc/externals.c Wed Jan 11
>> 12:29:49 2012
>> > @@ -621,11 +621,13 @@ close_file(void *file_baton,
>> >       {
>> >         new_checksum = eb->original_checksum;
>> >
>> > -        SVN_ERR(svn_wc__db_base_get_props(&actual_props, eb->db,
>> > -                                          eb->local_abspath, pool, pool));
>> > -        SVN_ERR(svn_wc__db_read_pristine_props(&base_props, eb->db,
>> > -                                               eb->local_abspath,
>> > -                                               pool, pool));
>> > +        if (eb->had_props)
>> > +          SVN_ERR(svn_wc__db_base_get_props(&base_props, eb->db,
>> > +                                            eb->local_abspath,
>> > +                                            pool, pool));
>>
>>
>> Hi Bert,
>>
>> Why did you replace svn_wc__db_read_pristine_props with
>> svn_wc__db_base_get_props here?  The log message doesn't touch upon
>> why you did.  It might be obvious to those with strong wcng-fu, but
>> I'm a bit puzzled :-)
>
> File externals are always stored in op_depth 0 (BASE) as they can't be 
> replaced/shadowed by higher level nodes.
>
> So simply to avoid checking the higher layers for no use, I used the BASE 
> specific functions. (All update/switch operations operate on op_depth 0)

Understood, thanks for the quick response.  I updated my vote for this
group in the 1.7.3 backport.

Paul

>        Bert
>>
>> Paul
>>
>>
>> > +        SVN_ERR(svn_wc__db_read_props(&actual_props, eb->db,
>> > +                                      eb->local_abspath, pool, pool));
>> >       }
>> >
>> >     if (!base_props)
>> >
>> >
>


Re: svn commit: r1229980 - /subversion/trunk/subversion/libsvn_wc/externals.c

2012-01-11 Thread Paul Burba
On Wed, Jan 11, 2012 at 7:29 AM,   wrote:
> Author: rhuijben
> Date: Wed Jan 11 12:29:49 2012
> New Revision: 1229980
>
> URL: http://svn.apache.org/viewvc?rev=1229980&view=rev
> Log:
> * subversion/libsvn_wc/externals.c
>  (close_file): Following up on r1229975, read the right set of properties in
>    each variable. Use had_props to avoid an unneeded database transaction in
>    some cases.
>
> Modified:
>    subversion/trunk/subversion/libsvn_wc/externals.c
>
> Modified: subversion/trunk/subversion/libsvn_wc/externals.c
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/externals.c?rev=1229980&r1=1229979&r2=1229980&view=diff
> ==
> --- subversion/trunk/subversion/libsvn_wc/externals.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/externals.c Wed Jan 11 12:29:49 2012
> @@ -621,11 +621,13 @@ close_file(void *file_baton,
>       {
>         new_checksum = eb->original_checksum;
>
> -        SVN_ERR(svn_wc__db_base_get_props(&actual_props, eb->db,
> -                                          eb->local_abspath, pool, pool));
> -        SVN_ERR(svn_wc__db_read_pristine_props(&base_props, eb->db,
> -                                               eb->local_abspath,
> -                                               pool, pool));
> +        if (eb->had_props)
> +          SVN_ERR(svn_wc__db_base_get_props(&base_props, eb->db,
> +                                            eb->local_abspath,
> +                                            pool, pool));


Hi Bert,

Why did you replace svn_wc__db_read_pristine_props with
svn_wc__db_base_get_props here?  The log message doesn't touch upon
why you did.  It might be obvious to those with strong wcng-fu, but
I'm a bit puzzled :-)

Paul


> +        SVN_ERR(svn_wc__db_read_props(&actual_props, eb->db,
> +                                      eb->local_abspath, pool, pool));
>       }
>
>     if (!base_props)
>
>


Re: svn commit: r1182574 - /subversion/branches/showing-merge-info/BRANCH-README

2011-11-11 Thread Paul Burba
On Wed, Oct 12, 2011 at 4:37 PM,   wrote:
> Author: julianfoad
> Date: Wed Oct 12 20:37:29 2011
> New Revision: 1182574
>
> URL: http://svn.apache.org/viewvc?rev=1182574&view=rev
> Log:
> On the 'showing-merge-info' branch: Add a BRANCH-README file describing the
> branch.
>
> Added:
>    subversion/branches/showing-merge-info/BRANCH-README
>
> Added: subversion/branches/showing-merge-info/BRANCH-README
> URL: 
> http://svn.apache.org/viewvc/subversion/branches/showing-merge-info/BRANCH-README?rev=1182574&view=auto
> ==
> --- subversion/branches/showing-merge-info/BRANCH-README (added)
> +++ subversion/branches/showing-merge-info/BRANCH-README Wed Oct 12 20:37:29 
> 2011
> @@ -0,0 +1,9 @@
> +This 'showing-merge-info' branch is a feature branch to experiment with
> +improvements to the user interface to merging.  As a 'feature' branch, it
> +is maintained by periodic 'catch-up' (aka 'sync') merges from trunk, but
> +it is not currently expected to be reintegrated to trunk as a whole.
>
> +### Many commits on this branch are intentionally experimental, unfinished,
> +### undocumented, and in other ways not of the quality expected on trunk.

Hi Julian,

I'm curious, does the above BRANCH-README still accurately describe
your intent for this branch?  That is, is this a sandbox for you to
play around with merge ideas or do you envision much of it finding its
way back to trunk?

I don't want to spend much time reviewing if the former, but don't
want to suddenly face a lot of merge churn in a short period of time
if the latter.

Paul


Re: svn commit: r1186252 - in /subversion/branches/1.7.x-r1180154: ./ subversion/include/svn_sorts.h subversion/libsvn_client/merge.c subversion/libsvn_subr/mergeinfo.c subversion/libsvn_subr/sorts.c

2011-10-19 Thread Paul Burba
On Wed, Oct 19, 2011 at 1:33 PM, Daniel Shahaf  wrote:
> Paul,
>
> pbu...@apache.org wrote on Wed, Oct 19, 2011 at 14:57:54 -:
>> Author: pburba
>> Date: Wed Oct 19 14:57:54 2011
>> New Revision: 1186252
>>
>> URL: http://svn.apache.org/viewvc?rev=1186252&view=rev
>> Log:
>> On the 1.7.x-r1180154 branch: Merge r1180154 from trunk.
>>
>> * src-branch-1.7.x
>>   Mergeinfo change only.
>>
>> * subversion/include/svn_sorts.h
>> * subversion/libsvn_client/merge.c
>> * subversion/libsvn_subr/sorts.c
>> * subversion/tests/libsvn_subr/mergeinfo-test.c
>>   Clean merge.
>>
>> * subversion/libsvn_subr/mergeinfo.c
>>   Got a lot of conflicts arising from the fact that r1149519 has not been
>>   backported, nor can it be because it revved svn_rangelist_merge to
>>   svn_rangelist_merge2 and added add a scratch pool.  It also claimed to
>>   modify the "rangelist in-place", but it didn't do all it could do on
>>   that front (hence r1180154).  Since r1180154 is effectively a full-on
>>   reimplementation of svn_rangelist_merge2 these conflicts don't really
>>   matter.  The only logical difference from r1180154 is that a local
>>   subpool is used in in place of svn_rangelist_merge2's scratch_pool.
>
> Could you clarify the last paragraph?  It definitely describes the
> history of the change, but I don't see where it answers the question
> "What happened to mergeinfo.c in r1186252?".

Hi Daniel,

It may be a moot point now, since this has already been backported,
but I adjusted the log message to better explain what I meant:
http://svn.apache.org/viewvc?view=revision&revision=1186252

Hopefully that clears things up, let me know if you still have any concerns.

Paul

> Thanks,
>
> Daniel
>


Re: svn commit: r1174517 - /subversion/branches/1.7.x/STATUS

2011-09-26 Thread Paul Burba
On Fri, Sep 23, 2011 at 1:16 AM,   wrote:
> Author: danielsh
> Date: Fri Sep 23 05:16:21 2011
> New Revision: 1174517
>
> URL: http://svn.apache.org/viewvc?rev=1174517&view=rev
> Log:
> * STATUS: Re-tweak my vote on the #4013 group.
>
> Modified:
>    subversion/branches/1.7.x/STATUS
>
> Modified: subversion/branches/1.7.x/STATUS
> URL: 
> http://svn.apache.org/viewvc/subversion/branches/1.7.x/STATUS?rev=1174517&r1=1174516&r2=1174517&view=diff
> ==
> --- subversion/branches/1.7.x/STATUS (original)
> +++ subversion/branches/1.7.x/STATUS Fri Sep 23 05:16:21 2011
> @@ -126,7 +126,10 @@ Candidate changes:
>      +1: pburba, philip
>      +0: ivan (r1173425 only, restart soak period for at least two weeks due
>                API and client/server protocol)
> -     +0: danielsh (I didn't review merge_tests.py or merge.c changes)
> +     +0: danielsh (I didn't review merge_tests.py.
> +                   All changes in merge.c look good, but I can't tell if
> +                   additional changes that should have been included are
> +                   missing, so not upgrading the vote to +1.)

Hi Daniel,

Does 
http://svn.apache.org/viewvc/subversion/branches/1.7.x/STATUS?r1=1174813&r2=1174812&pathrev=1174813
address your concerns?  Or was there something else that concerned
you?

Paul

>  * r1174111
>    Use the correct function to copy repositories in the testsuite.
>
>
>


Re: svn commit: r1127134 - in /subversion/trunk/subversion: libsvn_client/add.c libsvn_wc/adm_ops.c tests/cmdline/tree_conflict_tests.py

2011-09-06 Thread Paul Burba
On Tue, May 24, 2011 at 12:35 PM,   wrote:
> Author: stsp
> Date: Tue May 24 16:35:14 2011
> New Revision: 1127134
>
> URL: http://svn.apache.org/viewvc?rev=1127134&view=rev
> Log:
> As part of issue #3779, "actual-only nodes need regression tests",
> make 'svn add' detect tree conflict victims that do not exist on disk
> and prevent adding new nodes at that path with a meaningful error message.
>
> This implies that if users need to add a new node to resolve the conflict
> they need to mark the conflict as resolved first. I think this is safer
> than allowing accidental additions to take place. Since the node is not
> visible on disk the addition might be a mistake.
>
> * subversion/libsvn_wc/adm_ops.c
>  (check_can_add_node): Don't allow adding new items on top of nonexistent
>   conflicted nodes.
>
> * subversion/libsvn_client/add.c
>  (add): As previous.
>
> * subversion/tests/cmdline/tree_conflict_tests.py
>  (actual_only_node_behaviour): Adjust test cases for 'add' and 'mkdir'.
>
> Modified:
>    subversion/trunk/subversion/libsvn_client/add.c
>    subversion/trunk/subversion/libsvn_wc/adm_ops.c
>    subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py
>
> Modified: subversion/trunk/subversion/libsvn_client/add.c
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/add.c?rev=1127134&r1=1127133&r2=1127134&view=diff
> ==
> --- subversion/trunk/subversion/libsvn_client/add.c (original)
> +++ subversion/trunk/subversion/libsvn_client/add.c Tue May 24 16:35:14 2011
> @@ -537,10 +537,29 @@ add(void *baton, apr_pool_t *result_pool
>   else if (kind == svn_node_file)
>     err = add_file(b->local_abspath, b->ctx, scratch_pool);
>   else if (kind == svn_node_none)
> -    return svn_error_createf(SVN_ERR_WC_PATH_NOT_FOUND, NULL,
> -                             _("'%s' not found"),
> -                             svn_dirent_local_style(b->local_abspath,
> -                                                    scratch_pool));
> +    {
> +      svn_boolean_t tree_conflicted;
> +
> +      /* Provide a meaningful error message if the node does not exist
> +       * on disk but is a tree conflict victim. */
> +      err = svn_wc_conflicted_p3(NULL, NULL, &tree_conflicted,
> +                                 b->ctx->wc_ctx, b->local_abspath,
> +                                 scratch_pool);
> +      if (err)
> +        svn_error_clear(err);
> +      else if (tree_conflicted)
> +        return svn_error_createf(SVN_ERR_WC_FOUND_CONFLICT, NULL,
> +                                 _("'%s' is an existing item in conflict; "
> +                                   "please mark the conflict as resolved "
> +                                   "before adding a new item here"),
> +                                 svn_dirent_local_style(b->local_abspath,
> +                                                        scratch_pool));
> +      return svn_error_createf(SVN_ERR_WC_PATH_NOT_FOUND, NULL,
> +                               _("'%s' not found"),
> +                               svn_dirent_local_style(b->local_abspath,
> +                                                      scratch_pool));
> +    }
>   else
>     return svn_error_createf(SVN_ERR_UNSUPPORTED_FEATURE, NULL,
>                              _("Unsupported node kind for path '%s'"),
>
> Modified: subversion/trunk/subversion/libsvn_wc/adm_ops.c
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/adm_ops.c?rev=1127134&r1=1127133&r2=1127134&view=diff
> ==
> --- subversion/trunk/subversion/libsvn_wc/adm_ops.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Tue May 24 16:35:14 2011
> @@ -849,10 +849,12 @@ check_can_add_node(svn_node_kind_t *kind
>      adding the new node; if not, return an error. */
>   {
>     svn_wc__db_status_t status;
> +    svn_boolean_t conflicted;
>     svn_error_t *err
>       = svn_wc__db_read_info(&status, NULL, NULL, NULL, NULL, NULL, NULL,
>                              NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> -                             NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> +                             NULL, NULL, NULL, NULL, NULL, NULL,
> +                             &conflicted,
>                              NULL, NULL, NULL, NULL, NULL, NULL,
>                              db, local_abspath,
>                              scratch_pool, scratch_pool);
> @@ -870,6 +872,16 @@ check_can_add_node(svn_node_kind_t *kind
>       {
>         is_wc_root = FALSE;
>         exists = TRUE;
> +
> +        /* Note that the node may be in conflict even if it does not
> +         * exist on disk (certain tree conflict scenarios). */
> +        if (conflicted)
> +          return svn_error_createf(SVN_ERR_WC_FOUND_CONFLICT, NULL,
> +                                   _("'%s' is an existing item in conflict; "
> 

Re: svn commit: r1149105 - /subversion/trunk/subversion/libsvn_client/merge.c

2011-07-21 Thread Paul Burba
On Thu, Jul 21, 2011 at 6:53 AM,   wrote:
> Author: philip
> Date: Thu Jul 21 10:53:15 2011
> New Revision: 1149105
>
> URL: http://svn.apache.org/viewvc?rev=1149105&view=rev
> Log:
> Fix issue 3966, log_noop_revs in merge is far too slow.
>
> * subversion/libsvn_client/merge.c:
>  (rangelist_merge_revision): New, specialised rangelist merge.
>  (log_noop_revs): Use specialised rangelist merge.
>
> Modified:
>    subversion/trunk/subversion/libsvn_client/merge.c
>
> Modified: subversion/trunk/subversion/libsvn_client/merge.c
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1149105&r1=1149104&r2=1149105&view=diff
> ==
> --- subversion/trunk/subversion/libsvn_client/merge.c (original)
> +++ subversion/trunk/subversion/libsvn_client/merge.c Thu Jul 21 10:53:15 2011
> @@ -7868,6 +7868,36 @@ typedef struct log_noop_baton_t
>   apr_pool_t *pool;
>  } log_noop_baton_t;
>
> +/* Helper for log_noop_revs, this is not a general purpose rangelist
> +   merge.  Merge the single revision range REVISION-1 to REVISION into
> +   RANGELIST.  The existing ranges in RANGELIST must be ordered from
> +   highest/youngest to lowest/oldest.  */

Hi Philip,

The rangelist APIs expect[1] the elements to be sorted from to
lowest/oldest to highest/youngest, so you are creating an invalid
array which the svn_rangelist_APIs won't understand correctly.

In rr1149228 I switched remove_noop_subtree_ranges's call to
svn_ra_get_log2 to get the logs from oldest to youngest, so
log_noop_revs/rangelist_merge_revision get the revs in that order, can
build a valid rangelist, and can still avoid the worst-case
performance of svn_rangelist_merge when adding a single range younger
than any in the output rangelist.

Paul

[1] svn_mergeinfo.h:
 * (b) A "rangelist".  An array (@c apr_array_header_t *) of non-overlapping
 * merge ranges (@c svn_merge_range_t *), sorted as said by
 * @c svn_sort_compare_ranges().  An empty range list is represented by
 * an empty array.  Unless specifically noted otherwise, all APIs require
 * rangelists that describe only forward ranges, i.e. the range's start
 * revision is less than its end revision.


> +static svn_error_t *
> +rangelist_merge_revision(apr_array_header_t *rangelist,
> +                         svn_revnum_t revision,
> +                         apr_pool_t *result_pool)
> +{
> +  svn_merge_range_t *new_range;
> +  if (rangelist->nelts)
> +    {
> +      svn_merge_range_t *range = APR_ARRAY_IDX(rangelist, rangelist->nelts - 
> 1,
> +                                               svn_merge_range_t *);
> +      if (range->start == revision)
> +        {
> +          range->start = revision - 1;
> +          return SVN_NO_ERROR;
> +        }
> +    }
> +  new_range = apr_palloc(result_pool, sizeof(*new_range));
> +  new_range->start = revision - 1;
> +  new_range->end = revision;
> +  new_range->inheritable = TRUE;
> +
> +  APR_ARRAY_PUSH(rangelist, svn_merge_range_t *) = new_range;
> +
> +  return SVN_NO_ERROR;
> +}
> +
>  /* Implements the svn_log_entry_receiver_t interface.
>
>    BATON is an log_noop_baton_t *.
> @@ -7892,7 +7922,6 @@ log_noop_revs(void *baton,
>   svn_boolean_t log_entry_rev_required = FALSE;
>   apr_array_header_t *rl1;
>   apr_array_header_t *rl2;
> -  apr_array_header_t *rangelist;
>
>   /* The baton's pool is essentially an iterpool so we must clear it
>    * for each invocation of this function, preserving the result
> @@ -7909,12 +7938,10 @@ log_noop_revs(void *baton,
>
>   revision = log_entry->revision;
>
> -  rangelist = svn_rangelist__initialize(revision - 1, revision, TRUE,
> -                                        log_gap_baton->pool);
>   /* Unconditionally add LOG_ENTRY->REVISION to BATON->OPERATIVE_MERGES. */
> -  SVN_ERR(svn_rangelist_merge(&(log_gap_baton->operative_ranges),
> -                              rangelist,
> -                              log_gap_baton->pool));
> +  SVN_ERR(rangelist_merge_revision(log_gap_baton->operative_ranges,
> +                                   revision,
> +                                   log_gap_baton->pool));
>
>   /* Examine each path affected by LOG_ENTRY->REVISION.  If the explicit or
>      inherited mergeinfo for *all* of the corresponding paths under
> @@ -7977,6 +8004,10 @@ log_noop_revs(void *baton,
>       if (paths_explicit_rangelist)
>         {
>           apr_array_header_t *intersecting_range;
> +          apr_array_header_t *rangelist;
> +
> +          rangelist = svn_rangelist__initialize(revision - 1, revision, TRUE,
> +                                                log_gap_baton->pool);
>
>           /* If PATH inherited mergeinfo we must consider inheritance in the
>              event the inherited mergeinfo is actually non-inheritable. */
> @@ -7995,9 +8026,9 @@ log_noop_revs(void *baton,
>     }
>
>   if (!log_entry_rev_required)
> -    SVN_ERR(svn_rangelist_merge(

Re: svn commit: r1130916 - /subversion/branches/1.6.x/STATUS

2011-07-14 Thread Paul Burba
On Fri, Jun 3, 2011 at 2:48 AM,   wrote:
> Author: lgo
> Date: Fri Jun  3 06:48:20 2011
> New Revision: 1130916
>
> URL: http://svn.apache.org/viewvc?rev=1130916&view=rev
> Log:
> * STATUS: Propose r894014 and friends for backport.
>
> Modified:
>    subversion/branches/1.6.x/STATUS
>
> Modified: subversion/branches/1.6.x/STATUS
> URL: 
> http://svn.apache.org/viewvc/subversion/branches/1.6.x/STATUS?rev=1130916&r1=1130915&r2=1130916&view=diff
> ==
> --- subversion/branches/1.6.x/STATUS (original)
> +++ subversion/branches/1.6.x/STATUS Fri Jun  3 06:48:20 2011
> @@ -168,6 +168,18 @@ Veto-blocked changes:
>                    needs the same fix).
>      -1: markphip (tested multiple builds of this and never saw it work).
>
> + * r894014, 896247, 905705
> +   Use serf_connection_create2 instead of serf_connection_create so that
> +   1.6.x will be prepared to work with serf's upcoming ssl tunnel support
> +   (https over http proxy).
> +   Justification:
> +     Make use of new feature provided by the serf library, stop using
> +     an API that will be deprecated (although isn't yet).
> +   Note: this change doesn't require a change of the minimum serf 
> requirements
> +     for this branch.
> +   Votes:
> +     +1: lgo
> +

Lieven,

You put this in the 'Veto-blocked changes:' section.

Paul

>  Approved changes:
>  =
>
>
>
>


Re: svn propchange: r1141981 - svn:log

2011-07-01 Thread Paul Burba
On Fri, Jul 1, 2011 at 3:03 PM, Daniel Shahaf  wrote:
> Thanks.  (And sorry for the tone/implication in the previous email)

No offense taken Daniel, the log message was wrong after all.
Regardless, it's the Friday before a long weekend here, so not much is
going to bother me :-P

> pbu...@apache.org wrote on Fri, Jul 01, 2011 at 18:19:07 -:
>> Author: pburba
>> Revision: 1141981
>> Modified property: svn:log
>>
>> Modified: svn:log at Fri Jul  1 18:19:07 2011
>> --
>> --- svn:log (original)
>> +++ svn:log Fri Jul  1 18:19:07 2011
>> @@ -154,7 +154,7 @@ mechanism instead.
>>
>>  * subversion/mod_dav_svn/version.c
>>
>> -  (get_vsn_options): Add SVN_RA_SVN_CAP_VALIDATE_INHERITED_MERGEINFO
>> +  (get_vsn_options): Add SVN_DAV_NS_DAV_SVN_MERGEINFO_VALIDATION
>>     to reported capabilities list.
>>
>>  * subversion/svnserve/serve.c
>>
>


Re: svn commit: r1141981 - in /subversion/trunk/subversion: include/ libsvn_client/ libsvn_ra/ libsvn_ra_local/ libsvn_ra_neon/ libsvn_ra_serf/ libsvn_ra_svn/ mod_dav_svn/ mod_dav_svn/reports/ svnserv

2011-07-01 Thread Paul Burba
On Fri, Jul 1, 2011 at 1:14 PM, Daniel Shahaf  wrote:
> pbu...@apache.org wrote on Fri, Jul 01, 2011 at 16:31:22 -:
>> Author: pburba
>> Date: Fri Jul  1 16:31:21 2011
>> New Revision: 1141981
>>
>> URL: http://svn.apache.org/viewvc?rev=1141981&view=rev
>> Log:
>> Get svn_ra_get_mergeinfo2 out of the business of communicating server
>> capabilities via an in-out parameter and use our standard server capabilities
>> mechanism instead.
>>
>> * subversion/include/svn_dav.h
>>
>>   (SVN_DAV_NS_DAV_SVN_MERGEINFO_VALIDATION): New DAV header.
>>
>> * subversion/include/svn_ra.h
>>
>>   (svn_ra_get_mergeinfo2): Change the kludgey input-output parameter for
>>    inherited mergeinfo validation to an input only parameter that simply
>>    requests validation, whether or not the server can do it.
>>
>>   (SVN_RA_CAPABILITY_VALIDATE_INHERITED_MERGEINFO): New capability.
>>
>> * subversion/include/svn_ra_svn.h
>>
>>   (SVN_RA_SVN_CAP_VALIDATE_INHERITED_MERGEINFO): New ra_svn capability map
>>    to SVN_RA_CAPABILITY_VALIDATE_INHERITED_MERGEINFO.
>>
>> * subversion/libsvn_client/copy.c
>>
>>   (calculate_target_mergeinfo): Update call to
>>    svn_client__get_repos_mergeinfo().
>>
>> * subversion/libsvn_client/merge.c
>>
>>   (merge_cmd_baton_t.mergeinfo_validation_capable): New member.
>>
>>   (get_invalid_inherited_mergeinfo): Just try to get invalid mergeinfo, let
>>    the caller worry about whether the server has the capability or not.
>>
>>   (get_full_mergeinfo,
>>    inherit_implicit_mergeinfo_from_parent,
>>    ensure_implicit_mergeinfo,
>>    filter_merged_revisions,
>>    calculate_remaining_ranges): Add mergeinfo validation parameter indicating
>>    whether to validate inherited mergeinfo or not.
>>
>>   (populate_remaining_ranges,
>>    do_file_merge): Update calls to get_full_mergeinfo(),
>>    ensure_implicit_mergeinfo(), and calculate_remaining_ranges().
>>
>>   (do_merge): Initialize new merge_cmd_baton_t member.
>>
>>   (find_unmerged_mergeinfo,
>>    calculate_left_hand_side): Update call to svn_ra_get_mergeinfo2().
>>
>> * subversion/libsvn_client/mergeinfo.h
>>
>>   (svn_client__get_repos_mergeinfo,
>>    svn_client__get_repos_mergeinfo_catalog): Change the input-output
>>    parameter for inherited mergeinfo validation to an input only
>>    parameter.
>>
>> * subversion/libsvn_client/mergeinfo.c
>>
>>   (svn_client__get_repos_mergeinfo,
>>    svn_client__get_repos_mergeinfo_catalog): Account for the switch
>>    from in-out parameter to in-only parameter.
>>
>>   (svn_client__get_wc_or_repos_mergeinfo_catalog): Update call to
>>    svn_client__get_repos_mergeinfo_catalog().
>>
>>   (get_mergeinfo): Update call to svn_client__get_repos_mergeinfo_catalog().
>>
>> * subversion/libsvn_ra/deprecated.c
>>
>>   (svn_ra_get_mergeinfo): Update call to svn_ra_get_mergeinfo2().
>>
>> * subversion/libsvn_ra/ra_loader.c
>>
>>   (svn_ra_get_mergeinfo2): Account for changes to svn_ra_get_mergeinfo2().
>>
>> * subversion/libsvn_ra/ra_loader.h
>>
>>   (get_mergeinfo): Account for changes to svn_ra_get_mergeinfo2().
>>
>> * subversion/libsvn_ra_local/ra_plugin.c
>>
>>   (svn_ra_local__get_mergeinfo): Account for changes to
>>    svn_ra_get_mergeinfo2().
>>
>>   (svn_ra_local__has_capability): Check for new
>>    SVN_RA_CAPABILITY_VALIDATE_INHERITED_MERGEINFO capability.
>>
>> * subversion/libsvn_ra_neon/mergeinfo.c
>>
>>   (mergeinfo_baton): Remove validated_inherited_mergeinfo member.
>>
>>   (mergeinfo_report_elements): Remove ELEM_validate_inherited_mergeinfo
>>    element.
>>
>>   (start_element): Remove handler for ELEM_validate_inherited_mergeinfo.
>>
>>   (svn_ra_neon__get_mergeinfo): Account for changes to
>>    svn_ra_get_mergeinfo2().
>>
>> * subversion/libsvn_ra_neon/options.c
>>
>>   (parse_capabilities): Parse new
>>    SVN_RA_CAPABILITY_VALIDATE_INHERITED_MERGEINFO capability.
>>
>>   (svn_ra_neon__has_capability): Handle new
>>    SVN_RA_CAPABILITY_VALIDATE_INHERITED_MERGEINFO capability.
>>
>> * subversion/libsvn_ra_neon/ra_neon.h
>>
>>   (svn_ra_neon__get_mergeinfo): Account for changes to
>>    svn_ra_get_mergeinfo2().
>>
>> * subversion/libsvn_ra_serf/mergeinfo.c
>>
>>   (mergeinfo_state_e.MERGEINFO_VALIDATED): Remove.
>>
>>   (struct mergeinfo_context_t.validated_inherited_mergeinfo): Remove.
>>
>>   (start_element): Don't handle SVN_DAV__VALIDATE_INHERITED, no production
>>    server will ever send it.
>>
>>   (svn_ra_serf__get_mergeinfo): Account for changes to
>>    svn_ra_get_mergeinfo2().
>>
>> * subversion/libsvn_ra_serf/options.c
>>
>>   (capabilities_headers_iterator_callback): Parse new
>>    SVN_RA_CAPABILITY_VALIDATE_INHERITED_MERGEINFO capability.
>>
>>   (svn_ra_serf__has_capability): Handle new
>>    SVN_RA_CAPABILITY_VALIDATE_INHERITED_MERGEINFO capability.
>>
>>   (cdata_handler): Remove handler for defunct
>>    mergeinfo_state_e.MERGEINFO_VALIDATED.
>>
>> * subversion/libsvn_ra_serf/ra_serf.h
>>
>>   (svn_ra_serf__get_mergeinfo): Account for changes to
>>    svn_ra_get_mer

Re: svn commit: r1132561 - /subversion/trunk/subversion/tests/cmdline/copy_tests.py

2011-06-16 Thread Paul Burba
On Thu, Jun 16, 2011 at 2:47 PM, Paul Burba  wrote:
> On Mon, Jun 6, 2011 at 5:38 AM,   wrote:
>> Author: sbutler
>> Date: Mon Jun  6 09:38:30 2011
>> New Revision: 1132561
>>
>> URL: http://svn.apache.org/viewvc?rev=1132561&view=rev
>> Log:
>> Make copy_tests.py 102 XFAIL on OS X because APR doesn't support
>> case-insensitivity on Unix.
>>
>> * subversion/tests/cmdline/copy_tests.py
>>  (case_only_rename): Set XFAIL.
>>
>> Modified:
>>    subversion/trunk/subversion/tests/cmdline/copy_tests.py
>>
>> Modified: subversion/trunk/subversion/tests/cmdline/copy_tests.py
>> URL: 
>> http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/copy_tests.py?rev=1132561&r1=1132560&r2=1132561&view=diff
>> ==
>> --- subversion/trunk/subversion/tests/cmdline/copy_tests.py (original)
>> +++ subversion/trunk/subversion/tests/cmdline/copy_tests.py Mon Jun  6 
>> 09:38:30 2011
>> @@ -5176,6 +5176,8 @@ def copy_base_of_deleted(sbox):
>>  # Regression test for issue #3702: Unable to perform case-only rename
>>  # on windows.
>>  @Issue(3702)
>> +# APR's apr_filepath_merge() with APR_FILEPATH_TRUENAME is broken on OS X.
>> +@XFail(svntest.main.is_os_darwin)
>>  def case_only_rename(sbox):
>>   """case-only rename"""
>
> Hi Stephen,
>
> Since issue #3702 is fixed and this test still fails on OSX could you
> possibly and a new issue describing the problem and then associating
> this test with that issue?  I'd add one myself, but don't follow
> exactly what the problem is.
>
> No worries if you can't, I'm just trying to keep the number of failing
> tests with no associated failing issue to a minimum (preferably zero
> :-)
>
> Paul

Oh, I should add that this is the last thing standing between us and
completion of the 1.7 release task 'Test Review: Determine which XFail
and WIP tests should remain so, and which need to be fixed before
release.'  Every other xfailing test in the suite has an associated
issue with a target milestone.


Re: svn commit: r1132561 - /subversion/trunk/subversion/tests/cmdline/copy_tests.py

2011-06-16 Thread Paul Burba
On Mon, Jun 6, 2011 at 5:38 AM,   wrote:
> Author: sbutler
> Date: Mon Jun  6 09:38:30 2011
> New Revision: 1132561
>
> URL: http://svn.apache.org/viewvc?rev=1132561&view=rev
> Log:
> Make copy_tests.py 102 XFAIL on OS X because APR doesn't support
> case-insensitivity on Unix.
>
> * subversion/tests/cmdline/copy_tests.py
>  (case_only_rename): Set XFAIL.
>
> Modified:
>    subversion/trunk/subversion/tests/cmdline/copy_tests.py
>
> Modified: subversion/trunk/subversion/tests/cmdline/copy_tests.py
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/copy_tests.py?rev=1132561&r1=1132560&r2=1132561&view=diff
> ==
> --- subversion/trunk/subversion/tests/cmdline/copy_tests.py (original)
> +++ subversion/trunk/subversion/tests/cmdline/copy_tests.py Mon Jun  6 
> 09:38:30 2011
> @@ -5176,6 +5176,8 @@ def copy_base_of_deleted(sbox):
>  # Regression test for issue #3702: Unable to perform case-only rename
>  # on windows.
>  @Issue(3702)
> +# APR's apr_filepath_merge() with APR_FILEPATH_TRUENAME is broken on OS X.
> +@XFail(svntest.main.is_os_darwin)
>  def case_only_rename(sbox):
>   """case-only rename"""

Hi Stephen,

Since issue #3702 is fixed and this test still fails on OSX could you
possibly and a new issue describing the problem and then associating
this test with that issue?  I'd add one myself, but don't follow
exactly what the problem is.

No worries if you can't, I'm just trying to keep the number of failing
tests with no associated failing issue to a minimum (preferably zero
:-)

Paul


Re: svn commit: r1100321 - /subversion/trunk/subversion/libsvn_wc/cleanup.c

2011-05-06 Thread Paul Burba
On Fri, May 6, 2011 at 4:04 PM, Bert Huijben  wrote:
>
>
>> -Original Message-----
>> From: Paul Burba [mailto:ptbu...@gmail.com]
>> Sent: vrijdag 6 mei 2011 21:59
>> To: C. Michael Pilato
>> Cc: d...@subversion.apache.org; pbu...@apache.org;
>> commits@subversion.apache.org
>> Subject: Re: svn commit: r1100321 -
>> /subversion/trunk/subversion/libsvn_wc/cleanup.c
>>
>> On Fri, May 6, 2011 at 2:48 PM, C. Michael Pilato 
>> wrote:
>> > On 05/06/2011 02:43 PM, pbu...@apache.org wrote:
>> >> Author: pburba
>> >> Date: Fri May  6 18:43:55 2011
>> >> New Revision: 1100321
>> >>
>> >> URL: http://svn.apache.org/viewvc?rev=1100321&view=rev
>> >> Log:
>> >> Fix issue #3835
>> >>
>> >> * subversion/libsvn_wc/cleanup.c
>> >>
>> >>   (cleanup_internal): If the cleanup target is locked locked indirectly 
>> >> via a
>> >>    recursive lock on an ancestor, then recommend cleaning up the entire
>> WC.
>> >
>> > This is a minor nit, but is it possible to generate this wrapping error in
>> > the client itself?  I tend to think of our "suggestive error messages" as
>> > more appropriate when generated in client, which can then use more
>> explicit
>> > terminology:
>> >
>> >   Working copy locked; trying running 'svn cleanup' on the root of the
>> >   working copy (%s) instead.
>>
>> Done r1100349
>
> The client itself is 'svn', not libsvn_client :)

Ugh, that's what I get for rushing before I head out the door.  Will
fix tonight.

> TortoiseSVN has no such option as 'run svn cleanup'.
>
>        Bert
>
>


Re: svn commit: r1100321 - /subversion/trunk/subversion/libsvn_wc/cleanup.c

2011-05-06 Thread Paul Burba
On Fri, May 6, 2011 at 2:48 PM, C. Michael Pilato  wrote:
> On 05/06/2011 02:43 PM, pbu...@apache.org wrote:
>> Author: pburba
>> Date: Fri May  6 18:43:55 2011
>> New Revision: 1100321
>>
>> URL: http://svn.apache.org/viewvc?rev=1100321&view=rev
>> Log:
>> Fix issue #3835
>>
>> * subversion/libsvn_wc/cleanup.c
>>
>>   (cleanup_internal): If the cleanup target is locked locked indirectly via a
>>    recursive lock on an ancestor, then recommend cleaning up the entire WC.
>
> This is a minor nit, but is it possible to generate this wrapping error in
> the client itself?  I tend to think of our "suggestive error messages" as
> more appropriate when generated in client, which can then use more explicit
> terminology:
>
>   Working copy locked; trying running 'svn cleanup' on the root of the
>   working copy (%s) instead.

Done r1100349

Paul

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


Re: svn commit: r1096921 - in /subversion/trunk/subversion: libsvn_client/merge.c libsvn_client/repos_diff.c tests/cmdline/merge_tree_conflict_tests.py tests/cmdline/tree_conflict_tests.py

2011-04-28 Thread Paul Burba
On Tue, Apr 26, 2011 at 5:38 PM,   wrote:
> Author: rhuijben
> Date: Tue Apr 26 21:38:34 2011
> New Revision: 1096921
>
> URL: http://svn.apache.org/viewvc?rev=1096921&view=rev
> Log:
> Remove the last bit of working copy obstruction detection from the repository
> diff editor.
>
> This patch makes the merge editor aware of the list of added directories to
> allow dry run property merges to directories.
>
> * subversion/libsvn_client/merge.c
>  (merge_cmd_baton_t): Add hashtable of merge added paths.
>  (dry_run_added_p): New function.
>  (perform_obstruction_check): Specialize handling of merge-added paths.
>  (merge_dir_props_changed): Don't perform actual prop merging for added
>    directories when doing a dry-run merge.
>  (merge_file_added,
>   merge_dir_added,
>   merge_dir_deleted,
>   merge_dir_closed): Don't set optional output arguments to their default.
>
> * subversion/libsvn_client/repos_diff.c
>  (close_directory): When an obstruction is noticed, update the content state
>    and use updated notifications.
>
> * subversion/tests/cmdline/merge_tree_conflict_tests.py
>  (tree_conflicts_merge_edit_onto_missing,
>   tree_conflicts_merge_del_onto_missing): Expect more skip notifications.
>
> * subversion/tests/cmdline/tree_conflict_tests.py
>  (at_directory_external): Mark as XFail again. This change moves the failure
>    to where we step into the external for the first time. The merge code
>    detects this properly now, but wants to apply mergeinfo on the external
>    to notice that it wasn't merged. But it can't and shouldn't store this
>    information.

Fixed the failing tree_conflict test in r1097598.

Paul


Re: svn commit: r1096921 - in /subversion/trunk/subversion: libsvn_client/merge.c libsvn_client/repos_diff.c tests/cmdline/merge_tree_conflict_tests.py tests/cmdline/tree_conflict_tests.py

2011-04-28 Thread Paul Burba
On Tue, Apr 26, 2011 at 5:38 PM,   wrote:
> Author: rhuijben
> Date: Tue Apr 26 21:38:34 2011
> New Revision: 1096921
>
> URL: http://svn.apache.org/viewvc?rev=1096921&view=rev
> Log:
> Remove the last bit of working copy obstruction detection from the repository
> diff editor.
>
> This patch makes the merge editor aware of the list of added directories to
> allow dry run property merges to directories.
>
> * subversion/libsvn_client/merge.c
>  (merge_cmd_baton_t): Add hashtable of merge added paths.
>  (dry_run_added_p): New function.
>  (perform_obstruction_check): Specialize handling of merge-added paths.
>  (merge_dir_props_changed): Don't perform actual prop merging for added
>    directories when doing a dry-run merge.
>  (merge_file_added,
>   merge_dir_added,
>   merge_dir_deleted,
>   merge_dir_closed): Don't set optional output arguments to their default.
>
> * subversion/libsvn_client/repos_diff.c
>  (close_directory): When an obstruction is noticed, update the content state
>    and use updated notifications.
>
> * subversion/tests/cmdline/merge_tree_conflict_tests.py
>  (tree_conflicts_merge_edit_onto_missing,
>   tree_conflicts_merge_del_onto_missing): Expect more skip notifications.
>
> * subversion/tests/cmdline/tree_conflict_tests.py
>  (at_directory_external): Mark as XFail again. This change moves the failure
>    to where we step into the external for the first time. The merge code
>    detects this properly now, but wants to apply mergeinfo on the external
>    to notice that it wasn't merged. But it can't and shouldn't store this
>    information.
>
> Modified:
>    subversion/trunk/subversion/libsvn_client/merge.c
>    subversion/trunk/subversion/libsvn_client/repos_diff.c
>    subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py
>    subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py
>
> Modified: subversion/trunk/subversion/libsvn_client/merge.c
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1096921&r1=1096920&r2=1096921&view=diff
> ==
> --- subversion/trunk/subversion/libsvn_client/merge.c (original)
> +++ subversion/trunk/subversion/libsvn_client/merge.c Tue Apr 26 21:38:34 2011
> @@ -276,6 +276,10 @@ typedef struct merge_cmd_baton_t {
>      dry_run mode. */
>   apr_hash_t *dry_run_deletions;
>
> +  /* The list of paths for entries we've added, used only when in
> +     dry_run mode. */
> +  apr_hash_t *dry_run_added;
> +
>   /* The list of any paths which remained in conflict after a
>      resolution attempt was made.  We track this in-memory, rather
>      than just using WC entry state, since the latter doesn't help us
> @@ -353,6 +357,20 @@ dry_run_deleted_p(const merge_cmd_baton_
>                        APR_HASH_KEY_STRING) != NULL);
>  }
>
> +/* Return true iff we're in dry-run mode and WCPATH would have been
> +   added by now if we weren't in dry-run mode.
> +   Used to avoid spurious notifications (e.g. conflicts) from a merge
> +   attempt into an existing target which would have been deleted if we
> +   weren't in dry_run mode (issue #2584).  Assumes that WCPATH is
> +   still versioned (e.g. has an associated entry). */
> +static APR_INLINE svn_boolean_t
> +dry_run_added_p(const merge_cmd_baton_t *merge_b, const char *wcpath)
> +{
> +  return (merge_b->dry_run &&
> +          apr_hash_get(merge_b->dry_run_added, wcpath,
> +                       APR_HASH_KEY_STRING) != NULL);
> +}
> +
>  /* Return whether any WC path was put in conflict by the merge
>    operation corresponding to MERGE_B. */
>  static APR_INLINE svn_boolean_t
> @@ -408,20 +426,36 @@ perform_obstruction_check(svn_wc_notify_
>     *kind = svn_node_none;
>
>   /* In a dry run, make as if nodes "deleted" by the dry run appear so. */
> -  if (merge_b->dry_run && dry_run_deleted_p(merge_b, local_abspath))
> +  if (merge_b->dry_run)
>     {
> -      *obstruction_state = svn_wc_notify_state_inapplicable;
> +      if (dry_run_deleted_p(merge_b, local_abspath))
> +        {
> +          *obstruction_state = svn_wc_notify_state_inapplicable;
> +
> +          if (versioned)
> +            *versioned = TRUE;
> +          if (deleted)
> +            *deleted = TRUE;
> +
> +          if (expected_kind != svn_node_unknown
> +              && expected_kind != svn_node_none)
> +            *obstruction_state = svn_wc_notify_state_obstructed;
> +          return SVN_NO_ERROR;
> +        }
> +      else if (dry_run_added_p(merge_b, local_abspath))
> +        {
> +          *obstruction_state = svn_wc_notify_state_inapplicable;
>
> -      if (versioned)
> -        *versioned = TRUE;
> -      if (deleted)
> -        *deleted = TRUE;
> -
> -      if (expected_kind != svn_node_unknown
> -          && expected_kind != svn_node_none)
> -        *obstruction_state = svn_wc_notify_state_obstructed;
> -      return SVN_NO_ERROR;
> -    }
> +          if (versioned)
> +      

Re: svn commit: r1076726 - /subversion/trunk/subversion/tests/cmdline/log_tests.py

2011-03-08 Thread Paul Burba
On Sun, Mar 6, 2011 at 3:14 PM, Daniel Shahaf  wrote:
> pbu...@apache.org wrote on Thu, Mar 03, 2011 at 19:09:01 -:
>> Author: pburba
>> Date: Thu Mar  3 19:09:01 2011
>> New Revision: 1076726
>>
>> URL: http://svn.apache.org/viewvc?rev=1076726&view=rev
>> Log:
>> * subversion/tests/cmdline/log_tests.py
>>   (check_merge_results): Don't assume expected_reverse_merges is present, it
>>    may be None.
>>
>> Modified:
>>     subversion/trunk/subversion/tests/cmdline/log_tests.py
>>
>> Modified: subversion/trunk/subversion/tests/cmdline/log_tests.py
>> URL: 
>> http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/log_tests.py?rev=1076726&r1=1076725&r2=1076726&view=diff
>> ==
>> --- subversion/trunk/subversion/tests/cmdline/log_tests.py (original)
>> +++ subversion/trunk/subversion/tests/cmdline/log_tests.py Thu Mar  3 
>> 19:09:01 2011
>> @@ -1149,7 +1149,8 @@ def check_merge_results(log_chain, expec
>>    # Check to see if the number and values of the revisions is correct
>>    for log in log_chain:
>>      if (log['revision'] not in expected_merges
>> -        and log['revision'] not in expected_reverse_merges):
>> +        and (expected_reverse_merges is not None
>> +             and log['revision'] not in expected_reverse_merges)):
>
> I'm re-reading this and I'm still not convinced that it's correct:
> it means that if expected_reverse_merges is None, then the "Found
> unexpected revision" error will never be raised.  Is that the intended
> semantics?

Hi Daniel,

 You are quite right, that error will never be raised and we can get a
false pass if the expected revisions are not complete.  Also, this
function needs to handle the possibility of EXPECTED_MERGES being set
to none.  Fixed both of these in
http://svn.apache.org/viewvc?view=revision&revision=1079400

Thanks,

Paul


> Or did you mean this ---
>
>     if (log['revision'] not in expected_merges
>         and (expected_reverse_merges is not None
>              ? log['revision'] not in expected_reverse_merges
>              : True):
>
>>        raise SVNUnexpectedLogs("Found unexpected revision %d" %
>>                                log['revision'], log_chain)
>>
>>
>>
>


Re: [PATCH] Re: svn commit: r1071809 - in /subversion/trunk: build/run_tests.py subversion/tests/cmdline/svntest/main.py win-tests.py

2011-02-27 Thread Paul Burba
On Wed, Feb 23, 2011 at 9:59 PM, Noorul Islam K M  wrote:
> Noorul Islam K M  writes:
>
>> Paul Burba  writes:
>>
>>> If someone with the requisite linux skills/hardware could tweak
>>> makefile.in so it can take advantage of the --milestone-filter option,
>>> well that would be fabulous.
>>>
>>> Paul
>>>
>>> On Thu, Feb 17, 2011 at 5:09 PM,   wrote:
>>>> Author: pburba
>>>> Date: Thu Feb 17 22:09:02 2011
>>>> New Revision: 1071809
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1071809&view=rev
>>>> Log:
>>>> Add an option (--milestone-filter=REGEX) to the Python tests so we can 
>>>> list a
>>>> subset of the tests based on their associated issues' target milestone.
>>>>
>>>> This option is currently only available to win-tests.py and
>>>> subversion/tests/cmdline/svntest/main.py.  So it isn't quite as useful
>>>> on non-Windows platforms just yet.
>>>>
>>>> Now we can easily answer questions like, "What xfailing merge tests need to
>>>> be fixed before we can release 1.7?"
>>>>
>>>>  C:\SVN\src-trunk>win-tests.py --list --milestone-filter="(1.7.*)|(---)"
>>>>  --mode-filter xfail --log-to-stdout --test merge
>>>>  Listing Debug configuration on local repository.
>>>>  LISTING: merge_tests.py
>>>>  Test #  Mode   Test Description
>>>>  --  -  
>>>>    64    XFAIL  merge target with non inheritable mergeinfo
>>>>  [#2970(blue-sky),#3642(1.7.0)]
>>>>    75    XFAIL  merge added subtree [#1962(1.7-consider)]
>>>>
>>>> * build/run_tests.py
>>>>
>>>>  (TestHarness.__init__): Add mode_filter argument.
>>>>
>>>>  (TestHarness._run_c_test): Issue warning that --milestone-filter doesn't
>>>>   work when listing C tests.
>>>>
>>>>  (TestHarness._run_py_test): Accept --milestone-filter option.
>>>>
>>>> * subversion/tests/cmdline/svntest/main.py
>>>>
>>>>  (global): Import xml and urllib.
>>>>
>>>>  (TestSpawningThread.run_one): Support --milestone-filter option.
>>>>
>>>>  (TestRunner.list): Add optional argument mapping issues to target
>>>>   milestones.
>>>>
>>>>  (TestRunner.get_issues): New.
>>>>
>>>>  (_create_parser): Handle --milestone-filter.
>>>>
>>>>  (get_target_milestones_for_issues): New.
>>>>
>>>>  (execute_tests): Handle --milestone-filter.
>>>>
>>>> * win-tests.py
>>>>
>>>>  (_usage_exit): Add --milestone-filter to usage text.
>>>>
>>>>  (milestone_filter): New global variable.
>>>>
>>>>  (global): Accept --milestone-filter as a valid option, pass it to
>>>>   run_tests.TestHarness().
>>>>
>>>>
>>>> Modified:
>>>>    subversion/trunk/build/run_tests.py
>>>>    subversion/trunk/subversion/tests/cmdline/svntest/main.py
>>>>    subversion/trunk/win-tests.py
>>>>
>>>> Modified: subversion/trunk/build/run_tests.py
>>>> URL: 
>>>> http://svn.apache.org/viewvc/subversion/trunk/build/run_tests.py?rev=1071809&r1=1071808&r2=1071809&view=diff
>>>> ==
>>>> --- subversion/trunk/build/run_tests.py (original)
>>>> +++ subversion/trunk/build/run_tests.py Thu Feb 17 22:09:02 2011
>>>> @@ -79,7 +79,8 @@ class TestHarness:
>>>>                server_minor_version=None, verbose=None,
>>>>                cleanup=None, enable_sasl=None, parallel=None, 
>>>> config_file=None,
>>>>                fsfs_sharding=None, fsfs_packing=None,
>>>> -               list_tests=None, svn_bin=None, mode_filter=None):
>>>> +               list_tests=None, svn_bin=None, mode_filter=None,
>>>> +               milestone_filter=None):
>>>>     '''Construct a TestHarness instance.
>>>>
>>>>     ABS_SRCDIR and ABS_BUILDDIR are the source and build directories.
>>>> @@ -91,8 +92,12 @@ class TestHarness:
>>>>     HTTP_LIBRARY is the HTTP library for DAV-based communications.
>>>>     SERVER_MINOR_VERSION is the minor version of the server being tested.
>

Re: svn commit: r1071809 - in /subversion/trunk: build/run_tests.py subversion/tests/cmdline/svntest/main.py win-tests.py

2011-02-17 Thread Paul Burba
If someone with the requisite linux skills/hardware could tweak
makefile.in so it can take advantage of the --milestone-filter option,
well that would be fabulous.

Paul

On Thu, Feb 17, 2011 at 5:09 PM,   wrote:
> Author: pburba
> Date: Thu Feb 17 22:09:02 2011
> New Revision: 1071809
>
> URL: http://svn.apache.org/viewvc?rev=1071809&view=rev
> Log:
> Add an option (--milestone-filter=REGEX) to the Python tests so we can list a
> subset of the tests based on their associated issues' target milestone.
>
> This option is currently only available to win-tests.py and
> subversion/tests/cmdline/svntest/main.py.  So it isn't quite as useful
> on non-Windows platforms just yet.
>
> Now we can easily answer questions like, "What xfailing merge tests need to
> be fixed before we can release 1.7?"
>
>  C:\SVN\src-trunk>win-tests.py --list --milestone-filter="(1.7.*)|(---)"
>  --mode-filter xfail --log-to-stdout --test merge
>  Listing Debug configuration on local repository.
>  LISTING: merge_tests.py
>  Test #  Mode   Test Description
>  --  -  
>    64    XFAIL  merge target with non inheritable mergeinfo
>  [#2970(blue-sky),#3642(1.7.0)]
>    75    XFAIL  merge added subtree [#1962(1.7-consider)]
>
> * build/run_tests.py
>
>  (TestHarness.__init__): Add mode_filter argument.
>
>  (TestHarness._run_c_test): Issue warning that --milestone-filter doesn't
>   work when listing C tests.
>
>  (TestHarness._run_py_test): Accept --milestone-filter option.
>
> * subversion/tests/cmdline/svntest/main.py
>
>  (global): Import xml and urllib.
>
>  (TestSpawningThread.run_one): Support --milestone-filter option.
>
>  (TestRunner.list): Add optional argument mapping issues to target
>   milestones.
>
>  (TestRunner.get_issues): New.
>
>  (_create_parser): Handle --milestone-filter.
>
>  (get_target_milestones_for_issues): New.
>
>  (execute_tests): Handle --milestone-filter.
>
> * win-tests.py
>
>  (_usage_exit): Add --milestone-filter to usage text.
>
>  (milestone_filter): New global variable.
>
>  (global): Accept --milestone-filter as a valid option, pass it to
>   run_tests.TestHarness().
>
>
> Modified:
>    subversion/trunk/build/run_tests.py
>    subversion/trunk/subversion/tests/cmdline/svntest/main.py
>    subversion/trunk/win-tests.py
>
> Modified: subversion/trunk/build/run_tests.py
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/build/run_tests.py?rev=1071809&r1=1071808&r2=1071809&view=diff
> ==
> --- subversion/trunk/build/run_tests.py (original)
> +++ subversion/trunk/build/run_tests.py Thu Feb 17 22:09:02 2011
> @@ -79,7 +79,8 @@ class TestHarness:
>                server_minor_version=None, verbose=None,
>                cleanup=None, enable_sasl=None, parallel=None, 
> config_file=None,
>                fsfs_sharding=None, fsfs_packing=None,
> -               list_tests=None, svn_bin=None, mode_filter=None):
> +               list_tests=None, svn_bin=None, mode_filter=None,
> +               milestone_filter=None):
>     '''Construct a TestHarness instance.
>
>     ABS_SRCDIR and ABS_BUILDDIR are the source and build directories.
> @@ -91,8 +92,12 @@ class TestHarness:
>     HTTP_LIBRARY is the HTTP library for DAV-based communications.
>     SERVER_MINOR_VERSION is the minor version of the server being tested.
>     SVN_BIN is the path where the svn binaries are installed.
> -    mode_filter restricts the TestHarness to tests with the expected mode
> -    XFail, Skip, Pass, or All tests (default).
> +    MODE_FILTER restricts the TestHarness to tests with the expected mode
> +    XFail, Skip, Pass, or All tests (default).  MILESTONE_FILTER is a
> +    string representation of a valid regular expression pattern; when used
> +    in conjunction with LIST_TESTS, the only tests that are listed are
> +    those with an associated issue in the tracker which has a target
> +    milestone that matches the regex.
>     '''
>     self.srcdir = abs_srcdir
>     self.builddir = abs_builddir
> @@ -114,6 +119,7 @@ class TestHarness:
>     if config_file is not None:
>       self.config_file = os.path.abspath(config_file)
>     self.list_tests = list_tests
> +    self.milestone_filter = milestone_filter
>     self.svn_bin = svn_bin
>     self.mode_filter = mode_filter
>     self.log = None
> @@ -280,6 +286,8 @@ class TestHarness:
>     if not self.list_tests:
>       sys.stdout.write('.' * dot_count)
>       sys.stdout.flush()
> +    elif self.milestone_filter:
> +      print 'WARNING: --milestone-filter option does not currently work with 
> C tests'
>
>     if os.access(progbase, os.X_OK):
>       progname = './' + progbase
> @@ -349,6 +357,8 @@ class TestHarness:
>       svntest.main.options.server_minor_version = self.server_minor_version
>     if self.list_tests is not None:
>       svntest.main.options.list_tests = True
> +    if self.milestone_filter is not None:
> +      svntest.main.options.milestone_filter

Re: svn commit: r1036686 - /subversion/branches/1.6.x/STATUS

2010-11-19 Thread Paul Burba
On Fri, Nov 19, 2010 at 11:56 AM, Paul Burba  wrote:
> On Thu, Nov 18, 2010 at 8:12 PM, Paul Burba  wrote:
>> On Thu, Nov 18, 2010 at 7:11 PM,   wrote:
>>> Author: stsp
>>> Date: Fri Nov 19 00:11:15 2010
>>> New Revision: 1036686
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1036686&view=rev
>>> Log:
>>> * STATUS: Downgrade my vote for r1036429.
>>>
>>> Modified:
>>>    subversion/branches/1.6.x/STATUS
>>>
>>> Modified: subversion/branches/1.6.x/STATUS
>>> URL: 
>>> http://svn.apache.org/viewvc/subversion/branches/1.6.x/STATUS?rev=1036686&r1=1036685&r2=1036686&view=diff
>>> ==
>>> --- subversion/branches/1.6.x/STATUS (original)
>>> +++ subversion/branches/1.6.x/STATUS Fri Nov 19 00:11:15 2010
>>> @@ -111,7 +111,9 @@ Candidate changes:
>>>    Justification:
>>>      Avoids an assert in the server.
>>>    Votes:
>>> -     +1: philip, stsp
>>> +     +1: philip
>>> +     +0: stsp (it fixes the test, but pburba asked on IRC how this change
>>> +               will affect replaced directories and I'm not sure about 
>>> that)
>>
>> Hi Philip,
>>
>> My question was what happens if, in svnsync test 29, if in r3,
>> ^/trunk/H/B is not a replacement resulting from a copy, but is a
>> replacement without history from svn mkdir.  I momentarily thought
>> that we'd end up in replay.c:add_subdir and would hit this block:
>>
>>              if (! change->copyfrom_known)
>>                {
>>                  SVN_ERR(svn_fs_copied_from(&change->copyfrom_rev,
>>                                             &change->copyfrom_path,
>>                                             target_root, new_path, pool));
>>                  change->copyfrom_known = TRUE;
>>                }
>>
>> and call svn_fs_copied_from() for a path-rev that wasn't actually
>> copied.  But when looking closer into this that seems impossible,
>> because we'd never be in add_subdir() in the first place...I
>> *think*...this part of the code is quite unfamiliar to me :-\
>
> Scratch that, there *is* a problem:
>
> First apply this patch to break out of the svnsync tests before we
> remove the svn:sync* revprops:
>
> [[[
> Index: subversion/tests/cmdline/svnsync_tests.py
> ===
> --- subversion/tests/cmdline/svnsync_tests.py   (revision 1036857)
> +++ subversion/tests/cmdline/svnsync_tests.py   (working copy)
> @@ -159,6 +159,7 @@
>   run_sync(dest_sbox.repo_url)
>   run_copy_revprops(dest_sbox.repo_url)
>
> +  exit(0)
>   # Remove some SVNSync-specific housekeeping properties from the
>   # mirror repository in preparation for the comparison dump.
>   for prop_name in ("svn:sync-from-url", "svn:sync-from-uuid",
> ]]]
>
> ### Then run svnsync_tests.py 29 with BDB:
>
> C:\SVN\src-branch-1.6.x>run.python.test.DEBUG.bat svnsync 29 --fs-type bdb -v
> C:\SVN\src-branch-1.6.x>set TESTNAME=svnsync
> C:\SVN\src-branch-1.6.x>set CONFIG=Debug
> C:\SVN\src-branch-1.6.x>if not exist Debug\subversion\tests\cmdline
> mkdir Debug\subversion\tests\cmdline
> C:\SVN\src-branch-1.6.x>pushd Debug\subversion\tests\cmdline
> C:\SVN\src-branch-1.6.x\Debug\subversion\tests\cmdline>python
> C:\SVN\src-branch-1.6.x\\subversion\tests\cmdline\svnsync_tests.py 29
> --fs-type bdb -v
> CMD: svnadmin.exe create "svn-test-work\local_tmp\repos"
> --bdb-txn-nosync "--fs-type=bdb" 
> CMD: svn.exe import -m "Log message for revision 1."
> "svn-test-work\local_tmp\greekfiles"
> "file:///C%3A/SVN/src-branch-1.6.x/Debug/subversion/tests/cmdline/svn-
> test-work/local_tmp/repos" --config-dir
> "C:\SVN\src-branch-1.6.x\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\config"
> --password rayjandom --no-auth-c
> ache --username jrandom 
> Adding         svn-test-work\local_tmp\greekfiles\A
> Adding         svn-test-work\local_tmp\greekfiles\A\B
> Adding         svn-test-work\local_tmp\greekfiles\A\B\lambda
> Adding         svn-test-work\local_tmp\greekfiles\A\B\E
> Adding         svn-test-work\local_tmp\greekfiles\A\B\E\alpha
> Adding         svn-test-work\local_tmp\greekfiles\A\B\E\beta
> Adding         svn-test-work\local_tmp\greekfiles\A\B\F
> Adding         svn-test-work\local_tmp\greekfiles\A\mu
> Adding         svn-test-work\local_tmp\greekfiles\A\C
> Adding         svn-test-work\local_tm

Re: svn commit: r1036686 - /subversion/branches/1.6.x/STATUS

2010-11-19 Thread Paul Burba
On Thu, Nov 18, 2010 at 8:12 PM, Paul Burba  wrote:
> On Thu, Nov 18, 2010 at 7:11 PM,   wrote:
>> Author: stsp
>> Date: Fri Nov 19 00:11:15 2010
>> New Revision: 1036686
>>
>> URL: http://svn.apache.org/viewvc?rev=1036686&view=rev
>> Log:
>> * STATUS: Downgrade my vote for r1036429.
>>
>> Modified:
>>    subversion/branches/1.6.x/STATUS
>>
>> Modified: subversion/branches/1.6.x/STATUS
>> URL: 
>> http://svn.apache.org/viewvc/subversion/branches/1.6.x/STATUS?rev=1036686&r1=1036685&r2=1036686&view=diff
>> ==
>> --- subversion/branches/1.6.x/STATUS (original)
>> +++ subversion/branches/1.6.x/STATUS Fri Nov 19 00:11:15 2010
>> @@ -111,7 +111,9 @@ Candidate changes:
>>    Justification:
>>      Avoids an assert in the server.
>>    Votes:
>> -     +1: philip, stsp
>> +     +1: philip
>> +     +0: stsp (it fixes the test, but pburba asked on IRC how this change
>> +               will affect replaced directories and I'm not sure about that)
>
> Hi Philip,
>
> My question was what happens if, in svnsync test 29, if in r3,
> ^/trunk/H/B is not a replacement resulting from a copy, but is a
> replacement without history from svn mkdir.  I momentarily thought
> that we'd end up in replay.c:add_subdir and would hit this block:
>
>              if (! change->copyfrom_known)
>                {
>                  SVN_ERR(svn_fs_copied_from(&change->copyfrom_rev,
>                                             &change->copyfrom_path,
>                                             target_root, new_path, pool));
>                  change->copyfrom_known = TRUE;
>                }
>
> and call svn_fs_copied_from() for a path-rev that wasn't actually
> copied.  But when looking closer into this that seems impossible,
> because we'd never be in add_subdir() in the first place...I
> *think*...this part of the code is quite unfamiliar to me :-\

Scratch that, there *is* a problem:

First apply this patch to break out of the svnsync tests before we
remove the svn:sync* revprops:

[[[
Index: subversion/tests/cmdline/svnsync_tests.py
===
--- subversion/tests/cmdline/svnsync_tests.py   (revision 1036857)
+++ subversion/tests/cmdline/svnsync_tests.py   (working copy)
@@ -159,6 +159,7 @@
   run_sync(dest_sbox.repo_url)
   run_copy_revprops(dest_sbox.repo_url)

+  exit(0)
   # Remove some SVNSync-specific housekeeping properties from the
   # mirror repository in preparation for the comparison dump.
   for prop_name in ("svn:sync-from-url", "svn:sync-from-uuid",
]]]

### Then run svnsync_tests.py 29 with BDB:

C:\SVN\src-branch-1.6.x>run.python.test.DEBUG.bat svnsync 29 --fs-type bdb -v
C:\SVN\src-branch-1.6.x>set TESTNAME=svnsync
C:\SVN\src-branch-1.6.x>set CONFIG=Debug
C:\SVN\src-branch-1.6.x>if not exist Debug\subversion\tests\cmdline
mkdir Debug\subversion\tests\cmdline
C:\SVN\src-branch-1.6.x>pushd Debug\subversion\tests\cmdline
C:\SVN\src-branch-1.6.x\Debug\subversion\tests\cmdline>python
C:\SVN\src-branch-1.6.x\\subversion\tests\cmdline\svnsync_tests.py 29
--fs-type bdb -v
CMD: svnadmin.exe create "svn-test-work\local_tmp\repos"
--bdb-txn-nosync "--fs-type=bdb" 
CMD: svn.exe import -m "Log message for revision 1."
"svn-test-work\local_tmp\greekfiles"
"file:///C%3A/SVN/src-branch-1.6.x/Debug/subversion/tests/cmdline/svn-
test-work/local_tmp/repos" --config-dir
"C:\SVN\src-branch-1.6.x\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\config"
--password rayjandom --no-auth-c
ache --username jrandom 
Adding svn-test-work\local_tmp\greekfiles\A
Adding svn-test-work\local_tmp\greekfiles\A\B
Adding svn-test-work\local_tmp\greekfiles\A\B\lambda
Adding svn-test-work\local_tmp\greekfiles\A\B\E
Adding svn-test-work\local_tmp\greekfiles\A\B\E\alpha
Adding svn-test-work\local_tmp\greekfiles\A\B\E\beta
Adding svn-test-work\local_tmp\greekfiles\A\B\F
Adding svn-test-work\local_tmp\greekfiles\A\mu
Adding svn-test-work\local_tmp\greekfiles\A\C
Adding svn-test-work\local_tmp\greekfiles\A\D
Adding svn-test-work\local_tmp\greekfiles\A\D\gamma
Adding svn-test-work\local_tmp\greekfiles\A\D\G
Adding svn-test-work\local_tmp\greekfiles\A\D\G\pi
Adding svn-test-work\local_tmp\greekfiles\A\D\G\rho
Adding svn-test-work\local_tmp\greekfiles\A\D\G\tau
Adding svn-test-work\local_tmp\greekfiles\A\D\H
Adding svn-test-work\local_tmp\greekfiles\A\D\H\chi
Adding svn-test-work\local_tmp\greekfiles\A\D\H\omega
Adding

Re: svn commit: r1036686 - /subversion/branches/1.6.x/STATUS

2010-11-18 Thread Paul Burba
On Thu, Nov 18, 2010 at 7:11 PM,   wrote:
> Author: stsp
> Date: Fri Nov 19 00:11:15 2010
> New Revision: 1036686
>
> URL: http://svn.apache.org/viewvc?rev=1036686&view=rev
> Log:
> * STATUS: Downgrade my vote for r1036429.
>
> Modified:
>    subversion/branches/1.6.x/STATUS
>
> Modified: subversion/branches/1.6.x/STATUS
> URL: 
> http://svn.apache.org/viewvc/subversion/branches/1.6.x/STATUS?rev=1036686&r1=1036685&r2=1036686&view=diff
> ==
> --- subversion/branches/1.6.x/STATUS (original)
> +++ subversion/branches/1.6.x/STATUS Fri Nov 19 00:11:15 2010
> @@ -111,7 +111,9 @@ Candidate changes:
>    Justification:
>      Avoids an assert in the server.
>    Votes:
> -     +1: philip, stsp
> +     +1: philip
> +     +0: stsp (it fixes the test, but pburba asked on IRC how this change
> +               will affect replaced directories and I'm not sure about that)

Hi Philip,

My question was what happens if, in svnsync test 29, if in r3,
^/trunk/H/B is not a replacement resulting from a copy, but is a
replacement without history from svn mkdir.  I momentarily thought
that we'd end up in replay.c:add_subdir and would hit this block:

  if (! change->copyfrom_known)
{
  SVN_ERR(svn_fs_copied_from(&change->copyfrom_rev,
 &change->copyfrom_path,
 target_root, new_path, pool));
  change->copyfrom_known = TRUE;
}

and call svn_fs_copied_from() for a path-rev that wasn't actually
copied.  But when looking closer into this that seems impossible,
because we'd never be in add_subdir() in the first place...I
*think*...this part of the code is quite unfamiliar to me :-\

Paul

>  * r1036534
>    Allow perl bindings to compile within a symlinked working copy.
>
>
>


Re: svn commit: r1035894 - in /subversion/trunk: ./ subversion/include/ subversion/include/private/ subversion/libsvn_client/ subversion/libsvn_fs/ subversion/libsvn_fs_base/ subversion/libsvn_fs_fs/

2010-11-17 Thread Paul Burba
On Wed, Nov 17, 2010 at 5:45 AM, Julian Foad  wrote:
> Hi Paul.
>
> On Wed, 2010-11-17, pbu...@apache.org wrote:
>> Log:
>> Reintegrate the issue-3668-3669 branch.
>
> If you could insert a summary of this change here in the log message,
> that would be helpful.

Sure thing, I added a bit more explanation to the log message.  FWIW I
always figured the log messages of the feature branch served this
purpose, but your implied point is certainly valid, it is a lot more
convenient for everybody other than the branch author(s)!

For your convenience, here is the new log message:

[[[
Reintegrate the issue-3668-3669 branch.

This fixes issue #3668 'inheritance can result in self-referential mergeinfo'
and issue #3669 'inheritance can result in mergeinfo describing nonexistent
sources'.

The work for both of these issues was done in tandem for two reasons:

  First, because they are very similar, i.e. both are variations of the
  problem where merge tracking's simple inheritance model resulted in
  mergeinfo that contained invalid path-revs (either self-referential or
  simply non-existent).

  Second, the fix for issue #3668, while simple, behaves badly in cases where
  it is corrected but issue #3669 in not, see
  http://subversion.tigris.org/issues/show_bug.cgi?id=3668#desc5

The core of the issue #3669 fix is two-fold:

  First, rev svn_ra_get_mergeinfo() so it can request that the server
  validate any inherited mergeinfo it returns and second, rev
  svn_repos_fs_get_mergeinfo() so the server can actually perform this
  validation.

Note that since an RA API was revved, a lot of the noise in this change is
simply updating the plumbing of the various RA providers.  Any potential
reviewers wanting to see the heart of this change should probably start here:

  libsvn_fs/fs-loader.c:svn_fs_validate_mergeinfo(): Does the heavy
  lifting of validating mergeinfo.

  libsvn_client/merge.c:get_invalid_inherited_mergeinfo(): Indirectly uses
  the new RA API to determine what portion of WC path's inherited
  mergeinfo is invalid.

]]]

let me know if that's not what you had in mind.

> I see it's ... rather large.

As I mentioned in the new log, a lot of the code churn is RA plumbing
changes...hmmm, I guess this is exhibit 'A' as to why a more thorough
log message is helpful!

Thanks,

Paul

>> Modified:
>>     subversion/trunk/   (props changed)
>>     subversion/trunk/subversion/include/private/svn_dav_protocol.h
>>     subversion/trunk/subversion/include/svn_fs.h
>>     subversion/trunk/subversion/include/svn_ra.h
>>     subversion/trunk/subversion/include/svn_repos.h
>>     subversion/trunk/subversion/libsvn_client/copy.c
>>     subversion/trunk/subversion/libsvn_client/merge.c
>>     subversion/trunk/subversion/libsvn_client/mergeinfo.c
>>     subversion/trunk/subversion/libsvn_client/mergeinfo.h
>>     subversion/trunk/subversion/libsvn_fs/fs-loader.c
>>     subversion/trunk/subversion/libsvn_fs/fs-loader.h
>>     subversion/trunk/subversion/libsvn_fs_base/tree.c
>>     subversion/trunk/subversion/libsvn_fs_fs/tree.c
>>     subversion/trunk/subversion/libsvn_ra/deprecated.c
>>     subversion/trunk/subversion/libsvn_ra/ra_loader.c
>>     subversion/trunk/subversion/libsvn_ra/ra_loader.h
>>     subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c
>>     subversion/trunk/subversion/libsvn_ra_neon/mergeinfo.c
>>     subversion/trunk/subversion/libsvn_ra_neon/options.c
>>     subversion/trunk/subversion/libsvn_ra_neon/ra_neon.h
>>     subversion/trunk/subversion/libsvn_ra_serf/mergeinfo.c
>>     subversion/trunk/subversion/libsvn_ra_serf/options.c
>>     subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h
>>     subversion/trunk/subversion/libsvn_ra_svn/client.c
>>     subversion/trunk/subversion/libsvn_repos/fs-wrap.c
>>     subversion/trunk/subversion/mod_dav_svn/reports/mergeinfo.c
>>     subversion/trunk/subversion/svnserve/serve.c
>>     subversion/trunk/subversion/tests/cmdline/merge_reintegrate_tests.py
>>     subversion/trunk/subversion/tests/cmdline/merge_tests.py
>
>
>
>


Re: svn commit: r1002313 - /subversion/trunk/subversion/tests/cmdline/prop_tests.py

2010-09-28 Thread Paul Burba
Hi Daniel,

Comments inline.  All fixes mentioned were done in r1002372.

On Tue, Sep 28, 2010 at 3:30 PM, Daniel Shahaf  wrote:
> pbu...@apache.org wrote on Tue, Sep 28, 2010 at 18:38:19 -:
>> +  # Run propget -vR svn:mergeinfo and collect the stdout.
>> +  exit_code, pg_stdout, pg_stderr = svntest.actions.run_and_verify_svn(
>> +    None, None, [], 'propget', SVN_PROP_MERGEINFO, '-vR', wc_dir)
>> +
>
> exit_code and pg_stderr aren't checked anywhere.

Neither is pg_stdout...but that entire statement was cruft from an
earlier version of the test; removed it.

>> +  # Run propget -vR svn:mergeinfo, redirecting the stdout to a file.
>> +  arglist = ['svn.exe', 'propget', SVN_PROP_MERGEINFO, '-vR', wc_dir]
>
> s/.exe// ?

Actually that should ideally be 'svntest.main.svn_binary'.  Fixed that.

>> +  redir_file = open(redirect_file, 'wb')
>> +  pg_proc = subprocess.Popen(arglist, stdout=redir_file)
>
> Shouldn't this use the svntest/ infrastructure?  Compare
> svntest.actions.check_prop().

I didn't use it for two reasons:

First, svntest.actions.check_prop() only supports finding the props on
a single path (and as far as I can tell that works fine, no issue
#3721).

Second, Issue #3721 only occurs when the output of 'svn pg -vR' is
*redirected to a file* - see
http://subversion.tigris.org/issues/show_bug.cgi?id=3721#desc1.
check_prop() is (obviously) all processes and pipes underneath the
covers, so while it may also be possible to show the bug using it, I
wrote the test to hew as closely to the actual bug I witnessed as
possible.

>> +  pg_proc.wait()
>> +  redir_file.close()
>> +  pg_stdout_redir = open(redirect_file, 'r').readlines()
>> +
>> +  # Check if the redirected output of svn pg -vR is what we expect.
>> +  #
>> +  # Currently this fails because the mergeinfo for the three paths is
>> +  # interleaved and the lines endings are (at least on Windows) a mix
>> +  # of  and . See
>> +  # http://subversion.tigris.org/issues/show_bug.cgi?id=3721#desc1
>> +  unordered_expected_output = svntest.verify.UnorderedOutput([
>> +    "Properties on '" + B_path +  "':\n",
>> +    "Properties on '" + C_path +  "':\n",
>> +    "Properties on '" + D_path +  "':\n",
>> +    "  svn:mergeinfo\n",
>> +    "    /subversion/branches/1.5.x:872364-874936\n",
>> +    "    /subversion/branches/1.5.x-34184:874657-874741\n",
>> +    "    /subversion/branches/1.5.x-34432:874744-874798\n",
>
> So, 'unordered' also ignores repetitions?  (since the last 4 lines
> appear only once each, rather than three times each)

I think you mean the first 3 lines appear only once, all the other
lines appear 3 times each (because the test sets the same mergeinfo on
all three paths and the expected output is for svn pg -v***R***).

But yeah, this test isn't perfect as it would allow repetitions.  That
is the price we pay when using svntest.verify.UnorderedOutput(), which
is required here because there is no guarantee as to the order in
which svn pg -vR will report the three paths.  I tweaked the test to
check the expected number of lines in the actual redirected output, so
that we catch any dups.

Paul


Re: svn commit: r992042 - in /subversion/trunk/subversion: libsvn_client/merge.c tests/cmdline/merge_authz_tests.py tests/cmdline/merge_tests.py tests/cmdline/merge_tree_conflict_tests.py tests/cmdlin

2010-09-03 Thread Paul Burba
On Thu, Sep 2, 2010 at 5:52 PM, Julian Foad  wrote:
> pbu...@apache.org wrote:
>> Fix issue #2915 'Handle mergeinfo for subtrees missing due to removal by
>> non-svn command'.
>>
>> With this change, if you attempt a merge-tracking aware merge to a WC
>> which is missing subtrees due to an OS-level deletion, then an error is
>> raised before any editor drives begin.  The error message describes the
>> root of each missing path.
>
> +1 to this solution.
>
> Review comments below.
>
>> * subversion/libsvn_client/merge.c
>>
>>  (get_mergeinfo_walk_baton): Add some new members for tracking sub-
>>   directories' dirents.
>>
>>  (record_missing_subtree_roots): New function.
>>
>>  (get_mergeinfo_walk_cb): Use new function to flag missing subtree
>>   roots.
>>
>>  (get_mergeinfo_paths): Raise a SVN_ERR_CLIENT_NOT_READY_TO_MERGE error if
>>   any unexpectedly missing subtrees are found.
>>
>> * subversion/tests/cmdline/merge_authz_tests.py
>>
>>   (skipped paths get overriding mergeinfo): Don't test the file missing via
>>    OS-delete scenario, this is now covered in a much clearer manner in the
>>    new merge_tests.py.merge_with_os_deleted_subtrees test.  Also remove not
>>    about testing a missing directory, that is also covered in the new test.
>>
>> * subversion/tests/cmdline/merge_tests.py
>>
>>  (merge_into_missing): Use --ignore-ancestry during this test's merge to
>>   disregard mergeinfo and preserve the original intent of the test.
>>
>>  (skipped_files_get_correct_mergeinfo): Use a shallow WC to rather than an
>>   OS-level delete to test issue #3440.  Remove the second part of this test
>>   which has no relevance now that merge tracking doesn't tolerate subtrees
>>   missing via OS-deletion.
>>
>>  (merge_with_os_deleted_subtrees): New test.
>>
>>  (test_list): Add merge_with_os_deleted_subtrees.
>>
>> * subversion/tests/cmdline/merge_tree_conflict_tests.py
>>
>>   (tree_conflicts_merge_edit_onto_missing,
>>    tree_conflicts_merge_del_onto_missing): Use --ignore-ancestry during these
>>    tests' merges to disregard mergeinfo and preserve the original intent of
>>    the test.  Don't bother with the post-merge commit either, since there is
>>    nothing to commit as there are no mergeinfo changes.
>>
>> * subversion/tests/cmdline/svntest/actions.py
>>
>>   (deep_trees_run_tests_scheme_for_merge): Add new args allowing caller to
>>    use --ignore-ancestry during the merge and to skip the post merge commit
>>    if desired.
>
> [...]
>
>> @@ -5718,6 +5849,37 @@ get_mergeinfo_paths(apr_array_header_t *
>>                                       merge_cmd_baton->ctx->cancel_baton,
>>                                       scratch_pool));
>>
>> +  if (apr_hash_count(wb.missing_subtrees))
>> +    {
>> +      apr_hash_index_t *hi;
>> +      apr_pool_t *iterpool = svn_pool_create(scratch_pool);
>> +      const char *missing_subtree_err_str = NULL;
>> +
>> +      for (hi = apr_hash_first(iterpool, wb.missing_subtrees);

That's what I get for writing some quick-and-dirty code for testing
purposes and letting it morph in my mind to "complete" status while I
work on the "real" problem.  Fixed in
http://svn.apache.org/viewvc?view=revision&revision=992314.

> Iterpool isn't being used effectively here.  Normally, we would allocate
> the iterator in the outer pool ("hi =
> apr_hash_first(scratch_pool, ...)"), and use 'iterpool' for any
> temporary allocations within the loop and clear it each time round.  In
> this loop there aren't any, so it's redundant.
>
>> +           hi;
>> +           hi = apr_hash_next(hi))
>> +        {
>> +          const char *missing_abspath = svn__apr_hash_index_key(hi);
>> +
>> +          missing_subtree_err_str = apr_psprintf(
>> +            scratch_pool, "%s%s\n",
>> +            missing_subtree_err_str ? missing_subtree_err_str : "",
>> +            svn_dirent_local_style(missing_abspath, scratch_pool));
>
> In most realistic cases this way of constructing the string is fine, but
> it looks like it's quadratic in time and memory space which might become
> a problem when a merge is attempted into a pathological tree with
> several thousand individual missing subtree roots.
>
> Off the top of my head I would look at accumulating the string in an
> svn_stringbuf, which will expand and occasionally re-alloc as you append
> to it rather than re-allocating every time, and which remembers its
> length so appending is quick.

Ditto.

>> +        }
>> +
>> +      if (missing_subtree_err_str)
>
> Minor point: Either this "if" or the "if (apr_hash_count ...)" is
> redundant.

Gone.

> - Julian

On Fri, Sep 3, 2010 at 7:43 AM, Philip Martin
 wrote:
> pbu...@apache.org writes:
>
>> Author: pburba
>> Date: Thu Sep  2 18:10:01 2010
>> New Revision: 992042
>
>> @@ -5718,6 +5849,37 @@ get_mergeinfo_paths(apr_array_header_t *
>>   merge_cmd_baton->ctx->cancel_baton,
>>   scratch_pool));
>>
>> +  if (apr_hash_count(wb.missin

Re: svn commit: r992050 - /subversion/trunk/subversion/tests/cmdline/merge_tests.py

2010-09-02 Thread Paul Burba
On Thu, Sep 2, 2010 at 3:18 PM, Bert Huijben  wrote:
> $ python
> ActivePython 2.6.1.1 (ActiveState Software Inc.) based on
> Python 2.6.1 (r261:67515, Dec  5 2008, 13:58:38) [MSC v.1500 32 bit (Intel)] 
> on
> win32
> Type "help", "copyright", "credits" or "license" for more information.
 import os;
 print os.sep;
> \
 print os.pathsep;
> ;
 print os.path.sep;
> \
>
>
> Nice ;-)

And don't forget

>>> os.path.pathsep
';'


Re: svn commit: r982582 - /subversion/trunk/subversion/tests/libsvn_diff/parse-diff-test.c

2010-08-05 Thread Paul Burba
On Thu, Aug 5, 2010 at 8:41 AM,   wrote:
> Author: dannas
> Date: Thu Aug  5 12:41:51 2010
> New Revision: 982582
>
> URL: http://svn.apache.org/viewvc?rev=982582&view=rev
> Log:
> Adjust a C-unit test for parsing git diffs to have " b/" as part of
> the paths.
>
> The diff-parser searches for " b/" to find the start of old_path.
>
> * subversion/tests/libsvn_diff/parse-diff-tests.c
>  (git_diff_with_spaces_diff,
>   test_git_diffs_with_spaces_diff): See above.
>
> Modified:
>    subversion/trunk/subversion/tests/libsvn_diff/parse-diff-test.c
>
> Modified: subversion/trunk/subversion/tests/libsvn_diff/parse-diff-test.c
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_diff/parse-diff-test.c?rev=982582&r1=982581&r2=982582&view=diff
> ==
> --- subversion/trunk/subversion/tests/libsvn_diff/parse-diff-test.c (original)
> +++ subversion/trunk/subversion/tests/libsvn_diff/parse-diff-test.c Thu Aug  
> 5 12:41:51 2010
> @@ -220,9 +220,9 @@ static const char *bad_git_diff_header =
>   "new file mode 100644"                                                NL
>   "git --diff a/path one 1 b/path one 1"                                NL
>   "new file mode 100644"                                                NL
> -  "git --diff a/dir/b/path b/dir/b/path"                                NL
> +  "git --diff a/dir/ b/path b/dir/ b/path"                              NL
>   "new file mode 100644"                                                NL
> -  "git --diff a/b/path 1 b/b/path 1"                                    NL
> +  "git --diff a/ b/path 1 b/ b/path 1"                                  NL
>   "new file mode 100644"                                                NL;
>
>
> @@ -834,8 +834,9 @@ test_git_diffs_with_spaces_diff(apr_pool
>                                     FALSE, /* ignore_whitespace */
>                                     pool, pool));
>   SVN_TEST_ASSERT(patch);
> -  SVN_TEST_ASSERT(! strcmp(patch->old_filename, "dir/b/path"));
> -  SVN_TEST_ASSERT(! strcmp(patch->new_filename, "dir/b/path"));
> +  SVN_DBG(("%s\n", patch->old_filename));
   
Was this intentional?  If so, we need to conditionally exclude it so
we can build release configurations:

  #ifdef SVN_DEBUG
SVN_DBG(("%s\n", patch->old_filename));
  #endif

Paul


Re: svn commit: r952981 - /subversion/branches/1.6.x-issue3651/subversion/svn/copy-cmd.c

2010-06-09 Thread Paul Burba
On Wed, Jun 9, 2010 at 8:48 AM,   wrote:

> Author: stylesen
> Date: Wed Jun  9 12:48:05 2010
> New Revision: 952981
>
> URL: http://svn.apache.org/viewvc?rev=952981&view=rev
> Log:
> On the '1.6.x-issue3651' branch:
>
> Merge from trunk r952973 and use the private API to eat peg revision,
> since this API was made public in trunk recently.
>
> Modified:
>    subversion/branches/1.6.x-issue3651/subversion/svn/copy-cmd.c
>
> Modified: subversion/branches/1.6.x-issue3651/subversion/svn/copy-cmd.c
> URL: 
> http://svn.apache.org/viewvc/subversion/branches/1.6.x-issue3651/subversion/svn/copy-cmd.c?rev=952981&r1=952980&r2=952981&view=diff
> ==

Hi Senthil,

We want to add  #include "private/svn_opt_private.h" to avoid compiler
warnings about an undefined function yes?

Paul


> --- subversion/branches/1.6.x-issue3651/subversion/svn/copy-cmd.c (original)
> +++ subversion/branches/1.6.x-issue3651/subversion/svn/copy-cmd.c Wed Jun  9 
> 12:48:05 2010
> @@ -72,6 +72,8 @@ svn_cl__copy(apr_getopt_t *os,
>       APR_ARRAY_PUSH(sources, svn_client_copy_source_t *) = source;
>     }
>
> +  SVN_ERR(svn_opt__eat_peg_revisions(&targets, targets, pool));
> +
>   /* Figure out which type of trace editor to use.
>      If the src_paths are not homogeneous, setup_copy will return an error. */
>   src_path = APR_ARRAY_IDX(targets, 0, const char *);
>
>
>


Re: svn commit: r952439 - /subversion/branches/1.6.x/STATUS

2010-06-08 Thread Paul Burba
On Mon, Jun 7, 2010 at 5:10 PM,   wrote:
> Author: rhuijben
> Date: Mon Jun  7 21:10:22 2010
> New Revision: 952439
>
> URL: http://svn.apache.org/viewvc?rev=952439&view=rev
> Log:
> * STATUS: Cast some votes
>
> Modified:
>    subversion/branches/1.6.x/STATUS
>
> Modified: subversion/branches/1.6.x/STATUS
> URL: 
> http://svn.apache.org/viewvc/subversion/branches/1.6.x/STATUS?rev=952439&r1=952438&r2=952439&view=diff
> ==
> --- subversion/branches/1.6.x/STATUS (original)
> +++ subversion/branches/1.6.x/STATUS Mon Jun  7 21:10:22 2010
> @@ -137,6 +137,8 @@ Candidate changes:
>      Sanity checks should themselves be sane.
>    Votes:
>      +1: cmpilato
> +     -0: This introduces new issues like #3242 and might break reintegrate

   ^^^
 Needs your name :-)

> +         where it previously worked. (Untested)

Hi Bert,

I don't see how we would now provoke any issue #3242 problems.  We
still open the RA session to WC_REPOS_ROOT (i.e. the reintegrate
target), the same as we did prior to r952439.

All r952439 really does is fix the sanity check that the reintegrate
target and source are from the same repos; previously it always
compared the target root repos to itself!

If you are seeing something I am not please let me know.

Thanks,

Paul


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

2010-06-08 Thread Paul Burba
On Mon, Jun 7, 2010 at 8:47 PM,   wrote:
> Author: gstein
> Date: Tue Jun  8 00:47:22 2010
> New Revision: 952493
>
> URL: http://svn.apache.org/viewvc?rev=952493&view=rev
> Log:
> The query that we used to fetch all children in BASE_NODE and WORKING_NODE
> used a UNION between two SELECT statements. The idea was to have SQLite
> remove all duplicates for us in a single query. Unfortunately, this caused
> SQLite to create an ephemeral (temporary) table and place the results of
> each query into that table. It created an index to remove dupliates. Then
> it returned the values in that ephemeral table. For large numbers of
> nodes, the construction of the table and its index becomes very costly.
>
> This change rebuilds gather_children() in wc_db.c to do the duplicate
> removal manually using a hash table. It does some simple scanning straight
> into an array when it knows duplicates cannot exist (one of BASE or
> WORKING is empty).
>
> The performance problem of svn_wc__db_read_children() was first observed
> in issue #3499. The actual performance improvement is untested so far, but
> I'm assuming pburba can pick up this change and try in his scenario.

On Mon, Jun 7, 2010 at 8:53 PM, Greg Stein  wrote:
> Hey Paul,
>
> Can you try this change on your large-file-count working copies? I
> believe this should fix the performance problems you were seeing.

Greg,

Short Story: Hours to Seconds

Long Story: This does indeed solve the problems I was seeing:

My test repository was our test suite's Greek tree but with 17,000 1KB
files in a single directory:

Prior to r952493, update and status were taking *quite* some time:

  svn st  01:23:33
  svn up  Gave up after an hour (i.e. lasted longer than my lunch).

With your fix in place, performance improves dramatically:

  svn st  00:00:17
  svn up  00:00:11

Paul

P.S. Thanks!  I was nowhere near figuring this out :-\


Re: svn commit: r920424 - in /subversion/trunk/subversion: include/private/svn_wc_private.h libsvn_wc/lock.c libsvn_wc/util.c libsvn_wc/wc.h

2010-06-02 Thread Paul Burba
On Mon, Mar 8, 2010 at 2:05 PM,   wrote:
> Author: philip
> Date: Mon Mar  8 18:05:19 2010
> New Revision: 920424
>
> URL: http://svn.apache.org/viewvc?rev=920424&view=rev
> Log:
> Remove some svn_wc_entry_t using code.
>
> * subversion/include/private/svn_wc_private.h
>  (svn_wc__path_switched): Remove SVN_ERR_ENTRY_MISSING_URL from doc
>   string, the caller wasn't taking advantage of it.
>  (svn_wc__adm_open_anchor_in_context): Delete.
>
> * subversion/libsvn_wc/wc.h
>  (svn_wc__internal_path_switched): Delete.
>
> * subversion/libsvn_wc/util.c
>  (svn_wc__internal_path_switched): Delete.
>  (svn_wc__path_switched): Move to lock.c.
>
> * subversion/libsvn_wc/lock.c
>  (child_is_disjoint): Check for SVN_ERR_WC_NOT_DIRECTORY.
>  (svn_wc__adm_open_anchor_in_context): Delete.
>  (svn_wc__path_switched): Copied from util.c, just call child_is_disjoint.

Hi Philip,

This implies that a disjoint child is equivalent to a switched child,
but despite child_is_disjoint's lack of documentation, it seems clear
that a child can be disjoint but not switched.  Am I missing something
here?

Paul

> Modified:
>    subversion/trunk/subversion/include/private/svn_wc_private.h
>    subversion/trunk/subversion/libsvn_wc/lock.c
>    subversion/trunk/subversion/libsvn_wc/util.c
>    subversion/trunk/subversion/libsvn_wc/wc.h
>
> Modified: subversion/trunk/subversion/include/private/svn_wc_private.h
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_wc_private.h?rev=920424&r1=920423&r2=920424&view=diff
> ==
> --- subversion/trunk/subversion/include/private/svn_wc_private.h (original)
> +++ subversion/trunk/subversion/include/private/svn_wc_private.h Mon Mar  8 
> 18:05:19 2010
> @@ -77,9 +77,7 @@
>
>  /** Given a @a local_abspath with a @a wc_ctx, set @a *switched to
>  * TRUE if @a local_abspath is switched, otherwise set @a *switched to FALSE.
> - * If neither @a local_abspath or its parent have valid URLs, return
> - * @c SVN_ERR_ENTRY_MISSING_URL.  All temporaryallocations are done in
> - * @a scratch_pool.
> + * All temporary allocations are done in * @a scratch_pool.
>  */
>  svn_error_t *
>  svn_wc__path_switched(svn_boolean_t *switched,
> @@ -250,22 +248,6 @@
>                              void *cancel_baton,
>                              apr_pool_t *pool);
>
> -/** Like svn_wc_adm_open_anchor(), but with a svn_wc_context_t * to use
> - * when opening the access batons.
> - *
> - * NOT FOR NEW DEVELOPMENT!  (See note to svn_wc__adm_open_in_context().)
> - */
> -svn_error_t *
> -svn_wc__adm_open_anchor_in_context(svn_wc_adm_access_t **anchor_access,
> -                                   svn_wc_adm_access_t **target_access,
> -                                   const char **target,
> -                                   svn_wc_context_t *wc_ctx,
> -                                   const char *path,
> -                                   svn_boolean_t write_lock,
> -                                   int levels_to_lock,
> -                                   svn_cancel_func_t cancel_func,
> -                                   void *cancel_baton,
> -                                   apr_pool_t *pool);
>
>
>  /**
>
> Modified: subversion/trunk/subversion/libsvn_wc/lock.c
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/lock.c?rev=920424&r1=920423&r2=920424&view=diff
> ==
> --- subversion/trunk/subversion/libsvn_wc/lock.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/lock.c Mon Mar  8 18:05:19 2010
> @@ -1045,7 +1045,7 @@
>   err = svn_wc__db_read_children(&children, db, parent_abspath, scratch_pool,
>                                  scratch_pool);
>
> -  if (err && err->apr_err == SVN_ERR_WC_PATH_NOT_FOUND)
> +  if (err && err->apr_err == SVN_ERR_WC_NOT_DIRECTORY)
>     {
>       svn_error_clear(err);
>       *disjoint = TRUE;
> @@ -1329,23 +1329,6 @@
>                                       cancel_baton, pool));
>  }
>
> -svn_error_t *
> -svn_wc__adm_open_anchor_in_context(svn_wc_adm_access_t **anchor_access,
> -                                   svn_wc_adm_access_t **target_access,
> -                                   const char **target,
> -                                   svn_wc_context_t *wc_ctx,
> -                                   const char *path,
> -                                   svn_boolean_t write_lock,
> -                                   int levels_to_lock,
> -                                   svn_cancel_func_t cancel_func,
> -                                   void *cancel_baton,
> -                                   apr_pool_t *pool)
> -{
> -  return svn_error_return(open_anchor(anchor_access, target_access, target,
> -                                      wc_ctx->db, TRUE, path, write_lock,
> -                                      levels_to_lock, cancel_func,
> -               

Re: svn commit: r937524 - in /subversion/trunk/subversion: libsvn_wc/props.c tests/cmdline/prop_tests.py tests/cmdline/svntest/sandbox.py

2010-05-21 Thread Paul Burba
On Thu, May 20, 2010 at 4:59 PM, Greg Stein  wrote:
> Thanks for the ping.
>
> The patch looks good except for the incoming-delete case.

Hi Greg,

Which flavor of that case? Incoming delete on a local delete of the
same property with the same value?  Or something else?

> If the
> svn_string_compare() succeeds, but mine==NULL, then you get the crash.
> I think the mine==NULL needs to remain on the outer-if test.

I'm not entirely sure what you are referring to here.  Is it this
section of generate_conflict_message()?

[[[
  if (svn_string_compare(original, incoming_base))
{
  if (mine)
/* We were trying to delete the correct property, but an edit
   caused the conflict.  */
return svn_string_createf(result_pool,
  _("Trying to delete property '%s' with "
"value '%s',\nbut it has been modified "
"from '%s' to '%s'."),
  propname, incoming_base->data,
  original->data, mine->data);
}
  else if (mine == NULL)
{
  /* We were trying to delete the property, but we have locally
 deleted the same property, but with a different value. */
  return svn_string_createf(result_pool,
_("Trying to delete property '%s' with "
  "value '%s',\nbut property with value "
  "'%s' is locally deleted."),
propname, incoming_base->data,
original->data);
}

  /* We were trying to delete INCOMING_BASE but our ORIGINAL is
 something else entirely.  */
  SVN_ERR_ASSERT_NO_RETURN(!svn_string_compare(original, incoming_base));
]]]

If (ORIGINAL == INCOMING_BASE) && (MINE == INCOMING == NULL) then
we'll trigger that SVN_ERR_ASSERT_NO_RETURN.  But we shouldn't be
calling this function in the first place for this case, because the
function assumes there *is* a prop conflict of some kind.  It always
produces a conflict message or asserts trying.

At any rate, I'm a bit confused here.

Paul

> Other than that... looks great. Commit!
>
> Cheers,
> -g
>
> On Thu, May 20, 2010 at 15:26, Paul Burba  wrote:
>> Hi Greg,
>>
>> If you have a chance, let me know if you were planning on giving any
>> feedback on this.  Just want to be sure I answered your questions
>> before committing.
>>
>> Thanks,
>>
>> Paul
>>
>> On Fri, May 14, 2010 at 1:46 PM, Paul Burba  wrote:
>>> On Fri, Apr 23, 2010 at 5:22 PM,   wrote:
>>>> Author: gstein
>>>> Date: Fri Apr 23 21:22:52 2010
>>>> New Revision: 937524
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=937524&view=rev
>>>> Log:
>>>> Begin new infrastructure for generating prop conflict messages. This will
>>>> allow us to (re)generate a property reject file at will, given a record of
>>>> the property conflicts on a given node.
>>>>
>>>> There are two issues for discussion and fixing in a future revision:
>>>> - incoming-delete will remove local-add (it should conflict?)
>>>
>>> Hi Greg,
>>>
>>> I think the correct behavior is: An incoming-delete removes a local
>>> add only if the incoming base value is the *same* as the added value;
>>> otherwise there is a conflict.  This is analogous to how we treat an
>>> incoming file deletion on a local file addition.  It's only a tree
>>> conflict if the files differ.
>>>
>>> More below...
>>>
>>>> - incoming-delete will crash on a local-delete
>>>>
>>>> * subversion/libsvn_wc/props.c:
>>>>  (generate_conflict_message): new function to generate a property
>>>>    conflict message given the four property values involved in a 4-way
>>>>    merge.
>>>>  (apply_single_prop_delete): leave two notes about behavior in here (see
>>>>    the issues above). fix message generation: use OLD_VAL, not BASE_VAL
>>>>  (apply_single_generic_prop_change): the OLD_VAL parameter will always be
>>>>    not-NULL, so we can simplify an if condition.
>>>>  (svn_wc__merge_props): save away MINE_VAL, and then if we see a conflict
>>>>    message returned by the property merging functions, then assert that
>>>>    our new 

Re: svn commit: r937524 - in /subversion/trunk/subversion: libsvn_wc/props.c tests/cmdline/prop_tests.py tests/cmdline/svntest/sandbox.py

2010-05-20 Thread Paul Burba
Hi Greg,

If you have a chance, let me know if you were planning on giving any
feedback on this.  Just want to be sure I answered your questions
before committing.

Thanks,

Paul

On Fri, May 14, 2010 at 1:46 PM, Paul Burba  wrote:
> On Fri, Apr 23, 2010 at 5:22 PM,   wrote:
>> Author: gstein
>> Date: Fri Apr 23 21:22:52 2010
>> New Revision: 937524
>>
>> URL: http://svn.apache.org/viewvc?rev=937524&view=rev
>> Log:
>> Begin new infrastructure for generating prop conflict messages. This will
>> allow us to (re)generate a property reject file at will, given a record of
>> the property conflicts on a given node.
>>
>> There are two issues for discussion and fixing in a future revision:
>> - incoming-delete will remove local-add (it should conflict?)
>
> Hi Greg,
>
> I think the correct behavior is: An incoming-delete removes a local
> add only if the incoming base value is the *same* as the added value;
> otherwise there is a conflict.  This is analogous to how we treat an
> incoming file deletion on a local file addition.  It's only a tree
> conflict if the files differ.
>
> More below...
>
>> - incoming-delete will crash on a local-delete
>>
>> * subversion/libsvn_wc/props.c:
>>  (generate_conflict_message): new function to generate a property
>>    conflict message given the four property values involved in a 4-way
>>    merge.
>>  (apply_single_prop_delete): leave two notes about behavior in here (see
>>    the issues above). fix message generation: use OLD_VAL, not BASE_VAL
>>  (apply_single_generic_prop_change): the OLD_VAL parameter will always be
>>    not-NULL, so we can simplify an if condition.
>>  (svn_wc__merge_props): save away MINE_VAL, and then if we see a conflict
>>    message returned by the property merging functions, then assert that
>>    our new function comes up with the same message
>>
>> * subversion/tests/cmdline/prop_tests.py:
>>  (prop_reject_grind): new test function to grind thru all the variations
>>    of property conflicts.
>>  (test_list): add new test
>>
>> * subversion/tests/cmdline/svntest/sandbox.py:
>>  (Sandbox.simple_propset, Sandbox.simple_propdel): new methods
>>
>> Modified:
>>    subversion/trunk/subversion/libsvn_wc/props.c
>>    subversion/trunk/subversion/tests/cmdline/prop_tests.py
>>    subversion/trunk/subversion/tests/cmdline/svntest/sandbox.py
>>
>> Modified: subversion/trunk/subversion/libsvn_wc/props.c
>> URL: 
>> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/props.c?rev=937524&r1=937523&r2=937524&view=diff
>> ==
>> --- subversion/trunk/subversion/libsvn_wc/props.c (original)
>> +++ subversion/trunk/subversion/libsvn_wc/props.c Fri Apr 23 21:22:52 2010
>> @@ -709,6 +709,136 @@ svn_wc_merge_props3(svn_wc_notify_state_
>>  }
>>
>>
>> +/* Generate a message to describe the property conflict among these four
>> +   values.
>> +
>> +   Note that this function (currently) interprets the property values as
>> +   strings, but they could actually be binary values. We'll keep the
>> +   types as svn_string_t in case we fix this in the future.  */
>> +static const svn_string_t *
>> +generate_conflict_message(const char *propname,
>> +                          const svn_string_t *original,
>> +                          const svn_string_t *mine,
>> +                          const svn_string_t *incoming,
>> +                          const svn_string_t *incoming_base,
>> +                          apr_pool_t *result_pool)
>> +{
>> +  if (incoming_base == NULL)
>> +    {
>> +      /* Attempting to add the value INCOMING.  */
>> +      SVN_ERR_ASSERT_NO_RETURN(incoming != NULL);
>> +
>> +      if (mine)
>> +        {
>> +          /* To have a conflict, these must be different.  */
>> +          SVN_ERR_ASSERT_NO_RETURN(!svn_string_compare(mine, incoming));
>> +
>> +          /* Note that we don't care whether MINE is locally-added or
>> +             edited, or just something different that is a copy of the
>> +             pristine ORIGINAL.  */
>> +          return svn_string_createf(result_pool,
>> +                                    _("Trying to add new property '%s' with 
>> "
>> +                                      "value '%s',\nbut property already "
>> +                                      "exists with value '%s'."),
>> +                              

Re: svn commit: r937524 - in /subversion/trunk/subversion: libsvn_wc/props.c tests/cmdline/prop_tests.py tests/cmdline/svntest/sandbox.py

2010-05-14 Thread Paul Burba
On Fri, Apr 23, 2010 at 5:22 PM,   wrote:
> Author: gstein
> Date: Fri Apr 23 21:22:52 2010
> New Revision: 937524
>
> URL: http://svn.apache.org/viewvc?rev=937524&view=rev
> Log:
> Begin new infrastructure for generating prop conflict messages. This will
> allow us to (re)generate a property reject file at will, given a record of
> the property conflicts on a given node.
>
> There are two issues for discussion and fixing in a future revision:
> - incoming-delete will remove local-add (it should conflict?)

Hi Greg,

I think the correct behavior is: An incoming-delete removes a local
add only if the incoming base value is the *same* as the added value;
otherwise there is a conflict.  This is analogous to how we treat an
incoming file deletion on a local file addition.  It's only a tree
conflict if the files differ.

More below...

> - incoming-delete will crash on a local-delete
>
> * subversion/libsvn_wc/props.c:
>  (generate_conflict_message): new function to generate a property
>    conflict message given the four property values involved in a 4-way
>    merge.
>  (apply_single_prop_delete): leave two notes about behavior in here (see
>    the issues above). fix message generation: use OLD_VAL, not BASE_VAL
>  (apply_single_generic_prop_change): the OLD_VAL parameter will always be
>    not-NULL, so we can simplify an if condition.
>  (svn_wc__merge_props): save away MINE_VAL, and then if we see a conflict
>    message returned by the property merging functions, then assert that
>    our new function comes up with the same message
>
> * subversion/tests/cmdline/prop_tests.py:
>  (prop_reject_grind): new test function to grind thru all the variations
>    of property conflicts.
>  (test_list): add new test
>
> * subversion/tests/cmdline/svntest/sandbox.py:
>  (Sandbox.simple_propset, Sandbox.simple_propdel): new methods
>
> Modified:
>    subversion/trunk/subversion/libsvn_wc/props.c
>    subversion/trunk/subversion/tests/cmdline/prop_tests.py
>    subversion/trunk/subversion/tests/cmdline/svntest/sandbox.py
>
> Modified: subversion/trunk/subversion/libsvn_wc/props.c
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/props.c?rev=937524&r1=937523&r2=937524&view=diff
> ==
> --- subversion/trunk/subversion/libsvn_wc/props.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/props.c Fri Apr 23 21:22:52 2010
> @@ -709,6 +709,136 @@ svn_wc_merge_props3(svn_wc_notify_state_
>  }
>
>
> +/* Generate a message to describe the property conflict among these four
> +   values.
> +
> +   Note that this function (currently) interprets the property values as
> +   strings, but they could actually be binary values. We'll keep the
> +   types as svn_string_t in case we fix this in the future.  */
> +static const svn_string_t *
> +generate_conflict_message(const char *propname,
> +                          const svn_string_t *original,
> +                          const svn_string_t *mine,
> +                          const svn_string_t *incoming,
> +                          const svn_string_t *incoming_base,
> +                          apr_pool_t *result_pool)
> +{
> +  if (incoming_base == NULL)
> +    {
> +      /* Attempting to add the value INCOMING.  */
> +      SVN_ERR_ASSERT_NO_RETURN(incoming != NULL);
> +
> +      if (mine)
> +        {
> +          /* To have a conflict, these must be different.  */
> +          SVN_ERR_ASSERT_NO_RETURN(!svn_string_compare(mine, incoming));
> +
> +          /* Note that we don't care whether MINE is locally-added or
> +             edited, or just something different that is a copy of the
> +             pristine ORIGINAL.  */
> +          return svn_string_createf(result_pool,
> +                                    _("Trying to add new property '%s' with "
> +                                      "value '%s',\nbut property already "
> +                                      "exists with value '%s'."),
> +                                    propname, incoming->data, mine->data);
> +        }
> +
> +      /* To have a conflict, we must have an ORIGINAL which has been
> +         locally-deleted.  */
> +      SVN_ERR_ASSERT_NO_RETURN(original != NULL);
> +      return svn_string_createf(result_pool,
> +                                _("Trying to create property '%s' with "
> +                                  "value '%s',\nbut it has been locally "
> +                                  "deleted."),
> +                                propname, incoming->data);
> +    }
> +
> +  if (incoming == NULL)
> +    {
> +      /* Attempting to delete the value INCOMING_BASE.  */
> +      SVN_ERR_ASSERT_NO_RETURN(incoming_base != NULL);
> +
> +      /* A conflict can only occur if we originally had the property;
> +         otherwise, we would have merged the property-delete into the
> +         non-existent property.  */
> +      SVN_ERR_ASSERT_NO_RETURN(original != NULL);
> +
> +      

Re: svn commit: r937033 - /subversion/trunk/subversion/libsvn_repos/dump.c

2010-04-23 Thread Paul Burba
On Fri, Apr 23, 2010 at 4:14 AM, Bert Huijben  wrote:
>> -Original Message-
>> From: pbu...@apache.org [mailto:pbu...@apache.org]
>> Sent: donderdag 22 april 2010 21:40
>> To: commits@subversion.apache.org
>> Subject: svn commit: r937033 -
>> /subversion/trunk/subversion/libsvn_repos/dump.c
>>
>> Author: pburba
>> Date: Thu Apr 22 19:40:07 2010
>> New Revision: 937033
>>
>> URL: http://svn.apache.org/viewvc?rev=937033&view=rev
>> Log:
>> Issue an end-of-dump summary warning to supplement any in-line
>> copy-source warnings so the latter don't get missed in a sea of output.
>>
>> See http://svn.haxx.se/dev/archive-2010-04/0475.shtml
>>
>> * subversion/libsvn_repos/dump.c
>>
>>   (edit_baton): Add new member found_old_reference.
>>
>>   (dump_node): Set new edit baton member if an old reference warning is
>>    issued.
>>
>>   (svn_repos_dump_fs3): Possibly issue new summary warning.
>>
>> Modified:
>>     subversion/trunk/subversion/libsvn_repos/dump.c
>>
>> Modified: subversion/trunk/subversion/libsvn_repos/dump.c
>> URL:
>> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/d
>> ump.c?rev=937033&r1=937032&r2=937033&view=diff
>> ==
>> 
>> --- subversion/trunk/subversion/libsvn_repos/dump.c (original)
>> +++ subversion/trunk/subversion/libsvn_repos/dump.c Thu Apr 22 19:40:07
>> 2010
>> @@ -196,6 +196,10 @@ struct edit_baton
>>    /* The first revision dumped in this dumpstream. */
>>    svn_revnum_t oldest_dumped_rev;
>>
>> +  /* Set to true if any references to revisions older than
>> +     OLDEST_DUMPED_REV were found in the dumpstream. */
>> +  svn_boolean_t found_old_reference;
>> +
>>    /* reusable buffer for writing file contents */
>>    char buffer[SVN__STREAM_CHUNK_SIZE];
>>    apr_size_t bufsize;
>> @@ -428,7 +432,7 @@ dump_node(struct edit_baton *eb,
>>                         " into an empty repository\n"
>>                         "WARNING: will fail.\n"),
>>                       cmp_rev, eb->oldest_dumped_rev);
>> -
>> +              eb->found_old_reference = TRUE;
>>                SVN_ERR(eb->progress_func(eb->progress_baton,
>>                                          eb->oldest_dumped_rev, warning, 
>> pool));
>>              }
>> @@ -982,6 +986,7 @@ svn_repos_dump_fs3(svn_repos_t *repos,
>>    svn_revnum_t youngest;
>>    const char *uuid;
>>    int version;
>> +  svn_boolean_t found_old_reference = FALSE;
>>
>>    /* Determine the current youngest revision of the filesystem. */
>>    SVN_ERR(svn_fs_youngest_rev(&youngest, fs, pool));
>> @@ -1109,6 +1114,23 @@ svn_repos_dump_fs3(svn_repos_t *repos,
>>      loop_end:
>>        if (progress_func)
>>          SVN_ERR(progress_func(progress_baton, to_rev, NULL, subpool));
>> +
>> +      if (((struct edit_baton *)dump_edit_baton)->found_old_reference)
>> +        found_old_reference = TRUE;
>> +    }
>> +
>> +  /* Did we issue any warnings about references to revisions older than
>> +     the oldest dumped revision?  If so, then issue a final generic
>> +     warning, since the inline warnings already issued might easily be
>> +     missed. */
>> +  if (found_old_reference)
>> +    {
>> +      const char *warning = apr_psprintf(
>> +        subpool,
>> +        _("WARNING: The range of revisions dumped contained references
>> to\n"
>> +          "WARNING: copy sources outside that range.\n"));
>> +      SVN_ERR(progress_func(progress_baton, SVN_INVALID_REVNUM,
>> warning,
>> +                            subpool));
>>      }
>
> This block needs a test for progress_func being NULL. (And doesn't need the 
> apr_psprintf, but Greg already noticed that)

Thanks gents, fixed both.  Also tweaked svn_repos_dump_fs3's doc
string to note that progress_func may be null.

Paul


Re: svn commit: r931449 - in /subversion/trunk/subversion: libsvn_wc/entries.c tests/cmdline/switch_tests.py tests/cmdline/update_tests.py

2010-04-07 Thread Paul Burba
On Wed, Apr 7, 2010 at 3:05 AM,   wrote:
> Author: gstein
> Date: Wed Apr  7 07:05:28 2010
> New Revision: 931449
>
> URL: http://svn.apache.org/viewvc?rev=931449&view=rev
> Log:
> Fix the inheritance handling for entry->copyfrom_*. The mapping code was
> not properly considering the copyfrom data between "this" node and the
> ancestor nodes.
>
> This fixes merge_tests 34 and 134 (broken by r930162), but proceeds to
> break diff_tests 41 and merge_tests 8. Two steps forward...
>
> * subversion/libsvn_wc/entries.c:
>  (read_entries_new): rework the logic that analyzes copyfrom information
>    on "this" node and the ancestor nodes. it was not (properly)
>    considering that an ancestor may imply "this" node is part of a copy.
>
> * subversion/tests/cmdline/switch_tests.py:
>  (tree_conflicts_on_switch_2_1): note that we fail because an (A)dd is
>    incorrectly reported as a (M)odify, due to issues in wc_db.
>  (test_list): mark the above as an XFail
>
> * subversion/tests/cmdline/update_tests.py:
>  (tree_conflicts_on_update_2_1, tree_conflict_uc2_schedule_re_add): leave
>    comments about the breakage due to wc_db issues.
>  (test_list): mark the above two tests as XFail
>
> Modified:
>    subversion/trunk/subversion/libsvn_wc/entries.c
>    subversion/trunk/subversion/tests/cmdline/switch_tests.py
>    subversion/trunk/subversion/tests/cmdline/update_tests.py
>
> Modified: subversion/trunk/subversion/libsvn_wc/entries.c
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/entries.c?rev=931449&r1=931448&r2=931449&view=diff
> ==
> --- subversion/trunk/subversion/libsvn_wc/entries.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/entries.c Wed Apr  7 07:05:28 2010
> @@ -727,6 +727,8 @@ read_entries_new(apr_hash_t **result_ent
>                || status == svn_wc__db_status_obstructed_add)
>         {
>           svn_wc__db_status_t work_status;
> +          const char *op_root_abspath;
> +          const char *scanned_original_relpath;
>           svn_revnum_t original_revision;
>
>           /* For child nodes, pick up the parent's revision.  */
> @@ -817,17 +819,19 @@ read_entries_new(apr_hash_t **result_ent
>             }
>
>           SVN_ERR(svn_wc__db_scan_addition(&work_status,
> -                                           NULL,
> +                                           &op_root_abspath,
>                                            &repos_relpath,
>                                            &entry->repos,
>                                            &entry->uuid,
> -                                           NULL, NULL, NULL, 
> &original_revision,
> +                                           &scanned_original_relpath,
> +                                           NULL, NULL, /* original_root|uuid 
> */
> +                                           &original_revision,
>                                            db,
>                                            entry_abspath,
>                                            result_pool, iterpool));
> -
> +          SVN_DBG(("entry: work_status=%d  cmt_rev=%ld  relpath='%s'\n", 
> work_status, entry->cmt_rev, original_repos_relpath));
 ^
Hi Greg - looks like you left a scalpel in the patient.

Paul


Re: svn commit: r892189 - in /subversion/branches/1.6.x-r891672/subversion: libsvn_client/commit_util.c tests/cmdline/externals_tests.py

2010-01-25 Thread Paul Burba
On Fri, Dec 18, 2009 at 4:16 AM,   wrote:
> Author: stylesen
> Date: Fri Dec 18 09:16:18 2009
> New Revision: 892189
>
> URL: http://svn.apache.org/viewvc?rev=892189&view=rev
> Log:
> Merge r891672 from ^/subversion/trunk.
>
> Modify subversion/tests/cmdline/externals_tests.py to correct some
> latest changes on this test in trunk (r876917).
>
> Modified:
>    subversion/branches/1.6.x-r891672/subversion/libsvn_client/commit_util.c
>    
> subversion/branches/1.6.x-r891672/subversion/tests/cmdline/externals_tests.py

What client version of svn did you perform this merge with?  I'm
curious as to why that no mergeinfo was recorded.  Did you
--ignore-ancestry?  Or revert the svn:mergeinfo property changes?  Or
just not commit them?  Or something else?Paul

> Modified: 
> subversion/branches/1.6.x-r891672/subversion/libsvn_client/commit_util.c


Re: svn commit: r896522 - /subversion/trunk/subversion/tests/cmdline/resolve_tests.py

2010-01-06 Thread Paul Burba
On Wed, Jan 6, 2010 at 12:01 PM, Hyrum K. Wright
 wrote:
>
> On Jan 6, 2010, at 10:48 AM, pbu...@apache.org wrote:
>
>> Author: pburba
>> Date: Wed Jan  6 16:48:38 2010
>> New Revision: 896522
>>
>> URL: http://svn.apache.org/viewvc?rev=896522&view=rev
>> Log:
>> New test for 'svn resolve -R --accept [base | theirs-full | mine-full]'.
>>
>> This works fine on trunk but is currently failing on 1.6.x and the only
>> existing test coverage of this is in the Ruby bindings tests. See
>> http://svn.haxx.se/dev/archive-2010-01/0088.shtml
>>
>> * subversion/tests/cmdline/resolve_tests.py: New Python test file.  There are
>>  some 'svn resolve' tests scattered throughout other tests, but a subcommand
>>  deserves its own home.  Heck, maybe this will encourage some actual
>>  coverage!
>
> Should existing tests which focus on 'svn resolve' (if there be any) be moved 
> to this file?

Hi Hyrum,

I think so yes, it's on my own TODO list (albeit with very low
priority).  If anybody else wants to take a stab at it please do (/me
doesn't hold his breath :-)

Paul


Re: svn commit: r895653 - /subversion/trunk/subversion/bindings/swig/python/tests/mergeinfo.py

2010-01-04 Thread Paul Burba
On Mon, Jan 4, 2010 at 10:00 AM,   wrote:
> Author: hwright
> Date: Mon Jan  4 15:00:14 2010
> New Revision: 895653
>
> URL: http://svn.apache.org/viewvc?rev=895653&view=rev
> Log:
> Update a test expectation, as a result of recent mergeinfo sanitation.
>
> * subversion/bindings/swig/python/tests/mergeinfo.py
>  (test_mergeinfo_get): Expect to receive valid mergeinfo.
>
> Modified:
>    subversion/trunk/subversion/bindings/swig/python/tests/mergeinfo.py
>
> Modified: subversion/trunk/subversion/bindings/swig/python/tests/mergeinfo.py
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/swig/python/tests/mergeinfo.py?rev=895653&r1=895652&r2=895653&view=diff
> ==
> --- subversion/trunk/subversion/bindings/swig/python/tests/mergeinfo.py 
> (original)
> +++ subversion/trunk/subversion/bindings/swig/python/tests/mergeinfo.py Mon 
> Jan  4 15:00:14 2010
> @@ -129,10 +129,10 @@
>                                        False, None, None)
>     expected_mergeinfo = \
>       { '/trunk' :
> -          { 'branches/a' : [RevRange(2, 11)],
> -            'branches/b' : [RevRange(9, 13)],
> -            'branches/c' : [RevRange(2, 16)],
> -            'trunk'      : [RevRange(1, 9)],  },
> +          { '/branches/a' : [RevRange(2, 11)],
> +            '/branches/b' : [RevRange(9, 13)],
> +            '/branches/c' : [RevRange(2, 16)],
> +            '/trunk'      : [RevRange(1, 9)],  },
>       }
>     self.compare_mergeinfo_catalogs(mergeinfo, expected_mergeinfo)
>
>
>
>

Just making a record of something that puzzled me about the test
failure fixed by this commit...

The "mergeinfo sanitation" referred to in the log message is r888979
"Fix issue #3547 - 'svnadmin load --parent-dir PATH' can destroy
mergeinfo", rather than r889840 "When producing svn_mergeinfo_t from
string representations or vice-versa, tolerate relative merge source
paths, but convert such paths to absolute paths in the resulting
string/mergeinfo."

When I saw mention of this failing test on 1.6.x
(http://svn.haxx.se/dev/archive-2009-12/0508.shtml) I thought for sure
it was related to r889840, but the test still failed prior to that
change being backported on 1.6.x.

The test actually started failing with r888979 because the test repos
was loaded with an empty PARENT_DIR argument to svn_repos_load_fs2:

  def setUp(self):
"""Load the mergeinfo-full Subversion repository.  This dumpfile is
   created by dumping the repository generated for command line log
   tests 16.  If it needs to be updated (mergeinfo format changes, for
   example), we can go there to get a new version."""
dumpfile = open(os.path.join(os.path.split(__file__)[0],
 'data', 'mergeinfo.dump'))
# Remove any existing repository to ensure a fresh start
self.tearDown()
self.repos = repos.svn_repos_create(REPOS_PATH, '', '', None, None)
repos.svn_repos_load_fs2(self.repos, dumpfile, StringIO(),
 repos.svn_repos_load_uuid_ignore, '',
   ^^^

Thus issue #3547 manifested itself and the loaded repos had relative
merge source paths.  For some reason the test was incorrectly
expecting this corrupted mergeinfo, and prior to r888979 that is
exactly what it got.

Paul


Re: svn commit: r39163 - in branches/1.6.x-r38000: . build/ac-macros subversion/libsvn_client subversion/libsvn_wc subversion/mod_dav_svn subversion/tests/cmdline

2009-12-16 Thread Paul Burba
On Mon, Sep 7, 2009 at 9:07 AM, Stephen Butler  wrote:
> Author: sbutler
> Date: Mon Sep  7 07:07:09 2009
> New Revision: 39163
>
> Log:
> On ^/branches/1.6.x-r38000: bring up-to-date with ^/branches/1.6.x.
>
> Modified:
>   branches/1.6.x-r38000/   (props changed)
>   branches/1.6.x-r38000/CHANGES   (props changed)

Stephen,

Do you recall what client version you used to do this merge?

It created some strange mergeinfo on CHANGES

C:\SVN\src-branch-1.6.6>svn diff
https://svn.collab.net/repos/svn/branches/1.6.x-r38000/chan...@39163
-r39162:39163

Property changes on: CHANGES
___
Modified: svn:mergeinfo
   Merged /branches/1.6.x-r38927/CHANGES:r38928-39102
   Merged /branches/1.6.x/CHANGES:r39009-39162
   Merged /trunk/CHANGES:r2-1281,38872,38875,38881,38927,39031,39074
 ^^^
  These revs predate the earliest history of
   CHANGES, which was created in r1282.

I'm trying to reconstruct what happened here because this looks to be
causing us problems now when reintegrating other branches to 1.6.x
(http://mail-archives.apache.org/mod_mbox/subversion-commits/200912.mbox/browser).

Paul


Re: svn commit: r40041 - branches/1.6.x

2009-12-14 Thread Paul Burba
On Wed, Oct 14, 2009 at 8:23 PM, Julian Foad  wrote:
> Author: julianfoad
> Date: Wed Oct 14 18:23:05 2009
> New Revision: 40041
>
> Log:
> * STATUS: Update my review status on r39019.
>
> Modified:
>   branches/1.6.x/STATUS
>
> Modified: branches/1.6.x/STATUS
> URL: 
> http://svn.collab.net/viewvc/svn/branches/1.6.x/STATUS?pathrev=40041&r1=40040&r2=40041
> ==
> --- branches/1.6.x/STATUS       Wed Oct 14 13:36:15 2009        (r40040)
> +++ branches/1.6.x/STATUS       Wed Oct 14 18:23:05 2009        (r40041)
> @@ -114,8 +114,7 @@ Candidate changes:
>      ^/branches/1.6.x-r39109
>    Votes:
>      +1: pburba
> -     -0: julianfoad (tried to review but got stuck trying to understand what
> -           combine_with_lastrange() is meant to do,
> +     -0: julianfoad (reviewed the changes in mergeinfo.c only;
>            and also it seems to sneak in another bug fix in
>            fix_deleted_subtree_ranges(). I suggest splitting up this patch.)

Hi Julian,

Here is why that change is included:

svn_rangelist_* APIs have always required forward rangelists:

 * (b) A "rangelist".  An array (@c apr_array_header_t *) of non-overlapping
 * merge ranges (@c svn_merge_range_t *), sorted as said by
 * @c svn_sort_compare_ranges().  An empty range list is represented by
 * an empty array.  Unless specifically noted otherwise, all APIs require
 * rangelists that describe only forward ranges, i.e. the range's start
 * revision is less than its end revision.

Prior to r879093(r40041)
libsvn_client/merge.c:fix_deleted_subtree_ranges() called
svn_rangelist_diff() with reversed ranges.  We simply lucked out that
this has never caused any *known* problems.  AFAICT this abuse of the
svn_rangelist_diff API in fix_deleted_subtree_ranges() could never
cause an incorrect merge/error/assert in practice(1) prior to
r879093...

...But r879093 added the helper function
libsvn_subr/mergeinfo.c:get_type_of_intersection() which is ultimately
used by all the svn_rangelist_* and svn_mergeinfo_* APIs and is
written assuming only forward merges are passed in; the function
asserts if reverse ranges are passed to it.  So if I were to create a
backport branch of r879093 minus the change to
fix_deleted_subtree_ranges(), this assert would get triggered in
previously working merges.  In our test suite this can be seen in
merge_tests.py 65 'child having different rev ranges to merge'.  I'm
pretty certain nobody would approve a backport which breaks the test
suite.

I endeavor to keep each commit a logically discrete change, but in
this case I think the fix of fix_deleted_subtree_ranges()'s API abuse
is so interdependent with the core fix of r879093 that it made little
sense to commit them separately.  Even if I had, they would both be
nominated as a group for backport.

Paul

(1) I hesitate to get into to much detail why this is (it hardly seems
relevant), but briefly this seems never to have bitten us due to the
call to filter_merged_revisions() immediately preceding
svn_rangelist_diff() in
fix_deleted_subtree_ranges(calculate_remaining_ranges on the 1.6.x
branch).  This removes any requested reverse merges which are not
represented in explicit/implicit mergeinfo.  In combination with the
temporary inheritable mergeinfo created by a merge under any paths
with non-inheritable mergeinfo in the merge target, I can't find any
combination or merges that would create a situation where a reverse
merge would break, though this is quite dependent on the current
implementation of get_type_of_intersection().  Assuming there were no
asserts, an equally valid implementation of get_type_of_intersection
could easily produce incorrect merges.


Re: svn commit: r886164 - in /subversion/trunk/subversion: svn/main.c tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout

2009-12-02 Thread Paul Burba
On Wed, Dec 2, 2009 at 9:52 AM,   wrote:
> Author: cmpilato
> Date: Wed Dec  2 14:52:49 2009
> New Revision: 886164
>
> URL: http://svn.apache.org/viewvc?rev=886164&view=rev
> Log:
> Fixup a blatant, flagrant, evil lie of the Devil found in the usage
> messages of 'svn update' and 'svn switch'.
>
> * subversion/svn/main.c
>  (svn_cl__cmd_table): Stop claiming that --set-depth can't make
>    directories more shallow, because since 1.6 was released, it
>    certainly can.

Hey Mike,

You only updated the help text for switch, update still has the
incorrect message.

Paul

> * subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
>  Update expected output from 'svn help switch'.
>
> Modified:
>    subversion/trunk/subversion/svn/main.c
>    
> subversion/trunk/subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
>
> Modified: subversion/trunk/subversion/svn/main.c
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/main.c?rev=886164&r1=886163&r2=886164&view=diff
> ==
> --- subversion/trunk/subversion/svn/main.c (original)
> +++ subversion/trunk/subversion/svn/main.c Wed Dec  2 14:52:49 2009
> @@ -1013,9 +1013,7 @@
>      "     are applied to the obstructing path.\n"
>      "\n"
>      "     Use the --set-depth option to set a new working copy depth on 
> the\n"
> -     "     targets of this operation.  Currently, the depth of a working 
> copy\n"
> -     "     directory can only be increased (telescoped more deeply); you 
> cannot\n"
> -     "     make a directory more shallow.\n"
> +     "     targets of this operation.\n"
>      "\n"
>      "  2. Rewrite working copy URL metadata to reflect a syntactic change 
> only.\n"
>      "     This is used when repository's root URL changes (such as a 
> scheme\n"
>
> Modified: 
> subversion/trunk/subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout?rev=886164&r1=886163&r2=886164&view=diff
> ==
> --- 
> subversion/trunk/subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
>  (original)
> +++ 
> subversion/trunk/subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
>  Wed Dec  2 14:52:49 2009
> @@ -88,9 +88,7 @@
>      are applied to the obstructing path.
>
>      Use the --set-depth option to set a new working copy depth on the
> -     targets of this operation.  Currently, the depth of a working copy
> -     directory can only be increased (telescoped more deeply); you cannot
> -     make a directory more shallow.
> +     targets of this operation.
>
>   2. Rewrite working copy URL metadata to reflect a syntactic change only.
>      This is used when repository's root URL changes (such as a scheme
>
>
>


Re: svn commit: r40510(r880584) - in trunk: . subversion subversion/include

2009-11-19 Thread Paul Burba
On Mon, Nov 16, 2009 at 2:16 PM, Paul Burba  wrote:
> On Sat, Nov 14, 2009 at 2:42 PM, Arfrever Frehtes Taifersar Arahesis
>  wrote:
>> Author: arfrever
>> Date: Sat Nov 14 11:42:26 2009
>> New Revision: 40510
>>
>> Log:
>> Move inclusion of svn_debug.h from svn_types.h to svn_private_config.h.
>>
>> * subversion/include/svn_types.h: Don't include "private/svn_debug.h".
>>
>> * configure.ac: Include "private/svn_debug.h" in generated 
>> svn_private_config.h.in
>>   file.
>>
>> * subversion/svn_private_config.hw: Include "private/svn_debug.h".
>>
>> Review by: gstein
>>
>> Modified:
>>   trunk/configure.ac
>>   trunk/subversion/include/svn_types.h
>>   trunk/subversion/svn_private_config.hw
>>
>> Modified: trunk/configure.ac
>> URL: 
>> http://svn.collab.net/viewvc/svn/trunk/configure.ac?pathrev=40510&r1=40509&r2=40510
>> ==
>> --- trunk/configure.ac  Sat Nov 14 00:02:24 2009        (r40509)
>> +++ trunk/configure.ac  Sat Nov 14 11:42:26 2009        (r40510)
>> @@ -615,7 +615,15 @@ if test "$enable_nls" = "yes"; then
>>   fi
>>  fi
>>
>> -AH_BOTTOM(
>> +AH_BOTTOM([/*
>> + * Subversion developers may want to use some additional debugging 
>> facilities
>> + * while working on the code. We'll pull that in here, so individual source
>> + * files don't have to include this header manually.
>> + */
>> +#ifdef SVN_DEBUG
>> +#include "private/svn_debug.h"
>> +#endif
>> +
>>  #define N_(x) x
>>  #define U_(x) x
>>  #ifdef ENABLE_NLS
>> @@ -629,7 +637,7 @@ AH_BOTTOM(
>>  #define gettext(x) (x)
>>  #define dgettext(domain, x) (x)
>>  #endif
>> -)
>> +])
>>
>>  dnl Used to simulate makefile conditionals.
>>  GETTEXT_CODESET=\#
>>
>> Modified: trunk/subversion/include/svn_types.h
>> URL: 
>> http://svn.collab.net/viewvc/svn/trunk/subversion/include/svn_types.h?pathrev=40510&r1=40509&r2=40510
>> ==
>> --- trunk/subversion/include/svn_types.h        Sat Nov 14 00:02:24 2009     
>>    (r40509)
>> +++ trunk/subversion/include/svn_types.h        Sat Nov 14 11:42:26 2009     
>>    (r40510)
>> @@ -1144,14 +1144,4 @@ typedef unsigned long svn_linenum_t;
>>  #include "svn_error.h"
>>
>>
>> -/*
>> - * Subversion developers may want to use some additional debugging 
>> facilities
>> - * while working on the code. We'll pull that in here, so individual source
>> - * files don't have to include this header manually.
>> - */
>> -#ifdef SVN_DEBUG
>> -#include "private/svn_debug.h"
>> -#endif
>> -
>> -
>>  #endif /* SVN_TYPES_H */
>>
>> Modified: trunk/subversion/svn_private_config.hw
>> URL: 
>> http://svn.collab.net/viewvc/svn/trunk/subversion/svn_private_config.hw?pathrev=40510&r1=40509&r2=40510
>> ==
>> --- trunk/subversion/svn_private_config.hw      Sat Nov 14 00:02:24 2009     
>>    (r40509)
>> +++ trunk/subversion/svn_private_config.hw      Sat Nov 14 11:42:26 2009     
>>    (r40510)
>> @@ -91,3 +91,12 @@
>>  #define APU_WANT_DB
>>  #include 
>>  #endif
>> +
>> +/*
>> + * Subversion developers may want to use some additional debugging 
>> facilities
>> + * while working on the code. We'll pull that in here, so individual source
>> + * files don't have to include this header manually.
>> + */
>> +#ifdef SVN_DEBUG
>> +#include "private/svn_debug.h"
>> +#endif
>
> Hi Arfrever,
>
> This change breaks the debug build on Windows since __attribute__ is
> not longer defined, see __attribute__no.longer.defined.txt.
>
> I thought that simply adding #include  to svn_debug.h would fix
> this, but I end up with even more weirdness, see include.apr.h.txt.
>
> I hate to admit I can't quite figure this out.  I've reverted this
> change locally so I can build, but please look into this when you can.

Hi Arfrever,

I've been unable to fix the broken Windows debug build and it doesn't
appear anyone else is working on this.  Edmund has confirmed the
breakage on a his machine.  So, per our rules in HACKING, I
regrettably reverted this change in r882182.

I'm more than happy to test a new version of r40510(r880584) or assist
in any way I can.

Sorry,

Paul