Re: post-update script to update wc - suggestions welcome

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 20, 2007, 08:52 UTC
Message-ID: <7vr6o7uhwz.fsf@assigned-by-dhcp.pobox.com>
In-Reply-To: <E1I0tZY-0001Uz-00@www.watts.utsl.gen.nz>

Sam Vilain <samv@utsl.gen.nz> writes:

Show 12 quoted lines

> #!/bin/sh
> #
> # An example hook script to prepare a packed repository for use over
> # dumb transports.
> #
> # To enable this hook, make this file executable by "chmod +x post-update".
>
> git-update-server-info
>
> ref=$1
> active=`git-symbolic-ref HEAD`
> if [ "$ref" = "$active" ]

post-update hook takes list of refs that have been updated, so the above check is bogus, I think.

Better do something like:

	case " $* " in
        *" $active "*)
        	: current branch was updated...
		;;
	*)
        	: do not have to worry about this one
                exit 0
                ;;
	esac
	# ... rest of the code here ...

Also you would want to see if the thing is bare.

Unfortunately, git-sh-setup's is_bare_repository does a wrong thing because receive-pack already runs GIT_DIR set to '.' after chdir to it. Matthias's long set of patches currently parked in 'pu' is supposed to clarify the semantics of bare/non-bare, so it may help though (it has "rev-parse --is-bare-repository).

I think 80% of the time (because other 20% of bare repositories are initialized, following an often repeated incorrect procedure of making a worktree-full repository and moving its .git/ to another place, $name.git --- that is wrong for at least three reasons), if $GIT_DIR/index exists, you can treat it as a non-bare repository.

So in short, I would follow the above quoted one with:

	test -f "$GIT_DIR/index" || exit 0

to ignore bare repositories.

Matthias, do you know if your patchset correctly handle this case? That is:

	$ git push site:my.git set of refs

will:

        (1) spawn git-receive-pack with my.git as its
            argument;

        (2) git-receive-pack finds the repository and chdir()'s
            to the directory that has refs/ and HEAD in it
            (i.e. if my.git is bare, the directory itself, if
            my.git has my.git/.git and possibly a checkout in
            my.git/{Makefile,README,...}, then it chdir()'s to
            my.git/.git);

	(3) then it sets GIT_DIR=. and exports it.

and from that environment, post-update hook is called.

I think Matthias's patch series may need to (conditionally) set GIT_WORK_TREE environment when it finds the repository and see if there is an associated work tree at its natural place (that is, if $something/.git is found in (2) and $something/.git/index exists, then worktree should be rooted at $something/, unless core.worktree is set) .

> then
>   echo "Pushing to checked out branch - updating working copy" >&2
>   export GIT_DIR=`cd $GIT_DIR; pwd`

... and Matthias's patch series probably needs to update this part of your patch as well.

>   cd ..
>   success=
>   if git-diff-files
>   then

I would suggest running update-index --refresh before running diff-files. The user may have cache-dirty file and only that.

Show 12 quoted lines

>     git-diff-index -z -R --name-status HEAD | perl -n0 -le \
> 	'if ($z^=1) {
> 		$status=$_;
> 	 }
> 	 else {
>         	$filename=$_;
> 		printf STDERR "$status\t$filename\n";
> 		if($status eq "D"){
> 			unlink($filename)
> 				or die("unlink($filename) failed; $!")
> 		}
>          }' &&

With "--name-only --diff-filter=D" you would not have to do most of the above ;-) Use --diff-filter=A without -R, perhaps, to make it even shorter.

>     git-reset --hard HEAD && success=1
>   fi

Wouldn't "reset --hard HEAD" pretty much unconditionally nuke your local changes, including added files to the index? For example, if I do this:

	$ >foo && git add foo && git reset --hard HEAD

it would remove the newly added 'foo' from both the index and the working tree. So I am not quite sure what you are trying to achieve with "diff-index | perl" magic.

Show 15 quoted lines

>   if [ -z "$success" ]
>   then
>     (
>     echo "Non-bare repository checkout is not clean - not updating it"
>     echo "However I AM going to update the index.  Any in-progress commit"
>     echo "happening in that checkout will be thrown away, but on the bright"
>     echo "side this is probably the least confusing thing for us to do and"
>     echo "at least we're not throwing any files somebody has changed away"
>     git-reset --mixed HEAD
>     echo 
>     echo "This is the new status of the upstream working copy:"
>     git-status
>     ) >&2
>   fi
> fi

I'll cook up an alternative patch perhaps tomorrow.