Re: gc changes permissions of files

From

Johannes Schindelin <johannes.schindelin@gmx.de>

Date

Jun 15, 2008, 05:38 UTC

Message-ID

<alpine.DEB.1.00.0806150551010.6439@racer>

In-Reply-To

<20080615035917.GA13414@lifeintegrity.com>

Hi,

On Sat, 14 Jun 2008, Allan Wind wrote:

> Is anyone looking into fixing gc so it does not change permission on 
> files?  I would think it should consider current permissions of files 
> before umask.
> 
> I believe Nigel reported this issue with:
> http://thread.gmane.org/gmane.comp.version-control.git/79213/focus=79237
> and still seeing issues with 1.5.5.4.

It would have been nice to briefly illustrate the problem again.

However, I cannot reproduce at all:

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t1301-shared-repo.sh |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh
index 5e4252a..c4d3239 100755
--- a/t/t1301-shared-repo.sh
+++ b/t/t1301-shared-repo.sh
@@ -43,6 +43,24 @@ test_expect_success 'update-server-info honors core.sharedRepository' '
 	esac
 '
 
+test_expect_success 'pack-refs honors core.sharedRepository' '
+
+	(umask 0217 &&
+	 : > a2 &&
+	 git pack-refs) &&
+	actual="$(ls -l .git/packed-refs)" &&
+	case "$actual" in
+	-r--r--r--*)
+		: happy
+		;;
+	*)
+		echo Oops, .git/packed-refs is not 0444
+		false
+		;;
+	esac
+
+'
+
 for u in	0660:rw-rw---- \
 		0640:rw-r----- \
 		0600:rw------- \
-- snap --

This succeeds.

Indeed, it should succeed, since the relevant part in lockfile.c was fixed 
in version v1.4.0~4.

However, I suspect from the original message that the reflogs did not have 
the correct permissions.  But again, a simple test succeeds:

-- snip --
[PATCH] Add a test for logrefs in bare repositories

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t1301-shared-repo.sh |   36 ++++++++++++++++++++++++++++++++++++
 1 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh
index 5e4252a..8530a56 100755
--- a/t/t1301-shared-repo.sh
+++ b/t/t1301-shared-repo.sh
@@ -24,6 +24,39 @@ test_expect_success 'shared=all' '
 	test 2 = $(git config core.sharedrepository)
 '
 
+test_expect_success 'reflogs honor core.sharedRepository' '
+	git config core.logAllRefUpdates true &&
+	echo 2 > a1 &&
+	git add a1 &&
+	test_tick &&
+	git commit -m a1 &&
+	actual="$(ls -l .git/logs/refs/heads/master)" &&
+	case "$actual" in
+	-rw-rw-r--*)
+		: happy
+		;;
+	*)
+		echo Oops, .git/logs/refs/heads/master is not 0664
+		false
+		;;
+	esac &&
+	(mkdir bare.git &&
+	 cd bare.git &&
+	 git --bare init --shared &&
+	 git config core.logAllRefUpdates true) &&
+	git push bare.git master:refs/heads/new &&
+	actual="$(ls -l bare.git/logs/refs/heads/new)" &&
+	case "$actual" in
+	-rw-rw-*)
+		: happy
+		;;
+	*)
+		echo Oops, bare.git/logs/refs/heads/new is not 0664
+		false
+		;;
+	esac
+'
+
 test_expect_success 'update-server-info honors core.sharedRepository' '
 	: > a1 &&
 	git add a1 &&

-- 
1.5.5.1.780.g346de
-- snap --

So now I got curious and tested it with v1.5.4.4, and indeed, it failed.

If you are curious who/what fixed it, go ahead, bisect it.

I wrote all this down so that the next time, you could verify yourself if 
the bug was fixed or not, instead of having to ask here.

Ciao,
Dscho