Get deps from pypi during build #3310

joemarshall · 2022-12-01T16:55:15Z

Description

Further extension to the work on pyodide build. This adds two new arguments:

--build-dependencies

Get dependency list from wheel and build (or download) wheels for all dependencies alongside the main wheel.

--skip-dependency NAME
Skip a dependency - NAME can be either a package name, or the path to a json package list file in either repodata.json format (ie. Built in pyodide packages) or jupyterlite all.json format (i.e. jupyterlite wheel folder).

It uses resolvelib to do dependency resolution because it can handle backtracking etc. If you call pyodide build to build something then chuck all the wheel files into jupyterlite pypi directory it should now be able to load without hitting pypi (as piplite uses the version provided locally by default).

Needs tests but I thought it worth chucking up as soon as it was working for people to see

Checklists

Add a CHANGELOG entry
Add / update tests
Add new / update outdated documentation

rth

Thanks for working on this @joemarshall !

I'm wondering if we actually want to manage the dependency resolution ourselves. The other alternative could be to use something like pip-tools to generate a dependency tree and output the requirements.txt with pinned versions that can then be built.

I guess here we could do a more accurate resolution for the emscripten_wasm32 platform, but I'm a bit concerned about the feature creap. In particular,
a) it might be better to list the output of packages to build, so the user can double-check them before actually building them. So I wonder even if we implement this, if wouldn't be better to write a requirements.txt files with some new CLI command and then run build on that input.
b) there are a lot of ways in which dependency resolution could produce results different to what the user wanted. So we need to consider whether we want to deal with such issues in the future, or if we consider this out of scope, and rather rely on some existing tool to do some of this.

Overall I agree this is useful functionality, and I'm not opposed to this PR, but I just want to be sure we also consider existing alternatives (if any) that would allow solving part of this use case.

rth · 2022-12-02T08:20:51Z

pyodide-build/pyodide_build/out_of_tree/pypi.py

+
+
+@contextmanager
+def stream_redirected(to=os.devnull, stream=None):


Maybe you could use contextlib.redirect_stderr and redirect_stdout to avoid re-implementing it.

contextlib doesn't redirect subprocess output - that's why this one is needed. Otherwise one would need to delve into anywhere subprocess is called (i.e. deep inside build) to redirect that.

Could you please write a comment about this for the people who read this code in the future?

joemarshall · 2022-12-02T11:49:37Z

I was a bit torn about doing it like this with resolvelib (which is basically the resolution logic from pip made into a library) vs pip-tools (which uses more of pip internals, but is basically a similar thing).

I think the pain with pip-tools is that there are a bunch of use cases that it doesn't easily support which are really super-useful for emscripten. In particular:

It doesn't support ignoring packages. Unless pyodide starts entirely using pypi packages and not using the pyodide package repository, this is a real pain; especially if the ignored packages have complex dependency trees, unwrapping them from the piptools output is a right pain. I also have the particular use case that there are various packages that use stuff to do cool multiprocessing stuff or processor specific stuff, which have dependencies on compiled packages that will just never compile on emscripten.
It doesn't really support cross-compiling - this isn't too bad right now as most packages don't deliberately support emscripten, but with things like jupyterlite, I think increasingly there will be packages that know about emscripten. I've got a couple of packages that drop deps on emscripten for example where they are used to run servers, or to do graphics.
It doesn't support building emscripten wheels for sdist packages. This is important because
a) a surprising number of packages are sdist only anyway, and many packages are also sdist only on emscripten
b) you need to build the wheel before you can tell what the dependencies are, so you can't reasonably separate building wheels from resolving dependencies.

It's easy enough to make this logic output pinned requirements.txt files if required (and building from them is pretty trivial anyway given we support pypi builds already), but I do think having the core bit that calls resolvelib and makes wheels at the same time is important to support correctly resolving for emscripten.

I have to moan about python packaging again btw, I get that in theory different builds could have different build-time generated dependencies, and that there are all sorts of build tools, but the fact that there is no standard metadata file included in an sdist which just specifies dependencies for each platform makes me sad. Having to compile wheels in order to find dependencies is a bit awful...

