You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by gb...@apache.org on 2013/09/26 17:00:15 UTC
svn commit: r1526539 - in
/subversion/branches/invoke-diff-cmd-feature/subversion: include/svn_io.h
libsvn_subr/io.c tests/cmdline/diff_tests.py
Author: gbg
Date: Thu Sep 26 15:00:15 2013
New Revision: 1526539
URL: http://svn.apache.org/r1526539
Log:
On the invoke-diff-cmd branch: Add refactored version
__create_custom_diff_cmd as a handrolled O(n) alternative offering to
(old and suspect) svn_io_create_custom_diff_cmd. Update diff_tests.py
Nr. 49.
* subversion/include/svn_io.h
(__create_custom_diff_cmd): New internal function.(temporary
installation for comparison)
* subversion/libsvn_subr/io.c
(__create_custom_diff_cmd): New temporary internal function,
refactored copy of svn_io_create_custom_diff_cmd().
(svn_io_run_external_diff): Change call temporarily from
svn_io_create_custom_diff_cmd() to __create_custom_diff_cmd().
Adjust code to conform to 80 char limit.
* subversion/tests/cmdline/diff_tests.py
(diff_invoke_external_diffcmd): Fix typo in description, improve
tests.
Modified:
subversion/branches/invoke-diff-cmd-feature/subversion/include/svn_io.h
subversion/branches/invoke-diff-cmd-feature/subversion/libsvn_subr/io.c
subversion/branches/invoke-diff-cmd-feature/subversion/tests/cmdline/diff_tests.py
Modified: subversion/branches/invoke-diff-cmd-feature/subversion/include/svn_io.h
URL: http://svn.apache.org/viewvc/subversion/branches/invoke-diff-cmd-feature/subversion/include/svn_io.h?rev=1526539&r1=1526538&r2=1526539&view=diff
==============================================================================
--- subversion/branches/invoke-diff-cmd-feature/subversion/include/svn_io.h (original)
+++ subversion/branches/invoke-diff-cmd-feature/subversion/include/svn_io.h Thu Sep 26 15:00:15 2013
@@ -2393,6 +2393,18 @@ svn_io_create_custom_diff_cmd(const char
const char *cmd,
apr_pool_t *pool);
+/* temporary hand rolled version of the above */
+const char **
+__create_custom_diff_cmd(const char *label1,
+ const char *label2,
+ const char *label3,
+ const char *from,
+ const char *to,
+ const char *base,
+ const char *cmd,
+ apr_pool_t *pool);
+
+
/** Run the external diff command defined by the invoke-diff-cmd
* option.
*
Modified: subversion/branches/invoke-diff-cmd-feature/subversion/libsvn_subr/io.c
URL: http://svn.apache.org/viewvc/subversion/branches/invoke-diff-cmd-feature/subversion/libsvn_subr/io.c?rev=1526539&r1=1526538&r2=1526539&view=diff
==============================================================================
--- subversion/branches/invoke-diff-cmd-feature/subversion/libsvn_subr/io.c (original)
+++ subversion/branches/invoke-diff-cmd-feature/subversion/libsvn_subr/io.c Thu Sep 26 15:00:15 2013
@@ -3104,6 +3104,123 @@ svn_io_create_custom_diff_cmd(const char
return result;
}
+const char **
+__create_custom_diff_cmd(const char *label1,
+ const char *label2,
+ const char *label3,
+ const char *from,
+ const char *to,
+ const char *base,
+ const char *cmd,
+ apr_pool_t *pool)
+{
+ /* The idea is that we can use the delimiter's characters to find
+ the correct indexes, and thus avoid repeated str(str|cmp|etc)
+ operation which easily turn this into an O(n^3) operation. This
+ handrolled function is O(n). */
+
+ struct replace_tokens_t
+ {
+ const char *token;
+ } tokens_tab[] = { /* corresponds to delimiter*/
+ { from }, /* f1 */
+ { to }, /* f2 */
+ { base }, /* f3 */
+ { label1 }, /* l1 */
+ { label2 }, /* l2 */
+ { label3 }, /* l3 */
+ };
+
+ apr_array_header_t *words;
+ const char ** result;
+ int argv;
+ apr_pool_t *scratch_pool = svn_pool_create(pool);
+
+ /* split the user command into an array of words */
+ words = svn_cstring_split(cmd, " ", TRUE, scratch_pool);
+
+ result = apr_palloc(pool,
+ (words->nelts+1) * words->elt_size*sizeof(char *) );
+
+ for (argv = 0; argv < words->nelts; argv++)
+ {
+ int j = 0;
+ svn_stringbuf_t *word;
+
+ word = svn_stringbuf_create_empty(scratch_pool);
+ svn_stringbuf_appendcstr(word, APR_ARRAY_IDX(words, argv, char *));
+
+ /* a delimiter token always starts with a ';' */
+ if (word->data[j] == ';')
+ {
+ /* there are some cases where a '+' or other non-alphanumeric
+ character can be appended to a file name. It's simpler if
+ this is removed for the check and added back onto the result. */
+ char special_end_marker = 0;
+
+ /* if the last char is a non-alphanumeric, ie, + in ;f1+, remove it */
+ if (!isalnum(word->data[word->len-1]))
+ {
+ special_end_marker = word->data[word->len-1];
+ svn_stringbuf_remove(word, word->len-1, 1);
+ }
+
+ if (word->data[++j] == ';') /* likely to be a protected delimiter */
+ {
+ /* ensure that there is an unbroken line of ';' */
+ while (j < word->len && word->data[j++] == ';') ;
+
+ /* ensure that the end characters are [f|l][1-3]. */
+ if ( (j == (word->len - 1))
+
+ /* check the letters */
+ && (word->data[j-1] == 'f' || word->data[j-1] == 'l')
+
+ /*check that next char is 1,2 or 3 */
+ && (word->data[j] > 48) && (word->data[j] < 52) )
+ {
+ /* it's a protected delimiter, so we consume one ';' */
+ svn_stringbuf_remove(word, 0, 1);
+
+ /* put the special character back if it exists */
+ if (special_end_marker)
+ svn_stringbuf_appendbyte(word, special_end_marker);
+ }
+ }
+ else /* check that this is a regular delimiter (length 3) */
+ if ( (word->len == 3)
+
+ /* check the letters */
+ && (word->data[1] == 'f' || word->data[1] == 'l')
+
+ /*check that next char is 1,2 or 3 */
+ && (word->data[2] > 48) && (word->data[2] < 52) )
+ {
+ /* extract the delimiter's integer 1,2 or 3 */
+ j = (int)word->data[2]-49;
+
+ /* if it's 'l' it will be in position 4,5 or 6 */
+ if (word->data[1] == 'l')
+ j+=3;
+
+ /* pick the correct string parameter out of the tokens_tab */
+ svn_stringbuf_replace(word, 0,
+ 3, /* delimiter is always length 3 */
+ tokens_tab[j].token,
+ strlen(tokens_tab[j].token));
+
+ /* put the special character back if it exists */
+ if (special_end_marker)
+ svn_stringbuf_appendbyte(word, special_end_marker);
+ }
+ }
+ result[argv] = word->data;
+ }
+ result[argv] = NULL;
+ svn_pool_destroy(scratch_pool);
+ return result;
+}
+
svn_error_t *
svn_io_run_external_diff(const char *dir,
const char *label1,
@@ -3122,9 +3239,9 @@ svn_io_run_external_diff(const char *dir
if (0 == strlen(external_diff_cmd))
return svn_error_createf(SVN_ERR_INCORRECT_PARAMS, NULL, NULL);
- cmd = svn_io_create_custom_diff_cmd(label1, label2, NULL,
- tmpfile1, tmpfile2, NULL,
- external_diff_cmd, pool);
+ cmd = __create_custom_diff_cmd(label1, label2, NULL,
+ tmpfile1, tmpfile2, NULL,
+ external_diff_cmd, pool);
if (pexitcode == NULL)
pexitcode = &exitcode;
@@ -3137,7 +3254,8 @@ svn_io_run_external_diff(const char *dir
const char *failed_command = "";
for (i = 0; cmd[i]; ++i)
- failed_command = apr_pstrcat(pool, failed_command, cmd[i], " ", (char*) NULL);
+ failed_command = apr_pstrcat(pool, failed_command,
+ cmd[i], " ", (char*) NULL);
return svn_error_createf(SVN_ERR_EXTERNAL_PROGRAM, NULL,
_("'%s' was expanded to '%s' and returned %d"),
Modified: subversion/branches/invoke-diff-cmd-feature/subversion/tests/cmdline/diff_tests.py
URL: http://svn.apache.org/viewvc/subversion/branches/invoke-diff-cmd-feature/subversion/tests/cmdline/diff_tests.py?rev=1526539&r1=1526538&r2=1526539&view=diff
==============================================================================
--- subversion/branches/invoke-diff-cmd-feature/subversion/tests/cmdline/diff_tests.py (original)
+++ subversion/branches/invoke-diff-cmd-feature/subversion/tests/cmdline/diff_tests.py Thu Sep 26 15:00:15 2013
@@ -3262,7 +3262,7 @@ def diff_external_diffcmd(sbox):
# Check the correct parsing of arguments for an external diff tool
def diff_invoke_external_diffcmd(sbox):
- "svn diff --diff-invoke-cmd passes correct args"
+ "svn diff --invoke-diff-cmd passes correct args"
diff_script_path = os.path.abspath(".")+"/diff"
@@ -3281,16 +3281,28 @@ def diff_invoke_external_diffcmd(sbox):
expected_output = svntest.verify.ExpectedOutput([
"Index: iota\n",
"===================================================================\n",
- "iota (revision 1)\n",
+ # assert correct label ;l1
+ "iota (revision 1)\n",
+ # assert correct file ;f1
os.path.abspath(svntest.wc.text_base_path("iota")) + "\n",
+ # assert correct label ;l2
"iota (working copy)\n",
- os.path.abspath("iota") + "\n"])
+ # assert correct file ;f2
+ os.path.abspath("iota") + "\n",
+ # assert special end char is added ;f1+ -> (file)+
+ os.path.abspath(svntest.wc.text_base_path("iota")) + "+\n",
+ ";f1\n", # assert removal of ';' @ lower boundary '1' ;;f1 -> ;f1
+ ";;l3\n", # assert removal of ';' @ high boundary '3' ;;;l3 -> ;;l3
+ ";;fo\n", # assert only eligible tokens are modified ;;fo -> ;;fo
+ ";f1o\n" # assert correct length test of token ;f1o -> ;f1o
+ ])
svntest.actions.run_and_verify_svn(None, expected_output, [],
'diff',
- '--invoke-diff-cmd='+diff_script_path+' ;l1 ;f1 ;l2 ;f2',
+ '--invoke-diff-cmd='+diff_script_path+' ;l1 ;f1 ;l2 ;f2 ;f1+ ;;f1 ;;;l3 ;;fo ;f1o',
iota_path)
+
#----------------------------------------------------------------------
# Diffing an unrelated repository URL against working copy with
# local modifications (i.e. not committed). This is issue #3295 (diff