pfmoore / editables

MIT License
39 stars 11 forks source link

Rename build_editable? #5

Closed pfmoore closed 3 years ago

pfmoore commented 3 years ago

The name of the main entry point here, build_editable, doesn't actually read particularly well to me in "real world" code. I'm considering changing it - if we do so better to do it now, while we don't have many users.

I'm going to think about alternative names, but if anyone has strong opinions, please speak up.

@ofek I know you've already started using this package - would a name change be an issue for you? I can keep the old name as an alias easily enough, if necessary.

ofek commented 3 years ago

Nope, not a problem!

pfmoore commented 3 years ago

@ofek Please take a look at the new "redirector" branch. It's a rough prototype at the moment, but it makes the following changes:

The API is now

from editables.redirector import Redirector

# Project name is for now, I may remove the need for it later
r = Redirector("project")
r.redirect(module, target_location)

for name, content in r.files():
    # add to the wheel

The implementation no longer uses "self-replacing" modules, instead it uses an import meta-hook. The meta-hook is currently included with editables, so editable wheels will depend on that at runtime, but if this concept works, I'll split it out into a separate package.

This is very much still proof of concept for feedback only

ofek commented 3 years ago

What should be passed to Redirector.redirect?

pfmoore commented 3 years ago

Soory - module name (what you "import") and the path of the file that gets loaded for that import.

So if you want import xxx to use /tmp/work/xxx.py you'd do r.redirect("xxx", "/tmp/work/xxx.py").

Edit: Actually, in normal usage, the module name can be deduced from the basename of the target, so I'll do that.

ofek commented 3 years ago

I'd prefer mod be kept at least as a kwarg because my backend already has a notion of relative path (src/pkg) vs distribution path (pkg)

edit: then again nvm, that'd be a simple change, carry on 👍

pfmoore commented 3 years ago

Can you point me to your code? I'd be interested in seeing how you're doing things.

Some specific things I want to remove from the current API:

  1. The hide argument in build_editable. It would add complexity in the metapath implementation, and I can't think of any real use case for it, so I'm inclined to leave it out.
  2. Exposing subpackages (foo.bar as opposed to foo) doesn't work properly at the moment, and again it's not at all clear what use it would be (except for namespace packages, which are broken in the current implementation).

