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
