Обсуждение: [pgadmin-hackers] [pgAdmin4] [PATCH] Simplify Server's python setup

Поиск
Список
Период
Сортировка

[pgadmin-hackers] [pgAdmin4] [PATCH] Simplify Server's python setup

От
Jan Alexander Steffens
Дата:
Hi,

in the process of packaging pgAdmin4 in Arch Linux I ran into trouble getting it to start, which I reduced to a bad sys.path.

The attached patch simplifies the Python initialization. If the program name is set to match the venv's interpreter executable, Py_Initialize will set up sys.path to match the virtualenv.

This means that, assuming the venv is in the right directory, no further configuration of the PythonPath should be necessary.

I tested this using Python 2.7.13 and Python 3.6. I could not test this on Windows or OSX, but I made an effort at supporting them. Since this patch offloads all the path gathering to Python itself, the differences between the platforms are minimal.

If relevant, the script I use to package is at https://pkgbuild.com/~heftig/packages/pgadmin4/PKGBUILD . The interesting stuff is in the build() and package() functions, which should be self-explanatory.

Thanks,
Jan Steffens




Вложения

Re: [pgadmin-hackers] [pgAdmin4] [PATCH] Simplify Server's python setup

От
Dave Page
Дата:
Hi

On Wed, Jan 25, 2017 at 9:21 AM, Jan Alexander Steffens
<jan.steffens@gmail.com> wrote:
> Hi,
>
> in the process of packaging pgAdmin4 in Arch Linux I ran into trouble
> getting it to start, which I reduced to a bad sys.path.
>
> The attached patch simplifies the Python initialization. If the program name
> is set to match the venv's interpreter executable, Py_Initialize will set up
> sys.path to match the virtualenv.
>
> This means that, assuming the venv is in the right directory, no further
> configuration of the PythonPath should be necessary.
>
> I tested this using Python 2.7.13 and Python 3.6. I could not test this on
> Windows or OSX, but I made an effort at supporting them. Since this patch
> offloads all the path gathering to Python itself, the differences between
> the platforms are minimal.
>
> If relevant, the script I use to package is at
> https://pkgbuild.com/~heftig/packages/pgadmin4/PKGBUILD . The interesting
> stuff is in the build() and package() functions, which should be
> self-explanatory.

As far as I can see, this breaks usage in development environments, or
any configurations where the Python code may be separated from the
runtime. Am I missing something?

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


Re: [pgadmin-hackers] [pgAdmin4] [PATCH] Simplify Server's python setup

От
Jan Alexander Steffens
Дата:
On Wed, Jan 25, 2017 at 12:39 PM Dave Page <dpage@pgadmin.org> wrote:
As far as I can see, this breaks usage in development environments, or
any configurations where the Python code may be separated from the
runtime. Am I missing something?

What kind of setup are you talking about? It's not clear to me.

If the venv doesn't exist, the program name becomes "/python" and you get the standard environment including whatever packages you installed with "pip install" or "pip install --user".

Granted, "/python" is nonsense and should probably just be "python". Also, I think PATH doesn't need to be touched at all anymore, so the lines involving pathEnv can be removed.

How about a settings key to force the program name? That would allow choosing any other virtualenv to run with.

Re: [pgadmin-hackers] [pgAdmin4] [PATCH] Simplify Server's python setup

От
Dave Page
Дата:
On Wed, Jan 25, 2017 at 1:43 PM, Jan Alexander Steffens
<jan.steffens@gmail.com> wrote:
> On Wed, Jan 25, 2017 at 12:39 PM Dave Page <dpage@pgadmin.org> wrote:
>>
>> As far as I can see, this breaks usage in development environments, or
>> any configurations where the Python code may be separated from the
>> runtime. Am I missing something?
>
>
> What kind of setup are you talking about? It's not clear to me.

One where the virtualenv is in a path that's not related to the
location of the runtime. If I'm reading the code correctly, you're
taking the appDir (the location of the runtime executable) and
appending a platform-specific path to the virtual env (e.g. on Linux,
"/../venv/bin/"), and expecting to find the Python executable in the
resulting directory.

