Page MenuHomePhabricator

packaging: add support for PyOxidizer
ClosedPublic

Authored by indygreg on Nov 16 2019, 4:21 PM.

Details

Summary

I've successfully built Mercurial on the development tip of
PyOxidizer on Linux and Windows. It mostly "just works" on Linux.
Windows is a bit more finicky.

In-memory resource files are probably not all working correctly
due to bugs in PyOxidizer's naming of modules. PyOxidizer now
now supports installing files next to the produced binary. (We
do this for templates in the added file.) So a workaround
should be available.

Also, since the last time I submitted support for PyOxidizer,
PyOxidizer gained the ability to auto-generate Rust projects
to build executables. So we don't need to worry about vendoring
any Rust code to initially support PyOxidizer. However, at some
point we will likely want to write our own command line driver
that embeds a Python interpreter via PyOxidizer so we can run
Rust code outside the confines of a Python interpreter. But that
will be a follow-up.

I would also like to add packaging.py CLI commands to build
PyOxidizer distributions. This can come later, if ever.
PyOxidizer's new "targets" feature makes it really easy to define
packaging tasks in its Starlark configuration file. While not
much is implemented yet, eventually we should be able to produce
MSIs, etc using a pyoxidizer build one-liner. We'll get there...

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

indygreg created this revision.Nov 16 2019, 4:21 PM
mharbison72 added inline comments.
rust/hgcli/pyoxidizer.bzl
26

For some reason, this caused it to complain about not being able to find hgdemandimport with pyoxidizer v0.4.0-58-gc7d9c79. I commented it out, and now I can get basic commands running. I can live with that for now (though I saw the comment about external templates elsewhere, so maybe it needs to work eventually).

CC: @martinvonz

main.rs and the Cargo files already exist, so this fails to apply.

What;s the status of this? is it both flagged as "need review" and "INCOMPLETE".

indygreg retitled this revision from INCOMPLETE pyoxidizer to packaging: add support for PyOxidizer.Sun, Jan 26, 2:16 AM
indygreg edited the summary of this revision. (Show Details)
indygreg updated this revision to Diff 19619.

