Hi, It appears a regression was introduced in b68294e ("git diffs: Support file mode changes") - patch will change the permissions of a file even though no permission changes are specified in the git diff patch.
This happens when the file to be patched has different permissions than the ones on the 'index sha1..sha1 <PERMS>' line, where git apply would just apply the patch and warn about the disreptancy: mkdir test && git init test cd test echo 'ksplice' > f && chmod 644 f git add f ; git commit -s -m 'Initial commit.' echo 'ksplice rocks!' > f git add f ; git commit -s -m 'Second commit.' git checkout -b test_git_apply master^ cat f ksplice git apply <(git show master | sed 's/100644/100755/') warning: f has type 100644, expected 100755 cat f ksplice rocks! stat -c %a f 644 This is emphasized in the attached patches, where before the last patch the new no-mode-change-git-diff test would fail. I'm calling this a regression because when this happens, reverting the patch doesn't bring back the old mode, since that information is lost at that point. So applying a patch and then reverting it actually leaves some changes other than access/modified/change stat information. Quentin
>From 98549535731976fa814abe3242f584a72ca2828b Mon Sep 17 00:00:00 2001 From: Quentin Casasnovas <quentin.casasno...@oracle.com> Date: Tue, 27 Jan 2015 13:31:14 +0100 Subject: [PATCH 1/3] test-lib.sh: factorize require_* functions. Since the code is identical when just checking if a utility is present on the system or not, we can factorize it. Signed-off-by: Quentin Casasnovas <quentin.casasno...@oracle.com> --- tests/asymmetric-hunks | 2 +- tests/backup-prefix-suffix | 2 +- tests/bad-usage | 2 +- tests/concat-git-diff | 2 +- tests/copy-rename | 2 +- tests/corrupt-reject-files | 2 +- tests/create-delete | 4 ++-- tests/criss-cross | 2 +- tests/crlf-handling | 4 ++-- tests/dash-o-append | 2 +- tests/empty-files | 2 +- tests/fifo | 2 +- tests/file-modes | 4 ++-- tests/filename-choice | 2 +- tests/git-binary-diff | 2 +- tests/global-reject-files | 2 +- tests/inname | 2 +- tests/line-numbers | 4 ++-- tests/mangled-numbers-abort | 2 +- tests/merge | 6 +++--- tests/mixed-patch-types | 2 +- tests/munged-context-format | 4 ++-- tests/need-filename | 4 ++-- tests/no-newline-triggers-assert | 2 +- tests/preserve-c-function-names | 4 ++-- tests/preserve-mode-and-timestamp | 4 ++-- tests/quoted-filenames | 2 +- tests/read-only-files | 2 +- tests/reject-format | 6 +++--- tests/remember-backup-files | 2 +- tests/remember-reject-files | 2 +- tests/remove-directories | 2 +- tests/symlinks | 2 +- tests/test-lib.sh | 18 +++++++----------- tests/unmodified-files | 2 +- 35 files changed, 53 insertions(+), 57 deletions(-) diff --git a/tests/asymmetric-hunks b/tests/asymmetric-hunks index 6929c4a..d6979d9 100644 --- a/tests/asymmetric-hunks +++ b/tests/asymmetric-hunks @@ -6,7 +6,7 @@ . $srcdir/test-lib.sh -require_cat +require cat use_local_patch use_tmpdir diff --git a/tests/backup-prefix-suffix b/tests/backup-prefix-suffix index a51142c..e37d602 100644 --- a/tests/backup-prefix-suffix +++ b/tests/backup-prefix-suffix @@ -6,7 +6,7 @@ . $srcdir/test-lib.sh -require_cat +require cat use_local_patch use_tmpdir diff --git a/tests/bad-usage b/tests/bad-usage index 022eeda..551553a 100644 --- a/tests/bad-usage +++ b/tests/bad-usage @@ -6,7 +6,7 @@ . $srcdir/test-lib.sh -require_cat +require cat use_local_patch use_tmpdir diff --git a/tests/concat-git-diff b/tests/concat-git-diff index c78da53..f8bf911 100644 --- a/tests/concat-git-diff +++ b/tests/concat-git-diff @@ -6,7 +6,7 @@ . $srcdir/test-lib.sh -require_cat +require cat use_local_patch use_tmpdir diff --git a/tests/copy-rename b/tests/copy-rename index 40f53d1..fd5fd64 100644 --- a/tests/copy-rename +++ b/tests/copy-rename @@ -8,7 +8,7 @@ . $srcdir/test-lib.sh -require_cat +require cat use_local_patch use_tmpdir diff --git a/tests/corrupt-reject-files b/tests/corrupt-reject-files index 17215aa..ad001f8 100644 --- a/tests/corrupt-reject-files +++ b/tests/corrupt-reject-files @@ -6,7 +6,7 @@ . $srcdir/test-lib.sh -require_cat +require cat use_local_patch use_tmpdir diff --git a/tests/create-delete b/tests/create-delete index 404d99e..2ec80df 100644 --- a/tests/create-delete +++ b/tests/create-delete @@ -6,8 +6,8 @@ . $srcdir/test-lib.sh -require_cat -require_sed +require cat +require sed use_local_patch use_tmpdir diff --git a/tests/criss-cross b/tests/criss-cross index 7b5a706..5e7b611 100644 --- a/tests/criss-cross +++ b/tests/criss-cross @@ -8,7 +8,7 @@ . $srcdir/test-lib.sh -require_cat +require cat use_local_patch use_tmpdir diff --git a/tests/crlf-handling b/tests/crlf-handling index b704dc8..239149c 100644 --- a/tests/crlf-handling +++ b/tests/crlf-handling @@ -8,8 +8,8 @@ . $srcdir/test-lib.sh -require_gnu_diff -require_sed +require gnu_diff +require sed use_local_patch use_tmpdir diff --git a/tests/dash-o-append b/tests/dash-o-append index 1633699..c6c2d4a 100644 --- a/tests/dash-o-append +++ b/tests/dash-o-append @@ -6,7 +6,7 @@ . $srcdir/test-lib.sh -require_cat +require cat use_local_patch use_tmpdir diff --git a/tests/empty-files b/tests/empty-files index 66319ec..6c8708f 100644 --- a/tests/empty-files +++ b/tests/empty-files @@ -8,7 +8,7 @@ . $srcdir/test-lib.sh -require_cat +require cat use_local_patch use_tmpdir umask 022 diff --git a/tests/fifo b/tests/fifo index 9e07558..a669aba 100644 --- a/tests/fifo +++ b/tests/fifo @@ -8,7 +8,7 @@ . $srcdir/test-lib.sh -require_cat +require cat use_local_patch use_tmpdir diff --git a/tests/file-modes b/tests/file-modes index e71c209..b615099 100644 --- a/tests/file-modes +++ b/tests/file-modes @@ -8,8 +8,8 @@ . $srcdir/test-lib.sh -require_cat -require_sed +require cat +require sed use_local_patch use_tmpdir diff --git a/tests/filename-choice b/tests/filename-choice index 88fc805..1d011bc 100644 --- a/tests/filename-choice +++ b/tests/filename-choice @@ -8,7 +8,7 @@ . $srcdir/test-lib.sh -require_cat +require cat use_local_patch use_tmpdir diff --git a/tests/git-binary-diff b/tests/git-binary-diff index 2b3f3a2..21c5df1 100644 --- a/tests/git-binary-diff +++ b/tests/git-binary-diff @@ -6,7 +6,7 @@ . $srcdir/test-lib.sh -require_cat +require cat use_local_patch use_tmpdir diff --git a/tests/global-reject-files b/tests/global-reject-files index 65c57dd..6426d03 100644 --- a/tests/global-reject-files +++ b/tests/global-reject-files @@ -8,7 +8,7 @@ . $srcdir/test-lib.sh -require_cat +require cat use_local_patch use_tmpdir diff --git a/tests/inname b/tests/inname index 080a0cc..b5ec9f3 100644 --- a/tests/inname +++ b/tests/inname @@ -8,7 +8,7 @@ . $srcdir/test-lib.sh -require_cat +require cat use_local_patch use_tmpdir diff --git a/tests/line-numbers b/tests/line-numbers index fc83042..e9b0cfe 100644 --- a/tests/line-numbers +++ b/tests/line-numbers @@ -6,8 +6,8 @@ . $srcdir/test-lib.sh -require_cat -require_sed +require cat +require sed use_local_patch use_tmpdir diff --git a/tests/mangled-numbers-abort b/tests/mangled-numbers-abort index 8826ae9..d05c171 100644 --- a/tests/mangled-numbers-abort +++ b/tests/mangled-numbers-abort @@ -6,7 +6,7 @@ . $srcdir/test-lib.sh -require_cat +require cat use_local_patch use_tmpdir diff --git a/tests/merge b/tests/merge index 8e01edc..22d787b 100644 --- a/tests/merge +++ b/tests/merge @@ -8,9 +8,9 @@ . $srcdir/test-lib.sh -require_cat -require_gnu_diff -require_sed +require cat +require gnu_diff +require sed use_local_patch use_tmpdir diff --git a/tests/mixed-patch-types b/tests/mixed-patch-types index da17c75..2465c0b 100644 --- a/tests/mixed-patch-types +++ b/tests/mixed-patch-types @@ -6,7 +6,7 @@ . $srcdir/test-lib.sh -require_cat +require cat use_local_patch use_tmpdir umask 022 diff --git a/tests/munged-context-format b/tests/munged-context-format index 3cd166c..50c95c8 100644 --- a/tests/munged-context-format +++ b/tests/munged-context-format @@ -8,8 +8,8 @@ . $srcdir/test-lib.sh -require_cat -require_sed +require cat +require sed use_local_patch use_tmpdir diff --git a/tests/need-filename b/tests/need-filename index 3748064..8b92848 100644 --- a/tests/need-filename +++ b/tests/need-filename @@ -8,8 +8,8 @@ . $srcdir/test-lib.sh -require_cat -require_sed +require cat +require sed use_local_patch use_tmpdir diff --git a/tests/no-newline-triggers-assert b/tests/no-newline-triggers-assert index 7c2e7eb..855ba70 100644 --- a/tests/no-newline-triggers-assert +++ b/tests/no-newline-triggers-assert @@ -8,7 +8,7 @@ . $srcdir/test-lib.sh -require_cat +require cat use_local_patch use_tmpdir diff --git a/tests/preserve-c-function-names b/tests/preserve-c-function-names index 813360e..789fa61 100644 --- a/tests/preserve-c-function-names +++ b/tests/preserve-c-function-names @@ -8,8 +8,8 @@ . $srcdir/test-lib.sh -require_cat -require_gnu_diff +require cat +require gnu_diff use_local_patch use_tmpdir diff --git a/tests/preserve-mode-and-timestamp b/tests/preserve-mode-and-timestamp index 170aff5..18c3c91 100644 --- a/tests/preserve-mode-and-timestamp +++ b/tests/preserve-mode-and-timestamp @@ -6,8 +6,8 @@ . $srcdir/test-lib.sh -require_cat -require_sed +require cat +require sed use_local_patch use_tmpdir diff --git a/tests/quoted-filenames b/tests/quoted-filenames index 5fb9f12..08f312f 100644 --- a/tests/quoted-filenames +++ b/tests/quoted-filenames @@ -8,7 +8,7 @@ . $srcdir/test-lib.sh -require_cat +require cat use_local_patch use_tmpdir diff --git a/tests/read-only-files b/tests/read-only-files index 918b97a..bcc11a4 100644 --- a/tests/read-only-files +++ b/tests/read-only-files @@ -8,7 +8,7 @@ . $srcdir/test-lib.sh -require_cat +require cat use_local_patch use_tmpdir diff --git a/tests/reject-format b/tests/reject-format index 0b02af6..3c533c6 100644 --- a/tests/reject-format +++ b/tests/reject-format @@ -8,9 +8,9 @@ . $srcdir/test-lib.sh -require_cat -require_sed -require_gnu_diff +require cat +require sed +require gnu_diff use_local_patch use_tmpdir diff --git a/tests/remember-backup-files b/tests/remember-backup-files index b544329..7ca0e86 100644 --- a/tests/remember-backup-files +++ b/tests/remember-backup-files @@ -9,7 +9,7 @@ . $srcdir/test-lib.sh -require_cat +require cat use_local_patch use_tmpdir diff --git a/tests/remember-reject-files b/tests/remember-reject-files index 93b6413..78c6335 100644 --- a/tests/remember-reject-files +++ b/tests/remember-reject-files @@ -6,7 +6,7 @@ . $srcdir/test-lib.sh -require_cat +require cat use_local_patch use_tmpdir diff --git a/tests/remove-directories b/tests/remove-directories index 6acdc49..07112eb 100644 --- a/tests/remove-directories +++ b/tests/remove-directories @@ -6,7 +6,7 @@ . $srcdir/test-lib.sh -require_cat +require cat use_local_patch use_tmpdir diff --git a/tests/symlinks b/tests/symlinks index 04a9b73..16e601c 100644 --- a/tests/symlinks +++ b/tests/symlinks @@ -8,7 +8,7 @@ . $srcdir/test-lib.sh -require_cat +require cat use_local_patch use_tmpdir diff --git a/tests/test-lib.sh b/tests/test-lib.sh index 051f1a0..be0d7e3 100644 --- a/tests/test-lib.sh +++ b/tests/test-lib.sh @@ -7,13 +7,6 @@ # FIXME: Requires a version of diff that understands "-u". -require_cat() { - if ! type cat > /dev/null 2> /dev/null; then - echo "This test requires the cat utility" >&2 - exit 77 - fi -} - require_gnu_diff() { case "`diff --version 2> /dev/null`" in *GNU*) @@ -24,9 +17,12 @@ require_gnu_diff() { esac } -require_sed() { - if ! type sed > /dev/null 2> /dev/null; then - echo "This test requires the sed utility" >&2 +require() { + utility="$1" + if type require_${utility} > /dev/null 2> /dev/null; then + require_${utility} + elif ! type "${utility}" > /dev/null 2> /dev/null; then + echo "This test requires the ${utility} utility" >&2 exit 77 fi } @@ -163,7 +159,7 @@ if ! type seq > /dev/null 2> /dev/null; then )} fi -require_cat +require cat clean_env checks_succeeded=0 diff --git a/tests/unmodified-files b/tests/unmodified-files index a9e00c6..fd5eee6 100644 --- a/tests/unmodified-files +++ b/tests/unmodified-files @@ -6,7 +6,7 @@ . $srcdir/test-lib.sh -require_cat +require cat use_local_patch use_tmpdir -- 2.0.5
>From 96381a459bb2b23cee9c2ae58fde59c5e7a1292d Mon Sep 17 00:00:00 2001 From: Quentin Casasnovas <quentin.casasno...@oracle.com> Date: Tue, 27 Jan 2015 13:33:20 +0100 Subject: [PATCH 2/3] tests: add a test case for unwanted mode changes. Signed-off-by: Quentin Casasnovas <quentin.casasno...@oracle.com> --- tests/Makefile.am | 1 + tests/no-mode-change-git-diff | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 tests/no-mode-change-git-diff diff --git a/tests/Makefile.am b/tests/Makefile.am index cfc4f37..3bdff2f 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -42,6 +42,7 @@ TESTS = \ mixed-patch-types \ munged-context-format \ need-filename \ + no-mode-change-git-diff \ no-newline-triggers-assert \ preserve-c-function-names \ preserve-mode-and-timestamp \ diff --git a/tests/no-mode-change-git-diff b/tests/no-mode-change-git-diff new file mode 100644 index 0000000..5df03ea --- /dev/null +++ b/tests/no-mode-change-git-diff @@ -0,0 +1,35 @@ +# Copyright (C) 2010-2012 Free Software Foundation, Inc. +# +# Copying and distribution of this file, with or without modification, +# in any medium, are permitted without royalty provided the copyright +# notice and this notice are preserved. + +. $srcdir/test-lib.sh + +require cat +require chmod +require stat +use_local_patch +use_tmpdir + +echo 'ksplice' > f +chmod 755 f + +cat > simple.diff <<EOF +diff --git a/f b/f +index 422a422a..736b6c7063690a 100644 +--- a/f ++++ b/f +@@ -1 +1 @@ +-ksplice ++ksplice rocks! +EOF + +check 'patch -p1 < simple.diff || echo "Status: $?"' <<EOF +patching file f +EOF + +check 'stat -c "%a" f'<<EOF +755 +EOF + -- 2.0.5
>From db9d794fbab604536fbbc6f9e3e171bb87b9fe6b Mon Sep 17 00:00:00 2001 From: Quentin Casasnovas <quentin.casasno...@oracle.com> Date: Tue, 27 Jan 2015 14:03:57 +0100 Subject: [PATCH 3/3] patch: git-diff mode: do not change permissions if there isn't an explicit mode change. Signed-off-by: Quentin Casasnovas <quentin.casasno...@oracle.com> --- src/patch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/patch.c b/src/patch.c index cb4dbb2..88940ae 100644 --- a/src/patch.c +++ b/src/patch.c @@ -540,7 +540,7 @@ main (int argc, char **argv) enum file_attributes attr = 0; struct timespec new_time = pch_timestamp (! reverse); mode_t mode = file_type | - ((new_mode ? new_mode : instat.st_mode) & S_IRWXUGO); + ((set_mode ? new_mode : instat.st_mode) & S_IRWXUGO); if ((set_time | set_utc) && new_time.tv_sec != -1) { -- 2.0.5