Re: [PATCH] Convert emailing part of hooks--update to hooks--post-receive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Andy Parkins <andyparkins@xxxxxxxxx> writes:

> On Saturday 2007, March 24, Junio C Hamano wrote:
> ...
>> You are doing this one branch at a time even though a single
>> push may update more than one branch.  I wonder what happens if
>> branch A and B are pushed, and they both contain shared new
>> material?  When this is run for A, the entire contribution of B
>> (after push happened) is excluded.  Then when this is run for
>> B, the entire contribution of A is excluded.  Doesn't that mean
>> the commits new to this repository that are shared between A and
>> B appear for neither branches?  There must be something obvious
>> I am missing...
>
> Hmmm, that's a good point.  That's an assumption from the old update 
> hook that is no longer true.  I have no answer for that.

I think you can do something along the following line.

 (1) You say "for-each-ref --all" to get the ref information
     that is after update.

 (2) Instead of looping and processing one at a time, grab all
     information upfront.  replace the object name you obtained
     in (1) for the ref being updated with the oldrev info you
     discovered.

 (3) Handle the branch one at a time, but instead of using --all,
     use the result of (2) as the "state of the repository and
     its refs before this update".

In other words, you have enough information to reconstruct the
view of the repository before your push took place.  After
running "for-each-ref --all" once upfront, you reconstruct such
a view, and after that you do not ever look at where the real
refs are in the repository.  That would perhaps alleviate the
issue of racing pushers as a side effect.

>> > +
>> > +#
>> > +# Called for the deletion of a branch
>> > +#
>> > +generate_delete_branch_email()
>> > +{
>> > +	echo "       was  $oldrev"
>> > +	echo ""
>> > +}
>>
>> Perhaps with a one-line?
>
> Don't understand.  Do you mean throw the excessive size?

Sorry, what I meant was:

	git show -s --pretty=oneline "$oldrev"

>> Perfect example of using for-each-ref and nicely done.  Contrary
>> to what you said earlier, I see a hyphen between git and
>> for-each-ref, though.
>
> Fixed.  I wish I could take credit, but it was Shawn's idea :-)

Actually it was not quite "nicely done".  If taggername has an
unusual character then dq pair you hard coded there may not
quote the string correctly.

> +	# Use git-for-each-ref to pull out the individual fields from the tag
> +	eval $(git-for-each-ref --format='
> +	tagobject="%(*objectname)"
> +	tagtype="%(*objecttype)"
> +	tagger="%(taggername)"
> +	tagged="%(taggerdate)"' $refname
> +	)

I even suspect that a malicious tagger could do:

	GIT_COMMITTER_NAME='Andy `rm -fr .` Parkins' \
        git tag -a my-malicious-tag

and push it to you.  Telling for-each-ref to quote properly for
the host language, like this:

	git-for-each-ref --shell --format='
        	o=%(*objectname)
                t=%(*objecttype)
                n=%(taggername)
                d=%(taggerdate)' $refname

would be safer.

>> > +		# If the tagged object is a commit, then we assume this is a
>> > +		# release, and so we calculate which tag this tag is replacing
>> > +		prevtag=$(git describe --abbrev=0 $newrev^ 2>/dev/null)
>>
>> What if $newrev is a merge, I wonder...
>
> Oh yes; but that one is Andreas's fault:
>
> (Andreas Ericsson   54)  prev=$(git describe "$3^" | sed 's/-g.*//')
>
> Can we hope that even if it is a merge, that either parent would be on 
> the same previous tag?
>
>  X --- * -- * --- * --- * -- Y
>         \                   /
>          * --- * --- * --- *
>
> Not guaranteed, but what else is valid?

You could describe all the parents and see if they differ.  If
they reach different tag, we could see which one is newer.  Or
something like that.  In the special (but usual) case of a
single parent commit, "describing all the parents" is what you
are already doing, so it is not any more expensive in the normal
case.

>> This goes on the Subject: header; you'd better make sure the
>> repository is not misconfigured to have more than one lines in
>> it.
>
> projectdesc=$(cat $GIT_DIR/description | sed -ne '1p')
>
> ?

Please do not have cat on either side of a pipe.  That makes a
shell script look ... eh, you know the word ;-).

	sed -e 1q "$GIT_DIR/description"

-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]