On Wed, 2 Feb 2011, Dongsheng Song wrote:
> Index: hooks/update_datestamp
> ===================================================================
> --- hooks/update_datestamp      (revision 0)
> +++ hooks/update_datestamp      (revision 0)
> @@ -0,0 +1,51 @@
> +#!/bin/sh
> +
> +REPOS="$1"
> +REV="$2"
> +
> +PATH=/usr/local/bin:/usr/pkg/bin:/usr/bin:/bin

/usr/local/bin on gcc.gnu.org is scary, and I think not needed; 
/usr/pkg/bin does not actually exist.

> +IGNORE_BRANCHES='gcc-(2_95|3_0|3_1|3_2|3_3|3_4|4_0|4_1|4_2)-branch'

I believe we can omit this altogether.  I see thhis script is a clone
of the one running once a day to bump on all branches, and there we
wanted to limit where that bumping happens.  As long as a branch is
committed to, I don't see any problem with bumping the data stamp.

Anyone disagrees?

> +BRANCHES=`svnlook -r ${REV} dirs-changed "${REPOS}" \

Do we really need to worry about more than branch being hit in one
commit?  I wasn't aware that SVN supports this, but I guess it's
defensive programming. :-)

> +| grep -E "^trunk/|^branches/gcc-[0-9]+_[0-9]+-branch/" \

Can you make this one a variable at the top, ${BRANCH_REGEXP} or
something like that?

> +  if ! svn commit -m "Daily bump." gcc/DATESTAMP; then
> +    # If we could not commit the files, indicate failure.
> +    RESULT=1
> +  fi

Can we also issue an error message here?

> Index: hooks/post-commit
> ===================================================================
> --- hooks/post-commit   (revision 169520)
> +++ hooks/post-commit   (working copy)
> @@ -17,3 +17,5 @@
>     --repository "${REPOS}" --revision "${REV}" --background
> 
>  ${REPOS}/hooks/synchooks.sh "${REPOS}" "${REV}"
> +
> +${REPOS}/hooks/update_version_svn ${REPOS} ${REV} &

This should have been hooks/update_datestamp, no?  We could, in fact,
use update_version_svn, too, that would be a bit of a performance issue, 
though. :-)

Gerald

Reply via email to