[PATCH v2 0/3] safe.directory clean-up
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 23, 2024, 02:18 UTC
- Message-ID
- <20240723021900.388020-1-gitster@pobox.com>
- In-Reply-To
- <20240720220915.2933266-1-gitster@pobox.com>
Changes since v1:
- two "extra" patches for optimization that are unnecessary have been dropped. - a patch to add test for safe.directory="." use case has been added.
Recently we discussed what we should do when either the path configured in the safe.directory configuration or coming from the caller of ensure_valid_ownership() function as a result of repository discovery is not normalized and textual equality check is not sufficient. See the thread the contains
https://lore.kernel.org/git/6d5b75a6-639d-429b-bd37-232fc6f475af@gmail.com/
Here are three patches that implement the comparison between normalized path and configuration value.
Imagine that you have a repository at /mnt/disk4/repos/frotz directory but in order to make it simpler to manage and use, you have your users use /projects/frotz to access the repository. A symlink /projects/frotz pointing at /mnt/disk4/repos/frotz directory allows you to do so.
- The first patch normalizes the path to the directory that we suspect is a usable repository, before comparing it with the safe.directory configuration variable. The safe.directory may say /mnt/disk4/repos/frotz or /mnt/disk4/repos/*, but the path to the repository for the users may be /mnt/disk4/repos/frotz or /projects/frotz, depending on where they come from and what their $PWD makes getcwd() to say.
- The second patch normalizes the value of the safe.directory variable. This allows safe.directory to say /projects/frotz or /projects/* and have them match /mnt/disk4/repos/frotz (which is how the first patch normalizes the repository path to).
- The third patch only adds a test to illustrate what happens when the safe.directory configuration is set to ".", which was a plausible setting for those who run "git daemon". The normalization done by the first two patches is sufficient to make this just work.
Junio C Hamano (3):
safe.directory: normalize the checked path
safe.directory: normalize the configured path
safe.directory: setting safe.directory="." allows the "current"
directorysetup.c | 28 ++++++-- t/t0033-safe-directory.sh | 146 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 170 insertions(+), 4 deletions(-)
Range-diff against v1:
1: 20e1160946 ! 1: 86d5c83ee7 safe.directory: normalize the checked path
@@ t/t0033-safe-directory.sh: test_expect_success 'local clone of unowned repo acce
+ sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
+ git config --global safe.directory "$(pwd)/repository"
+ ) &&
-+ git -C repository/ for-each-ref >/dev/null &&
-+ git -C repo/ for-each-ref
++ git -C repository for-each-ref &&
++ git -C repository/ for-each-ref &&
++ git -C repo for-each-ref &&
++ git -C repo/ for-each-ref &&
++ test_must_fail git -C repository/.git for-each-ref &&
++ test_must_fail git -C repository/.git/ for-each-ref &&
++ test_must_fail git -C repo/.git for-each-ref &&
++ test_must_fail git -C repo/.git/ for-each-ref
+'
+
+test_expect_success SYMLINKS 'checked leading paths are normalized' '
@@ t/t0033-safe-directory.sh: test_expect_success 'local clone of unowned repo acce
+ sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
+ git config --global safe.directory "$(pwd)/repository/*"
+ ) &&
-+ git -C repository/s for-each-ref >/dev/null &&
-+ git -C repo/s for-each-ref
++ git -C repository/s for-each-ref &&
++ git -C repository/s/ for-each-ref &&
++ git -C repo/s for-each-ref &&
++ git -C repo/s/ for-each-ref &&
++ git -C repository/s/.git for-each-ref &&
++ git -C repository/s/.git/ for-each-ref &&
++ git -C repo/s/.git for-each-ref &&
++ git -C repo/s/.git/ for-each-ref
+'
+
test_done
2: 06de9038b7 ! 2: c4998076da safe.directory: normalize the configured path
@@ setup.c: static int safe_directory_cb(const char *key, const char *value,
## t/t0033-safe-directory.sh ##
@@ t/t0033-safe-directory.sh: test_expect_success SYMLINKS 'checked leading paths are normalized' '
- git -C repo/s for-each-ref
+ git -C repo/s/.git/ for-each-ref
'
+test_expect_success SYMLINKS 'configured paths are normalized' '
@@ t/t0033-safe-directory.sh: test_expect_success SYMLINKS 'checked leading paths a
+ sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
+ git config --global safe.directory "$(pwd)/repo"
+ ) &&
-+ git -C repository/ for-each-ref >/dev/null &&
-+ git -C repo/ for-each-ref
++ git -C repository for-each-ref &&
++ git -C repository/ for-each-ref &&
++ git -C repo for-each-ref &&
++ git -C repo/ for-each-ref &&
++ test_must_fail git -C repository/.git for-each-ref &&
++ test_must_fail git -C repository/.git/ for-each-ref &&
++ test_must_fail git -C repo/.git for-each-ref &&
++ test_must_fail git -C repo/.git/ for-each-ref
+'
+
+test_expect_success SYMLINKS 'configured leading paths are normalized' '
@@ t/t0033-safe-directory.sh: test_expect_success SYMLINKS 'checked leading paths a
+ git init repository/s &&
+ ln -s repository repo &&
+ (
-+ cd repository &&
++ cd repository/s &&
+ sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
+ test_commit sample
+ ) &&
@@ t/t0033-safe-directory.sh: test_expect_success SYMLINKS 'checked leading paths a
+ sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
+ git config --global safe.directory "$(pwd)/repo/*"
+ ) &&
-+ git -C repository/s for-each-ref >/dev/null &&
-+ git -C repo/s for-each-ref
++ git -C repository/s for-each-ref &&
++ git -C repository/s/ for-each-ref &&
++ git -C repository/s/.git for-each-ref &&
++ git -C repository/s/.git/ for-each-ref &&
++ git -C repo/s for-each-ref &&
++ git -C repo/s/ for-each-ref &&
++ git -C repo/s/.git for-each-ref &&
++ git -C repo/s/.git/ for-each-ref
+'
+
test_done
-: ---------- > 3: ffc3f7364e safe.directory: setting safe.directory="." allows the "current" directory-- 2.46.0-rc1-52-gda884b23f2