However, in a dev environment, we may have the venv in
~/.virtualenvs/pgadmin4 and the runtime in
~/git/pgadmin4/runtime/<builddir> for example. In an RPM based
installation, there is no virtualenv - the Python code is installed in
the system site-packages directory, and the runtime in /usr/bin (which
may or may not be where python is found - assuming it's called that,
and not python3).

> If the venv doesn't exist, the program name becomes "/python" and you get
> the standard environment including whatever packages you installed with "pip
> install" or "pip install --user".
>
> Granted, "/python" is nonsense and should probably just be "python". Also, I
> think PATH doesn't need to be touched at all anymore, so the lines involving
> pathEnv can be removed.
>
> How about a settings key to force the program name? That would allow
> choosing any other virtualenv to run with.

That might be a better option, and is basically how (for example)
Pycharms does it. In that case I'd argue that we should search
relative known locations first (such as the installers on Windows/Mac
would lay down), and then fall back to the configured Python
executable. That would allow installed copies to remain independent of
development copies. The only possible wrinkle would be whether a
relocatable installation such as the aforementioned installers would
be able to find their environments based purely on the executable
name.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


Re: [pgadmin-hackers] [pgAdmin4] [PATCH] Simplify Server's python setup

От
Jan Alexander Steffens
Дата:


On Wed, Jan 25, 2017 at 2:57 PM Dave Page <dpage@pgadmin.org> wrote:
One where the virtualenv is in a path that's not related to the
location of the runtime. If I'm reading the code correctly, you're
taking the appDir (the location of the runtime executable) and
appending a platform-specific path to the virtual env (e.g. on Linux,
"/../venv/bin/"), and expecting to find the Python executable in the
resulting directory.

However, in a dev environment, we may have the venv in
~/.virtualenvs/pgadmin4 and the runtime in
~/git/pgadmin4/runtime/<builddir> for example. In an RPM based
installation, there is no virtualenv - the Python code is installed in
the system site-packages directory, and the runtime in /usr/bin (which
may or may not be where python is found - assuming it's called that,
and not python3).

> Granted, "/python" is nonsense and should probably just be "python". Also, I
> think PATH doesn't need to be touched at all anymore, so the lines involving
> pathEnv can be removed.

I've now implemented this (see attached updated patch) and Python will configure itself to match whatever usable "python" it finds first in the PATH, including any activated virtualenv.

I think the RPM setup will still work properly, as the non-virtual environment with all system packages is the fallback case.
 
>
> How about a settings key to force the program name? That would allow
> choosing any other virtualenv to run with.

That might be a better option, and is basically how (for example)
Pycharms does it. In that case I'd argue that we should search
relative known locations first (such as the installers on Windows/Mac
would lay down), and then fall back to the configured Python
executable. That would allow installed copies to remain independent of
development copies. The only possible wrinkle would be whether a
relocatable installation such as the aforementioned installers would
be able to find their environments based purely on the executable
name.

Wouldn't the installed Python on Windows or OSX be caught be the PATH search Python does, too? Unfortunately I can't test either.
Вложения

Re: [pgadmin-hackers] [pgAdmin4] [PATCH] Simplify Server's python setup

От
Dave Page
Дата:
Hi

