classification
Title: _uuidmodule.c cannot build on AIX - different typedefs of uuid_t, etc..
Type: behavior Stage: resolved
Components: Extension Modules Versions: Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Michael.Felt, pitrou
Priority: normal Keywords: patch

Created on 2017-12-21 18:49 by Michael.Felt, last changed 2017-12-30 21:40 by pitrou. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 4974 merged Michael.Felt, 2017-12-22 10:17
Messages (7)
msg308894 - (view) Author: Michael Felt (Michael.Felt) * Date: 2017-12-21 18:49
I was hoping for something simple - as in:

    +1  #define PY_SSIZE_T_CLEAN
    +2
    +3  #include "Python.h"
    +4  #ifndef _AIX
    +5  #include <uuid/uuid.h>
    +6  #else
    +7  #include <uuid.h>
    +8  #endif

However, it dies - instantly.

       11 | static PyObject *
       12 | py_uuid_generate_time_safe(void)
       13 | {
       14 | #ifdef HAVE_UUID_GENERATE_TIME_SAFE
       15 |     uuid_t out;
       16 |     int res;
       17 |
       18 |     res = uuid_generate_time_safe(out);
       19 |     return Py_BuildValue("y#i", (const char *) out, sizeof(out), res);
       20 | #else
       21 |     uuid_t out;
       22 |     uuid_generate_time(out);
       23 |     return Py_BuildValue("y#O", (const char *) out, sizeof(out), Py_None);
       23 +     return _Py_BuildValue_SizeT("y#O", (const char *) out, sizeof(out), (&_Py_NoneStruct));
"/data/prj/python/git/python3-3.7.0.a3/Modules/_uuidmodule.c", line 23.48: 1506-117 (S) Operand must be a scalar type.
       24 | #endif
       25 | }
       26 |

On a linux system I see:

typedef unsigned char uuid_t[16];

while on AIX the typedef is:

/*
 * Universal Unique Identifier (UUID) types.
 */
typedef struct _uuid_t
{
    unsigned32          time_low;
    unsigned16          time_mid;
    unsigned16          time_hi_and_version;
    unsigned8           clock_seq_hi_and_reserved;
    unsigned8           clock_seq_low;
    byte               node[6];
} uuid_t, *uuid_p_t;

So, mentioning this for now - as I do not yet know the module. If someone with intimate knowledge of the current implementation is willing to help me - I'll dabble and learn - and see if we can make it work on AIX as well.

p.s. - guessing on the "Extension Modules" label. If not the right choice, please update.
msg308896 - (view) Author: Michael Felt (Michael.Felt) * Date: 2017-12-21 19:13
So - KISS principle:

This diff shows what can compile:

diff --git a/Modules/_uuidmodule.c b/Modules/_uuidmodule.c
index d4bc3c7..5550705 100644
--- a/Modules/_uuidmodule.c
+++ b/Modules/_uuidmodule.c
@@ -1,7 +1,11 @@
 #define PY_SSIZE_T_CLEAN

 #include "Python.h"
+#ifndef _AIX
 #include <uuid/uuid.h>
+#else
+#include <uuid.h>
+#endif


 static PyObject *
@@ -16,7 +20,11 @@ py_uuid_generate_time_safe(void)
 #else
     uuid_t out;
     uuid_generate_time(out);
+#ifndef _AIX
     return Py_BuildValue("y#O", (const char *) out, sizeof(out), Py_None);
+#else
+    return Py_BuildValue("y#O", (const char *) &out, sizeof(out), Py_None);
+#endif
 #endif
 }

However, no uuid_generate_time(). So, ends with:
ld: 0711-317 ERROR: Undefined symbol: .uuid_generate_time
msg308911 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2017-12-21 22:41
Keep in mind _uuid is an optional C extension that is used to accelerate the uuid module when available.  It doesn't bring any additional functionality by itself.
msg308929 - (view) Author: Michael Felt (Michael.Felt) * Date: 2017-12-22 09:00
Understood.

What I have learned.

Although the types are quite different, they are both 16 bytes.

Starting with AIX 6.1, libc includes uuid_create(&uuid, &status)
which fills "uuid" with a uuid.uuid1() like result.
After calling uuid_to_string( &uuid, &uuid_string, &status); the result:
"13d866fa-e6f1-11e7-8017-fad18cf76204"

Further reading of the documentation within """ and """ in Lib/uuid.py helps me realize that the AIX approach aligns with the UUID fields description:

        fields      a tuple of the six integer fields of the UUID,
                    which are also available as six individual attributes
                    and two derived attributes:

            time_low                the first 32 bits of the UUID
            time_mid                the next 16 bits of the UUID
            time_hi_version         the next 16 bits of the UUID
            clock_seq_hi_variant    the next 8 bits of the UUID
            clock_seq_low           the next 8 bits of the UUID
            node                    the last 48 bits of the UUID


So - with this: there is also more than can be done for AIX re: https://bugs.python.org/issue28009

More to come...
msg308952 - (view) Author: Michael Felt (Michael.Felt) * Date: 2017-12-23 10:26
As the 'basics' seem to be 'accepted', going to work on the PR so that configure.ac can prepare the right choices, rather than rely on a hard-coded _AIX determined macro.
msg308953 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2017-12-23 11:09
Does this approach allow test_uuid to pass for you?
msg309262 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2017-12-30 21:39
New changeset 0d3ccb4395cccb11a50289c84c9a0dbbac03c647 by Antoine Pitrou (Michael Felt) in branch 'master':
bpo-32399: Starting with AIX6.1 there is support in libc.a for uuid (RFC4122) (#4974)
https://github.com/python/cpython/commit/0d3ccb4395cccb11a50289c84c9a0dbbac03c647
History
Date User Action Args
2017-12-30 21:40:46pitrousetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2017-12-30 21:39:23pitrousetmessages: + msg309262
2017-12-23 11:09:36pitrousetmessages: + msg308953
2017-12-23 10:26:22Michael.Feltsetmessages: + msg308952
2017-12-22 10:17:02Michael.Feltsetkeywords: + patch
stage: patch review
pull_requests: + pull_request4865
2017-12-22 09:00:38Michael.Feltsettype: behavior
2017-12-22 09:00:24Michael.Feltsetmessages: + msg308929
2017-12-21 22:41:09pitrousetnosy: + pitrou
messages: + msg308911
2017-12-21 19:13:40Michael.Feltsetmessages: + msg308896
2017-12-21 18:49:07Michael.Feltcreate