This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8575

hooklib: fix detection of successors for changeset_obsoleted
ClosedPublic

Authored by joerg.sonnenberger on May 21 2020, 1:07 PM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHG04ef381000a8: hooklib: fix detection of successors for changeset_obsoleted
Summary

Provide a hook for obsutil.getobsolete to be used with either a
transaction or the changes item of the transaction, since hooks only
have access to the latter. Use that to find the correct list of
revisions with obsmarkers, even new ones, and then filter out revisions
with known successors.

Move the processing from pretxnclose to txnclose as the transaction
access itself is no longer necessary. This is more in line with notify
and ensures that sanity checks can abort the transaction first.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

pulkit added subscribers: marmoute, pulkit.Jun 9 2020, 6:58 AM

Adding @marmoute as he knows this topic the best.

pulkit added inline comments.Jul 7 2020, 2:25 PM
mercurial/obsutil.py
491

This should be converted into some if-else with error messages probably. This is some amazing assert which should be avoided.

joerg.sonnenberger added inline comments.Jul 7 2020, 3:21 PM
mercurial/obsutil.py
491

It is exactly the condition from the comment? It's a sanity check for the API contract from my perspective, nothing more.

pulkit added inline comments.Jul 10 2020, 5:59 AM
mercurial/obsutil.py
491

It's better to raise ProgrammingError in such cases.

pulkit accepted this revision.Jul 11 2020, 4:18 AM
This revision is now accepted and ready to land.Jul 11 2020, 4:18 AM

Revision Contents
Changeset List

Diff 21860

hgext/hooklib/changeset_obsoleted.py

ui.write(b'\n') ui.write(b'\n')
else: else:
ui.status(_(b'notify_obsoleted: sending mail for %d\n') % ctx.rev()) ui.status(_(b'notify_obsoleted: sending mail for %d\n') % ctx.rev())
mail.sendmail( mail.sendmail(
ui, emailutils.parseaddr(msg['From'])[1], subs, msgtext, mbox=n.mbox ui, emailutils.parseaddr(msg['From'])[1], subs, msgtext, mbox=n.mbox
) )
def has_successor(repo, rev):
return any(
r for r in obsutil.allsuccessors(repo.obsstore, [rev]) if r != rev
)
def hook(ui, repo, hooktype, node=None, **kwargs): def hook(ui, repo, hooktype, node=None, **kwargs):
if hooktype != b"pretxnclose": if hooktype != b"txnclose":
raise error.Abort( raise error.Abort(
_(b'Unsupported hook type %r') % pycompat.bytestr(hooktype) _(b'Unsupported hook type %r') % pycompat.bytestr(hooktype)
) )
for rev in obsutil.getobsoleted(repo, repo.currenttransaction()): for rev in obsutil.getobsoleted(repo, changes=kwargs['changes']):
_report_commit(ui, repo, repo.unfiltered()[rev]) ctx = repo.unfiltered()[rev]
if not has_successor(repo, ctx.node()):
_report_commit(ui, repo, ctx)

mercurial/obsutil.py

# obsutil.py - utility functions for obsolescence # obsutil.py - utility functions for obsolescence
# #
# Copyright 2017 Boris Feld <boris.feld@octobus.net> # Copyright 2017 Boris Feld <boris.feld@octobus.net>
# #
# This software may be used and distributed according to the terms of the # This software may be used and distributed according to the terms of the
# GNU General Public License version 2 or any later version. # GNU General Public License version 2 or any later version.
from __future__ import absolute_import from __future__ import absolute_import
import re import re
from .i18n import _ from .i18n import _
from . import ( from . import (
diffutil, diffutil,
encoding, encoding,
error,
node as nodemod, node as nodemod,
phases, phases,
pycompat, pycompat,
util, util,
) )
from .utils import dateutil from .utils import dateutil
### obsolescence marker flag ### obsolescence marker flag
# Check if the diff has changed # Check if the diff has changed
if not _cmpdiff(source, changectx): if not _cmpdiff(source, changectx):
effects |= DIFFCHANGED effects |= DIFFCHANGED
return effects return effects
def getobsoleted(repo, tr): def getobsoleted(repo, tr=None, changes=None):
"""return the set of pre-existing revisions obsoleted by a transaction""" """return the set of pre-existing revisions obsoleted by a transaction
Either the transaction or changes item of the transaction (for hooks)
must be provided, but not both.
"""
if (tr is None) == (changes is None):
pulkitUnsubmitted
Not Done

This should be converted into some if-else with error messages probably. This is some amazing assert which should be avoided.

pulkit: This should be converted into some if-else with error messages probably. This is some amazing…
joerg.sonnenbergerAuthorUnsubmitted
Not Done

It is exactly the condition from the comment? It's a sanity check for the API contract from my perspective, nothing more.

joerg.sonnenberger: It is exactly the condition from the comment? It's a sanity check for the API contract from my…
pulkitUnsubmitted
Not Done

