sbutler_at_tigris.org wrote:
> Author: sbutler
> Date: Sat Nov 22 15:46:26 2008
> New Revision: 34345
>
> Log:
> Properly loggify the recording of tree conflict data during update and
> switch. The desired end result is the same as before; only the timing
> of the entry-writing has changed. It's now consistent with the other
> loggy commands.
>
> * subversion/libsvn_wc/tree_conflicts.c
> (svn_wc__loggy_add_tree_conflict): Move to log.c, where the other
> svn_wc__loggy_* functions are.
>
> * subversion/libsvn_wc/log.c
> (#include tree_conflicts.h): We need tree conflict helper functions.
> (svn_wc__loggy_add_tree_conflict): Move here from tree_conflicts.
> Change this function to simply write an <add-tree-conflict> log
> element in the victim's parent's log.
> (SVN_WC__LOG_ADD_TREE_CONFLICT,
> SVN_WC__LOG_ATTR_DATA): New XML element and attribute names.
> (log_runner): Add fields tree_conflicts_added (boolean) and
> tree_conflicts (array). Add field result_pool to hold the tree
> conflicts.
> (log_do_add_tree_conflict): Sanity check a new tree conflict and
> add it to loggy->tree_conflicts.
> (start_handler): Support <add-tree-conflict> log elements.
> (run_log_from_memory): Note: no tree conflict support needed because
> this function doesn't update the entry.
> (run_log): If the log_runner has tree conflicts, encode them as text
> data and include them in the entry rewrite.
>
> * subversion/tests/cmdline/depth_tests.py
> (make_depth_tree_conflicts): Correct errors in expected status.
> (verify_lines): New function borrowed from merge tests.
This isn't necessary.
In my first "fix" that was reverted later, I also changed this to use
UnorderedOutput expectations. These lines aren't regexes, so that's
sufficient. Reinstating my first fix of the tests in r34428.
> (tree_conflicts_resolved_depth_immediates,
> tree_conflicts_resolved_depth_infinity): Allow expected output in
> any order.
>
> Modified:
> trunk/subversion/libsvn_wc/log.c
> trunk/subversion/libsvn_wc/tree_conflicts.c
> trunk/subversion/tests/cmdline/depth_tests.py
>
> Modified: trunk/subversion/libsvn_wc/log.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/log.c?pathrev=34345&r1=34344&r2=34345
> ==============================================================================
> --- trunk/subversion/libsvn_wc/log.c Sat Nov 22 15:34:51 2008 (r34344)
> +++ trunk/subversion/libsvn_wc/log.c Sat Nov 22 15:46:26 2008 (r34345)
> @@ -41,6 +41,7 @@
> #include "lock.h"
> #include "translate.h"
> #include "questions.h"
> +#include "tree_conflicts.h"
>
> #include "private/svn_wc_private.h"
> #include "svn_private_config.h"
> @@ -103,6 +104,9 @@
> /* Set SVN_WC__LOG_ATTR_NAME to have timestamp SVN_WC__LOG_ATTR_TIMESTAMP. */
> #define SVN_WC__LOG_SET_TIMESTAMP "set-timestamp"
>
> +/* Add a new tree conflict to the parent entry's tree-conflict-data. */
> +#define SVN_WC__LOG_ADD_TREE_CONFLICT "add-tree-conflict"
> +
>
> /* Handle closure after a commit completes successfully:
> *
> @@ -135,6 +139,7 @@
> #define SVN_WC__LOG_ATTR_PROPVAL "propval"
> #define SVN_WC__LOG_ATTR_FORMAT "format"
> #define SVN_WC__LOG_ATTR_FORCE "force"
> +#define SVN_WC__LOG_ATTR_DATA "data"
>
> /* This one is for SVN_WC__LOG_MERGE. */
> #define SVN_WC__LOG_ATTR_ARG_1 "arg1"
> @@ -185,14 +190,17 @@
> /*** Userdata for the callbacks. ***/
> struct log_runner
> {
> - apr_pool_t *pool;
> + apr_pool_t *pool; /* cleared before processing each log element */
> + apr_pool_t *result_pool;
> svn_xml_parser_t *parser;
> svn_boolean_t entries_modified;
> svn_boolean_t wcprops_modified;
> svn_boolean_t rerun;
> svn_wc_adm_access_t *adm_access; /* the dir in which all this happens */
> const char *diff3_cmd; /* external diff3 cmd, or null if none */
> -
> + svn_boolean_t tree_conflicts_added;
> + apr_array_header_t *tree_conflicts; /* array of pointers to
> + svn_wc_conflict_description_t. */
> /* Which top-level log element we're on for this logfile. Some
> callers care whether a failure happened on the first element or
> on some later element (e.g., 'svn cleanup').
> @@ -1485,6 +1493,45 @@ log_do_upgrade_format(struct log_runner
> }
>
>
> +static svn_error_t *
> +log_do_add_tree_conflict(struct log_runner *loggy,
> + const char **atts)
> +{
> + apr_array_header_t *new_conflicts;
> + const svn_wc_conflict_description_t *new_conflict;
> + const char* dir_path =svn_wc_adm_access_path(loggy->adm_access);
There's a space missing after "=".
> +
> + /* Convert the text data to a conflict. */
> + new_conflict = apr_pcalloc(loggy->pool,
> + sizeof(svn_wc_conflict_description_t *));
You are setting new_conflict, ...
> + new_conflicts = apr_array_make(loggy->pool, 1,
> + sizeof(svn_wc_conflict_description_t *));
> + SVN_ERR(svn_wc__read_tree_conflicts(&new_conflicts,
> + svn_xml_get_attr_value(SVN_WC__LOG_ATTR_DATA, atts),
> + dir_path, loggy->pool));
> + new_conflict = APR_ARRAY_IDX(new_conflicts, 0,
> + svn_wc_conflict_description_t *);
...and you are setting it again? Am I not getting something?
> +
> + /* Re-adding an existing tree conflict victim is an error. */
> + if (svn_wc__tree_conflict_exists(loggy->tree_conflicts,
> + svn_path_basename(new_conflict->path,
> + loggy->pool),
> + loggy->pool))
> + return svn_error_create(SVN_ERR_WC_CORRUPT, NULL,
> + _("Attempt to add tree conflict that already exists"));
> +
> + /* Copy the new conflict to to the result pool. Add its pointer to
to to
> + the array of existing conflicts. */
> + APR_ARRAY_PUSH(loggy->tree_conflicts,
> + const svn_wc_conflict_description_t *) =
> + svn_wc__conflict_description_dup(new_conflict,
> + loggy->result_pool);
> +
> + loggy->tree_conflicts_added = TRUE;
> +
> + return SVN_NO_ERROR;
> +}
> +
> static void
> start_handler(void *userData, const char *eltname, const char **atts)
> {
> @@ -1570,6 +1617,9 @@ start_handler(void *userData, const char
> else if (strcmp(eltname, SVN_WC__LOG_UPGRADE_FORMAT) == 0) {
> err = log_do_upgrade_format(loggy, atts);
> }
> + else if (strcmp(eltname, SVN_WC__LOG_ADD_TREE_CONFLICT) == 0) {
> + err = log_do_add_tree_conflict(loggy, atts);
> + }
> else
> {
> SIGNAL_ERROR
> @@ -1686,6 +1736,7 @@ run_log_from_memory(svn_wc_adm_access_t
> loggy = apr_pcalloc(pool, sizeof(*loggy));
> loggy->adm_access = adm_access;
> loggy->pool = svn_pool_create(pool);
> + loggy->result_pool = svn_pool_create(pool);
> loggy->parser = svn_xml_make_parser(loggy, start_handler,
> NULL, NULL, pool);
> loggy->entries_modified = FALSE;
> @@ -1693,6 +1744,8 @@ run_log_from_memory(svn_wc_adm_access_t
> loggy->rerun = rerun;
> loggy->diff3_cmd = diff3_cmd;
> loggy->count = 0;
> + loggy->tree_conflicts_added = FALSE;
> + loggy->tree_conflicts = NULL;
>
> parser = loggy->parser;
> /* Expat wants everything wrapped in a top-level form, so start with
> @@ -1720,6 +1773,7 @@ run_log(svn_wc_adm_access_t *adm_access,
> int log_number;
> apr_pool_t *iterpool = svn_pool_create(pool);
> svn_boolean_t killme, kill_adm_only;
> + const svn_wc_entry_t *entry;
>
> /* kff todo: use the tag-making functions here, now. */
> const char *log_start
> @@ -1736,12 +1790,24 @@ run_log(svn_wc_adm_access_t *adm_access,
> parser = svn_xml_make_parser(loggy, start_handler, NULL, NULL, pool);
> loggy->adm_access = adm_access;
> loggy->pool = svn_pool_create(pool);
> + loggy->result_pool = svn_pool_create(pool);
> loggy->parser = parser;
> loggy->entries_modified = FALSE;
> loggy->wcprops_modified = FALSE;
> loggy->rerun = rerun;
> loggy->diff3_cmd = diff3_cmd;
> loggy->count = 0;
> + loggy->tree_conflicts_added = FALSE;
> + loggy->tree_conflicts = apr_array_make(pool, 0,
> + sizeof(svn_wc_conflict_description_t *));
> +
> + /* Populate the tree conflict array with the existing tree conflicts. */
> + SVN_ERR(svn_wc_entry(&entry, svn_wc_adm_access_path(adm_access), adm_access,
> + TRUE, pool));
> + SVN_ERR(svn_wc__read_tree_conflicts(&(loggy->tree_conflicts),
> + entry->tree_conflict_data,
> + svn_wc_adm_access_path(adm_access),
> + pool));
Nice, only one read of existing conflicts per parent dir.
But, wait a minute, that means that the conflicts are all written out only
*after* the rest of the loggy commands have been executed?
Could that be a problem? I don't know enough about it, so I'm paranoid about
changing the order in any way... :P
Does it make sense to write out the new state of tree_conflicts after each
consecutive series of tree-conflicts-add log commands?
>
> /* Expat wants everything wrapped in a top-level form, so start with
> a ghost open tag. */
> @@ -1794,7 +1860,29 @@ run_log(svn_wc_adm_access_t *adm_access,
> goto rerun;
> #endif
>
> - if (loggy->entries_modified == TRUE)
> + /* If the logs included tree conflicts, write them to the entry. */
> + if (loggy->tree_conflicts_added)
> + {
> + char *conflict_data;
> + svn_wc_entry_t tmp_entry;
> + svn_error_t *err;
> +
> + SVN_ERR(svn_wc__write_tree_conflicts(&conflict_data,
> + loggy->tree_conflicts, pool));
> +
> + tmp_entry.tree_conflict_data = apr_pstrdup(pool, conflict_data);
> + err = svn_wc__entry_modify(adm_access, SVN_WC_ENTRY_THIS_DIR,
> + &tmp_entry,
> + SVN_WC__ENTRY_MODIFY_TREE_CONFLICT_DATA,
> + FALSE, pool);
> + if (err)
> + return svn_error_createf(pick_error_code(loggy), err,
> + _("Error recording tree conflicts in '%s'"),
> + svn_wc_adm_access_path(adm_access));
> +
> + loggy->entries_modified = TRUE;
> + }
> + if (loggy->entries_modified)
> {
> apr_hash_t *entries;
> SVN_ERR(svn_wc_entries_read(&entries, adm_access, TRUE, pool));
> @@ -2367,6 +2455,33 @@ svn_wc__loggy_upgrade_format(svn_stringb
> return SVN_NO_ERROR;
> }
>
> +svn_error_t *
> +svn_wc__loggy_add_tree_conflict(svn_stringbuf_t **log_accum,
> + const svn_wc_conflict_description_t *conflict,
> + svn_wc_adm_access_t *adm_access,
> + apr_pool_t *pool)
> +{
> + char *conflict_data;
> + apr_array_header_t *conflicts;
> +
> + /* ### TODO: implement write_one_tree_conflict(). */
> + conflicts = apr_array_make(pool, 1,
> + sizeof(svn_wc_conflict_description_t *));
> + APR_ARRAY_PUSH(conflicts, const svn_wc_conflict_description_t *) = conflict;
> +
> + SVN_ERR(svn_wc__write_tree_conflicts(&conflict_data, conflicts, pool));
> +
> + svn_xml_make_open_tag(log_accum, pool, svn_xml_self_closing,
> + SVN_WC__LOG_ADD_TREE_CONFLICT,
> + SVN_WC__LOG_ATTR_NAME,
> + SVN_WC_ENTRY_THIS_DIR,
> + SVN_WC__LOG_ATTR_DATA,
> + conflict_data,
> + NULL);
> +
> + return SVN_NO_ERROR;
> +}
> +
>
>
> /*** Helper to write log files ***/
>
> Modified: trunk/subversion/libsvn_wc/tree_conflicts.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/tree_conflicts.c?pathrev=34345&r1=34344&r2=34345
> ==============================================================================
> --- trunk/subversion/libsvn_wc/tree_conflicts.c Sat Nov 22 15:34:51 2008 (r34344)
> +++ trunk/subversion/libsvn_wc/tree_conflicts.c Sat Nov 22 15:46:26 2008 (r34345)
> @@ -629,50 +629,6 @@ svn_wc__loggy_del_tree_conflict(svn_stri
> }
>
> svn_error_t *
> -svn_wc__loggy_add_tree_conflict(svn_stringbuf_t **log_accum,
> - const svn_wc_conflict_description_t *conflict,
> - svn_wc_adm_access_t *adm_access,
> - apr_pool_t *pool)
> -{
> - const char *dir_path;
> - const svn_wc_entry_t *entry;
> - apr_array_header_t *conflicts;
> - char *conflict_data;
> - svn_wc_entry_t tmp_entry;
> -
> - /* Make sure the node is a directory.
> - * Otherwise we should not have been called. */
> - dir_path = svn_wc_adm_access_path(adm_access);
> - SVN_ERR(svn_wc_entry(&entry, dir_path, adm_access, TRUE, pool));
> - SVN_ERR_ASSERT(entry->kind == svn_node_dir);
> -
> - conflicts = apr_array_make(pool, 0,
> - sizeof(svn_wc_conflict_description_t *));
> - SVN_ERR(svn_wc__read_tree_conflicts(&conflicts, entry->tree_conflict_data,
> - dir_path, pool));
> -
> - /* If CONFLICTS has a tree conflict with the same victim path as the
> - * new conflict, then the working copy has been corrupted. */
> - if (svn_wc__tree_conflict_exists(conflicts,
> - svn_path_basename(conflict->path, pool),
> - pool))
> - return svn_error_create(SVN_ERR_WC_CORRUPT, NULL,
> - _("Attempt to add tree conflict that already exists"));
> -
> - APR_ARRAY_PUSH(conflicts, const svn_wc_conflict_description_t *) = conflict;
> -
> - SVN_ERR(svn_wc__write_tree_conflicts(&conflict_data, conflicts, pool));
> - tmp_entry.tree_conflict_data = apr_pstrdup(pool, conflict_data);
> -
> - SVN_ERR(svn_wc__loggy_entry_modify(log_accum, adm_access, dir_path,
> - &tmp_entry,
> - SVN_WC__ENTRY_MODIFY_TREE_CONFLICT_DATA,
> - pool));
> -
> - return SVN_NO_ERROR;
> -}
> -
> -svn_error_t *
> svn_wc__get_tree_conflict(svn_wc_conflict_description_t **tree_conflict,
> const char *victim_path,
> svn_wc_adm_access_t *adm_access,
>
> Modified: trunk/subversion/tests/cmdline/depth_tests.py
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/tests/cmdline/depth_tests.py?pathrev=34345&r1=34344&r2=34345
> ==============================================================================
> --- trunk/subversion/tests/cmdline/depth_tests.py Sat Nov 22 15:34:51 2008 (r34344)
> +++ trunk/subversion/tests/cmdline/depth_tests.py Sat Nov 22 15:46:26 2008 (r34345)
> @@ -19,7 +19,7 @@
> ######################################################################
>
> # General modules
> -import os
> +import os, re
>
> # Our testing module
> import svntest
> @@ -2327,10 +2327,11 @@ def make_depth_tree_conflicts(sbox):
> expected_status.tweak('A/mu',
> 'A/B', 'A/B/lambda',
> 'A/B/E', 'A/B/E/alpha', 'A/B/E/beta',
> + 'A/B/F',
> 'A/D/gamma',
> - status='D ')
> + status='D ', wc_rev=1)
> expected_status.tweak('A/mu', 'A/B', 'A/D/gamma',
> - treeconflict='C', wc_rev='1')
> + treeconflict='C', wc_rev=1)
>
> svntest.actions.run_and_verify_update(wc,
> expected_output,
> @@ -2339,6 +2340,27 @@ def make_depth_tree_conflicts(sbox):
> None, None, None, None, None, False,
> wc)
>
> +# Helper for text output.
> +def verify_lines(lines, regexes):
> + """Return True if each of the given regular expressions matches
> + exactly one line in the list of lines."""
> + for regex in regexes:
> + num_patterns_found = 0
> + for line in lines:
> + if re.search(regex, line):
> + num_patterns_found += 1
> + if num_patterns_found != 1:
> + print(("UNEXPECTED OUTPUT: " + str(num_patterns_found) +
> + " occurrences of '" + regex + "'"))
> + if svntest.main.verbose_mode:
> + print(" Actual output:")
> + for line in lines:
> + sys.stdout.write(" %s" % line)
> + print(" Expected regexes:")
> + for regex in regexes:
> + sys.stdout.write(" %s\n" % regex)
> + return False
> + return True
>
>
> def tree_conflicts_resolved_depth_empty(sbox):
> @@ -2383,11 +2405,13 @@ def tree_conflicts_resolved_depth_immedi
> m = j(A, 'mu')
> B = j(A, 'B')
>
> - svntest.actions.run_and_verify_svn(None,
> - ["Resolved conflicted state of '%s'\n" % m,
> - "Resolved conflicted state of '%s'\n" % B],
> - [],
> - 'resolved', '--depth=immediates', A)
> + exit_code, output, error = svntest.main.run_svn(None, 'resolved',
> + '--depth=immediates', A)
> + if not verify_lines(output, [
> + "Resolved conflicted state of '%s'\n" % m,
> + "Resolved conflicted state of '%s'\n" % B,
> + ]):
> + raise svntest.Failure("Unexpected tree-conflict resolution output")
>
>
> def tree_conflicts_resolved_depth_infinity(sbox):
> @@ -2403,12 +2427,14 @@ def tree_conflicts_resolved_depth_infini
> g = j(A, 'D', 'gamma')
>
>
> - svntest.actions.run_and_verify_svn(None,
> - ["Resolved conflicted state of '%s'\n" % m,
> - "Resolved conflicted state of '%s'\n" % B,
> - "Resolved conflicted state of '%s'\n" % g],
> - [],
> - 'resolved', '--depth=infinity', A)
> + exit_code, output, error = svntest.main.run_svn(None, 'resolved',
> + '--depth=infinity', A)
> + if not verify_lines(output, [
> + "Resolved conflicted state of '%s'\n" % m,
> + "Resolved conflicted state of '%s'\n" % B,
> + "Resolved conflicted state of '%s'\n" % g,
> + ]):
> + raise svntest.Failure("Unexpected tree-conflict resolution output")
>
>
> #----------------------------------------------------------------------
> @@ -2448,10 +2474,10 @@ test_list = [ None,
> excluded_path_update_operation,
> excluded_path_misc_operation,
> excluded_receive_remote_removal,
> - XFail(tree_conflicts_resolved_depth_empty),
> - XFail(tree_conflicts_resolved_depth_files),
> - XFail(tree_conflicts_resolved_depth_immediates),
> - XFail(tree_conflicts_resolved_depth_infinity),
> + tree_conflicts_resolved_depth_empty,
> + tree_conflicts_resolved_depth_files,
> + tree_conflicts_resolved_depth_immediates,
> + tree_conflicts_resolved_depth_infinity,
> ]
>
> if __name__ == "__main__":
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe_at_subversion.tigris.org
> For additional commands, e-mail: svn-help_at_subversion.tigris.org
>
Nice one.
~Neels
Received on 2008-11-26 04:00:43 CET