A few comments inline. > -----Oorspronkelijk bericht----- > Van: [email protected] [mailto:[email protected]] > Verzonden: zaterdag 26 september 2015 02:47 > Aan: [email protected] > Onderwerp: svn commit: r1705391 - in /subversion/trunk: ./ > subversion/include/ subversion/include/private/ subversion/libsvn_client/ > subversion/libsvn_diff/ subversion/libsvn_fs_x/ subversion/tests/cmdline/ > subversion/tests/libsvn_diff/ > > Author: danielsh > Date: Sat Sep 26 00:46:52 2015 > New Revision: 1705391 > > URL: http://svn.apache.org/viewvc?rev=1705391&view=rev > Log: > patch: Parse 'old mode' / 'new mode' line from git diffs and translate them to > svn:executable propchanges. > > This merges the 'patch-exec' branch into trunk. > > --- > > The following are the major changes. See the branch for detailed changes. > > * subversion/include/svn_diff.h > (svn_patch_t.old_executable_p, svn_patch_t.new_executable_p): New > members. > > * subversion/libsvn_client/patch.c > (contradictory_executability): New function. > (init_patch_target, apply_one_patch): > Translate mode changes to svn:executable changes. > > * subversion/include/private/svn_diff_private.h > (svn_diff_hunk__create_adds_single_line, > svn_diff_hunk__create_deletes_single_line): New functions. > > * subversion/libsvn_diff/parse-diff.c > (add_or_delete_single_line, > svn_diff_hunk__create_adds_single_line, > svn_diff_hunk__create_deletes_single_line): New functions. > (parse_state::state_old_mode_seen, > parse_state::state_git_mode_seen): New enumerators. > (parse_bits_into_executability, git_old_mode, git_new_mode): New > functions. > (git_deleted_file, git_new_file): Parse file mode. > (transitions): Parse "old mode" and "new mode" lines. > > * subversion/tests/cmdline/patch_tests.py, > * subversion/tests/libsvn_diff/parse-diff-test.c: > Add unit tests. > > Modified: > subversion/trunk/ (props changed) > subversion/trunk/subversion/include/private/svn_diff_private.h > subversion/trunk/subversion/include/svn_diff.h > subversion/trunk/subversion/libsvn_client/patch.c > subversion/trunk/subversion/libsvn_diff/parse-diff.c > subversion/trunk/subversion/libsvn_fs_x/ (props changed) > subversion/trunk/subversion/tests/cmdline/patch_tests.py > subversion/trunk/subversion/tests/libsvn_diff/parse-diff-test.c > > Propchange: subversion/trunk/ > ------------------------------------------------------------------------------ > --- svn:mergeinfo (original) > +++ svn:mergeinfo Sat Sep 26 00:46:52 2015 > @@ -62,6 +62,7 @@ > /subversion/branches/multi-layer-moves:1239019-1300930 > /subversion/branches/nfc-nfd-aware-client:870276,870376 > /subversion/branches/node_pool:1304828-1305388 > +/subversion/branches/patch-exec:1692717-1705390 > > /subversion/branches/performance:979193,980118,981087,981090,981189, > 981194,981287,981684,981827,982043,982355,983398,983406,983430,98347 > 4,983488,983490,983760,983764,983766,983770,984927,984973,984984,985 > 014,985037,985046,985472,985477,985482,985487- > 985488,985493,985497,985500,985514,985601,985603,985606,985669,98567 > 3,985695,985697,986453,986465,986485,986491- > 986492,986517,986521,986605,986608,986817,986832,987865,987868- > 987869,987872,987886- > 987888,987893,988319,988898,990330,990533,990535- > 990537,990541,990568,990572,990574- > 990575,990600,990759,992899,992904,992911,993127,993141,994956,99547 > 8,995507,995603,998012,998858,999098,1001413,1001417,1004291,1022668 > ,1022670,1022676,1022715,1022719,1025660,1025672,1027193,1027203,102 > 7206,1027214,1027227,1028077,1028092,1028094,1028104,1028107,102811 > 1,1028354,1029038,1029042-1029043,1029054-1029055,1029062- > 1029063,1029078,1029080,1029090,1029092- > 1029093,1029111,1029151,1029158,1029229-1029230,1029232,1029335- > 1029336,1029339-1029340,1029342,10 > > 29344,1030763,1030827,1031203,1031235,1032285,1032333,1033040,10330 > 57,1033294,1035869,1035882,1039511,1043705,1053735,1056015,1066452,1 > 067683,1067697-1078365 > /subversion/branches/pin-externals:1643757-1659392 > /subversion/branches/py-tests-as-modules:956579-1033052 >
<snip> > Modified: subversion/trunk/subversion/include/svn_diff.h > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_diff > .h?rev=1705391&r1=1705390&r2=1705391&view=diff > ================================================================ > ============== > --- subversion/trunk/subversion/include/svn_diff.h (original) > +++ subversion/trunk/subversion/include/svn_diff.h Sat Sep 26 00:46:52 > +++ 2015 > @@ -1288,6 +1288,24 @@ typedef struct svn_patch_t { > * to apply it as parsed from the file. > * @since New in 1.10. */ > svn_diff_binary_patch_t *binary_patch; > + > + /** The old and new executability bits, as retrieved from the patch file. > + * > + * A patch may specify an executability change via @a old_executable_p > and > + * / @a new_executable_p, via a #SVN_PROP_EXECUTABLE propchange > hunk, or both > + * ways; however, if both ways are used, they must specify the same > semantic > + * change. > + * > + * #svn_tristate_unknown indicates the patch does not specify the > + * corresponding bit. > + * > + * @since New in 1.10. > + */ > + /* ### This is currently not parsed out of "index" lines (where it > + * ### serves as an assertion of the executability state, without > + * ### changing it). */ > + svn_tristate_t old_executable_p; > + svn_tristate_t new_executable_p; > } svn_patch_t; I'm not sure if we should leave this comment here in the long term... > > /** An opaque type representing an open patch file. > > Modified: subversion/trunk/subversion/libsvn_client/patch.c > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/pa > tch.c?rev=1705391&r1=1705390&r2=1705391&view=diff > ================================================================ > ============== > --- subversion/trunk/subversion/libsvn_client/patch.c (original) > +++ subversion/trunk/subversion/libsvn_client/patch.c Sat Sep 26 00:46:52 > 2015 > @@ -46,6 +46,7 @@ > #include "private/svn_eol_private.h" > #include "private/svn_wc_private.h" > #include "private/svn_dep_compat.h" > +#include "private/svn_diff_private.h" > #include "private/svn_string_private.h" > #include "private/svn_subr_private.h" > #include "private/svn_sorts_private.h" > @@ -914,6 +915,41 @@ choose_target_filename(const svn_patch_t > return (old < new) ? patch->old_filename : patch->new_filename; > } > > +/* Return whether the svn:executable proppatch and the out-of-band > + * executability metadata contradict each other, assuming both are present. > + */ > +static svn_boolean_t > +contradictory_executability(const svn_patch_t *patch, > + const prop_patch_target_t *target) > +{ > + switch (target->operation) > + { > + case svn_diff_op_added: > + return patch->new_executable_p == svn_tristate_false; > + > + case svn_diff_op_deleted: > + return patch->new_executable_p == svn_tristate_true; > + > + case svn_diff_op_unchanged: > + /* ### Can this happen? */ > + return (patch->old_executable_p != svn_tristate_unknown > + && patch->new_executable_p != svn_tristate_unknown > + && patch->old_executable_p != patch->new_executable_p); > + > + case svn_diff_op_modified: > + /* Can't happen: the property should only ever be added or deleted, > + * but never modified from one valid value to another. */ > + return (patch->old_executable_p != svn_tristate_unknown > + && patch->new_executable_p != svn_tristate_unknown > + && patch->old_executable_p == patch->new_executable_p); These can happen when the svn:executable property just changes value (E.g. from '*' to 'x'). That should not happen, but it can. > + > + default: > + /* Can't happen: the proppatch parser never generates other values. > */ > + SVN_ERR_MALFUNCTION_NO_RETURN(); > + } > +} > + > + > /* Attempt to initialize a *PATCH_TARGET structure for a target file > * described by PATCH. Use working copy context WC_CTX. > * STRIP_COUNT specifies the number of leading path components > @@ -956,6 +992,9 @@ init_patch_target(patch_target_t **patch > target->kind_on_disk = svn_node_none; > target->content = content; > target->prop_targets = apr_hash_make(result_pool); > + if (patch->new_executable_p != svn_tristate_unknown) > + /* May also be set by apply_hunk(). */ > + target->has_prop_changes = TRUE; > > SVN_ERR(resolve_target_path(target, choose_target_filename(patch), > root_abspath, strip_count, has_text_changes, > @@ -1129,6 +1168,7 @@ init_patch_target(patch_target_t **patch > if (! target->skipped) > { > apr_hash_index_t *hi; > + prop_patch_target_t *prop_executable_target; > > for (hi = apr_hash_first(result_pool, patch->prop_patches); > hi; > @@ -1145,6 +1185,99 @@ init_patch_target(patch_target_t **patch > result_pool, scratch_pool)); > svn_hash_sets(target->prop_targets, prop_name, prop_target); > } > + > + /* Now, check for an out-of-band mode change and convert it to > + * an svn:executable property patch. */ > + prop_executable_target = svn_hash_gets(target->prop_targets, > + SVN_PROP_EXECUTABLE); > + if (patch->new_executable_p != svn_tristate_unknown > + && prop_executable_target) > + { > + if (contradictory_executability(patch, prop_executable_target)) > + /* Invalid input: specifies both git-like "new mode" lines > and > + * svn-like addition/removal of svn:executable. > + * > + * If this were merely a hunk that didn't apply, we'd reject > it > + * and move on. However, this is a self-contradictory hunk; > + * it has no unambiguous interpretation. Therefore: */ > + return svn_error_createf(SVN_ERR_INVALID_INPUT, NULL, > + _("Invalid patch: specifies " > + "contradicting mode changes and " > + "%s changes (for '%s')"), > + SVN_PROP_EXECUTABLE, > + target->local_abspath); > + else > + /* We have two representations of the same change. > + * > + * In the caller, there will be two hunk_info_t's for the > + * patch: one generated from the property diff and one > + * generated from the out-of-band mode change. Both hunks > will > + * be processed, but the output will be as though there was > + * just one hunk. > + * > + * The reason there will be only a single notification is not > + * specific to SVN_PROP_EXECUTABLE but generic to all > property > + * patches: if a patch file contains two identical property > + * hunks (e.g., > + * svn ps k v iota; svn diff iota >patch; svn diff iota > >>patch > + * ), applying the patch file will only produce a single ' U' > + * notification. > + * > + * In contrast, the same for file-content hunks will result > in > + * a 'U' notification followed by a 'G' notification. (The > 'U' > + * for the first copy of the hunk; the 'G' for the second.) > + */ > + ; > + } > + else if (patch->new_executable_p != svn_tristate_unknown > + && !prop_executable_target) > + { > + svn_diff_operation_kind_t operation; > + svn_boolean_t nothing_to_do = FALSE; > + prop_patch_target_t *prop_target; > + > + if (patch->old_executable_p == patch->new_executable_p) > + { > + /* Noop change. */ > + operation = svn_diff_op_unchanged; > + } > + else switch (patch->old_executable_p) > + { > + case svn_tristate_false: > + /* Made executable. */ > + operation = svn_diff_op_added; > + break; > + > + case svn_tristate_true: > + /* Made non-executable. */ > + operation = svn_diff_op_deleted; > + break; > + > + case svn_tristate_unknown: > + if (patch->new_executable_p == svn_tristate_true) > + /* New, executable file. */ > + operation = svn_diff_op_added; > + else > + /* New, non-executable file. That's not a change. */ > + nothing_to_do = TRUE; > + break; > + > + default: > + /* NOTREACHED */ > + SVN_ERR_MALFUNCTION(); > + } > + > + if (! nothing_to_do) > + { > + SVN_ERR(init_prop_target(&prop_target, > + SVN_PROP_EXECUTABLE, > + operation, > + wc_ctx, target->local_abspath, > + result_pool, scratch_pool)); > + svn_hash_sets(target->prop_targets, SVN_PROP_EXECUTABLE, > + prop_target); > + } > + } > } > } > > @@ -2425,6 +2558,50 @@ apply_one_patch(patch_target_t **patch_t > } > } > > + /* Match implied property hunks. */ > + if (patch->new_executable_p != svn_tristate_unknown > + && svn_hash_gets(target->prop_targets, SVN_PROP_EXECUTABLE)) > + { > + hunk_info_t *hi; > + svn_diff_hunk_t *hunk; > + prop_patch_target_t *prop_target = svn_hash_gets(target- > >prop_targets, > + SVN_PROP_EXECUTABLE); > + const char *const value = SVN_PROP_EXECUTABLE_VALUE; > + > + switch (prop_target->operation) > + { > + case svn_diff_op_added: > + SVN_ERR(svn_diff_hunk__create_adds_single_line(&hunk, value, > patch, > + result_pool, > + iterpool)); > + break; > + > + case svn_diff_op_deleted: > + SVN_ERR(svn_diff_hunk__create_deletes_single_line(&hunk, value, > + patch, > + result_pool, > + iterpool)); > + break; > + > + case svn_diff_op_unchanged: > + /* ### What to do? */ > + break; > + > + default: > + SVN_ERR_MALFUNCTION(); > + } > + > + /* Derive a hunk_info from hunk. */ > + SVN_ERR(get_hunk_info(&hi, target, prop_target->content, > + hunk, 0 /* fuzz */, 0 /* previous_offset */, > + ignore_whitespace, > + TRUE /* is_prop_hunk */, > + cancel_func, cancel_baton, > + result_pool, iterpool)); > + if (! hi->already_applied) > + APR_ARRAY_PUSH(prop_target->content->hunks, hunk_info_t *) = hi; > + } > + > /* Apply or reject property hunks. */ > for (hash_index = apr_hash_first(scratch_pool, target->prop_targets); > hash_index; > > Modified: subversion/trunk/subversion/libsvn_diff/parse-diff.c > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_diff/pars > e-diff.c?rev=1705391&r1=1705390&r2=1705391&view=diff > ================================================================ > ============== > --- subversion/trunk/subversion/libsvn_diff/parse-diff.c (original) > +++ subversion/trunk/subversion/libsvn_diff/parse-diff.c Sat Sep 26 00:46:52 > 2015 > @@ -40,6 +40,7 @@ > > #include "private/svn_eol_private.h" > #include "private/svn_dep_compat.h" > +#include "private/svn_diff_private.h" > #include "private/svn_sorts_private.h" > > #include "diff.h" > @@ -108,6 +109,116 @@ struct svn_diff_binary_patch_t { > svn_filesize_t dst_filesize; /* Expanded/final size */ > }; > > +/* Common guts of svn_diff_hunk__create_adds_single_line() and > + * svn_diff_hunk__create_deletes_single_line(). > + * > + * ADD is TRUE if adding and FALSE if deleting. > + */ > +static svn_error_t * > +add_or_delete_single_line(svn_diff_hunk_t **hunk_out, > + const char *line, > + svn_patch_t *patch, > + svn_boolean_t add, > + apr_pool_t *result_pool, > + apr_pool_t *scratch_pool) > +{ > + svn_diff_hunk_t *hunk = apr_palloc(result_pool, sizeof(*hunk)); As pcalloc() would be safer here... > + static const char *hunk_header[] = { "@@ -1 +0,0 @@\n", "@@ -0,0 +1 > @@\n" }; > + const unsigned len = strlen(line); > + /* The +1 is for the 'plus' start-of-line character. */ > + const apr_off_t end = STRLEN_LITERAL(hunk_header[add]) + (1 + len); > + /* The +1 is for the second \n. */ > + svn_stringbuf_t *buf = svn_stringbuf_create_ensure(end + 1, > scratch_pool); > + > + hunk->patch = patch; > + > + /* hunk->apr_file is created below. */ > + > + hunk->diff_text_range.start = STRLEN_LITERAL(hunk_header[add]); > + hunk->diff_text_range.current = STRLEN_LITERAL(hunk_header[add]); > + hunk->diff_text_range.end = end; > + > + if (add) > + { > + hunk->original_text_range.start = 0; /* There's no "original" text. */ > + hunk->original_text_range.current = 0; > + hunk->original_text_range.end = 0; > + > + hunk->modified_text_range.start = STRLEN_LITERAL(hunk_header[add]); > + hunk->modified_text_range.current = > STRLEN_LITERAL(hunk_header[add]); > + hunk->modified_text_range.end = end; > + > + hunk->original_start = 0; > + hunk->original_length = 0; > + > + hunk->modified_start = 1; > + hunk->modified_length = 1; > + } > + else /* delete */ > + { > + hunk->original_text_range.start = STRLEN_LITERAL(hunk_header[add]); > + hunk->original_text_range.current = > STRLEN_LITERAL(hunk_header[add]); > + hunk->original_text_range.end = end; > + > + hunk->modified_text_range.start = 0; /* There's no "original" text. */ > + hunk->modified_text_range.current = 0; > + hunk->modified_text_range.end = 0; > + > + hunk->original_start = 1; > + hunk->original_length = 1; > + > + hunk->modified_start = 0; > + hunk->modified_length = 0; /* setting to '1' works too */ > + } > + > + hunk->leading_context = 0; > + hunk->trailing_context = 0; As this patch forgets to set the new booleans added on trunk to signal no EOL markers here. You might need to set one of them to true though, as I think you are trying to add a property with no end of line. > + > + /* Create APR_FILE and put just a hunk in it (without a diff header). > + * Save the offset of the last byte of the diff line. */ > + svn_stringbuf_appendbytes(buf, hunk_header[add], > + STRLEN_LITERAL(hunk_header[add])); > + svn_stringbuf_appendbyte(buf, add ? '+' : '-'); > + svn_stringbuf_appendbytes(buf, line, len); > + svn_stringbuf_appendbyte(buf, '\n'); > + > + SVN_ERR(svn_io_open_unique_file3(&hunk->apr_file, NULL /* filename > */, > + NULL /* system tempdir */, > + svn_io_file_del_on_pool_cleanup, > + result_pool, scratch_pool)); > + SVN_ERR(svn_io_file_write_full(hunk->apr_file, > + buf->data, buf->len, > + NULL, scratch_pool)); > + /* No need to seek. */ > + > + *hunk_out = hunk; > + return SVN_NO_ERROR; > +} > + > +svn_error_t * > +svn_diff_hunk__create_adds_single_line(svn_diff_hunk_t **hunk_out, > + const char *line, > + svn_patch_t *patch, > + apr_pool_t *result_pool, > + apr_pool_t *scratch_pool) > +{ > + SVN_ERR(add_or_delete_single_line(hunk_out, line, patch, TRUE, > + result_pool, scratch_pool)); > + return SVN_NO_ERROR; > +} > + > +svn_error_t * > +svn_diff_hunk__create_deletes_single_line(svn_diff_hunk_t **hunk_out, > + const char *line, > + svn_patch_t *patch, > + apr_pool_t *result_pool, > + apr_pool_t *scratch_pool) > +{ > + SVN_ERR(add_or_delete_single_line(hunk_out, line, patch, FALSE, > + result_pool, scratch_pool)); > + return SVN_NO_ERROR; > +} > + > void > svn_diff_hunk_reset_diff_text(svn_diff_hunk_t *hunk) > { > @@ -1240,6 +1351,8 @@ enum parse_state > state_git_tree_seen, /* a tree operation, rather than content change > */ > state_git_minus_seen, /* --- /dev/null; or --- a/ */ > state_git_plus_seen, /* +++ /dev/null; or +++ a/ */ > + state_old_mode_seen, /* old mode 100644 */ > + state_git_mode_seen, /* new mode 100644 */ > state_move_from_seen, /* rename from foo.c */ > state_copy_from_seen, /* copy from foo.c */ > state_minus_seen, /* --- foo.c */ > @@ -1472,6 +1585,85 @@ git_plus(enum parse_state *new_state, ch > return SVN_NO_ERROR; > } > > +/* Helper for git_old_mode() and git_new_mode(). Translate the git > + * file mode MODE_STR into a binary "executable?" notion EXECUTABLE_P. > */ > +static svn_error_t * > +parse_bits_into_executability(svn_tristate_t *executable_p, > + const char *mode_str) > +{ > + apr_uint64_t mode; > + SVN_ERR(svn_cstring_strtoui64(&mode, mode_str, > + 0 /* min */, > + 0777777 /* max: six octal digits */, > + 010 /* radix (octal) */)); > + > + /* Note: 0644 and 0755 are the only modes that can occur for plain files. > + * We deliberately choose to parse only those values: we are strict in what > + * we accept _and_ in what we produce. > + * > + * (Having said that, though, we could consider relaxing the parser to also > + * map > + * (mode & 0111) == 0000 -> svn_tristate_false > + * (mode & 0111) == 0111 -> svn_tristate_true > + * [anything else] -> svn_tristate_unknown > + * .) > + */ > + > + switch (mode & 0777) > + { > + case 0644: > + *executable_p = svn_tristate_false; > + break; > + > + case 0755: > + *executable_p = svn_tristate_true; > + break; > + > + default: > + /* Ignore unknown values. */ > + *executable_p = svn_tristate_unknown; > + break; > + } > + > + return SVN_NO_ERROR; > +} > + > +/* Parse the 'old mode ' line of a git extended unidiff. */ > +static svn_error_t * > +git_old_mode(enum parse_state *new_state, char *line, svn_patch_t > *patch, > + apr_pool_t *result_pool, apr_pool_t *scratch_pool) > +{ > + SVN_ERR(parse_bits_into_executability(&patch->old_executable_p, > + line + STRLEN_LITERAL("old mode "))); > + > +#ifdef SVN_DEBUG > + /* If this assert trips, the "old mode" is neither ...644 nor ...755 . */ > + SVN_ERR_ASSERT(patch->old_executable_p != svn_tristate_unknown); > +#endif I don't think we should leave these enabled even in maintainer mode. This will break abilities to apply diffs generated from other tools. (Perhaps even from older Subversion versions as I think you recently changed what Subversion generates itself on trunk) > + > + *new_state = state_old_mode_seen; > + return SVN_NO_ERROR; > +} > + > +/* Parse the 'new mode ' line of a git extended unidiff. */ > +static svn_error_t * > +git_new_mode(enum parse_state *new_state, char *line, svn_patch_t > *patch, > + apr_pool_t *result_pool, apr_pool_t *scratch_pool) > +{ > + SVN_ERR(parse_bits_into_executability(&patch->new_executable_p, > + line + STRLEN_LITERAL("new mode "))); > + > +#ifdef SVN_DEBUG > + /* If this assert trips, the "old mode" is neither ...644 nor ...755 . */ > + SVN_ERR_ASSERT(patch->new_executable_p != svn_tristate_unknown); > +#endif Same here. > + > + /* Don't touch patch->operation. */ > + > + *new_state = state_git_mode_seen; > + return SVN_NO_ERROR; > +} > + > /* Parse the 'rename from ' line of a git extended unidiff. */ > static svn_error_t * > git_move_from(enum parse_state *new_state, char *line, svn_patch_t > *patch, > @@ -1532,6 +1724,10 @@ static svn_error_t * > git_new_file(enum parse_state *new_state, char *line, svn_patch_t *patch, > apr_pool_t *result_pool, apr_pool_t *scratch_pool) > { > + SVN_ERR( > + parse_bits_into_executability(&patch->new_executable_p, > + line + STRLEN_LITERAL("new file mode "))); > + > patch->operation = svn_diff_op_added; > > /* Filename already retrieved from diff --git header. */ > @@ -1545,6 +1741,10 @@ static svn_error_t * > git_deleted_file(enum parse_state *new_state, char *line, svn_patch_t > *patch, > apr_pool_t *result_pool, apr_pool_t *scratch_pool) > { > + SVN_ERR( > + parse_bits_into_executability(&patch->old_executable_p, > + line + STRLEN_LITERAL("deleted file mode > "))); > + > patch->operation = svn_diff_op_deleted; > > /* Filename already retrieved from diff --git header. */ > @@ -1808,15 +2008,22 @@ static struct transition transitions[] = > > {"diff --git", state_start, git_start}, > {"--- a/", state_git_diff_seen, git_minus}, > + {"--- a/", state_git_mode_seen, git_minus}, > {"--- a/", state_git_tree_seen, git_minus}, > + {"--- /dev/null", state_git_mode_seen, git_minus}, > {"--- /dev/null", state_git_tree_seen, git_minus}, > {"+++ b/", state_git_minus_seen, git_plus}, > {"+++ /dev/null", state_git_minus_seen, git_plus}, > > + {"old mode ", state_git_diff_seen, git_old_mode}, > + {"new mode ", state_old_mode_seen, git_new_mode}, > + > {"rename from ", state_git_diff_seen, git_move_from}, > + {"rename from ", state_git_mode_seen, git_move_from}, > {"rename to ", state_move_from_seen, git_move_to}, > > {"copy from ", state_git_diff_seen, git_copy_from}, > + {"copy from ", state_git_mode_seen, git_copy_from}, > {"copy to ", state_copy_from_seen, git_copy_to}, > > {"new file ", state_git_diff_seen, git_new_file}, > @@ -1850,6 +2057,8 @@ svn_diff_parse_next_patch(svn_patch_t ** > } > > patch = apr_pcalloc(result_pool, sizeof(*patch)); > + patch->old_executable_p = svn_tristate_unknown; > + patch->new_executable_p = svn_tristate_unknown; > > pos = patch_file->next_patch_offset; > SVN_ERR(svn_io_file_seek(patch_file->apr_file, APR_SET, &pos, > scratch_pool)); > @@ -1896,7 +2105,8 @@ svn_diff_parse_next_patch(svn_patch_t ** > /* We have a valid diff header, yay! */ > break; > } > - else if (state == state_git_tree_seen && line_after_tree_header_read > + else if ((state == state_git_tree_seen || state == state_git_mode_seen) > + && line_after_tree_header_read > && !valid_header_line) > { > /* git patches can contain an index line after the file mode line > */ > @@ -1911,7 +2121,8 @@ svn_diff_parse_next_patch(svn_patch_t ** > break; > } > } > - else if (state == state_git_tree_seen) > + else if (state == state_git_tree_seen > + || state == state_git_mode_seen) > { > line_after_tree_header_read = TRUE; > } > > Propchange: subversion/trunk/subversion/libsvn_fs_x/ > ------------------------------------------------------------------------------ > --- svn:mergeinfo (original) > +++ svn:mergeinfo Sat Sep 26 00:46:52 2015 > @@ -61,6 +61,7 @@ > /subversion/branches/multi-layer-moves/subversion/libsvn_fs_x:1239019- > 1300930 > /subversion/branches/nfc-nfd-aware- > client/subversion/libsvn_fs_x:870276,870376 > /subversion/branches/node_pool/subversion/libsvn_fs_x:1304828-1305388 > +/subversion/branches/patch-exec/subversion/libsvn_fs_x:1692717- > 1705390 > > /subversion/branches/performance/subversion/libsvn_fs_x:979193,980118, > 981087,981090,981189,981194,981287,981684,981827,982043,982355,98339 > 8,983406,983430,983474,983488,983490,983760,983764,983766,983770,984 > 927,984973,984984,985014,985037,985046,985472,985477,985482,985487- > 985488,985493,985497,985500,985514,985601,985603,985606,985669,98567 > 3,985695,985697,986453,986465,986485,986491- > 986492,986517,986521,986605,986608,986817,986832,987865,987868- > 987869,987872,987886- > 987888,987893,988319,988898,990330,990533,990535- > 990537,990541,990568,990572,990574- > 990575,990600,990759,992899,992904,992911,993127,993141,994956,99547 > 8,995507,995603,998012,998858,999098,1001413,1001417,1004291,1022668 > ,1022670,1022676,1022715,1022719,1025660,1025672,1027193,1027203,102 > 7206,1027214,1027227,1028077,1028092,1028094,1028104,1028107,102811 > 1,1028354,1029038,1029042-1029043,1029054-1029055,1029062- > 1029063,1029078,1029080,1029090,1029092- > 1029093,1029111,1029151,1029158,1029229-1029230,1029232,1029335- > 1029336,102 > 9339- > 1029340,1029342,1029344,1030763,1030827,1031203,1031235,1032285,103 > 2333,1033040,1033057,1033294,1035869,1035882,1039511,1043705,105373 > 5,1056015,1066452,1067683,1067697-1078365 > /subversion/branches/pin-externals/subversion/libsvn_fs_x:1643757- > 1659392 > /subversion/branches/py-tests-as-modules/subversion/libsvn_fs_x:956579- > 1033052 > > Modified: subversion/trunk/subversion/tests/cmdline/patch_tests.py > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/p > atch_tests.py?rev=1705391&r1=1705390&r2=1705391&view=diff > ================================================================ > ============== > --- subversion/trunk/subversion/tests/cmdline/patch_tests.py (original) > +++ subversion/trunk/subversion/tests/cmdline/patch_tests.py Sat Sep 26 > 00:46:52 2015 > @@ -5962,6 +5962,189 @@ def patch_final_eol(sbox): > expected_status, expected_skip, > [], False, True, '--reverse-diff') > > +def patch_adds_executability_nocontents(sbox): > + """patch adds svn:executable, without contents""" > + > + sbox.build(read_only=True) > + wc_dir = sbox.wc_dir > + > + unidiff_patch = ( > + "diff --git a/iota b/iota\n" > + "old mode 100644\n" > + "new mode 100755\n" > + ) > + patch_file_path = make_patch_path(sbox) > + svntest.main.file_write(patch_file_path, unidiff_patch) > + > + expected_output = [ > + ' U %s\n' % sbox.ospath('iota'), > + ] > + expected_disk = svntest.main.greek_state.copy() > + # "*" is SVN_PROP_EXECUTABLE_VALUE aka SVN_PROP_BOOLEAN_TRUE > + expected_disk.tweak('iota', props={'svn:executable': '*'}) > + > + expected_status = svntest.actions.get_virginal_state(wc_dir, 1) > + expected_status.tweak('iota', status=' M') > + > + expected_skip = wc.State('', { }) > + > + svntest.actions.run_and_verify_patch(wc_dir, > os.path.abspath(patch_file_path), > + expected_output, expected_disk, > + expected_status, expected_skip, > + check_props=True) > + > +def patch_adds_executability_yescontents(sbox): > + """patch adds svn:executable, with contents""" > + > + sbox.build(read_only=True) > + wc_dir = sbox.wc_dir > + > + mu_new_contents = ( > + "This is the file 'mu'.\n" > + "with text mods too\n" > + ) > + > + unidiff_patch = ( > + "diff --git a/A/mu b/A/mu\n" > + "old mode 100644\n" > + "new mode 100755\n" > + "index 8a0f01c..dfad3ac\n" > + "--- a/A/mu\n" > + "+++ b/A/mu\n" > + "@@ -1 +1,2 @@\n" > + " This is the file 'mu'.\n" > + "+with text mods too\n" > + ) > + patch_file_path = make_patch_path(sbox) > + svntest.main.file_write(patch_file_path, unidiff_patch) > + > + expected_output = [ > + 'UU %s\n' % sbox.ospath('A/mu'), > + ] > + expected_disk = svntest.main.greek_state.copy() > + # "*" is SVN_PROP_EXECUTABLE_VALUE aka SVN_PROP_BOOLEAN_TRUE > + expected_disk.tweak('A/mu', props={'svn:executable': '*'}) > + expected_disk.tweak('A/mu', contents=mu_new_contents) > + > + expected_status = svntest.actions.get_virginal_state(wc_dir, 1) > + expected_status.tweak('A/mu', status='MM') > + > + expected_skip = wc.State('', { }) > + > + svntest.actions.run_and_verify_patch(wc_dir, > os.path.abspath(patch_file_path), > + expected_output, expected_disk, > + expected_status, expected_skip, > + check_props=True) > + > +def patch_deletes_executability(sbox): > + """patch deletes svn:executable""" > + > + sbox.build(read_only=True) > + wc_dir = sbox.wc_dir > + > + ## Set up the basic state. > + sbox.simple_propset('svn:executable', 'yes', 'iota') > + #sbox.simple_commit(target='iota', message="Make 'iota' executable.") > + > + unidiff_patch = ( > + "diff --git a/iota b/iota\n" > + "old mode 100755\n" > + "new mode 100644\n" > + ) > + patch_file_path = make_patch_path(sbox) > + svntest.main.file_write(patch_file_path, unidiff_patch) > + > + expected_output = [ > + ' U %s\n' % sbox.ospath('iota'), > + ] > + expected_disk = svntest.main.greek_state.copy() > + expected_disk.tweak('iota') # props=None by default > + > + expected_status = svntest.actions.get_virginal_state(wc_dir, 1) > + expected_status.tweak('iota', status=' ') > + > + expected_skip = wc.State('', { }) > + > + svntest.actions.run_and_verify_patch(wc_dir, > os.path.abspath(patch_file_path), > + expected_output, expected_disk, > + expected_status, expected_skip, > + check_props=True) > + > +def patch_ambiguous_executability_contradiction(sbox): > + """patch ambiguous svn:executable, bad""" > + > + sbox.build(read_only=True) > + wc_dir = sbox.wc_dir > + > + unidiff_patch = ( > + "Index: iota\n" > + > "=============================================================== > ====\n" > + "diff --git a/iota b/iota\n" > + "old mode 100755\n" > + "new mode 100644\n" > + "Property changes on: iota\n" > + "-------------------------------------------------------------------\n" > + "Added: svn:executable\n" > + "## -0,0 +1 ##\n" > + "+*\n" > + ) This specifies the addition of the svn:executable property with the non canonical "*\n" value. Is this really what you want to test? I think the actual property set code will change it to the proper value, but I would say you need the " \ No newline at end of property" marker here. The generated patchfile doesn't need it, but it does need the boolean to note the same thing. > + patch_file_path = make_patch_path(sbox) > + svntest.main.file_write(patch_file_path, unidiff_patch) > + > + expected_output = [] > + > + expected_disk = svntest.main.greek_state.copy() > + > + expected_status = svntest.actions.get_virginal_state(wc_dir, 1) > + > + expected_skip = wc.State('', { }) > + > + error_re_string = r'.*Invalid patch:.*contradicting.*mode.*svn:executable' > + svntest.actions.run_and_verify_patch(wc_dir, > os.path.abspath(patch_file_path), > + expected_output, expected_disk, > + expected_status, expected_skip, > + error_re_string=error_re_string, > + check_props=True) > + > +def patch_ambiguous_executability_consistent(sbox): > + """patch ambiguous svn:executable, good""" > + > + sbox.build(read_only=True) > + wc_dir = sbox.wc_dir > + > + unidiff_patch = ( > + "Index: iota\n" > + > "=============================================================== > ====\n" > + "diff --git a/iota b/iota\n" > + "old mode 100644\n" > + "new mode 100755\n" > + "Property changes on: iota\n" > + "-------------------------------------------------------------------\n" > + "Added: svn:executable\n" > + "## -0,0 +1 ##\n" > + "+*\n" > + ) Same in this test. > + patch_file_path = make_patch_path(sbox) > + svntest.main.file_write(patch_file_path, unidiff_patch) > + > + expected_output = [ > + ' U %s\n' % sbox.ospath('iota'), > + ] > + > + expected_disk = svntest.main.greek_state.copy() > + expected_disk.tweak('iota', props={'svn:executable': '*'}) > + > + expected_status = svntest.actions.get_virginal_state(wc_dir, 1) > + expected_status.tweak('iota', status=' M') > + > + expected_skip = wc.State('', { }) > + > + svntest.actions.run_and_verify_patch(wc_dir, > os.path.abspath(patch_file_path), > + expected_output, expected_disk, > + expected_status, expected_skip, > + error_re_string=None, > + check_props=True) > + > > ################################################################ > ######## > #Run the tests > > @@ -6027,6 +6210,11 @@ test_list = [ None, > patch_delete_nodes, > patch_delete_missing_eol, > patch_final_eol, > + patch_adds_executability_nocontents, > + patch_adds_executability_yescontents, > + patch_deletes_executability, > + patch_ambiguous_executability_contradiction, > + patch_ambiguous_executability_consistent, > ] > > if __name__ == '__main__': > <snip> Bert
