Re: GCC Git hooks

[Jonathan Wakely <jwakely.gcc@gmail.com>]

On Thu, 9 Jan 2020 at 22:07, Joseph Myers wrote:
>
> @@ -63,6 +63,8 @@ class BranchUpdate(AbstractUpdate):
>          # the update unless we have had a chance to verify that these hooks
>          # work well with those branches.
>          assert (self.ref_name.startswith('refs/heads/')
> +                or self.ref_name.startswith('refs/users/')
> +                or self.ref_name.startswith('refs/vendors/')
>                  # Namespaces used by Gerrit.
>                  or self.ref_name.startswith('refs/meta/')
>                  or self.ref_name.startswith('refs/publish/')
> @@ -80,6 +82,20 @@ class BranchUpdate(AbstractUpdate):
>          # irrelevant.
>          if not is_null_rev(self.old_rev):
>              check_fast_forward(self.ref_name, self.old_rev, self.new_rev)
> +            # GCC-specific: do not allow updates introducing ancestry
> +            # based on the old git-svn repository, to ensure people
> +            # rebase onto the new history rather than merging branches
> +            # based on git-svn history into those based on the new history.
> +            rev_list = git.rev_list(
> +                self.new_rev, '^%s' % self.old_rev)
> +        else:
> +            rev_list = git.rev_list(
> +                self.new_rev)
> +        if '3cf0d8938a953ef13e57239613d42686f152b4fe' in rev_list:
> +            raise InvalidUpdate(
> +                'Refs not based on the git-svn history must not be '
> +                'updated to be based on it, and new branches may not be '
> +                'based on the old history.')

Could you avoid the double negative here? And the error message could
be more specific to the actual error by testing the two cases
separately, e.g.

        if not is_null_rev(self.old_rev):
            check_fast_forward(self.ref_name, self.old_rev, self.new_rev)
            # GCC-specific: do not allow updates introducing ancestry
            # based on the old git-svn repository, to ensure people
            # rebase onto the new history rather than merging branches
            # based on git-svn history into those based on the new history.
            rev_list = git.rev_list(
                self.new_rev, '^%s' % self.old_rev)
            if '3cf0d8938a953ef13e57239613d42686f152b4fe' in rev_list:
                raise InvalidUpdate(
                'Refs must not be updated to introduce history from '
                'the old git-svn repo.')
        else:
            rev_list = git.rev_list(
                self.new_rev)
            if '3cf0d8938a953ef13e57239613d42686f152b4fe' in rev_list:
                raise InvalidUpdate(
                'New branches must not be based on the old git-svn repo.')