joemarshall · 2022-12-02T13:20:30Z

it might be better to list the output of packages to build, so the user can double-check them before actually building them. So I wonder even if we implement this, if wouldn't be better to write a requirements.txt files with some new CLI command and then run build on that input.

The problem I ran into is things that are sdist only, or don't have pyodide specific wheels. There's no reliable way to determine requirements for an sdist other than to compile it. So then my script to generate the requirements.txt had to build all the non-pure python packages anyway, before it could output a set of resolutions, and having it done in one go saves doubling time spent compiling.

Basically I think anything that makes the requirements.txt files will
a) have to be integrated with and hence call into the pyodide build system to make wheels during the resolution process.
b) Need to use some kind of resolver library, either libresolve or pip-tools

It could in theory be a separate pyodide command that does this, although that would lose the benefit of being able to build everything for a project from scratch whilst only building wheels once. Dunno how important that is?

joemarshall · 2022-12-06T14:31:15Z

@rth Any thoughts on the above:

I had a bit of a play with pip-tools again, but until there's a full cross-build environment for pyodide including support for using pyodide built-in packages in pip, it just isn't trivial to do this using pip-tools. I tried with pyodide pyenv but pip didn't really work for me there.

You end up hacking an isolated virtual environment and then hacking sys to change the values of sys_platform etc., then telling it a whole load of packages are unsafe, and the unsafe list has to include any dependencies of any packages that you already have, as it still traverses dependencies of unsafe packages. It freaks me out slightly as you end up relying on the internal behaviour of pip-tools, which in turn is tied to the internal behaviour of pip, both of which don't really support running as libraries as opposed to scripts.

Personally I think that one script in pyodide build that does the downloading and building of dependencies (and of requirements.txt files) using libraries that are meant to be used for dependency resolution is probably less fragile and less work to maintain than a script to hack a virtual environment so that pip-tools works, which will end up needing maintenance as pip-tools upgrades things or changes behaviour.

It would be possible with a load of work on cross-env and pyodide to get things to a state where one could just build entirely using pip, but I think that's some way off? I really want reliable package building for complex dependency trees to be there soon, as it just makes everything so much simpler when porting anything complicated that uses multiple libraries across.

hoodmane · 2022-12-18T20:31:46Z

It would be possible with a load of work on cross-env and pyodide to get things to a state where one could just build entirely using pip, but I think that's some way off? I really want reliable package building for complex dependency trees to be there soon, as it just makes everything so much simpler when porting anything complicated that uses multiple libraries across.

I looked into this a bit and I agree it looks quite complicated and fragile. If there is an alternative to writing some complicated hack to make pip's internals understand what we need then it is preferable. It would be cool to get editable installs working correctly though. Currently with my venvs we can do editable installs of pure Python packages, but not of binary packages.

pyodide-build/pyodide_build/out_of_tree/pypi.py

pyodide-build/pyodide_build/cli/build.py

pyodide-build/pyodide_build/out_of_tree/pypi.py

hoodmane · 2022-12-18T23:30:39Z

pyodide-build/pyodide_build/out_of_tree/pypi.py

+
+
+if TYPE_CHECKING:
+    APBase = AbstractProvider[Requirement, Candidate, str]


What goes wrong if you write AbstractProvider[Requirement, Candidate, str] when not TYPE_CHECKING?

pyodide-build/pyodide_build/out_of_tree/pypi.py

for more information, see https://pre-commit.ci

joemarshall · 2022-12-27T23:12:11Z

@hoodmane I got round to doing the review changes for this. I also added thorough tests which serve a very basic fake pypi to test things like unresolvable requirements and complicated extra handling.

I think at the moment it feels like it is reliable enough to merge. I have a couple of todos that would be worth putting in which is to read and resolve requirements.txt files, and to write out a flat requirements.txt but ideally I'd like to put those in a separate pr?

hoodmane · 2022-12-27T23:50:00Z

Sounds good to me. This pr is already quite large any additional features would be best left to follow ups. I'll try to look this over one more time and merge it in a bit.

