apply: don't use core.sharedRepository to create working tree files

core.sharedRepository defines which permissions Git should set when
creating files in $GIT_DIR, so that the repository may be shared with
other users. But (in its current form) the setting shouldn't affect how
files are created in the working tree. This is not respected by apply
and am (which uses apply), when creating leading directories:

$ cat d.patch
 diff --git a/d/f b/d/f
 new file mode 100644
 index 0000000..e69de29

Apply without the setting:
$ umask 0077
$ git apply d.patch
$ ls -ld d
 drwx------

Apply with the setting:
$ umask 0077
$ git -c core.sharedRepository=0770 apply d.patch
$ ls -ld d
 drwxrws---

Only the leading directories are affected. That's because they are
created with safe_create_leading_directories(), which calls
adjust_shared_perm() to set the directories' permissions based on
core.sharedRepository. To fix that, let's introduce a variant of this
function that ignores the setting, and use it in apply. Also add a
regression test and a note in the function documentation about the use
of each variant according to the destination (working tree or git
dir).

Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/apply.c b/apply.c
index 76dba93c974b3814117e3856d875e7e4381d2f62..023e8da6b9e35a731b502e727bbbd49448e90a5f 100644 (file)
--- a/apply.c
+++ b/apply.c
@@ -4409,7 +4409,7 @@ static int create_one_file(struct apply_state *state,
                return 0;
 
        if (errno == ENOENT) {
-               if (safe_create_leading_directories(path))
+               if (safe_create_leading_directories_no_share(path))
                        return 0;
                res = try_create_file(state, path, mode, buf, size);
                if (res < 0)

diff --git a/cache.h b/cache.h
index c0072d43b1a78101f92df1283f9f34712f4c0265..c161ddc1f06c846837d6e92c92c382180ce3efd5 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -1255,7 +1255,11 @@ int adjust_shared_perm(const char *path);
  * safe_create_leading_directories() temporarily changes path while it
  * is working but restores it before returning.
  * safe_create_leading_directories_const() doesn't modify path, even
- * temporarily.
+ * temporarily. Both these variants adjust the permissions of the
+ * created directories to honor core.sharedRepository, so they are best
+ * suited for files inside the git dir. For working tree files, use
+ * safe_create_leading_directories_no_share() instead, as it ignores
+ * the core.sharedRepository setting.
  */
 enum scld_error {
        SCLD_OK = 0,
@@ -1266,6 +1270,7 @@ enum scld_error {
 };
 enum scld_error safe_create_leading_directories(char *path);
 enum scld_error safe_create_leading_directories_const(const char *path);
+enum scld_error safe_create_leading_directories_no_share(char *path);
 
 /*
  * Callback function for raceproof_create_file(). This function is

diff --git a/sha1-file.c b/sha1-file.c
index dd65bd5c681513c1f7c9bf908dc9f8675e24a829..c3c49d2fa5587a86766d82539bb87b298b1e40ed 100644 (file)
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -291,7 +291,7 @@ int mkdir_in_gitdir(const char *path)
        return adjust_shared_perm(path);
 }
 
-enum scld_error safe_create_leading_directories(char *path)
+static enum scld_error safe_create_leading_directories_1(char *path, int share)
 {
        char *next_component = path + offset_1st_component(path);
        enum scld_error ret = SCLD_OK;
@@ -337,7 +337,7 @@ enum scld_error safe_create_leading_directories(char *path)
                                ret = SCLD_VANISHED;
                        else
                                ret = SCLD_FAILED;
-               } else if (adjust_shared_perm(path)) {
+               } else if (share && adjust_shared_perm(path)) {
                        ret = SCLD_PERMS;
                }
                *slash = slash_character;
@@ -345,6 +345,16 @@ enum scld_error safe_create_leading_directories(char *path)
        return ret;
 }
 
+enum scld_error safe_create_leading_directories(char *path)
+{
+       return safe_create_leading_directories_1(path, 1);
+}
+
+enum scld_error safe_create_leading_directories_no_share(char *path)
+{
+       return safe_create_leading_directories_1(path, 0);
+}
+
 enum scld_error safe_create_leading_directories_const(const char *path)
 {
        int save_errno;

diff --git a/t/t4129-apply-samemode.sh b/t/t4129-apply-samemode.sh
index 5cdd76dfa726d32226b1dd625d21d6280125afb7..41818d8315f32359b0eabd1cb91684c90c070098 100755 (executable)
--- a/t/t4129-apply-samemode.sh
+++ b/t/t4129-apply-samemode.sh
@@ -73,4 +73,30 @@ test_expect_success FILEMODE 'bogus mode is rejected' '
        test_i18ngrep "invalid mode" err
 '
 
+test_expect_success POSIXPERM 'do not use core.sharedRepository for working tree files' '
+       git reset --hard &&
+       test_config core.sharedRepository 0666 &&
+       (
+               # Remove a default ACL if possible.
+               (setfacl -k newdir 2>/dev/null || true) &&
+               umask 0077 &&
+
+               # Test both files (f1) and leading dirs (d)
+               mkdir d &&
+               touch f1 d/f2 &&
+               git add f1 d/f2 &&
+               git diff --staged >patch-f1-and-f2.txt &&
+
+               rm -rf d f1 &&
+               git apply patch-f1-and-f2.txt &&
+
+               echo "-rw-------" >f1_mode.expected &&
+               echo "drwx------" >d_mode.expected &&
+               test_modebits f1 >f1_mode.actual &&
+               test_modebits d >d_mode.actual &&
+               test_cmp f1_mode.expected f1_mode.actual &&
+               test_cmp d_mode.expected d_mode.actual
+       )
+'
+
 test_done

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 8d59b90348ea898ca983e8bbd0742de495a8d36d..7195d55c907cc60aacdb14a2ff4d52ba06b54822 100644 (file)
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -367,9 +367,9 @@ test_chmod () {
        git update-index --add "--chmod=$@"
 }
 
-# Get the modebits from a file.
+# Get the modebits from a file or directory.
 test_modebits () {
-       ls -l "$1" | sed -e 's|^\(..........\).*|\1|'
+       ls -ld "$1" | sed -e 's|^\(..........\).*|\1|'
 }
 
 # Unset a configuration variable, but don't fail if it doesn't exist.