On Wed, Jan 25, 2017 at 2:35 PM, Jan Alexander Steffens
<jan.steffens@gmail.com> wrote:
>
>
> On Wed, Jan 25, 2017 at 2:57 PM Dave Page <dpage@pgadmin.org> wrote:
>>
>> One where the virtualenv is in a path that's not related to the
>> location of the runtime. If I'm reading the code correctly, you're
>> taking the appDir (the location of the runtime executable) and
>> appending a platform-specific path to the virtual env (e.g. on Linux,
>> "/../venv/bin/"), and expecting to find the Python executable in the
>> resulting directory.
>>
>> However, in a dev environment, we may have the venv in
>> ~/.virtualenvs/pgadmin4 and the runtime in
>> ~/git/pgadmin4/runtime/<builddir> for example. In an RPM based
>> installation, there is no virtualenv - the Python code is installed in
>> the system site-packages directory, and the runtime in /usr/bin (which
>> may or may not be where python is found - assuming it's called that,
>> and not python3).
>>
>> > Granted, "/python" is nonsense and should probably just be "python".
>> > Also, I
>> > think PATH doesn't need to be touched at all anymore, so the lines
>> > involving
>> > pathEnv can be removed.
>
>
> I've now implemented this (see attached updated patch) and Python will
> configure itself to match whatever usable "python" it finds first in the
> PATH, including any activated virtualenv.
>
> I think the RPM setup will still work properly, as the non-virtual
> environment with all system packages is the fallback case.
>
>>
>> >
>> > How about a settings key to force the program name? That would allow
>> > choosing any other virtualenv to run with.
>>
>> That might be a better option, and is basically how (for example)
>> Pycharms does it. In that case I'd argue that we should search
>> relative known locations first (such as the installers on Windows/Mac
>> would lay down), and then fall back to the configured Python
>> executable. That would allow installed copies to remain independent of
>> development copies. The only possible wrinkle would be whether a
>> relocatable installation such as the aforementioned installers would
>> be able to find their environments based purely on the executable
>> name.
>
>
> Wouldn't the installed Python on Windows or OSX be caught be the PATH search
> Python does, too? Unfortunately I can't test either.

I'll test that as part of a deeper review.

In the meantime, I'm thinking something like the attached patch would
be more appropriate. The only issue I can find with it at the moment
(having just tested on a Mac dev environment for now) is that if
initialisation fails and you enter a new Python Executable path, the
Py_Finalize()/Py_Initialize cycle isn't enough to make the change - it
seems to be because Py_SetProgramName() doesn't have any effect when
called again. In other words, you can give it the correct interpreter
and hit OK to attempt to run the server again, but it still won't work
until you actually restart the app.

Any ideas?

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Вложения

Re: [pgadmin-hackers] [pgAdmin4] [PATCH] Simplify Server's python setup

От
Jan Alexander Steffens
Дата:
On Thu, Jan 26, 2017 at 3:58 PM Dave Page <dpage@pgadmin.org> wrote:
In the meantime, I'm thinking something like the attached patch would
be more appropriate. The only issue I can find with it at the moment
(having just tested on a Mac dev environment for now) is that if
initialisation fails and you enter a new Python Executable path, the
Py_Finalize()/Py_Initialize cycle isn't enough to make the change - it
seems to be because Py_SetProgramName() doesn't have any effect when
called again. In other words, you can give it the correct interpreter
and hit OK to attempt to run the server again, but it still won't work
until you actually restart the app.

Any ideas?

Looking through the cpython code, Py_SetPath(NULL) clears the calculated module search path again. Try calling this before the Py_Initialize.

Re: [pgadmin-hackers] [pgAdmin4] [PATCH] Simplify Server's python setup

От
Dave Page
Дата:
On Thu, Jan 26, 2017 at 7:22 PM, Jan Alexander Steffens
<jan.steffens@gmail.com> wrote:
> On Thu, Jan 26, 2017 at 3:58 PM Dave Page <dpage@pgadmin.org> wrote:
>>
>> In the meantime, I'm thinking something like the attached patch would
>> be more appropriate. The only issue I can find with it at the moment
>> (having just tested on a Mac dev environment for now) is that if
>> initialisation fails and you enter a new Python Executable path, the
>> Py_Finalize()/Py_Initialize cycle isn't enough to make the change - it
>> seems to be because Py_SetProgramName() doesn't have any effect when
>> called again. In other words, you can give it the correct interpreter
>> and hit OK to attempt to run the server again, but it still won't work
>> until you actually restart the app.
>>
>> Any ideas?
>
>
> Looking through the cpython code, Py_SetPath(NULL) clears the calculated
> module search path again. Try calling this before the Py_Initialize.

