Patchwork util: document we want Python type mapping to be temporary

login
register
mail settings
Submitter Gregory Szorc
Date Oct. 8, 2016, 5:16 p.m.
Message ID <d15d8ac73cfd2d1ddbd4.1475947014@gps-mbp.local>
Download mbox | patch
Permalink /patch/16958/
State Accepted
Headers show

Comments

Gregory Szorc - Oct. 8, 2016, 5:16 p.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1475947010 -7200
#      Sat Oct 08 19:16:50 2016 +0200
# Node ID d15d8ac73cfd2d1ddbd443262ccad9c68ee69406
# Parent  266ad9c9faa524a8b3f473c924db409681cb205e
util: document we want Python type mapping to be temporary

I think remapping Python C API types and functions is not a great
approach. I'd prefer this whole #ifdef disappeared. Add a comment
so we don't forget about it.
Jun Wu - Oct. 8, 2016, 9:16 p.m.
These look good to me. (Including related patches:
  util: define PyInt_Type on Python 3
  parsers: return NULL from PyInit_parsers on Python 3)

I think we can always iterate on Python 3 later.

Excerpts from Gregory Szorc's message of 2016-10-08 19:16:54 +0200:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1475947010 -7200
> #      Sat Oct 08 19:16:50 2016 +0200
> # Node ID d15d8ac73cfd2d1ddbd443262ccad9c68ee69406
> # Parent  266ad9c9faa524a8b3f473c924db409681cb205e
> util: document we want Python type mapping to be temporary
> 
> I think remapping Python C API types and functions is not a great
> approach. I'd prefer this whole #ifdef disappeared. Add a comment
> so we don't forget about it.
> 
> diff --git a/mercurial/util.h b/mercurial/util.h
> --- a/mercurial/util.h
> +++ b/mercurial/util.h
> @@ -12,8 +12,11 @@
>  
>  #if PY_MAJOR_VERSION >= 3
>  
>  #define IS_PY3K
> +/* The mapping of Python types is meant to be temporary to get Python
> + * 3 to compile. We should remove this once Python 3 support is fully
> + * supported and proper types are used in the extensions themselves. */
>  #define PyInt_Type PyLong_Type
>  #define PyInt_FromLong PyLong_FromLong
>  #define PyInt_AsLong PyLong_AsLong
>
Pierre-Yves David - Oct. 8, 2016, 9:38 p.m.
On 10/08/2016 07:16 PM, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1475947010 -7200
> #      Sat Oct 08 19:16:50 2016 +0200
> # Node ID d15d8ac73cfd2d1ddbd443262ccad9c68ee69406
> # Parent  266ad9c9faa524a8b3f473c924db409681cb205e
> util: document we want Python type mapping to be temporary
>
> I think remapping Python C API types and functions is not a great
> approach. I'd prefer this whole #ifdef disappeared. Add a comment
> so we don't forget about it.

I cannot find anywhere to apply this. This code does not seems to exists 
in our current public repository. Am I missing something?
Jun Wu - Oct. 8, 2016, 9:42 p.m.
It should be PATCH 3 of the 
[PATCH 1 of 2] parsers: return NULL from PyInit_parsers on Python 3

Note: hgpatch.lihdd.net supports getting a branch across email threads,
based on "Parent" information:
http://hgpatch.lihdd.net/d15d8ac73cfd2d1ddbd443262ccad9c68ee69406/branch

Excerpts from Pierre-Yves David's message of 2016-10-08 23:38:46 +0200:
> 
> On 10/08/2016 07:16 PM, Gregory Szorc wrote:
> > # HG changeset patch
> > # User Gregory Szorc <gregory.szorc@gmail.com>
> > # Date 1475947010 -7200
> > #      Sat Oct 08 19:16:50 2016 +0200
> > # Node ID d15d8ac73cfd2d1ddbd443262ccad9c68ee69406
> > # Parent  266ad9c9faa524a8b3f473c924db409681cb205e
> > util: document we want Python type mapping to be temporary
> >
> > I think remapping Python C API types and functions is not a great
> > approach. I'd prefer this whole #ifdef disappeared. Add a comment
> > so we don't forget about it.
> 
> I cannot find anywhere to apply this. This code does not seems to exists 
> in our current public repository. Am I missing something?
>
Yuya Nishihara - Oct. 9, 2016, 6:22 a.m.
On Sat, 08 Oct 2016 19:16:54 +0200, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1475947010 -7200
> #      Sat Oct 08 19:16:50 2016 +0200
> # Node ID d15d8ac73cfd2d1ddbd443262ccad9c68ee69406
> # Parent  266ad9c9faa524a8b3f473c924db409681cb205e
> util: document we want Python type mapping to be temporary

Queued this, too.

Patch

diff --git a/mercurial/util.h b/mercurial/util.h
--- a/mercurial/util.h
+++ b/mercurial/util.h
@@ -12,8 +12,11 @@ 
 
 #if PY_MAJOR_VERSION >= 3
 
 #define IS_PY3K
+/* The mapping of Python types is meant to be temporary to get Python
+ * 3 to compile. We should remove this once Python 3 support is fully
+ * supported and proper types are used in the extensions themselves. */
 #define PyInt_Type PyLong_Type
 #define PyInt_FromLong PyLong_FromLong
 #define PyInt_AsLong PyLong_AsLong