[RFC] scripts/setlocalversion on write-protected source tree

[RFC] scripts/setlocalversion on write-protected source tree

[Christian Kujau]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1826 bytes --]

Hi,

I just stumbled across another[0] issue when scripts/setlocalversion 
operates on a write-protected source tree. Back then[0] the source tree 
was on an read-only NFS share, so "test -w" was introduced before "git 
update-index" was run.

This time, the source tree is on read/write NFS share, but the permissions 
are world-readable and only a specific user (or root) can write. 
Thus, "test -w ." returns "0" and then runs "git update-index", 
producing the following message (on a dirty tree):

  fatal: Unable to create '/usr/local/src/linux-git/.git/index.lock': Permission denied

While it says "fatal", compilation continues just fine.

The patch below adds yet another (clumsy) check before "git 
update-index", but I'm not sure if this is the right way to go.

On a side note, I don't think a kernel compilation should alter the source 
tree (or the .git directory) in any way and I don't see how removing 
"git update-index" could do any harm. The Mercurial and SVN routines in 
scripts/setlocalversion don't have any tree-modifying commands, AFAICS. 
So, maybe the attached patch would be acceptable.

Thoughts?
Christian.

[0] https://patchwork.kernel.org/patch/29718/

diff --git a/scripts/setlocalversion b/scripts/setlocalversion
old mode 100755
new mode 100644
index 84b88f1..2560718
--- a/scripts/setlocalversion
+++ b/scripts/setlocalversion
@@ -72,7 +72,8 @@ scm_version()
 		fi
 
 		# Update index only on r/w media
-		[ -w . ] && git update-index --refresh --unmerged > /dev/null
+		[ -w . ] && touch .git/index.lock 2>/dev/null && rm -f .git/index.lock && \
+			git update-index --refresh --unmerged > /dev/null
 
 		# Check for uncommitted changes
 		if git diff-index --name-only HEAD | grep -qv "^scripts/package"; then


-- 
BOFH excuse #359:

YOU HAVE AN I/O ERROR -> Incompetent Operator error

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: TEXT/x-diff; name=setlocalversion_no-update-index.diff, Size: 492 bytes --]

diff --git a/scripts/setlocalversion b/scripts/setlocalversion
index 84b88f1..d105a44 100755
--- a/scripts/setlocalversion
+++ b/scripts/setlocalversion
@@ -71,9 +71,6 @@ scm_version()
 			printf -- '-svn%s' "`git svn find-rev $head`"
 		fi
 
-		# Update index only on r/w media
-		[ -w . ] && git update-index --refresh --unmerged > /dev/null
-
 		# Check for uncommitted changes
 		if git diff-index --name-only HEAD | grep -qv "^scripts/package"; then
 			printf '%s' -dirty

Re: [RFC] scripts/setlocalversion on write-protected source tree

[Nico Schottelius]

Hey Christian,

Christian Kujau [Sun, Jun 09, 2013 at 05:50:49PM -0700]:
> Hi,
> 
> [errors when using git update-index - different scenarios]
> [...]
> 
> On a side note, I don't think a kernel compilation should alter the source 
> tree (or the .git directory) in any way and I don't see how removing 
> "git update-index" could do any harm. The Mercurial and SVN routines in 
> scripts/setlocalversion don't have any tree-modifying commands, AFAICS. 
> So, maybe the attached patch would be acceptable.
> 
> [...]

As --refresh is being used, I guess (!) the motivation was to update the
index for stat changes that may have taken place elsewhere (like on
NFS) - not sure whether this actually matches (can somebody confirm /
overturn this theory?).

I do however agree with you that setlocalversion should probably not
try to fix this problem, but leave it to the user to update the index,
if necessary.

So I'd personally go for your second patch.

Cheers,

Nico

> diff --git a/scripts/setlocalversion b/scripts/setlocalversion
> index 84b88f1..d105a44 100755
> --- a/scripts/setlocalversion
> +++ b/scripts/setlocalversion
> @@ -71,9 +71,6 @@ scm_version()
>  			printf -- '-svn%s' "`git svn find-rev $head`"
>  		fi
>  
> -		# Update index only on r/w media
> -		[ -w . ] && git update-index --refresh --unmerged > /dev/null
> -
>  		# Check for uncommitted changes
>  		if git diff-index --name-only HEAD | grep -qv "^scripts/package"; then
>  			printf '%s' -dirty


-- 
PGP key: 7ED9 F7D3 6B10 81D7 0EC5  5C09 D7DC C8E4 3187 7DF0