Unfortunately that just crashes:

Process:               pgAdmin4 [72302]
Path:                  /Users/USER/*/pgAdmin4.app/Contents/MacOS/pgAdmin4
Identifier:            org.pgadmin.pgAdmin4
Version:               1.1.0 (1.1.0)
Code Type:             X86-64 (Native)
Parent Process:        Qt Creator [71857]
Responsible:           pgAdmin4 [72302]
User ID:               501

Date/Time:             2017-01-27 11:04:08.929 +0000
OS Version:            Mac OS X 10.12.1 (16B2555)
Report Version:        12
Anonymous UUID:        8EDD1C6E-ECA1-3406-3699-E93A83594C48

Sleep/Wake UUID:       2F80B265-AF26-4822-8D8F-556844AAD638

Time Awake Since Boot: 280000 seconds
Time Since Wake:       7900 seconds

System Integrity Protection: enabled

Crashed Thread:        0  Dispatch queue: com.apple.main-thread

Exception Type:        EXC_BAD_ACCESS (SIGSEGV)
Exception Codes:       KERN_INVALID_ADDRESS at 0x0000000000000000
Exception Note:        EXC_CORPSE_NOTIFY

Termination Signal:    Segmentation fault: 11
Termination Reason:    Namespace SIGNAL, Code 0xb
Terminating Process:   exc handler [0]

VM Regions Near 0:
-->
    __TEXT                 000000010da00000-000000010da2c000 [  176K]
r-x/rwx SM=COW  /Users/USER/*/pgAdmin4.app/Contents/MacOS/pgAdmin4

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   libsystem_platform.dylib       0x00007fffc21d5f3c
_platform_strchr$VARIANT$Haswell + 28
1   org.python.python             0x000000010daec349 PySys_SetPath + 24
2   org.pgadmin.pgAdmin4           0x000000010da105a6
Server::Server(unsigned short) + 1046
3   org.pgadmin.pgAdmin4           0x000000010da04ddb main + 795
4   org.pgadmin.pgAdmin4           0x000000010da04a34 start + 52


--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


Re: [pgadmin-hackers] [pgAdmin4] [PATCH] Simplify Server's python setup

От
Jan Alexander Steffens
Дата:
On Fri, Jan 27, 2017 at 12:04 PM Dave Page <dpage@pgadmin.org> wrote:
On Thu, Jan 26, 2017 at 7:22 PM, Jan Alexander Steffens
<jan.steffens@gmail.com> wrote:
> Looking through the cpython code, Py_SetPath(NULL) clears the calculated
> module search path again. Try calling this before the Py_Initialize.

Unfortunately that just crashes:
 
1   org.python.python             0x000000010daec349 PySys_SetPath + 24

