Patchwork [2,of,6] phases: fix error return with no exception from computephases()

login
register
mail settings
Submitter Yuya Nishihara
Date July 18, 2020, 10:12 a.m.
Message ID <fb15ffcb12fae7e7c73d.1595067178@mimosa>
Download mbox | patch
Permalink /patch/46797/
State Accepted
Headers show

Comments

Yuya Nishihara - July 18, 2020, 10:12 a.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1595063621 -32400
#      Sat Jul 18 18:13:41 2020 +0900
# Node ID fb15ffcb12fae7e7c73dfbbc271874f0f3418193
# Parent  bf0637f0915c1365c58d37e55f511323af0dc706
phases: fix error return with no exception from computephases()

PySet_Check() does not set an exception. Since we don't have to care whether
the roots is a set or any other iterable type, let's simply relax the type
checking.
Joerg Sonnenberger - July 18, 2020, 6:54 p.m.
On Sat, Jul 18, 2020 at 07:12:58PM +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1595063621 -32400
> #      Sat Jul 18 18:13:41 2020 +0900
> # Node ID fb15ffcb12fae7e7c73dfbbc271874f0f3418193
> # Parent  bf0637f0915c1365c58d37e55f511323af0dc706
> phases: fix error return with no exception from computephases()
> 
> PySet_Check() does not set an exception. Since we don't have to care whether
> the roots is a set or any other iterable type, let's simply relax the type
> checking.

This is the one part I don't agree with. I'd really prefer to keep the
check strict here since the behavior of the different iterators is
subtile different across the various containers.

Joerg
Yuya Nishihara - July 19, 2020, 8:48 a.m.
On Sat, 18 Jul 2020 20:54:20 +0200, Joerg Sonnenberger wrote:
> On Sat, Jul 18, 2020 at 07:12:58PM +0900, Yuya Nishihara wrote:
> > # HG changeset patch
> > # User Yuya Nishihara <yuya@tcha.org>
> > # Date 1595063621 -32400
> > #      Sat Jul 18 18:13:41 2020 +0900
> > # Node ID fb15ffcb12fae7e7c73dfbbc271874f0f3418193
> > # Parent  bf0637f0915c1365c58d37e55f511323af0dc706
> > phases: fix error return with no exception from computephases()
> > 
> > PySet_Check() does not set an exception. Since we don't have to care whether
> > the roots is a set or any other iterable type, let's simply relax the type
> > checking.
> 
> This is the one part I don't agree with. I'd really prefer to keep the
> check strict here since the behavior of the different iterators is
> subtile different across the various containers.

Okay, I'll add PyErr_SetString() then.

Please drop only this patch. The other patch won't conflict.

Patch

diff --git a/mercurial/cext/revlog.c b/mercurial/cext/revlog.c
--- a/mercurial/cext/revlog.c
+++ b/mercurial/cext/revlog.c
@@ -757,8 +757,6 @@  static int add_roots_get_min(indexObject
 	int rev, minrev = -1;
 	char *node;
 
-	if (!PySet_Check(roots))
-		return -2;
 	iterator = PyObject_GetIter(roots);
 	if (iterator == NULL)
 		return -2;