Passing mx_include_dir via pip install --global-option (or detect egenix-mx-base automatically?)

[cthart]

We have a legacy dependency on mxDateTime and need to build pscyopg2 with support for it.

Currently it's impossible to build psycopg2 with support for it using pip install, or even passed as a parameter to setup.py when building from source; I need to download the source and edit the setup.cfg file.

Would it be possible to add support for passing this via --global-option or some other parameter to pip install?
I tried to determine how to do this and can see options for use-pydatetime, have-ssl, and static-libpq but can't see where they are used.

Or would it even be possible to detect that egenix-mx-base is installed, and build support for mxDateTime automatically?

[dvarrazzo]

I'm not an expert with pip, and usually have a hard time figuring setuptools/distutils internals.

If you would like to work in it and provide a patch we'll be happy to apply, otherwise I'm not keen to do work on mx.

[cthart]

I have now started looking at this. I now understand what needs to be done, but it requires some restructuring in setup.py

I can submit a patch soon, but it would help me if I could get some input:

Who has written the code that parses setup.cfg?
There is code that adds include dirs etc based on parameters in setup.cfg This code need needs to go inside the build_ext class so that those additions get done correctly whether they are specified in setup.cfg or passed as a parameter to setup.py
So it would help me if I could get in touch with the original author so that I can get the reasoning behind the code the way it is today.

IMHO the setup.py could benefit with some structuring and cleanup

• the variable have_mxdatetime isn't being used (I grepped the whole codebase, and even my entire Python virtual environment) -- though HAVE_MXDATETIME is set later
• mxincludedir is used as name in one place mx_include_dir in another
• get_python_inc() doesn't find include files inside a virtual environment (I'm still trying to see if this can be fixed too)
• passing --pg-config to setup.py build_ext is I think, broken (not sure on that)

What's the best way of communicating with the developers on this? I can devote some time to this over the next few months and get this cleaned up properly, if you want me to do this.

[fogzot]

I think that code was written by me more than 15 years ago. You can ask your questions here and I'll try to answer and help as I can but note that I didn't do any Python distutils in the last 10 years. ;)

[dvarrazzo]

Who has written the code that parses setup.cfg?

parsing setup.cfg is part of distutils/setuptools, it's not something we have in our setup.py.

There is code that adds include dirs etc based on parameters in setup.cfg This code need needs to go inside the build_ext class so that those additions get done correctly whether they are specified in setup.cfg or passed as a parameter to setup.py

My experience with setup() is that it follows a quite convoluted logic. If you can wrap your head around it that's ok for me.

So it would help me if I could get in touch with the original author so that I can get the reasoning behind the code the way it is today.

IMHO the setup.py could benefit with some structuring and cleanup

* the variable have_mxdatetime isn't being used (I grepped the whole codebase, and even my entire Python virtual environment) -- though HAVE_MXDATETIME is set later

Probably that's about having mx as the default datetime adapter. That was a choice reasonable like in python 2.2, I can't remember when datetime was introduced. It was dropped a few years ago because was unused/untested and probably outright bitrotten. Support from the code was removed actually not long ago - May 2019 according to git - probably that's a leftover and it can go.

Any other improvement and care for consistency is welcome.

What's the best way of communicating with the developers on this? I can devote some time to this over the next few months and get this cleaned up properly, if you want me to do this.

You can use this issue or the mailing list.

[dvarrazzo]

You can use this issue or the mailing list.

Or open a merge request, which actually works ok for code review.

[cthart]

Maybe it’s high time for an overhaul then. I’ll do my best to clean it up but at the same time to keep the patch small... As I said, I now understand how to add a parameter to enable building of MX support from the command line and also why autodetection of the MX includes doesn’t work when using venv (the second would be really nice to fix too, but we can live with passing a parameter, as long as it can be passed to pip install).

…

On 4 May 2020, at 16:54, Federico Di Gregorio ***@***.***> wrote:  I think that code was written by me more than 15 years ago. You can ask your questions here and I'll try to answer and help as I can but note that I didn't do any Python distutils in the last 10 years. ;) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

[dvarrazzo]

but we can live with passing a parameter, as long as it can be passed to pip install

Given the scope of your patch (not many people use MX anymore) I'd keep this as simple as possible: whatever fulfils your needs and doesn't break the rest would work for us, even if it's not complete (e.g. if command line works but setup.cfg doesn't, no big deal). I would add an option in user_options.extend() and try to use it mimicking other options that are already there (note the mangling of - and _: the --have-ssl param becomes an option have_ssl in the parser etc.

You could probably get better help from the Python Packaging Authority people than from us to understand how the setup machinery works.

[cthart]

Well, I ended up finding out how to locate mxDateTime.h automatically. I've submitted a pull request to add this.