Use reusable venvs #199

jayvdb · 2019-11-18T18:21:37Z

Replace pip install --target with venvs.
As setuptools breakes out of the venv to load distutils build_ext
command, first create a copy of the Python distribution and hack
it.

This also has the benefit that the venv can be used to build
extensions outside of PyOxidizer.

Also add venv_path to PipRequirementsFile, allowing the same
venv to be incrementally populated in multiple rules,
and the venv re-used across PyOxidizer build runs.

Fixes #162
Fixes #170
Closes #194

jayvdb · 2019-11-19T05:10:07Z

The next step IMO is to add a cli subcommand for creating a hacked venv, before running a full build. This involves splitting the build process a bit, enough to decompress the standalone Python distribution to disk so it can be used for the venvs. I was working on this, but that section of code was being heavily revised so I dropped it in order to get this PR up.

jayvdb · 2019-11-19T05:14:03Z

Here is the macOS failure with cryptography. It is just a build env problem, needing openssl, but that doesnt need to be solved to validate the venv changes. And there are problems with openssl specifically, as the host openssl may have different header files than used for the standalone Python build, causing link failures even on Linux. So for macOS I am using bcrypt, and I suspect the same will be needed for Windows.

warsaw · 2019-11-20T22:35:36Z

While I can get farther in my problems with #202 it's not a complete solution. I have to put the same venv_path in all my packing rules. That does get me farther, but I still have mysterious, hard to debug packaging failures.

indygreg · 2019-11-24T18:24:54Z

I think supporting reusing a virtualenv makes sense.

But I'm going to hold off reviewing and landing this because I'm overhauling how the config files work and it will be much easier to do advanced things once that work is done. My plan is to leverage Starlark as a "scripting" language and for the config files to more resemble Python scripts than static data structures. e.g. the packaging rules will be attached to the PythonDistribution instances and will emit objects that represent resources which can be packaged - either embedded in a binary or manifested on the filesystem. In this world, we could have a PythonDistribution.create_virtualenv() API that returns a Virtualenv instance and we could call .run_pip(...) on it. This will give vastly more control and flexibility and should result in tons of feature requests being satisfied.

jayvdb · 2019-11-25T01:40:00Z

@indygreg , I'm almost finished getting cryptography working on all platforms, and need a bit of time to tidy up and re-push here. Please could you please avoid conflicting me by fiddling in the same part of the codebase. It causes useless merge conflicts for me, and the changes you are making do not help solve the problem. The real problems are not here -- they are indygreg/python-build-standalone#19 and indygreg/python-build-standalone#22 - I have workarounds for those, but real fixes for those are desperately needed.

indygreg · 2019-11-25T06:21:42Z

Don't worry about resolving the merge conflicts: I will rebase your PRs for you.

jayvdb · 2019-11-25T09:11:04Z

@indygreg , I'd rather do the rebasing myself, and have purple PRs in my history than lots of red ones and need to go figure out what changes you've made to my code.

jayvdb · 2019-11-25T09:12:37Z

I've done a quick push of what I have. There are still some bits that are no longer needed, to be removed. I'll lightly annotate it now.

jayvdb · 2019-11-25T09:14:27Z

ci/azure-pipelines-template.yml

+  - ${{ if eq(parameters.name, 'Linux') }}:
+    - script: |
+        cargo run --bin pyoxidizer -- init --pip-install cryptography ~/pyapp
+        cat ci/pyapp.py | cargo run --bin pyoxidizer -- run ~/pyapp


#208 is needed for all of these enhancements to the sample app CI. This approach is taken because otherwise the lines are very long, and also to avoid horrid Windows CMD quoting problems.

Fix included in #200

jayvdb · 2019-11-25T09:15:11Z

ci/azure-pipelines-template.yml

+      displayName: Build Oxidized Application
+
+  - ${{ if eq(parameters.name, 'macOS') }}:
+    - script: brew install [email protected]


this is unnecessary, as it is pre-installed, but it is only a few seconds. Happy to remove it though.

jayvdb · 2019-11-25T09:18:36Z

ci/azure-pipelines-template.yml

+  - ${{ if eq(parameters.name, 'Windows') }}:
+    - task: UsePythonVersion@0
+      inputs:
+        versionSpec: '3.7'


this is necessary because the windows job needs a functional Python, mostly due to indygreg/python-build-standalone#22

indygreg/python-build-standalone#19 is also problematic, but it can be worked around by using virtualenv instead of venv. Currently the code in the PR does use virtualenv because I was initially attempting to use the standalone python, but cffi put a halt to that plan, so I am going to rip that out so that all OS use venv, simplifying a few areas of the changes.

jayvdb · 2019-11-25T09:19:51Z

ci/pyapp.py

@@ -0,0 +1,15 @@
+import cryptography.fernet


this is copied from jamesabel/spafit#2

It is a good smoketest for basic cryptography.

pyoxidizer/src/py_packaging/distribution.rs

jayvdb · 2019-11-25T09:25:34Z

pyoxidizer/src/py_packaging/distribution.rs

