Re: git pull aborts in 50% of cases

Johannes Schindelin [off-list ref] writes:

quoted

0.99.9k is clearly bad.

Huh? It could be slower, and it could therefore hit the maximum client 
count faster, but it should not be bad.

All changes to pull were done in a manner so as to be backward compatible. 
In both ways.

I do not think the fetch-pack common computation changes is
involved in this problem at all.

What is suspect is the repository validity check code,
specifically (quoting from diff between 0.99.9j and 0.99.9k
daemon.c::path_ok() function):

+               /* The validation is done on the paths after enter_repo
+                * canonicalization, so whitelist should be written in
+                * terms of real pathnames (i.e. after ~user is expanded
+                * and symlinks resolved).
+                */

I suspect (but have not heard back from HPA to confirm) that
kernel.org runs git-daemon with /pub/scm as the whitelist, but
there is a symbolic link (or bind mount?) involved, and the real
path checked based on getcwd() return value is somewhere else.

Junio C Hamano wrote:

Johannes Schindelin [off-list ref] writes:

quoted

quoted

0.99.9k is clearly bad.

Huh? It could be slower, and it could therefore hit the maximum client 
count faster, but it should not be bad.

All changes to pull were done in a manner so as to be backward compatible. 
In both ways.

I do not think the fetch-pack common computation changes is
involved in this problem at all.

What is suspect is the repository validity check code,
specifically (quoting from diff between 0.99.9j and 0.99.9k
daemon.c::path_ok() function):

+               /* The validation is done on the paths after enter_repo
+                * canonicalization, so whitelist should be written in
+                * terms of real pathnames (i.e. after ~user is expanded
+                * and symlinks resolved).
+                */

I suspect (but have not heard back from HPA to confirm) that
kernel.org runs git-daemon with /pub/scm as the whitelist, but
there is a symbolic link (or bind mount?) involved, and the real
path checked based on getcwd() return value is somewhere else.

/pub is a symbolic link.  We shouldn't rely on getcwd() for this kind of 
stuff; it's bad for a whole bunch of reasons.

	-hpa