This produces a working hg executable with templates support on Linux and Windows (I haven't tested macOS). I'd feel comfortable landing if it will unblock others to hack on things.

The test harness currently has a ~25% failure rate with a PyOxidizer hg binary. The reason is it is picking up hg as the python executable and scripts with python in the shebang all fail. We'll need to produce a separate executable that functions like python for use in the test harness. Or we'll need to teach the test harness to invoke a real Python for scripts there. Ideally we would use an oxidized Python binary in the test harness, otherwise we may fail to test some code in the PyOxidizer environment. I'll probably add a feature to PyOxidizer to enable creation of executables that function like python. Then, it should be a few lines of Starlark to produce a second executable binary :)

This produces a working hg executable with templates support on Linux and Windows (I haven't tested macOS). I'd feel comfortable landing if it will unblock others to hack on things.

Nice! I won’t get to test this latest revision until tomorrow, but I’d like to get something official landed to make things easier.

I’ve run into the issue where *.py isn’t treated as a resource, which I think is a known issue [1]. Has enough been solidified to get something working yet? Mostly this affects discovering bundled extensions and displaying help.

@martinvonz and I talked a bit about bundling templates and possibly 3rd party extensions a couple days ago in IRC, in case you missed it. So having that would be helpful.

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

I don't have strong opinions about bundling templates and other resources internally versus externally. I do think it would be cool to have a fully self-contained executable. But that would require a command to write out embedded resources to the filesystem so people can modify them. That's a bit obscure.

We may want a policy-like primitive to control where to load resources by default. And/or we can establish a clear order for resolution. e.g. try CLI argument, environment variable, then config option, then user directory, then global directory, etc.

On Windows (which is where we have the biggest pressing concern for PyOxidizer at the moment since we aren't producing Python 3 installers yet), it doesn't much matter since the installer can install multiple files easily. I think installing files next to the executable mimicking today's install layout is the path of least resistance.

indygreg updated this revision to Diff 19620.Sun, Jan 26, 1:34 PM
indygreg updated this revision to Diff 19628.Sun, Jan 26, 7:25 PM

I just released PyOxidizer 0.5.0 and have updated the instructions in this file accordingly.

mharbison72 requested changes to this revision.Sun, Jan 26, 11:21 PM

I think this can land after the Windows options are added.

So we don't need to worry about vendoring any Rust code to initially support PyOxidizer.

Does that mean the parent of this can go away?

For anybody else following along, start by updating Rust, especially on Windows. IDR what version I had and it scrolled off the screen at this point, but I installed it in late October/early November. Building PyOxidizer 0.5.0 cause my antivirus to go nuts when it was building some dependencies in MSYS. (Being totally new to Rust, I'm wondering how we know that we can trust the numerous dependencies). I switched to cmd.exe, and I started getting popups about CRT issues. Updating seems to have fixed both of these. (I've got rustc 1.40.0 now.)

It also looks like something creates paths longer than MAX_PATH, which most tools can't deal with. (I know there's a registry option to enable support globally, but I wonder if make clean should be delegating to a tool smart enough to handle long paths)

$ make clean>/dev/null
'build\lib.win-amd64-2.7' does not exist -- can't clean it
'build\bdist.win-amd64' does not exist -- can't clean it
'build\scripts-2.7' does not exist -- can't clean it
rm: cannot lstat `build/pyoxidizer/python_distributions/python.86a3260edabeed314c6f32a931e60dd097fa854b1346561443353e1bc90e3edd/python/install/Lib/site-packag
es/pip-19.3.1-py3.7.egg/pip/_vendor/urllib3/contrib/_securetransport/__pycache__/bindings.cpython-37.pyc': File or path name too long
rm: cannot lstat `build/pyoxidizer/python_distributions/python.86a3260edabeed314c6f32a931e60dd097fa854b1346561443353e1bc90e3edd/python/install/Lib/site-packag
es/pip-19.3.1-py3.7.egg/pip/_vendor/urllib3/contrib/_securetransport/__pycache__/low_level.cpython-37.pyc': File or path name too long
rm: cannot lstat `build/pyoxidizer/python_distributions/python.86a3260edabeed314c6f32a931e60dd097fa854b1346561443353e1bc90e3edd/python/install/Lib/site-packag
es/pip-19.3.1-py3.7.egg/pip/_vendor/urllib3/contrib/_securetransport/__pycache__/__init__.cpython-37.pyc': File or path name too long
rm: cannot lstat `build/pyoxidizer/python_distributions/python.86a3260edabeed314c6f32a931e60dd097fa854b1346561443353e1bc90e3edd/python/install/Lib/site-packag
es/pip-19.3.1-py3.7.egg/pip/_vendor/urllib3/packages/ssl_match_hostname/__pycache__/_implementation.cpython-37.pyc': File or path name too long
rm: cannot lstat `build/pyoxidizer/python_distributions/python.86a3260edabeed314c6f32a931e60dd097fa854b1346561443353e1bc90e3edd/python/install/Lib/site-packag
es/pip-19.3.1-py3.7.egg/pip/_vendor/urllib3/packages/ssl_match_hostname/__pycache__/__init__.cpython-37.pyc': File or path name too long
make: *** [cleanbutpackages] Error 1

Another fun thing to handle will be stacktrace differences in tests. Note the '.' vs '/' separator. Is this related to the module naming issue that you referenced, and/or the unresolved issues behind the PyOxidizer issue I referenced here earlier?

@@ -646,7 +634,7 @@
 Even though the extension fails during uisetup, hg is still basically usable:
   $ hg --config extensions.baduisetup=$TESTTMP/baduisetup.py version
   Traceback (most recent call last):
-    File "*/mercurial/extensions.py", line *, in _runuisetup (glob)
+    File "mercurial.extensions", line 251, in _runuisetup
       uisetup(ui)
     File "$TESTTMP/baduisetup.py", line 2, in uisetup
       1 / 0
contrib/packaging/pyoxidizer.bzl
25 ↗(On Diff #19628)

I'm not sure why this worked for you on Windows, but I also needed to add:

legacy_windows_stdio = True,
legacy_windows_fs_encoding = True,

(I'm not sure if fs_encoding is really needed, but that was in the config file I was hacking with back in early December.) Without the stdio option, I got no output from the hg config command. Maybe because that wants to spin up a pager?

This revision now requires changes to proceed.Sun, Jan 26, 11:21 PM

I don't have strong opinions about bundling templates and other resources internally versus externally. I do think it would be cool to have a fully self-contained executable. But that would require a command to write out embedded resources to the filesystem so people can modify them. That's a bit obscure.

Yeah, these were a few of the things we talked about. I think we're all on the same page here, and I'm hoping for a single executable. Martin mentioned early in the IRC chat, maybe putting the files under /etc/mercurial/templates (which is fine now that Windows has a global area for hg data), but then we'd minimally need an HGRCPATH analog for templates for running tests.

The other tricky thing that came up is maybe teaching %include how to include resources. At work, we point to a custom map-cmdline file that includes the default one, and then overrides just enough so that hg log -v will put each file on a newline, instead of making them space delimited.

We may want a policy-like primitive to control where to load resources by default. And/or we can establish a clear order for resolution. e.g. try CLI argument, environment variable, then config option, then user directory, then global directory, etc.

I thought about adding a user level search path, but it will probably get complicated to tell resource based vs user file based files when reading without adding a known prefix to the resource names. So I set that aside, because I'm not sure how useful it would actually be for users to install templates.

I think this can land after the Windows options are added.

So we don't need to worry about vendoring any Rust code to initially support PyOxidizer.

Does that mean the parent of this can go away?

Nevermind this. It looks like phabread --stack doesn't pay attention to the fact that it was abandoned, so I didn't realize it.

I'd be very happy to queue this as soon as @mharbison72's request for Windows has been addressed (I think that was all, right?). We (Google) would like to use it for macOS packaging too. We may also end up using it on Linux so we don't break when the system python3 changes from 3.7 of 3.8 (because we currently install .so files specific to a minor Python version, IIUC). Thanks a lot for your work on this, @indygreg. And thanks for reviewing and testing it, @mharbison72.

I'd be very happy to queue this as soon as @mharbison72's request for Windows has been addressed (I think that was all, right?).

Yes.

I'm still curious about the *.py-as-resources issue[1], but don't want this held up.

[1] https://phab.mercurial-scm.org/D7450#118094

I'd be very happy to queue this as soon as @mharbison72's request for Windows has been addressed (I think that was all, right?).

Yes.
I'm still curious about the *.py-as-resources issue[1], but don't want this held up.
[1] https://phab.mercurial-scm.org/D7450#118094

I think @indygreg doesn't have much time for Mercurial these days. @mharbison72, what do you think about taking this patch as is and then you can send fixes for Windows on top?

mharbison72 accepted this revision.Thu, Jan 30, 2:28 PM

I think @indygreg doesn't have much time for Mercurial these days. @mharbison72, what do you think about taking this patch as is and then you can send fixes for Windows on top?

Sounds good to me.

This revision now requires review to proceed.Thu, Jan 30, 2:28 PM
martinvonz accepted this revision.Thu, Jan 30, 4:12 PM
This revision is now accepted and ready to land.Thu, Jan 30, 4:12 PM
This revision was automatically updated to reflect the committed changes.