It's better to raise ProgrammingError in such cases.

pulkit: It's better to raise `ProgrammingError` in such cases.
e = b"exactly one of tr and changes must be provided"
raise error.ProgrammingError(e)
torev = repo.unfiltered().changelog.index.get_rev torev = repo.unfiltered().changelog.index.get_rev
phase = repo._phasecache.phase phase = repo._phasecache.phase
succsmarkers = repo.obsstore.successors.get succsmarkers = repo.obsstore.successors.get
public = phases.public public = phases.public
addedmarkers = tr.changes[b'obsmarkers'] if changes is None:
origrepolen = tr.changes[b'origrepolen'] changes = tr.changes
addedmarkers = changes[b'obsmarkers']
origrepolen = changes[b'origrepolen']
seenrevs = set() seenrevs = set()
obsoleted = set() obsoleted = set()
for mark in addedmarkers: for mark in addedmarkers:
node = mark[0] node = mark[0]
rev = torev(node) rev = torev(node)
if rev is None or rev in seenrevs or rev >= origrepolen: if rev is None or rev in seenrevs or rev >= origrepolen:
continue continue
seenrevs.add(rev) seenrevs.add(rev)

tests/test-hooklib-changeset_obsoleted.t

> * = baz > * = baz
> EOF > EOF
$ hg init a $ hg init a
$ hg --cwd a debugbuilddag +2 $ hg --cwd a debugbuilddag +2
$ hg init b $ hg init b
$ cat <<EOF >> b/.hg/hgrc $ cat <<EOF >> b/.hg/hgrc
> [hooks] > [hooks]
> incoming.notify = python:hgext.notify.hook > incoming.notify = python:hgext.notify.hook
> pretxnclose.changeset_obsoleted = python:hgext.hooklib.changeset_obsoleted.hook > txnclose.changeset_obsoleted = python:hgext.hooklib.changeset_obsoleted.hook
> EOF > EOF
$ hg --cwd b pull ../a | "$PYTHON" $TESTDIR/unwrap-message-id.py $ hg --cwd b pull ../a | "$PYTHON" $TESTDIR/unwrap-message-id.py
pulling from ../a pulling from ../a
requesting all changes requesting all changes
adding changesets adding changesets
adding manifests adding manifests
adding file changes adding file changes
added 2 changesets with 0 changes to 0 files added 2 changesets with 0 changes to 0 files
1 new obsolescence markers 1 new obsolescence markers
obsoleted 1 changesets obsoleted 1 changesets
1 new orphan changesets 1 new orphan changesets
$ hg --cwd a push ../b --hidden | "$PYTHON" $TESTDIR/unwrap-message-id.py $ hg --cwd a push ../b --hidden | "$PYTHON" $TESTDIR/unwrap-message-id.py
1 new orphan changesets 1 new orphan changesets
pushing to ../b pushing to ../b
searching for changes searching for changes
no changes found no changes found
1 new obsolescence markers
obsoleted 1 changesets
Subject: changeset abandoned Subject: changeset abandoned
In-reply-to: <hg.81c297828fd2d5afaadf2775a6a71b74143b6451dfaac09fac939e9107a50d01@example.com> In-reply-to: <hg.81c297828fd2d5afaadf2775a6a71b74143b6451dfaac09fac939e9107a50d01@example.com>
Message-Id: <hg.d6329e9481594f0f3c8a84362b3511318bfbce50748ab1123f909eb6fbcab018@example.com> Message-Id: <hg.d6329e9481594f0f3c8a84362b3511318bfbce50748ab1123f909eb6fbcab018@example.com>
Date: * (glob) Date: * (glob)
From: test@example.com From: test@example.com
To: baz@example.com To: baz@example.com
This changeset has been abandoned. This changeset has been abandoned.
Check that known changesets with known successors do not result in a mail.
$ hg init c
$ hg init d
$ cat <<EOF >> d/.hg/hgrc
> [hooks]
> incoming.notify = python:hgext.notify.hook
> txnclose.changeset_obsoleted = python:hgext.hooklib.changeset_obsoleted.hook
> EOF
$ hg --cwd c debugbuilddag '.:parent.*parent'
$ hg --cwd c push ../d -r 1
pushing to ../d
searching for changes
adding changesets
adding manifests
adding file changes
added 2 changesets with 0 changes to 0 files
$ hg --cwd c debugobsolete $(hg --cwd c log -T '{node}' -r 1) $(hg --cwd c log -T '{node}' -r 2)
1 new obsolescence markers
obsoleted 1 changesets
$ hg --cwd c push ../d | "$PYTHON" $TESTDIR/unwrap-message-id.py
pushing to ../d
searching for changes
adding changesets
adding manifests
adding file changes
added 1 changesets with 0 changes to 0 files (+1 heads)
1 new obsolescence markers 1 new obsolescence markers
obsoleted 1 changesets obsoleted 1 changesets