Dan,
As mentioned in IRC I added a new merge test in r25101 that tests some
practical applications of this fix, most importantly merging to a path
then reversing that merge to a subtree of the path:
svn merge -rX:Y URL/SOURCE PATH
svn merge -rY:X URL/SOURCE/CHILD PATH/CHILD
PATH/CHILD should end up with mergeinfo for SOURCE/CHILD with an empty
rev range, overriding the merge info on PATH.
Trunk currently sets no mergeinfo on PATH/CHILD. Attached is a patch
which fixes this.
[[[
Reversing the subtree of a merge puts empty rev range mergeinfo on the
subtree.
* subversion/libsvn_client/merge.c
(svn_client__elide_mergeinfo): Add optional predicate to tell caller
if
elision was performed.
(update_wc_merge_info): Allow rangelists with no ranges.
(do_merge, do_single_file_merge): Update calls to
svn_client__elide_mergeinfo().
(clear_empty_range_mergeinfo): New file private function that clears
merge info on a WC path if consists only of empty ranges and doesn't
override
any ancestor path.
(svn_client_merge3, svn_client_merge_peg3): Use new
clear_empty_range_mergeinfo() as post-elision step on merge target.
* subversion/libsvn_client/mergeinfo.h
(svn_client__elide_mergeinfo): Update doc string and arg list for new
predicate.
* subversion/libsvn_client/switch.c (svn_client__switch_internal):
* subversion/libsvn_client/update.c (svn_client__update_internal):
Update calls to svn_client__elide_mergeinfo().
* subversion/tests/cmdline/merge_tests.py
(test_list): Remove Xfail from empty_rev_range_mergeinfo.
]]]
There's a problem with this patch however: If a path has mergeinfo from
multiple sources, removing all the ranges only for one source path
leaves spurious empty range mergeinfo for that path. For example, we
have a greek tree where:
A is copied to A_COPY in r1
A_COPY/D/G/rho is modified in r2:
A_COPY/D/H/psi is modified in r3:
>svn pl -vR merge_tests-1
Properties on 'merge_tests-1\A_COPY':
svn:mergeinfo : /A:1
# Set some fake mergeinfo on A/D and commit at r5
>svn ps svn:mergeinfo "/A_COPY/B:2" merge_tests-1\A\D
property 'svn:mergeinfo' set on 'merge_tests-1\A\D'
>svn ci -m "" merge_tests-1
Sending merge_tests-1\A\D
Committed revision 5.
# Merge the two text changes into A/D/H
>svn merge %URL%/A_COPY/D/H merge_tests-1\A\D\H -r2:4
U merge_tests-1\A\D\H\psi
>svn pl -vR merge_tests-1
Properties on 'merge_tests-1\A\D':
svn:mergeinfo : /A_COPY/B:2
Properties on 'merge_tests-1\A\D\H':
svn:mergeinfo : /A_COPY/B/H:2
/A_COPY/D/H:3-4
Properties on 'merge_tests-1\A_COPY':
svn:mergeinfo : /A:1
# Reverse the two test changes out of a
# child of the previous merge target
>svn merge %URL%/A_COPY/D/H merge_tests-1\A\D\H -r4:2
G merge_tests-1\A\D\H\psi
# My patch still leaves us with spurious mergeinfo for /A_COPY/D/H on
A/D/H:
>svn pl -vR merge_tests-1
Properties on 'merge_tests-1\A\D':
svn:mergeinfo : /A_COPY/B:2
Properties on 'merge_tests-1\A\D\H':
svn:mergeinfo : /A_COPY/B/H:2
/A_COPY/D/H:
Properties on 'merge_tests-1\A_COPY':
svn:mergeinfo : /A:1
This isn't a really serious problem in this simple example, but
eventually these meaningless empty rev ranges would litter the prop
space.
Any thoughts appreciated.
Paul
> -----Original Message-----
> From: dlr@tigris.org [mailto:dlr@tigris.org]
> Sent: Saturday, May 19, 2007 3:12 PM
> To: svn@subversion.tigris.org
> Subject: svn commit: r25068 - in trunk/subversion:
> libsvn_client libsvn_fs_util libsvn_subr tests/libsvn_subr
>
> Author: dlr
> Date: Sat May 19 12:11:55 2007
> New Revision: 25068
>
> Log:
> More changes necessary to fix issue #2769, to allow for removal of "N"
> or "all" merge sources for a path.
>
> Note: Users of (currently unreleased) APR 1.3.0 will require at least
> r538391 from the ASF's Subversion repository to make
> apr_array_clear() available.
>
> * subversion/libsvn_fs_util/mergeinfo-sqlite-index.c
> Include svn_compat.h for the APR_VERSION_AT_LEAST() macro.
> (get_merge_info_for_path): Add declaration to allow for forward
> reference.
> (no_mergeinfo): New constant representing "no merge info".
> (index_path_merge_info): Support removal of all merge sources for a
> path.
> (parse_mergeinfo_from_db): Filter out invalid revision numbers,
> which are assumed to represent dummy records indicating that a
> merge source has no merge info.
>
> * subversion/libsvn_subr/mergeinfo.c
> Include ctype.h.
> (parse_revision, parse_revision_line): Include range list
> in error text.
> (parse_revlist): Don't return an error when an empty revision list
> is encountered for a merge source. This is expected, in the case
> where other merge sources still have revision lists, but the merge
> source in question does not. Include range list in error text.
>
> * subversion/libsvn_client/merge.c
> Include apr_tables.h and apr_hash.h.
> (adjust_mergeinfo_source_paths): New function that adjusts merge
> source paths. Factored out of get_wc_merge_info().
> (get_wc_merge_info, get_wc_or_repos_merge_info, mergeinfo_elides,
> svn_client__elide_mergeinfo, calculate_merge_ranges,
> update_wc_merge_info): Account for potentially NULL merge info.
> Adjust doc string accordingly.
>
> * subversion/libsvn_client/copy.c
> (extend_wc_merge_info, wc_to_repos_copy): Account for potentially
> NULL merge info.
>
> * subversion/libsvn_client/mergeinfo.h
> * subversion/libsvn_client/mergeinfo.c
> (svn_client__parse_merge_info): Return NULL merge info if there is
> no merge info set on WCPATH. Adjust doc string accordingly.
>
> * subversion/tests/libsvn_subr/mergeinfo-test.c
> (NBR_BROKEN_MERGEINFO_VALS): Decrement to 4.
> (broken_mergeinfo_vals): Remove "/missing-revs-with-colon:
> " test case.
> (mergeinfo7): Add a multi-line merge info with empty range lists.
> (test_parse_multi_line_mergeinfo): Use mergeinfo7.
>
> Review by: kameshj
>
>
> Modified:
> trunk/subversion/libsvn_client/copy.c
> trunk/subversion/libsvn_client/merge.c
> trunk/subversion/libsvn_client/mergeinfo.c
> trunk/subversion/libsvn_client/mergeinfo.h
> trunk/subversion/libsvn_fs_util/mergeinfo-sqlite-index.c
> trunk/subversion/libsvn_subr/mergeinfo.c
> trunk/subversion/tests/libsvn_subr/mergeinfo-test.c
>
> Modified: trunk/subversion/libsvn_client/copy.c
> URL:
> http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_clien
> t/copy.c?pathrev=25068&r1=25067&r2=25068
> ==============================================================
> ================
> --- trunk/subversion/libsvn_client/copy.c (original)
> +++ trunk/subversion/libsvn_client/copy.c Sat May 19 12:11:55 2007
> @@ -447,8 +447,11 @@
> adm_access, ctx, pool));
>
> /* Combine the provided merge info with any merge info
> from the WC. */
> - SVN_ERR(svn_mergeinfo_merge(&wc_mergeinfo, mergeinfo,
> - pool));
> + if (wc_mergeinfo)
> + SVN_ERR(svn_mergeinfo_merge(&wc_mergeinfo, mergeinfo,
> + pool)); else
> + wc_mergeinfo = mergeinfo;
>
> return svn_client__record_wc_merge_info(target_wcpath,
> wc_mergeinfo,
> adm_access, pool);
> @@ -1241,7 +1244,8 @@
> SVN_ERR(svn_client__parse_merge_info(&wc_mergeinfo, entry,
> pair->src,
> adm_access, ctx,
> pool));
> - SVN_ERR(svn_mergeinfo_merge(&mergeinfo, wc_mergeinfo, pool));
> + if (wc_mergeinfo)
> + SVN_ERR(svn_mergeinfo_merge(&mergeinfo, wc_mergeinfo, pool));
> SVN_ERR(svn_mergeinfo__to_string((svn_string_t **)
> &mergeinfo_prop->value,
> mergeinfo, pool));
>
> Modified: trunk/subversion/libsvn_client/merge.c
> URL:
> http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_clien
> t/merge.c?pathrev=25068&r1=25067&r2=25068
> ==============================================================
> ================
> --- trunk/subversion/libsvn_client/merge.c (original)
> +++ trunk/subversion/libsvn_client/merge.c Sat May 19 12:11:55 2007
> @@ -23,6 +23,8 @@
> /*** Includes. ***/
>
> #include <apr_strings.h>
> +#include <apr_tables.h>
> +#include <apr_hash.h>
> #include "svn_types.h"
> #include "svn_hash.h"
> #include "svn_wc.h"
> @@ -852,9 +854,32 @@
> >
> /*** Retrieving merge info. ***/
>
> +/* Adjust merge sources in MERGEINFO (which is assumed to be
> non-NULL).
> +*/ static APR_INLINE void adjust_mergeinfo_source_paths(apr_hash_t
> +*mergeinfo, const char *walk_path,
> + apr_hash_t *wc_mergeinfo, apr_pool_t
> +*pool) {
> + apr_hash_index_t *hi;
> + const void *merge_source;
> + void *rangelist;
> + const char *path;
> +
> + for (hi = apr_hash_first(NULL, wc_mergeinfo); hi; hi =
> apr_hash_next(hi))
> + {
> + /* Copy inherited merge info into our output hash,
> adjusting the
> + merge source as appropriate. */
> + apr_hash_this(hi, &merge_source, NULL, &rangelist);
> + path = svn_path_join((const char *) merge_source, walk_path,
> + apr_hash_pool_get(mergeinfo));
> + /* ### If pool has a different lifetime than mergeinfo->pool,
> + ### this use of "rangelist" will be a problem... */
> + apr_hash_set(mergeinfo, path, APR_HASH_KEY_STRING, rangelist);
> + }
> +}
> +
> /* Find explicit or inherited WC merge info for WCPATH, and return it
> - in *MERGEINFO. Set *INHERITED to whether the merge info
> was inherited
> - (TRUE or FALSE).
> + in *MERGEINFO (NULL if no merge info is set). Set *INHERITED to
> + whether the merge info was inherited (TRUE or FALSE).
>
> If INHERITED_ONLY is TRUE look only for inherited merge
> info and ignore
> explicit merge info on WCPATH.
> @@ -889,7 +914,7 @@
> interested in inherited mergeinfo. */
> if (inherited_only)
> {
> - wc_mergeinfo = apr_hash_make(pool);
> + wc_mergeinfo = NULL;
> inherited_only = FALSE;
> }
> else
> @@ -929,7 +954,7 @@
> SVN_ERR(svn_path_get_absolute(&wcpath, wcpath, pool));
> }
>
> - if (apr_hash_count(wc_mergeinfo) == 0 &&
> + if (wc_mergeinfo == NULL &&
> !svn_dirent_is_root(wcpath, strlen(wcpath)))
> {
> svn_error_t *err;
> @@ -976,24 +1001,17 @@
> else
> {
> /* Merge info may be inherited. */
> - apr_hash_index_t *hi;
> - void *val;
> - const void *key;
> - const char *path;
> - apr_array_header_t *rangelist;
> -
> - *inherited = apr_hash_count(wc_mergeinfo) > 0;
> - *mergeinfo = apr_hash_make(pool);
> -
> - for (hi = apr_hash_first(pool, wc_mergeinfo); hi; hi =
> apr_hash_next(hi))
> + if (wc_mergeinfo)
> {
> - /* Copy inherited merge info into our output hash,
> adjusting
> - the merge source as appropriate. */
> - apr_hash_this(hi, &key, NULL, &val);
> - path = svn_path_join((const char *) key, walk_path, pool);
> - rangelist = val;
> -
> - apr_hash_set(*mergeinfo, path,
> APR_HASH_KEY_STRING, rangelist);
> + *inherited = (wc_mergeinfo &&
> apr_hash_count(wc_mergeinfo) > 0);
> + *mergeinfo = apr_hash_make(pool);
> + adjust_mergeinfo_source_paths(*mergeinfo,
> walk_path, wc_mergeinfo,
> + pool);
> + }
> + else
> + {
> + *inherited = FALSE;
> + *mergeinfo = NULL;
> }
> }
>
> @@ -1010,9 +1028,9 @@
> RA_SESSION is NULL). Store any merge info obtained for the target
> (reflected by *ENTRY, which is also acquired and returned by this
> function) in *TARGET_MERGEINFO, if no merge info is found
> - *TARGET_MERGEINFO is an empty hash. If the target inherited any
> - merge info from a WC ancestor set *INHERITED to TRUE, set it to
> - FALSE otherwise. */
> + *TARGET_MERGEINFO is NULL. If the target inherited any merge info
> + *from a WC ancestor set *INHERITED to TRUE, set it to FALSE
> + *otherwise. */
> static svn_error_t *
> get_wc_or_repos_merge_info(apr_hash_t **target_mergeinfo,
> const svn_wc_entry_t **entry, @@
> -1091,7 +1109,7 @@
> pool));
>
> /* If there in no WC merge info check the repository. */
> - if (!apr_hash_count(*target_mergeinfo))
> + if (*target_mergeinfo == NULL)
> {
> apr_hash_t *repos_mergeinfo;
> if (ra_session == NULL)
> @@ -1116,8 +1134,9 @@
> /* Helper for svn_client__elide_mergeinfo and elide_children.
>
> Compare PARENT_MERGEINFO and CHILD_MERGEINFO to see if they are
> - identical. If PATH_SUFFIX is not NULL append PATH_SUFFIX to each
> - path in PARENT_MERGEINFO before performing the comparison.
> + identical (they may be NULL). If PATH_SUFFIX is not NULL append
> + PATH_SUFFIX to each path in PARENT_MERGEINFO before performing the
> + comparison.
>
> Set *ELIDES to whether PARENT_MERGEINFO and CHILD_MERGEINFO are
> identical (TRUE or FALSE). */
> @@ -1131,7 +1150,8 @@
> apr_pool_t *subpool = svn_pool_create(pool);
> apr_hash_t *mergeinfo;
>
> - if (apr_hash_count(parent_mergeinfo) !=
> apr_hash_count(child_mergeinfo))
> + if (parent_mergeinfo == NULL || child_mergeinfo == NULL ||
> + apr_hash_count(parent_mergeinfo) !=
> + apr_hash_count(child_mergeinfo))
> {
> *elides = FALSE;
> return SVN_NO_ERROR;
> @@ -1305,7 +1325,7 @@
>
> /* If TARGET_WCPATH has no explicit mergeinfo,
> there's nothing to
> elide, we're done. */
> - if (inherited || apr_hash_count(target_mergeinfo) == 0)
> + if (inherited || target_mergeinfo == NULL)
> return SVN_NO_ERROR;
>
> /* Get TARGET_WCPATH's inherited mergeinfo. */ @@
> -1317,7 +1337,7 @@
>
> /* If TARGET_WCPATH has no inherited mergeinfo, there's
> nowhere to elide to, we're done. */
> - if (apr_hash_count(mergeinfo) == 0)
> + if (mergeinfo == NULL)
> return SVN_NO_ERROR;
>
> SVN_ERR(mergeinfo_elides(&elides, mergeinfo,
> target_mergeinfo, @@ -1364,8 +1384,12 @@
> /* Subtract the revision ranges which have already been merged into
> the WC (if any) from the range requested for merging (to avoid
> repeated merging). */
> - target_rangelist = apr_hash_get(target_mergeinfo, rel_path,
> - APR_HASH_KEY_STRING);
> + if (target_mergeinfo)
> + target_rangelist = apr_hash_get(target_mergeinfo, rel_path,
> + APR_HASH_KEY_STRING); else
> + target_rangelist = NULL;
> +
> if (target_rangelist)
> {
> if (is_revert)
> @@ -1539,6 +1563,8 @@
> a fresh copy before using it to update the WC's
> merge info. */
> SVN_ERR(svn_client__parse_merge_info(&mergeinfo,
> entry, path, adm_access,
> ctx, subpool));
> + if (mergeinfo == NULL)
> + mergeinfo = apr_hash_make(subpool);
>
> /* ASSUMPTION: "target_wcpath" is always both a parent and
> prefix of "path". */
>
> Modified: trunk/subversion/libsvn_client/mergeinfo.c
> URL:
> http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_clien
> t/mergeinfo.c?pathrev=25068&r1=25067&r2=25068
> ==============================================================
> ================
> --- trunk/subversion/libsvn_client/mergeinfo.c (original)
> +++ trunk/subversion/libsvn_client/mergeinfo.c Sat May
> 19 12:11:55 2007
> @@ -75,7 +75,7 @@
> if (propval)
> SVN_ERR(svn_mergeinfo_parse(mergeinfo, propval->data, pool));
> else
> - *mergeinfo = apr_hash_make(pool);
> + *mergeinfo = NULL;
>
> return SVN_NO_ERROR;
> }
>
> Modified: trunk/subversion/libsvn_client/mergeinfo.h
> URL:
> http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_clien
> t/mergeinfo.h?pathrev=25068&r1=25067&r2=25068
> ==============================================================
> ================
> --- trunk/subversion/libsvn_client/mergeinfo.h (original)
> +++ trunk/subversion/libsvn_client/mergeinfo.h Sat May
> 19 12:11:55 2007
> @@ -30,8 +30,9 @@
> svn_revnum_t rev,
> apr_pool_t *pool);
>
> -/* Parse any merge info from WCPATH's ENTRY and store it in
> MERGEINFO.
> - If no merge info is available, set MERGEINFO to an empty hash. */
> +/* Parse any merge info from the WCPATH's ENTRY and store it in
> + MERGEINFO. If no record of any merge info exists, set
> MERGEINFO to
> + NULL. Does not acount for inherited merge info. */
> svn_error_t *
> svn_client__parse_merge_info(apr_hash_t **mergeinfo,
> const svn_wc_entry_t *entry,
>
> Modified: trunk/subversion/libsvn_fs_util/mergeinfo-sqlite-index.c
> URL:
> http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_fs_ut
> il/mergeinfo-sqlite-index.c?pathrev=25068&r1=25067&r2=25068
> ==============================================================
> ================
> --- trunk/subversion/libsvn_fs_util/mergeinfo-sqlite-index.c
> (original)
> +++ trunk/subversion/libsvn_fs_util/mergeinfo-sqlite-index.c
> Sat May 19 12:11:55 2007
> @@ -30,6 +30,7 @@
> #include "svn_path.h"
> #include "svn_mergeinfo.h"
>
> +#include "private/svn_compat.h"
> #include "private/svn_fs_mergeinfo.h"
> #include "../libsvn_fs/fs-loader.h"
> #include "svn_private_config.h"
> @@ -181,6 +182,19 @@
> return SVN_NO_ERROR;
> }
>
> +static svn_error_t *
> +get_merge_info_for_path(sqlite3 *db,
> + const char *path,
> + svn_revnum_t rev,
> + apr_hash_t *result,
> + apr_hash_t *cache,
> + svn_boolean_t include_parents,
> + apr_pool_t *pool);
> +
> +/* Represents "no merge info". */
> +static svn_merge_range_t no_mergeinfo = { SVN_INVALID_REVNUM,
> + SVN_INVALID_REVNUM };;
Extra ';'
> +
> /* Insert the necessary indexing data into the DB for all the merges
> on PATH as of NEW_REV, which is provided (unparsed) in
> MERGEINFO_STR. Use POOL for temporary allocations.*/ @@
> -191,10 +205,29 @@
> apr_hash_t *mergeinfo;
> apr_hash_index_t *hi;
> sqlite3_stmt *stmt;
> + svn_boolean_t remove_mergeinfo = FALSE;
>
> SVN_ERR(svn_mergeinfo_parse(&mergeinfo,
> mergeinfo_str->data, pool));
>
> - for (hi = apr_hash_first(pool, mergeinfo);
> + if (apr_hash_count(mergeinfo) == 0)
> + {
> + /* All merge info has been removed from PATH (or explicitly set
> + to "none", if there previously was no merge info). Find all
> + previous merge info, and (further below) insert
> dummy records
> + representing "no merge info" for all its previous merge
> + sources of PATH. */
> + apr_hash_t *cache = apr_hash_make(pool);
> + remove_mergeinfo = TRUE;
> + SVN_ERR(get_merge_info_for_path(db, path, new_rev,
> mergeinfo, cache,
> + TRUE, pool));
> + mergeinfo = apr_hash_get(mergeinfo, path, APR_HASH_KEY_STRING);
> + if (mergeinfo == NULL)
> + /* There was previously no merge info, inherited or explicit,
> + for PATH. */
> + return SVN_NO_ERROR;
> + }
> +
> + for (hi = apr_hash_first(NULL, mergeinfo);
> hi != NULL;
> hi = apr_hash_next(hi))
> {
> @@ -221,11 +254,23 @@
> db);
> SQLITE_ERR(sqlite3_bind_text(stmt, 3, path, -1,
> SQLITE_TRANSIENT),
> db);
> - for (i = 0; i < rangelist->nelts; i++)
> +
> + if (remove_mergeinfo)
> {
> - svn_merge_range_t *range;
> + /* Explicitly set "no merge info" for PATH,
> which may've
> + previously had only inherited merge info. */ #if
> +APR_VERSION_AT_LEAST(1, 3, 0)
> + apr_array_clear(rangelist); #else
> + rangelist = apr_array_make(pool, 1,
> +sizeof(&no_mergeinfo)); #endif
> + APR_ARRAY_PUSH(rangelist, svn_merge_range_t *)
> = &no_mergeinfo;
> + }
>
> - range = APR_ARRAY_IDX(rangelist, i,
> svn_merge_range_t *);
> + for (i = 0; i < rangelist->nelts; i++)
> + {
> + const svn_merge_range_t *range =
> + APR_ARRAY_IDX(rangelist, i, svn_merge_range_t *);
> SQLITE_ERR(sqlite3_bind_int64(stmt, 4, range->start),
> db);
> SQLITE_ERR(sqlite3_bind_int64(stmt, 5,
> range->end), @@ -365,8 +410,6 @@
>
> do
> {
> - svn_merge_range_t *temprange;
> -
> mergedfrom = (char *) sqlite3_column_text(stmt, 0);
> startrev = sqlite3_column_int64(stmt, 1);
> endrev = sqlite3_column_int64(stmt, 2); @@ -381,10
> +424,17 @@
> pathranges = apr_array_make(pool, 1,
>
> sizeof(svn_merge_range_t *));
> }
> - temprange = apr_pcalloc(pool, sizeof(*temprange));
> - temprange->start = startrev;
> - temprange->end = endrev;
> - APR_ARRAY_PUSH(pathranges, svn_merge_range_t *) =
> temprange;
> +
> + /* Filter out invalid revision numbers, which are
> assumed to
> + represent dummy records indicating that a merge source
> + has no merge info for PATH. */
> + if (SVN_IS_VALID_REVNUM(startrev) &&
> SVN_IS_VALID_REVNUM(endrev))
> + {
> + svn_merge_range_t *range = apr_pcalloc(pool,
> sizeof(*range));
> + range->start = startrev;
> + range->end = endrev;
> + APR_ARRAY_PUSH(pathranges, svn_merge_range_t
> *) = range;
> + }
>
> sqlite_result = sqlite3_step(stmt);
> lastmergedfrom = mergedfrom;
>
> Modified: trunk/subversion/libsvn_subr/mergeinfo.c
> URL:
> http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_subr/
> mergeinfo.c?pathrev=25068&r1=25067&r2=25068
> ==============================================================
> ================
> --- trunk/subversion/libsvn_subr/mergeinfo.c (original)
> +++ trunk/subversion/libsvn_subr/mergeinfo.c Sat May 19 12:11:55 2007
> @@ -16,6 +16,7 @@
> *
> ====================================================================
> */
> #include <assert.h>
> +#include <ctype.h>
>
> #include "svn_types.h"
> #include "svn_ctype.h"
> @@ -64,8 +65,9 @@
> svn_revnum_t result = strtol(curr, &endptr, 10);
>
> if (curr == endptr)
> - return svn_error_create(SVN_ERR_MERGE_INFO_PARSE_ERROR, NULL,
> - _("Invalid revision number"));
> + return svn_error_createf(SVN_ERR_MERGE_INFO_PARSE_ERROR, NULL,
> + _("Invalid revision number
> found parsing '%s'"),
> + curr);
>
> *revision = result;
>
> @@ -121,19 +123,28 @@
> const char *curr = *input;
> svn_merge_range_t *lastrange = NULL;
>
> - if (curr == end)
> - return svn_error_create(SVN_ERR_MERGE_INFO_PARSE_ERROR, NULL,
> - _("No revision list found"));
> + /* Eat any leading horizontal white-space before the
> rangelist. */
> + while (curr < end && *curr != '\n' && isspace(*curr))
> + curr++;
> +
> + if (*curr == '\n' || curr == end)
> + {
> + /* Empty range list. */
> + *input = curr;
> + return SVN_NO_ERROR;
> + }
>
> - while (curr < end)
> + while (curr < end && *curr != '\n')
> {
> + /* Parse individual revisions or revision ranges. */
> svn_merge_range_t *mrange = apr_pcalloc(pool, sizeof(*mrange));
> svn_revnum_t firstrev;
>
> SVN_ERR(parse_revision(&curr, end, &firstrev));
> if (*curr != '-' && *curr != '\n' && *curr != ',' &&
> curr != end)
> - return svn_error_create(SVN_ERR_MERGE_INFO_PARSE_ERROR, NULL,
> - _("Invalid character found
> in revision list"));
> + return
> svn_error_createf(SVN_ERR_MERGE_INFO_PARSE_ERROR, NULL,
> + _("Invalid character '%c'
> found in revision "
> + "list"), *curr);
> mrange->start = firstrev;
> mrange->end = firstrev;
>
> @@ -146,7 +157,6 @@
> mrange->end = secondrev;
> }
>
> - /* XXX: Watch empty revision list problem */
> if (*curr == '\n' || curr == end)
> {
> combine_with_lastrange(&lastrange, mrange, FALSE,
> revlist, pool); @@ -160,14 +170,16 @@
> }
> else
> {
> - return
> svn_error_create(SVN_ERR_MERGE_INFO_PARSE_ERROR, NULL,
> - _("Invalid character found
> in revision list"));
> + return
> svn_error_createf(SVN_ERR_MERGE_INFO_PARSE_ERROR, NULL,
> + _("Invalid character '%c'
> found in "
> + "range list"), *curr);
> }
>
> }
> if (*curr != '\n')
> return svn_error_create(SVN_ERR_MERGE_INFO_PARSE_ERROR, NULL,
> - _("Revision list parsing ended
> before hitting newline"));
> + _("Range list parsing ended
> before hitting "
> + "newline"));
> *input = curr;
> return SVN_NO_ERROR;
> }
> @@ -192,8 +204,9 @@
> SVN_ERR(parse_revlist(input, end, revlist, pool));
>
> if (*input != end && *(*input) != '\n')
> - return svn_error_create(SVN_ERR_MERGE_INFO_PARSE_ERROR, NULL,
> - _("Could not find end of line in
> revision line"));
> + return svn_error_createf(SVN_ERR_MERGE_INFO_PARSE_ERROR, NULL,
> + _("Could not find end of line
> in range list line "
> + "in '%s'"), *input);
>
> if (*input != end)
> *input = *input + 1;
>
> Modified: trunk/subversion/tests/libsvn_subr/mergeinfo-test.c
> URL:
> http://svn.collab.net/viewvc/svn/trunk/subversion/tests/libsvn
> _subr/mergeinfo-test.c?pathrev=25068&r1=25067&r2=25068
> ==============================================================
> ================
> --- trunk/subversion/tests/libsvn_subr/mergeinfo-test.c
> (original)
> +++ trunk/subversion/tests/libsvn_subr/mergeinfo-test.c
> Sat May 19 12:11:55 2007
> @@ -181,12 +181,11 @@
> }
>
>
> -#define NBR_BROKEN_MERGEINFO_VALS 5
> +#define NBR_BROKEN_MERGEINFO_VALS 4
> /* Invalid merge info values. */
> static const char * const
> broken_mergeinfo_vals[NBR_BROKEN_MERGEINFO_VALS] =
> {
> "/missing-revs",
> - "/missing-revs-with-colon: ",
> "/trunk: 5,7-9,10,11,13,14,",
> "/trunk 5,7-9,10,11,13,14",
> "/trunk:5 7--9 10 11 13 14"
> @@ -226,6 +225,7 @@
> static const char *mergeinfo4 = "/trunk: 15-25, 35-45";
> static const char *mergeinfo5 = "/trunk: 10-30, 35-45,
> 55-65"; static const char *mergeinfo6 = "/trunk: 15-25";
> +static const char *mergeinfo7 =
> +"/empty-rangelist:\n/with-trailing-space: ";
>
> static svn_error_t *
> test_parse_multi_line_mergeinfo(const char **msg, @@ -239,6 +239,9 @@
> return SVN_NO_ERROR;
>
> SVN_ERR(svn_mergeinfo_parse(&info1, mergeinfo1, pool));
> +
> + SVN_ERR(svn_mergeinfo_parse(&info1, mergeinfo7, pool));
> +
> return SVN_NO_ERROR;
> }
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue May 22 20:39:10 2007