A few comments inline.
> -----Oorspronkelijk bericht-----
> Van: danielsh_at_apache.org [mailto:danielsh_at_apache.org]
> Verzonden: zaterdag 26 september 2015 02:47
> Aan: commits_at_subversion.apache.org
> 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
Received on 2015-09-26 10:06:36 CEST