hoodmane · 2022-12-27T23:50:36Z

Or you could merge it if you like.

ryanking13

Thanks, @joemarshall. I have minor comments about some details. I wasn't following the full conversation in this PR, so I'll let others approve.

ryanking13 · 2022-12-27T23:49:20Z

pyodide-build/pyodide_build/cli/build.py

        if not package_path.is_dir():
            # a pure-python wheel has been downloaded - just copy to dist folder
            shutil.copy(str(package_path), str(curdir / "dist"))
            print(f"Successfully fetched: {package_path.name}")
-            return
+            return curdir / "dist" / package_path.name


Maybe make this into a variable and reuse in both shutil.copy and return value?

pyodide-build/pyodide_build/out_of_tree/pypi.py

ryanking13 · 2022-12-28T00:01:15Z

pyodide-build/pyodide_build/out_of_tree/pypi.py

+
+
+@contextmanager
+def stream_redirected(to=os.devnull, stream=None):


Could you please write a comment about this for the people who read this code in the future?

ryanking13 · 2022-12-28T00:11:19Z

pyodide-build/pyodide_build/tests/test_pypi.py

+)
+def test_host_in_node_test(runtime, fn, tmp_path):
+    if runtime.startswith("node"):
+        fn(tmp_path)


These tests are not actually running. I am trying to fix broken CLI tests in #3390. Maybe you will need to fix tests in a same way in #3390 (probably after it is merged).

…d-deps

joemarshall · 2022-12-29T08:37:36Z

Hi @hoodmane , I've done the review changes above now, if you could merge that would be great, I don't have merge access to this repo.

ryanking13

Thanks again for your work @joemarshall. Please update the changelog, and some remaining test issues need to be fixed.

pyodide-build/pyodide_build/cli/build_recipes.py

pyodide-build/pyodide_build/tests/test_pypi.py

joemarshall · 2023-01-03T06:55:18Z

@ryanking13 Thanks, done that now.

joemarshall · 2023-01-03T20:23:06Z

Damn, something broken in node tests, I'll sort that tomorrow

joemarshall · 2023-01-04T17:15:57Z

@ryanking13 tests run in ci correctly now

ryanking13

Thanks!

This switches to passing the source and build directories as arguments. It adds an output-directory argument to pyodide build allowing us to indicate where the output wheel should go independent of the build directory. I also did some cleanup of the logic added in pyodide#3310

This switches to passing the source and build directories as arguments. It adds an output-directory argument to pyodide build allowing us to indicate where the output wheel should go independent of the build directory. I also did some cleanup of the logic added in #3310

…ide#3746) This switches to passing the source and build directories as arguments. It adds an output-directory argument to pyodide build allowing us to indicate where the output wheel should go independent of the build directory. I also did some cleanup of the logic added in pyodide#3310

working version of get dependencies from pypi

1ab9187

rth reviewed Dec 2, 2022

View reviewed changes

joemarshall added 2 commits December 2, 2022 10:52

added resolvelib to deps

5f151fc

fixed typing

0b67b93

hoodmane reviewed Dec 18, 2022

View reviewed changes

joemarshall and others added 12 commits December 21, 2022 12:16

don't return requirement twice

e4a6506

build code tidying

a0485fa

simplified get_cached_built_wheel

87406de

use functools.cache for caching things

a4275a1

type annotations

a669d75

fake pypi to better test dependency building

955bb3f

Merge branch 'main' into build-deps

40791d8

[pre-commit.ci] auto fixes from pre-commit.com hooks

c4377b4

for more information, see https://pre-commit.ci

fake pypi fixes

f9480e3

Merge branch 'main' into build-deps

f3f006f

[pre-commit.ci] auto fixes from pre-commit.com hooks

fa05db4

for more information, see https://pre-commit.ci

update setup.cfg

a13e461

ryanking13 reviewed Dec 28, 2022

View reviewed changes

joemarshall added 3 commits December 29, 2022 08:26

review refactoring

0e0ba8d

Merge branch 'build-deps' of github.com:joemarshall/pyodide into buil…

