This patch implements fixed size keywords for Subversion, see issue #2095 for more details. Patch provided by Tom Molesworth , regression tests by Jani Averbach . * subversion/libsvn_subr/subst.c (translate_keyword_subst): Added fixed length keyword handling * subversion/tests/clients/cmdline/diff_tests.py (verify_exluded_output): New helper function (diff_keywords): New test, check that keywords won't show up on diff, and that they show when they should. * subversion/tests/clients/cmdline/trans_tests.py (keywords_from_birth): Added testing of fixed length keywords. (setup_working_copy): Signature changed, it now takes second argument: length of value field (value_len) for fixed keywords. (check_keywords): New helper function (fixed_length_keywords_path): New test path * subversion/tests/clients/cmdline/svntest/main.py (canonize_url): New helper function * doc/book/TODO Added entry for fixed length keywords. Index: subversion/libsvn_subr/subst.c =================================================================== --- subversion/libsvn_subr/subst.c (revision 11396) +++ subversion/libsvn_subr/subst.c (working copy) @@ -239,10 +239,59 @@ buf_ptr = buf + 1 + keyword_len; + /* Check for fixed-length expansion. + * The format of fixed length keyword and its data is + * Unexpanded keyword: "$keyword:: $" + * Expanded keyword: "$keyword:: value $" + * Expanded kw with filling: "$keyword:: value $" + * Truncated keyword: "$keyword:: longval#$" + */ + if ((buf_ptr[0] == ':') /* first char after keyword is ':' */ + && (buf_ptr[1] == ':') /* second char after keyword is ':' */ + && (buf_ptr[2] == ' ') /* third char after keyword is ' ' */ + && ((buf[*len - 2] == ' ') /* has ' ' for next to last character */ + || (buf[*len - 2] == '#')) /* .. or has '#' for next to last character */ + && ((6 + keyword_len) < *len)) /* holds "$kw:: x $" at least */ + { + /* This is fixed length keyword, so *len remains unchanged */ + apr_size_t max_value_len = *len - (6 + keyword_len); + + if (! value) + { + /* no value, so unexpand */ + buf_ptr += 2; + while (*buf_ptr != '$') + *(buf_ptr++) = ' '; + } + else if (value && (max_value_len > 0)) + { + if (value->len <= max_value_len) + { /* replacement not as long as template, pad with spaces */ + strncpy (buf_ptr + 3, value->data, value->len); + buf_ptr += 3 + value->len; + while (*buf_ptr != '$') + *(buf_ptr++) = ' '; + } + else + { + /* replacement needs truncating */ + strncpy (buf_ptr + 3, value->data, max_value_len); + buf[*len - 2] = '#'; + buf[*len - 1] = '$'; + } + } + else + { + /* if we are here, we have a logic error in the code */ + assert(1 == 0); + } + return TRUE; + } + /* Check for unexpanded keyword. */ - if ((buf_ptr[0] == '$') /* "$keyword$" */ - || ((buf_ptr[0] == ':') - && (buf_ptr[1] == '$'))) /* "$keyword:$" */ + else if ((buf_ptr[0] == '$') /* "$keyword$" */ + || ((buf_ptr[0] == ':') + && (buf_ptr[1] == '$'))) /* "$keyword:$" */ { /* unexpanded... */ if (value) Index: subversion/tests/clients/cmdline/diff_tests.py =================================================================== --- subversion/tests/clients/cmdline/diff_tests.py (revision 11396) +++ subversion/tests/clients/cmdline/diff_tests.py (working copy) @@ -104,6 +104,13 @@ else: raise svntest.Failure +def verify_excluded_output(diff_output, excluded): + "verify given line does not exist in diff output as diff line" + for line in diff_output: + if re.match("^(\\+|-)%s" % re.escape(excluded), line): + print 'Sought: %s' % excluded + print 'Found: %s' % line + raise svntest.Failure def extract_diff_path(line): l2 = line[(line.find("(")+1):] @@ -1571,6 +1578,90 @@ os.chdir(was_cwd) +#---------------------------------------------------------------------- +def diff_keywords(sbox): + "ensure that diff won't show keywords" + + sbox.build() + + iota_path = os.path.join(sbox.wc_dir, 'iota') + + svntest.actions.run_and_verify_svn(None, None, [], + 'ps', + 'svn:keywords', + 'Id Rev Date', + iota_path) + + fp = open(iota_path, 'w') + fp.write("$Date$\n") + fp.write("$Id$\n") + fp.write("$Rev$\n") + fp.write("$Date::%s$\n" % (' ' * 80)) + fp.write("$Id::%s$\n" % (' ' * 80)) + fp.write("$Rev::%s$\n" % (' ' * 80)) + fp.close() + + svntest.actions.run_and_verify_svn(None, None, [], + 'ci', '-m', 'keywords', sbox.wc_dir) + + svntest.main.file_append(iota_path, "bar\n") + svntest.actions.run_and_verify_svn(None, None, [], + 'ci', '-m', 'added bar', sbox.wc_dir) + + svntest.actions.run_and_verify_svn(None, None, [], + 'up', sbox.wc_dir) + + diff_output, err = svntest.actions.run_and_verify_svn(None, None, [], + 'diff', + '-r', 'prev:head', + sbox.wc_dir) + verify_expected_output(diff_output, "+bar") + verify_excluded_output(diff_output, "$Date:") + verify_excluded_output(diff_output, "$Rev:") + verify_excluded_output(diff_output, "$Id:") + + diff_output, err = svntest.actions.run_and_verify_svn(None, None, [], + 'diff', + '-r', 'head:prev', + sbox.wc_dir) + verify_expected_output(diff_output, "-bar") + verify_excluded_output(diff_output, "$Date:") + verify_excluded_output(diff_output, "$Rev:") + verify_excluded_output(diff_output, "$Id:") + + # Check fixed length keywords will show up + # when the length of keyword has changed + fp = open(iota_path, 'w') + fp.write("$Date$\n") + fp.write("$Id$\n") + fp.write("$Rev$\n") + fp.write("$Date::%s$\n" % (' ' * 79)) + fp.write("$Id::%s$\n" % (' ' * 79)) + fp.write("$Rev::%s$\n" % (' ' * 79)) + fp.close() + + svntest.actions.run_and_verify_svn(None, None, [], + 'ci', '-m', 'keywords 2', sbox.wc_dir) + svntest.actions.run_and_verify_svn(None, None, [], + 'up', sbox.wc_dir) + + diff_output, err = svntest.actions.run_and_verify_svn(None, None, [], + 'diff', + '-r', 'prev:head', + sbox.wc_dir) + # these should show up + verify_expected_output(diff_output, "+$Id:: ") + verify_expected_output(diff_output, "-$Id:: ") + verify_expected_output(diff_output, "-$Rev:: ") + verify_expected_output(diff_output, "+$Rev:: ") + verify_expected_output(diff_output, "-$Date:: ") + verify_expected_output(diff_output, "+$Date:: ") + # ... and these won't + verify_excluded_output(diff_output, "$Date: ") + verify_excluded_output(diff_output, "$Rev: ") + verify_excluded_output(diff_output, "$Id: ") + + ######################################################################## #Run the tests @@ -1600,6 +1691,7 @@ check_for_omitted_prefix_in_path_component, diff_renamed_file, diff_within_renamed_dir, + diff_keywords, ] if __name__ == '__main__': Index: subversion/tests/clients/cmdline/trans_tests.py =================================================================== --- subversion/tests/clients/cmdline/trans_tests.py (revision 11396) +++ subversion/tests/clients/cmdline/trans_tests.py (working copy) @@ -93,7 +93,20 @@ embd_author_rev_exp_path = '' embd_bogus_keywords_path = '' -def setup_working_copy(wc_dir): +def check_keywords(actual_kw, expected_kw, name): + """A Helper function to compare two keyword lists""" + + if len(actual_kw) != len(expected_kw): + print "Keyword lists are different by size" + raise svntest.Failure + + for i in range(0,len(actual_kw)): + if actual_kw[i] != expected_kw[i]: + print '%s, Expected: %s' % (name, expected_kw[i][:-1]) + print '%s, Got: %s' % (name, actual_kw[i][:-1]) + raise svntest.Failure + +def setup_working_copy(wc_dir, value_len): """Setup a standard test working copy, then create (but do not add) various files for testing translation.""" @@ -107,6 +120,7 @@ global embd_author_rev_unexp_path global embd_author_rev_exp_path global embd_bogus_keywords_path + global fixed_length_keywords_path # NOTE: Only using author and revision keywords in tests for now, # since they return predictable substitutions. @@ -123,6 +137,7 @@ embd_author_rev_unexp_path = os.path.join(wc_dir, 'embd_author_rev_unexp') embd_author_rev_exp_path = os.path.join(wc_dir, 'embd_author_rev_exp') embd_bogus_keywords_path = os.path.join(wc_dir, 'embd_bogus_keywords') + fixed_length_keywords_path = os.path.join(wc_dir, 'fixed_length_keywords') svntest.main.file_append (author_rev_unexp_path, "$Author$\n$Rev$") svntest.main.file_append (author_rev_exp_path, "$Author: blah $\n$Rev: 0 $") @@ -137,8 +152,30 @@ "blue $Author: blah $ fish$Rev: 0 $\nI fish") svntest.main.file_append (embd_bogus_keywords_path, "you fish $Arthur$then\n we$Rev0$ \n\nchew fish") - + keyword_test_targets = [ + # User tries to shoot him or herself on the foot + "$URL::$\n", + "$URL:: $\n", + "$URL:: $\n", + # Following are valid entries + "$URL:: $\n", + "$URL:: %s $\n" % (' ' * (value_len-1)), + "$URL:: %s $\n" % (' ' * value_len), + # Check we will clean the truncate marker when the value fits exactly + "$URL:: %s#$\n" % ('a' * value_len), + "$URL:: %s $\n" % (' ' * (value_len+1)), + # These are syntactically wrong + "$URL::x%s $\n" % (' ' * value_len), + "$URL:: %sx$\n" % (' ' * value_len), + "$URL::x%sx$\n" % (' ' * value_len) + ] + + for i in keyword_test_targets: + svntest.main.file_append (fixed_length_keywords_path, i) + + + ### Helper functions for setting/removing properties # Set the property keyword for PATH. Turn on all possible keywords. @@ -168,7 +205,13 @@ sbox.build() wc_dir = sbox.wc_dir - setup_working_copy (wc_dir) + canonized_repo_url = svntest.main.canonize_url(sbox.repo_url) + if canonized_repo_url[-1:] != '/': + url_expand_test_data = canonized_repo_url + '/fixed_length_keywords' + else: + url_expand_test_data = canonized_repo_url + 'fixed_length_keywords' + + setup_working_copy (wc_dir, len(url_expand_test_data)) # Add all the files expected_status = svntest.actions.get_virginal_state(wc_dir, 1) @@ -183,6 +226,7 @@ 'embd_author_rev_unexp' : Item(status='A ', wc_rev=0, repos_rev=1), 'embd_author_rev_exp' : Item(status='A ', wc_rev=0, repos_rev=1), 'embd_bogus_keywords' : Item(status='A ', wc_rev=0, repos_rev=1), + 'fixed_length_keywords' : Item(status='A ', wc_rev=0, repos_rev=1), }) svntest.main.run_svn (None, 'add', author_rev_unexp_path) @@ -195,6 +239,7 @@ svntest.main.run_svn (None, 'add', embd_author_rev_unexp_path) svntest.main.run_svn (None, 'add', embd_author_rev_exp_path) svntest.main.run_svn (None, 'add', embd_bogus_keywords_path) + svntest.main.run_svn (None, 'add', fixed_length_keywords_path) svntest.actions.run_and_verify_status(wc_dir, expected_status) @@ -205,6 +250,7 @@ keywords_on (id_unexp_path) keywords_on (id_exp_path) keywords_on (embd_author_rev_exp_path) + keywords_on (fixed_length_keywords_path) # Commit. expected_output = svntest.wc.State(wc_dir, { @@ -218,6 +264,7 @@ 'embd_author_rev_unexp' : Item(verb='Adding'), 'embd_author_rev_exp' : Item(verb='Adding'), 'embd_bogus_keywords' : Item(verb='Adding'), + 'fixed_length_keywords' : Item(verb='Adding'), }) svntest.actions.run_and_verify_commit (wc_dir, expected_output, @@ -259,8 +306,48 @@ print "Id expansion failed for", id_exp_path raise svntest.Failure fp.close() + + # Check fixed length keywords. + kw_workingcopy = [ + '$URL::$\n', + '$URL:: $\n', + '$URL:: $\n', + '$URL:: %s#$\n' % url_expand_test_data[0:1], + '$URL:: %s#$\n' % url_expand_test_data[:-1], + '$URL:: %s $\n' % url_expand_test_data, + '$URL:: %s $\n' % url_expand_test_data, + '$URL:: %s $\n'% url_expand_test_data, + '$URL::x%s $\n' % (' ' * len(url_expand_test_data)), + '$URL:: %sx$\n' % (' ' * len(url_expand_test_data)), + '$URL::x%sx$\n' % (' ' * len(url_expand_test_data)) + ] + fp = open(fixed_length_keywords_path, 'r') + actual_workingcopy_kw = fp.readlines() + fp.close() + check_keywords(actual_workingcopy_kw, kw_workingcopy, "working copy") + # Check text base for fixed length keywords. + kw_textbase = [ + '$URL::$\n', + '$URL:: $\n', + '$URL:: $\n', + '$URL:: $\n', + '$URL:: %s $\n' % (' ' * len(url_expand_test_data[:-1])), + '$URL:: %s $\n' % (' ' * len(url_expand_test_data)), + '$URL:: %s $\n' % (' ' * len(url_expand_test_data)), + '$URL:: %s $\n'% (' ' * len(url_expand_test_data)), + '$URL::x%s $\n' % (' ' * len(url_expand_test_data)), + '$URL:: %sx$\n' % (' ' * len(url_expand_test_data)), + '$URL::x%sx$\n' % (' ' * len(url_expand_test_data)) + ] + + fp = open(os.path.join(wc_dir, '.svn', 'text-base', + 'fixed_length_keywords.svn-base'), 'rb') + actual_textbase_kw = fp.readlines() + fp.close() + check_keywords(actual_textbase_kw, kw_textbase, "text base") + def enable_translation(sbox): "enable translation, check status, commit" Index: subversion/tests/clients/cmdline/svntest/main.py =================================================================== --- subversion/tests/clients/cmdline/svntest/main.py (revision 11396) +++ subversion/tests/clients/cmdline/svntest/main.py (working copy) @@ -410,6 +410,16 @@ current_repo_url = string.replace(current_repo_url, '\\', '/') +def canonize_url(input): + "Canonize the url, if the schema is unknown, returns intact input" + + m = re.match(r"^((file://)|((svn|svn\+ssh|http|https)(://)))", input) + if m: + schema = m.group(1) + return schema + re.sub(r'//*', '/', input[len(schema):]) + else: + return input + ###################################################################### # Sandbox handling Index: doc/book/TODO =================================================================== --- doc/book/TODO (revision 11396) +++ doc/book/TODO (working copy) @@ -91,3 +91,9 @@ ============================ * svn log --limit + * Fixed length keywords + The format of fixed length keyword and its data is + - Unexpanded keyword: "$keyword:: $" + - Expanded keyword: "$keyword:: value $" + - Expanded kw with filling: "$keyword:: value $" + - Truncated keyword: "$keyword:: longval#$"