This is an archive of the discontinued Mercurial Phabricator instance.

Differential D11308

pyoxidizer: add user-site to `sys.path` on Windows
ClosedPublic

Authored by mharbison72 on Aug 18 2021, 10:47 PM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHG95af358fcdfe: pyoxidizer: add user-site to `sys.path` on Windows
rHGc18210649334: pyoxidizer: add user-site to `sys.path` on Windows
Summary

This is a port of 53221078e0de to Windows to allow pip-installed extensions to
be loaded without specifying a path. It's a major headache to have an hg.exe on
PATH that needs to have the path to the extensions specified, because WSL
doesn't see the same path.

This is only for Windows for now, to match the currently shipping py2 behavior.
There is a better solution with using the site package, but this needs support
in PyOxidizer[1].

[1] https://github.com/indygreg/PyOxidizer/issues/430

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

mharbison72 created this revision.Aug 18 2021, 10:47 PM
marmoute added a subscriber: marmoute.Aug 19 2021, 6:21 AM

This looks good (i.e. similar to what is done in the ./hg script).

Do you think we could get a test for that in test-extensions ? (using a fake APPDATA for that part of the test) ?

mharbison72 added a comment.Aug 22 2021, 9:45 PM
In D11308#173544, @marmoute wrote:

This looks good (i.e. similar to what is done in the ./hg script).
Do you think we could get a test for that in test-extensions ? (using a fake APPDATA for that part of the test) ?

I haven't quite got the test working yet (should foo.py in site-packages be loadable, or does it need a directory with __init__.py?). But I see that site.getuserpackages() is available on py3, and it would be easier to write a test against that with support for all platforms. But that should (probably?) go on default, especially since the non pyoxidizer path needs to be updated to stay in sync. So how about this for stable, and a rework on default that supports py3 everywhere.

marmoute added a comment.Aug 23 2021, 4:53 AM
In D11308#173760, @mharbison72 wrote:
In D11308#173544, @marmoute wrote:

This looks good (i.e. similar to what is done in the ./hg script).
Do you think we could get a test for that in test-extensions ? (using a fake APPDATA for that part of the test) ?

I haven't quite got the test working yet (should foo.py in site-packages be loadable, or does it need a directory with __init__.py?). But I see that site.getuserpackages() is available on py3, and it would be easier to write a test against that with support for all platforms. But that should (probably?) go on default, especially since the non pyoxidizer path needs to be updated to stay in sync. So how about this for stable, and a rework on default that supports py3 everywhere.

You mean getting this specific patch for stable, without a test, and a more complete version for default, with a test ?

That would work for me.

mharbison72 added a comment.Aug 23 2021, 10:12 AM
In D11308#173761, @marmoute wrote:
In D11308#173760, @mharbison72 wrote:
In D11308#173544, @marmoute wrote:

This looks good (i.e. similar to what is done in the ./hg script).
Do you think we could get a test for that in test-extensions ? (using a fake APPDATA for that part of the test) ?

I haven't quite got the test working yet (should foo.py in site-packages be loadable, or does it need a directory with __init__.py?). But I see that site.getuserpackages() is available on py3, and it would be easier to write a test against that with support for all platforms. But that should (probably?) go on default, especially since the non pyoxidizer path needs to be updated to stay in sync. So how about this for stable, and a rework on default that supports py3 everywhere.

You mean getting this specific patch for stable, without a test, and a more complete version for default, with a test ?

Yes

marmoute added a comment.Aug 23 2021, 3:33 PM
In D11308#173762, @mharbison72 wrote:
In D11308#173761, @marmoute wrote:
In D11308#173760, @mharbison72 wrote:
In D11308#173544, @marmoute wrote:

This looks good (i.e. similar to what is done in the ./hg script).
Do you think we could get a test for that in test-extensions ? (using a fake APPDATA for that part of the test) ?

I haven't quite got the test working yet (should foo.py in site-packages be loadable, or does it need a directory with __init__.py?). But I see that site.getuserpackages() is available on py3, and it would be easier to write a test against that with support for all platforms. But that should (probably?) go on default, especially since the non pyoxidizer path needs to be updated to stay in sync. So how about this for stable, and a rework on default that supports py3 everywhere.

You mean getting this specific patch for stable, without a test, and a more complete version for default, with a test ?

Yes

That sounds good!

Alphare accepted this revision.Aug 25 2021, 5:59 AM
This revision is now accepted and ready to land.Aug 25 2021, 5:59 AM
mharbison72 added a commit: rHGc18210649334: pyoxidizer: add user-site to `sys.path` on Windows.Aug 25 2021, 6:08 AM
Closed by commit rHGc18210649334: pyoxidizer: add user-site to `sys.path` on Windows (authored by mharbison72). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mrust/hgcli/pyoxidizer.bzl (11 lines)

Diff 30012

rust/hgcli/pyoxidizer.bzl

import os import os
import sys import sys
extra_path = os.environ.get('PYTHONPATH') extra_path = os.environ.get('PYTHONPATH')
if extra_path is not None: if extra_path is not None:
# extensions and hooks expect a working python environment # extensions and hooks expect a working python environment
# We do not prepend the values because the Mercurial library wants to be in # We do not prepend the values because the Mercurial library wants to be in
# the front of the sys.path to avoid picking up other installations. # the front of the sys.path to avoid picking up other installations.
sys.path.extend(extra_path.split(os.pathsep)) sys.path.extend(extra_path.split(os.pathsep))
# Add user site to sys.path to load extensions without the full path
if os.name == 'nt':
vi = sys.version_info
sys.path.append(
os.path.join(
os.environ['APPDATA'],
'Python',
'Python%d%d' % (vi[0], vi[1]),
'site-packages',
)
)
import hgdemandimport; import hgdemandimport;
hgdemandimport.enable(); hgdemandimport.enable();
from mercurial import dispatch; from mercurial import dispatch;
dispatch.run(); dispatch.run();
""" """
set_build_path(ROOT + "/build/pyoxidizer") set_build_path(ROOT + "/build/pyoxidizer")