I did mean Py_SetPath, not PySys_SetPath. Unfortunately, after checking again it turns out this is Python 3 only. Python 2 has no means of changing the module_search_path from outside. So, there it definitely needs a restart. :-(

Re: [pgadmin-hackers] [pgAdmin4] [PATCH] Simplify Server's python setup

От
Dave Page
Дата:
On Fri, Jan 27, 2017 at 1:37 PM, Jan Alexander Steffens
<jan.steffens@gmail.com> wrote:
> On Fri, Jan 27, 2017 at 12:04 PM Dave Page <dpage@pgadmin.org> wrote:
>>
>> On Thu, Jan 26, 2017 at 7:22 PM, Jan Alexander Steffens
>> <jan.steffens@gmail.com> wrote:
>> > Looking through the cpython code, Py_SetPath(NULL) clears the calculated
>> > module search path again. Try calling this before the Py_Initialize.
>>
>> Unfortunately that just crashes:
>
>
>>
>> 1   org.python.python             0x000000010daec349 PySys_SetPath + 24
>
>
> I did mean Py_SetPath, not PySys_SetPath.

Ah, OK - it wouldn't compile with that (Python 2 :-/ ) so I assumed it
was a typo.

> Unfortunately, after checking
> again it turns out this is Python 3 only. Python 2 has no means of changing
> the module_search_path from outside. So, there it definitely needs a
> restart. :-(

Yeah :-(. How does the attached patch look to you?

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Вложения

Re: [pgadmin-hackers] [pgAdmin4] [PATCH] Simplify Server's python setup

От
Jan Alexander Steffens
Дата:
On Fri, Jan 27, 2017 at 5:56 PM Dave Page <dpage@pgadmin.org> wrote:
On Fri, Jan 27, 2017 at 1:37 PM, Jan Alexander Steffens
<jan.steffens@gmail.com> wrote:
> I did mean Py_SetPath, not PySys_SetPath.

Ah, OK - it wouldn't compile with that (Python 2 :-/ ) so I assumed it
was a typo.

> Unfortunately, after checking
> again it turns out this is Python 3 only. Python 2 has no means of changing
> the module_search_path from outside. So, there it definitely needs a
> restart. :-(

Yeah :-(. How does the attached patch look to you?

Looks good to me, thanks! I've tested both selecting a venv and the venv-beside-runtime.

Re: [pgadmin-hackers] [pgAdmin4] [PATCH] Simplify Server's python setup

От
Dave Page
Дата:
On Sat, Jan 28, 2017 at 1:00 AM, Jan Alexander Steffens
<jan.steffens@gmail.com> wrote:
> On Fri, Jan 27, 2017 at 5:56 PM Dave Page <dpage@pgadmin.org> wrote:
>>
>> On Fri, Jan 27, 2017 at 1:37 PM, Jan Alexander Steffens
>> <jan.steffens@gmail.com> wrote:
>> > I did mean Py_SetPath, not PySys_SetPath.
>>
>> Ah, OK - it wouldn't compile with that (Python 2 :-/ ) so I assumed it
>> was a typo.
>>
>> > Unfortunately, after checking
>> > again it turns out this is Python 3 only. Python 2 has no means of
>> > changing
>> > the module_search_path from outside. So, there it definitely needs a
>> > restart. :-(
>>
>> Yeah :-(. How does the attached patch look to you?
>
>
> Looks good to me, thanks! I've tested both selecting a venv and the
> venv-beside-runtime.

Cool. So we have a release coming up next wee so I'm going to put this
on hold for now, as we need to make some non-trivial changes to make
this work in the Windows and Mac packages:

- On Windows, the Python executable is shipped in the runtime
directory, not as part of the venv.

- On Mac, we don't ship the Python executable at all as we expect it
to be on the system anyway. Adding it to the package is not a simple
case of just adding the required files:

App: pgAdmin 4.app: Post-processing: .//Contents/Resources/venv/.Python
Rewriting library
/System/Library/Frameworks/Python.framework/Versions/2.7/Python to
@loader_path/../../../Contents/Frameworks/Python.framework/Versions/2.7/Python
in .//Contents/Resources/venv/.Python
install_name_tool -change
/System/Library/Frameworks/Python.framework/Versions/2.7/Python
@loader_path/../../../Contents/Frameworks/Python.framework/Versions/2.7/Python
.//Contents/Resources/venv/.Python
error: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/install_name_tool:
can't open input file: .//Contents/Resources/venv/.Python for writing
(Operation not permitted)
error: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/install_name_tool:
can't lseek to offset: 4096 in file:
.//Contents/Resources/venv/.Python for writing (Bad file descriptor)
error: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/install_name_tool:
can't write new headers in file: .//Contents/Resources/venv/.Python
(Bad file descriptor)
error: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/install_name_tool:
can't lseek to offset: 1257472 in file:
.//Contents/Resources/venv/.Python for writing (Bad file descriptor)
error: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/install_name_tool:
can't write new headers in file: .//Contents/Resources/venv/.Python
(Bad file descriptor)
error: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/install_name_tool:
can't close written on input file: .//Contents/Resources/venv/.Python
(Bad file descriptor)
complete-bundle.sh failed
make: *** [appbundle] Error 1

I've logged the work here: https://redmine.postgresql.org/issues/2123

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company