So the API I'm looking at now will essentially let you map a series of top-level import names (with no dots) to any of:

  1. A file, which will be treated as an importable module. This should handle .py files and extensions (.pyd or `.so) - at least, all it does is defer to the core import system, so it ought to!
  2. A directory containing an __init__.py, which will be treated as a package.
  3. One or more directories without __init__.py. In this case, the name will be treated as a namespace package consisting of those directories. (I need to do some additional work here to understand how the import system handles namespace packages).
ofek commented 3 years ago

I agree that there is no need to support hide.

Haven't push code to github yet but here is a version that supports your branch: https://pypi.org/project/hatchling/0.0.12/

Look in sdist hatch/core/builders/wheel.py

It uses https://www.python.org/dev/peps/pep-0621/ for config. Set:

[tool.hatch.version]
source = 'regex'
# Looks for a __version__ variable by default
path = 'foo/__about__.py'

[tool.hatch.build]
editable-mode = 'final'

Example build:

$ hatchling build dist -t wheel:editable
Location of artifacts: dist

[wheel]
hatch-1rc1-py3-none-any.whl

Also made a PR: https://github.com/pfmoore/editables/pull/6

pfmoore commented 3 years ago

Thanks, I'll take a look. Just a heads-up, I've had a chunk of free time this week, but that's coming to an end, so don't worry if I start to seem less responsive. I'm currently working on putting together a test suite covering the key behaviours of the "normal" import machinery, so we can be sure that any editable mechanism correctly emulates it.

pfmoore commented 3 years ago

https://bugs.python.org/issue43697 I'm not sure if namespace packages can be supported fully.

ofek commented 3 years ago

That's fine, pip install --editable ./path only supports multiple packages of a namespace as long as all are installed with --editable, so I'm used to it

pfmoore commented 3 years ago

Oh that's good to know. These sorts of thinks might well be worth including in the PEP, so there's a baseline of "minimum sufficient level of functionality".

Off the top of my head:

Any others? I'll post these two on discourse so they get included in the PEP.

ofek commented 3 years ago

Yup, I think just those 2 caveats

pfmoore commented 3 years ago

OK, so the rewrite branch is coming together. The new API looks basically like this:

# Create a project object. The project name will be used as the
# name of the files (.pth, etc) added to the wheel, the project_dir
# is used to resolve relative pathnames in subsequent calls.
project = EditableProject("myproject", project_dir)

# This arranges for "import xxx" to be satisfied by the given path
# It can be a single file, or a directory with an __init__.py.
project.map("xxx", project_path)

# This just adds the subdirectory to sys.path, using a .pth file
project.add_to_path(project_subdir)

for name, content in project.files():
    # add file to the wheel

for dep in project.dependencies():
    # add dep as a runtime dependency for the wheel

If you just use add_to_path, the resulting wheel will have no additional dependencies. The only dependency added will be if you use the map method, and it's a runtime dependency on the editables module. It would be possible to split out the runtime support into a standalone module, but honestly the module is so small at the moment, that doing so seems like an unnecessary complication. The dependencies method means we could make that change later without needing client changes.

ofek commented 3 years ago

Since the bootstrapping only uses RedirectingFinder which is entirely isolated in its own file, can it instead just copy the contents to avoid a runtime dependency?

pfmoore commented 3 years ago

Doing that would mean that each editable install would have its own copy, so there would be multiple copies on sys.meta_path, which would potentially slow the import system down. Probably not by much, but it seems like something to avoid and it's not exactly a massive cost to have a shared implementation.

ofek commented 3 years ago

I'm just trying to find a way to avoid a runtime dependency.

pfmoore commented 3 years ago

Understood. Out of curiosity, what's so bad about a runtime dependency?

ofek commented 3 years ago

I may be missing something but isn't the concept of runtime dependencies unrelated to the project itself entirely new? Like say you pip install -e .:

[build-system]
requires = ['backend']  # backends will depend on editables
build-backend = 'backend.build'

How would you define the dep? This?:

[project]
...
dependencies = [
  ...
  'editables',
]

then every standard/non-editable installation will come with it for no reason, right?

pfmoore commented 3 years ago

When you build the editable wheel, call project.dependencies and add the items returned to the dependency metadata. They aren't specified in the project configuration, and they aren't added to a normal wheel.

ofek commented 3 years ago

Oh I see, so backends will all have that extra logic. I understand now 👍

pfmoore commented 3 years ago

See #7. The new API feels ready to go now.

ofek commented 3 years ago

This isn't working for me.

>>> from editables import EditableProject
>>>
>>> editable_project = EditableProject('hatchling', 'C:\\Users\\ofek\\Desktop\\code\\hatch\\hatchling')
>>> editable_project.map('hatchling', 'hatchling')
>>>
>>> for filename, content in editable_project.files():
...     print(filename)
...     print(content)
...     print()
...
hatchling.pth
import _hatchling

_hatchling.py
from editables.redirector import RedirectingFinder as F
F.install()
F.map_module('hatchling', 'C:\\Users\\ofek\\Desktop\\code\\hatch\\hatchling\\hatchling\\__init__.py')

when installed:

$ python -c "from hatchling.cli import hatchling"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ModuleNotFoundError: No module named 'hatchling.cli'

I even tried appending the following to _hatchling.py to no avail:

F.map_module('hatchling.cli', 'C:\\Users\\ofek\\Desktop\\code\\hatch\\hatchling\\hatchling\\cli\\__init__.py')
pfmoore commented 3 years ago

Hmm, weird. I thought I had a test case that covered that. I'll take a look, but if you can provide a test case (in a form that could be added to the test suite) that demonstrates the issue, that would be very helpful.

ofek commented 3 years ago

I had an old .pth messing things up. I guess you can close this now, thanks!!!

pfmoore commented 3 years ago

Oh phew! I was worried there that something was badly wrong :-) Thanks for checking and reporting back.