1053548 - Reject likely unedited mq folds

Status: ASSIGNED

(Developer Services :: Mercurial: hg.mozilla.org, defect)

Description

[Mike Hommey [:glandium]]

See, for example: https://hg.mozilla.org/mozilla-central/rev/6b285759568c

hg log -k '* * *' suggests that happens quite often.

How this happens, essentially is when two patches are folded with mq, their summaries are concatenated and '* * *' is added in between. I think we should detect those '* * *' and reject changesets containing them.

Comment 1

[Ed Morley [:emorley]]

This would be good to do, there are just a few cases it would break at present, eg:

https://hg.mozilla.org/mozilla-central/rev/b72560289748 (|hg qbackout -s| style output [1])
https://hg.mozilla.org/mozilla-central/rev/eeb213ed89b1 (cairo backport, presumably done by script)

[1] https://hg.mozilla.org/hgcustom/version-control-tools/file/201f04d2215d/hgext/qbackout/__init__.py#l226

Comment 2

[Ed Morley [:emorley]]

Though we could just special-case the backouts for now.

Comment 3

[Gregory Szorc [:gps]]

Attachment: hghooks: reject likely unedited commit message folds (bug 1053548);

Various Mercurial operations that combine changesets will concatenate
commit messages together using a "* * *" line. The history of
mozilla-central shows a handful of commit messages like this. Most
instances appear to be unintentional and authors probably don't
realize they are doing it.

This commit modifies the commit message hook to reject commit messages
with these lines.

Review commit: https://reviewboard.mozilla.org/r/90742/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/90742/

Comment 4

[Mike Hommey [:glandium]]

Comment on attachment 8807648 [details]
hghooks: reject likely unedited commit message folds (bug 1053548);

https://reviewboard.mozilla.org/r/90742/#review91168

::: hghooks/mozhghooks/commit-message.py:35
(Diff revision 1)
>  ]]
>  
>  trySyntax = re.compile(r'\btry:')
>  
> +# A line with "* * *" is likely an unedited commit message fold.
> +RE_MESSAGE_FOLD = re.compile('^\* \* \*$', re.MULTILINE)

We may want to match ^***$ too. There are more than a few in the mercurial history.

::: hghooks/mozhghooks/commit-message.py:67
(Diff revision 1)
>          message("Rev {rev} contains 'r?' in the commit message. Please use 'r=' instead.")
>          return False
>  
> +    if RE_MESSAGE_FOLD.search(desc):
> +        message('Rev {rev} contains a likely unedited commit message fold. '
> +                'Please edit the commit message to remove `* * *`.')

Might be worth being more explicit here, because I can imagine people not looking further and just removing the separator line, and creating weird commit messages.

Comment 5

[Mike Hommey [:glandium]]

Forgot to add: this is better than nothing, but I don't quite like that this relies on a client-side hook. Not everyone using mercurial uses vct.

Comment 6

[Gregory Szorc [:gps]]

This hook runs on the server. It is also possible to run some of these hooks on clients. Although that isn't their target audience.