Thread: plpython win32
Hi! This patch attempts to fix the build of plpython on win32. Needs autoconf of course - don't have mine working on win32, so that part hasn't been 100% tested. My tests involved #:ing out all the code that would be included by that rule, and that makes it work, so I think we're safe.... //Magnus
Attachment
"Magnus Hagander" <mha@sollentuna.net> writes:
> This patch attempts to fix the build of plpython on win32.
How is python_includespec going to get set if we don't run the
autoconf test that finds it out? I'm quite unthrilled with hardwiring
the python version number, as well.
regards, tom lane
>> This patch attempts to fix the build of plpython on win32. > >How is python_includespec going to get set if we don't run the >autoconf test that finds it out? I'm quite unthrilled with hardwiring >the python version number, as well. We run the first part of the autoconf test. The one that sets python_includespec. (PGAC_PATH_PYTHON) We just skip the parts that tries to read the Makefile. If there is a good way, that subst command could/should be changed to just strip the last part of the directory. PGAC_PATH_PYTHON appends te python version, which is not correct on win32. //Magnus
Magnus Hagander wrote: > This patch attempts to fix the build of plpython on win32. Needs > autoconf of course - don't have mine working on win32, so that part > hasn't been 100% tested. My tests involved #:ing out all the code > that would be included by that rule, and that makes it work, so I > think we're safe.... Please do not use PORTNAME in external macros. I like to think that one can take these macros and put them in some other project without requiring the prior setup that the PostgreSQL configure.in does. Instead, use AC_REQUIRE([AC_CANONICAL_HOST]) and resolve the issue using $host_os. Also, add some comments to the magic you add in the makefiles. The hardcoded Python version number will of course not stand the test of time. -- Peter Eisentraut http://developer.postgresql.org/~petere/
Magnus Hagander wrote: > If there is a good way, that subst command could/should be changed to > just strip the last part of the directory. PGAC_PATH_PYTHON appends > te python version, which is not correct on win32. I'm curious to know how the code AC_PATH_PROG(PYTHON, python) "appends the python version". -- Peter Eisentraut http://developer.postgresql.org/~petere/
>> If there is a good way, that subst command could/should be changed to
>> just strip the last part of the directory. PGAC_PATH_PYTHON appends
>> te python version, which is not correct on win32.
>
>I'm curious to know how the code
>
>AC_PATH_PROG(PYTHON, python)
>
>"appends the python version".
No. Not that one. PGAC_PATH_PYTHON. That is a different line. It's
defined in config/python.m4. The line is:
python_includespec="-I${python_prefix}/include/python${python_version}"
//Magnus
Magnus Hagander wrote:
> No. Not that one. PGAC_PATH_PYTHON. That is a different line. It's
> defined in config/python.m4. The line is:
>
> python_includespec="-I${python_prefix}/include/python${python_version
>}"
Are we reading the same code?
# PGAC_PATH_PYTHON
# ----------------
# Look for Python and set the output variable 'PYTHON'
# to 'python' if found, empty otherwise.
AC_DEFUN([PGAC_PATH_PYTHON],
[AC_PATH_PROG(PYTHON, python)
if test x"$PYTHON" = x""; then
AC_MSG_ERROR([Python not found])
fi
])
--
Peter Eisentraut
http://developer.postgresql.org/~petere/
>> No. Not that one. PGAC_PATH_PYTHON. That is a different line. It's
>> defined in config/python.m4. The line is:
>>
>> python_includespec="-I${python_prefix}/include/python${python_version
>>}"
>
>Are we reading the same code?
>
># PGAC_PATH_PYTHON
># ----------------
># Look for Python and set the output variable 'PYTHON'
># to 'python' if found, empty otherwise.
>AC_DEFUN([PGAC_PATH_PYTHON],
>[AC_PATH_PROG(PYTHON, python)
>if test x"$PYTHON" = x""; then
> AC_MSG_ERROR([Python not found])
>fi
>])
Not quite. Further down in that same file. Seems I mean
PGAC_CHECK_PYTHON_DIRS :-) Not used to these files... I guess that means
that I'm off on the wrong path. :-(
Anyway. The issue is that there is no Makefile in $python_configdir.
That's the part that we need to get rid of. Any pointers? Some kind of
if in python.m4 then probably?
//Magnus
"Magnus Hagander" <mha@sollentuna.net> writes:
> We run the first part of the autoconf test. The one that sets
> python_includespec. (PGAC_PATH_PYTHON) We just skip the parts that tries
> to read the Makefile.
It would be better to put an "if" in the PGAC_CHECK_PYTHON_EMBED_SETUP
macro, and have it use some other technique for obtaining the info it
needs when on Windows.
If they don't have a Makefile, one would hope they have some other kind
of file that has the needed info. Or doesn't Python support embedding
on Windows?
regards, tom lane
>> We run the first part of the autoconf test. The one that sets >> python_includespec. (PGAC_PATH_PYTHON) We just skip the >parts that tries >> to read the Makefile. > >It would be better to put an "if" in the PGAC_CHECK_PYTHON_EMBED_SETUP >macro, and have it use some other technique for obtaining the info it >needs when on Windows. Ok. I'll look into that. >If they don't have a Makefile, one would hope they have some other kind >of file that has the needed info. Or doesn't Python support embedding >on Windows? They do. They just don't install the Makefile (could be because it's all built with MSVC). The distutils module has a get_python_inc() function which returns the include directory. If this one was used, we wouldn't have to hack up the include path as I do now. Is there any reason this is not used on Unix, instead of the hardcoded subdirectory-of-"python_prefix" way it is now? (in _PGAC_CHECK_PYTHON_DIRS) //Magnus
Magnus Hagander wrote: > The distutils module has a get_python_inc() function which returns the > include directory. If this one was used, we wouldn't have to hack up the > include path as I do now. Is there any reason this is not used on Unix, > instead of the hardcoded subdirectory-of-"python_prefix" way it is now? > (in _PGAC_CHECK_PYTHON_DIRS) Probably because until about 2 weeks ago, we didn't check for, or use, distutils at all ;-). Now we probably should. Joe
>> The distutils module has a get_python_inc() function which >returns the >> include directory. If this one was used, we wouldn't have to >hack up the >> include path as I do now. Is there any reason this is not >used on Unix, >> instead of the hardcoded subdirectory-of-"python_prefix" way >it is now? >> (in _PGAC_CHECK_PYTHON_DIRS) > >Probably because until about 2 weeks ago, we didn't check for, or use, >distutils at all ;-). Now we probably should. Thanks. That explains a lot :-) Digging a bit further into it, now that I knew that might be old code, I found that the parts that fail on win32 could be replaced with distutils code which does *not* fail on win32. So no port-specific hack required at all. So, here is a new patch. Summary: * Get python_includespec from distutils instead of assuming directory layout * Get python_libspec from distutils instead of manually grep:ing in the Makefile * Export the python version into Makefile.global for access from plpython/Makefile * Change plpython/Makefile to generate an import library from the installed python DLL. Now with comments on what and why. (This is the only if win32 part left) Hopefully this is better and more complete than the last attempt. I've tested it on Linux with python 2.3.3 (slackware 9.1), and the new autoconf stuff passes both there and on win32. I know it's close to the wrap-time, but it'd be great if this could get into beta3 (assuming it's Ok now). //Magnus
Attachment
Magnus Hagander wrote: > So, here is a new patch. Summary: There is still a hard-coded python version in "libpython23.a". -- Peter Eisentraut http://developer.postgresql.org/~petere/
> > So, here is a new patch. Summary:
>
> There is still a hard-coded python version in "libpython23.a".
Argh. I thought I caught them all. How the heck did I miss such an
obvious one.
Of cuorse, it's supposed to be libpython${pytverstr}.a... Same for the
.def file on the next line.
//mha
> > > So, here is a new patch. Summary:
> >
> > There is still a hard-coded python version in "libpython23.a".
>
> Argh. I thought I caught them all. How the heck did I miss
> such an obvious one.
> Of cuorse, it's supposed to be libpython${pytverstr}.a...
> Same for the .def file on the next line.
Here is an updated patch that fixes this. Apart from that, same as
before.
//Magnus
Attachment
Patch applied. Thanks.
---------------------------------------------------------------------------
Magnus Hagander wrote:
> > > > So, here is a new patch. Summary:
> > >
> > > There is still a hard-coded python version in "libpython23.a".
> >
> > Argh. I thought I caught them all. How the heck did I miss
> > such an obvious one.
> > Of cuorse, it's supposed to be libpython${pytverstr}.a...
> > Same for the .def file on the next line.
>
> Here is an updated patch that fixes this. Apart from that, same as
> before.
>
> //Magnus
>
Content-Description: plpython_win32.patch
[ Attachment, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo@postgresql.org so that your
> message can get through to the mailing list cleanly
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073