a383d05

…d-deps

review changes

9d78719

ryanking13 requested changes Jan 2, 2023

View reviewed changes

pyodide-build/pyodide_build/cli/build_recipes.py Show resolved Hide resolved

pyodide-build/pyodide_build/tests/test_pypi.py Outdated Show resolved Hide resolved

joemarshall added 3 commits January 3, 2023 06:41

Merge

4683ce0

make tests run only in pyodide

a8b5329

changelog

54ce9e2

test fixes for CI

e4e39a5

ryanking13 approved these changes Jan 4, 2023

View reviewed changes

ryanking13 merged commit 76581a7 into pyodide:main Jan 4, 2023

ryanking13 mentioned this pull request Jan 16, 2023

Add a CLI command to create a zipfile of Python libraries #3411

Merged

2 tasks

hoodmane mentioned this pull request Apr 6, 2023

Pass the source and build directory explicitly, don't use chdir #3746

Merged

3 tasks

rth mentioned this pull request Dec 14, 2023

[resolver] Use resolvelib for the dependency resolution pyodide/micropip#93

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Get deps from pypi during build #3310

Get deps from pypi during build #3310

joemarshall commented Dec 1, 2022 •

edited by ryanking13

Loading

rth left a comment

rth Dec 2, 2022

joemarshall Dec 2, 2022

ryanking13 Dec 28, 2022 •

edited

Loading

joemarshall commented Dec 2, 2022

joemarshall commented Dec 2, 2022

joemarshall commented Dec 6, 2022

hoodmane commented Dec 18, 2022

hoodmane Dec 18, 2022

joemarshall commented Dec 27, 2022

hoodmane commented Dec 27, 2022

hoodmane commented Dec 27, 2022

ryanking13 left a comment

ryanking13 Dec 27, 2022

ryanking13 Dec 28, 2022 •

edited

Loading

ryanking13 Dec 28, 2022 •

edited

Loading

joemarshall commented Dec 29, 2022

ryanking13 left a comment

joemarshall commented Jan 3, 2023

joemarshall commented Jan 3, 2023

joemarshall commented Jan 4, 2023

ryanking13 left a comment



		@contextmanager
		def stream_redirected(to=os.devnull, stream=None):



		if TYPE_CHECKING:
		APBase = AbstractProvider[Requirement, Candidate, str]

Get deps from pypi during build #3310

Get deps from pypi during build #3310

Conversation

joemarshall commented Dec 1, 2022 • edited by ryanking13 Loading

Description

Checklists

rth left a comment

Choose a reason for hiding this comment

rth Dec 2, 2022

Choose a reason for hiding this comment

joemarshall Dec 2, 2022

Choose a reason for hiding this comment

ryanking13 Dec 28, 2022 • edited Loading

Choose a reason for hiding this comment

joemarshall commented Dec 2, 2022

joemarshall commented Dec 2, 2022

joemarshall commented Dec 6, 2022

hoodmane commented Dec 18, 2022

hoodmane Dec 18, 2022

Choose a reason for hiding this comment

joemarshall commented Dec 27, 2022

hoodmane commented Dec 27, 2022

hoodmane commented Dec 27, 2022

ryanking13 left a comment

Choose a reason for hiding this comment

ryanking13 Dec 27, 2022

Choose a reason for hiding this comment

ryanking13 Dec 28, 2022 • edited Loading

Choose a reason for hiding this comment

ryanking13 Dec 28, 2022 • edited Loading

Choose a reason for hiding this comment

joemarshall commented Dec 29, 2022

ryanking13 left a comment

Choose a reason for hiding this comment

joemarshall commented Jan 3, 2023

joemarshall commented Jan 3, 2023

joemarshall commented Jan 4, 2023

ryanking13 left a comment

Choose a reason for hiding this comment

joemarshall commented Dec 1, 2022 •

edited by ryanking13

Loading

ryanking13 Dec 28, 2022 •

edited

Loading

ryanking13 Dec 28, 2022 •

edited

Loading

ryanking13 Dec 28, 2022 •

edited

Loading