+            .unwrap()
+            .display()
+            .to_string();
+        if site_packages_s.starts_with("\\\\?\\") {


This prefix is added by Path.canonicalize, and I wasnt sure if Python could handle it.
I'm not sure if this is necessary. Need to try removing it.

Also it seems I need to set PYTHONHOME. I've seen warnings that it wasnt set whilst pip is installing stuff.

I read venv bin/activate again, and I am not sure about PYTHONHOME

The message I had seen is appearing on macOS only, and is

... modifying distutils/unixccompiler.py for oxidation creating venv /var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/pyoxidizer-temp-venv.TMkXswfSsJy9/venv Could not find platform dependent libraries <exec_prefix> Consider setting $PYTHONHOME to <prefix>[:<exec_prefix>] Could not find platform dependent libraries <exec_prefix> Consider setting $PYTHONHOME to <prefix>[:<exec_prefix>] Collecting gevent Downloading ...

It looks like we are seeing some effects of https://bugs.python.org/issue22213 - when I used virtualenv instead of venv, the problem goes away.

jayvdb · 2019-11-25T09:29:36Z

pyoxidizer/src/py_packaging/distribution.rs

+        fs::create_dir_all(&python_paths.pyoxidizer_state_dir).unwrap();
+
+        extra_envs.insert(
+            "PYOXIDIZER_DISTUTILS_STATE_DIR".to_string(),


this is moved out of distutils.rs because most of the venv environ stuff is here, and it feels a bit odd to getting only a single path returned from prepare_hacked_distutils and that value does not need to be any specific value for the distutils hack to work.

Also being able to run commands in the venv outside of PyOxidizer provides a lot of flexibility not currently possible with the config file. For that to happen either:

the distutils hacking pushes the state directory name into the .py files, or

the distutils .py and rust code both use the same state directory path

I have opt for the latter, as that seems to be more appropriate in a venv which has a 'prefix'

jayvdb · 2019-11-25T09:30:06Z

pyoxidizer/src/py_packaging/distribution.rs

@@ -694,6 +955,8 @@ pub fn analyze_python_distribution_data(
        };
    }

+    let venv_base = dist_dir.parent().unwrap().join("hacked_base");


"hacked_base" probably should be given a better name

jayvdb · 2019-11-25T09:32:02Z

pyoxidizer/src/py_packaging/distutils.rs

    );

-    let orig_distutils_path = dist.stdlib_path.join("distutils");


This is a separate bug, but it doesnt seem useful to split it off to a separate commit: distutils can not be anywhere - it must be the real one, as setuptools hunts for the real one, even navigating through a venv to find it.

pyoxidizer/src/py_packaging/distutils.rs

jayvdb force-pushed the reusable-venvs branch 4 times, most recently from 3bc32f9 to e20c4a3 Compare November 19, 2019 04:06

jayvdb mentioned this pull request Nov 19, 2019

Allow Pip rules to use wheels #193

Open

jayvdb force-pushed the reusable-venvs branch from e20c4a3 to 6a26af9 Compare November 19, 2019 04:30

jayvdb mentioned this pull request Nov 19, 2019

Packaging with Poetry and other PEP-517 based packages #201

Open

jayvdb force-pushed the reusable-venvs branch from 73733b1 to 583cc48 Compare November 19, 2019 07:22

This was referenced Nov 19, 2019

pyca/cryptography: cffi build and __file__ #26

Closed

Windows archive is missing ensurepip, venv, tests and tk indygreg/python-build-standalone#19

Closed

No way to build packages with setup.py requirements #202

Open

Offline mode #204

Open

jayvdb force-pushed the reusable-venvs branch 8 times, most recently from 814309a to 5fea374 Compare November 20, 2019 16:56

jayvdb force-pushed the reusable-venvs branch from 120aa64 to 6a1fd91 Compare November 25, 2019 09:00

jayvdb commented Nov 25, 2019

View reviewed changes

indygreg force-pushed the main branch 25 times, most recently from 296b083 to 13e08f6 Compare November 23, 2020 01:46

indygreg force-pushed the main branch 3 times, most recently from 8f998b7 to 5c5ac33 Compare December 1, 2021 03:16

indygreg force-pushed the main branch 2 times, most recently from 5e0524b to 789c62e Compare May 31, 2022 04:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use reusable venvs #199

Use reusable venvs #199

jayvdb commented Nov 18, 2019 •

edited

Loading

jayvdb commented Nov 19, 2019

jayvdb commented Nov 19, 2019

warsaw commented Nov 20, 2019

indygreg commented Nov 24, 2019

jayvdb commented Nov 25, 2019

indygreg commented Nov 25, 2019

jayvdb commented Nov 25, 2019

jayvdb commented Nov 25, 2019

jayvdb Nov 25, 2019

jayvdb Nov 25, 2019

jayvdb Nov 25, 2019

jayvdb Nov 25, 2019

jayvdb Nov 25, 2019

jayvdb Nov 25, 2019

jayvdb Nov 26, 2019

jayvdb Nov 27, 2019

jayvdb Dec 1, 2019

jayvdb Nov 25, 2019

jayvdb Nov 26, 2019

jayvdb Nov 25, 2019

jayvdb Nov 25, 2019

		);

		let orig_distutils_path = dist.stdlib_path.join("distutils");

Use reusable venvs #199

Are you sure you want to change the base?

Use reusable venvs #199

Conversation

jayvdb commented Nov 18, 2019 • edited Loading

jayvdb commented Nov 19, 2019

jayvdb commented Nov 19, 2019

warsaw commented Nov 20, 2019

indygreg commented Nov 24, 2019

jayvdb commented Nov 25, 2019

indygreg commented Nov 25, 2019

jayvdb commented Nov 25, 2019

jayvdb commented Nov 25, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jayvdb commented Nov 18, 2019 •

edited

Loading