Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP prototype pep517-style editable #8154

Closed
wants to merge 3 commits into from
Closed

Conversation

dholth
Copy link
Member

@dholth dholth commented Apr 28, 2020

See https://discuss.python.org/t/third-try-on-editable-installs/3986/22

A new editable hook does an in-place build, populates a directory that will be installed into PLATLIB.

@dholth dholth marked this pull request as draft May 1, 2020 13:17
@RonnyPfannschmidt
Copy link
Contributor

is there any plan to support arbitrary namespace package mappings

i would really like to be able to map a src folder to a installed package name of something like myns.packagename

@pfmoore
Copy link
Member

pfmoore commented May 7, 2020

@RonnyPfannschmidt That would be up to the backend, I'd expect. Or honestly, a 3rd party tool could be written that built a wheel that exposed anything you like, in any layout you wanted.

@sbidoul
Copy link
Member

sbidoul commented May 7, 2020

i would really like to be able to map a src folder to a installed package name of something like myns.packagename

@RonnyPfannschmidt I do have such a use case too, so I'll do some experiments in the coming days/weeks to confirm it's feasible with .pth + import hook.

@RonnyPfannschmidt
Copy link
Contributor

I already have such a thing prototyped in gumby elf, just not pip integrated

@pfmoore
Copy link
Member

pfmoore commented May 7, 2020

Neat. I don't anticipate any more complexity in what pip exposes than pip install -e provides. Backends can add extra features as they choose, of course 🙂

with open(os.path.join(self.metadata_directory, 'RECORD'), 'w+'):
pass

install_unpacked_parsed_wheel(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the need for this. Why can't the backend just provide a wheel, which we install using the normal install_wheel route? We already have a lot of concepts and we're now adding an "unpacked parsed wheel" that's used to communicate between backend and frontend. That seems like a recipe for confusion...

Copy link
Member Author

@dholth dholth May 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only the easiest way to copy files into site-packages with pip. Remember the older PEP 376 concept which is a directory with a *.dist-info. That concept also appears in the PEP 517 metadata hook. An unhappy accident that the *.data/ directory would technically work?

If it was a complete wheel it would be a strange one that only worked on this machine and shared the name of the package but not its contents. It would be a hassle to re-implement bdist_wheel in the develop command class, or to create an ephemeral ZipFile and compute RECORD in another build system.

As for pip that function should be broken up even without editable installs. It has a TODO. I've split out the only part that requires a ZipFile.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I notice that this doesn't populate RECORD when it's installed. Thought that was happening before, but maybe not.

@@ -777,18 +777,49 @@ def install(

global_options = global_options if global_options is not None else []
if self.editable:
Copy link
Member

@sbidoul sbidoul May 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somehow I think this should become if self.editable and not self.is_wheel then legacy editable install, else go ahead and install wheel with the regular install_wheel() below, no other change necessary in this function.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll have to decide if this prototype makes sense or if it is just the way pip works. The prototyped approach is to put extra files into the directory into which we have already generated our *.dist-info directory as part of another hook. It's not quite a wheel. It's not necessarily easy for the build system, accustomed to building a regular wheel, to build a completely different kind of wheel. Stub modules in place of the regular contents, no need for compatibility tags, wheel caching, *.data directories, or the RECORD associated with a full wheel.

@BrownTruck
Copy link
Contributor

Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the master branch into this pull request or rebase this pull request against master then it will be eligible for code review and hopefully merging!

@BrownTruck BrownTruck added the needs rebase or merge PR has conflicts with current master label Jul 3, 2020
@ofek
Copy link
Contributor

ofek commented Jul 4, 2020

Any update on this?

@dholth
Copy link
Member Author

dholth commented Jul 5, 2020

Another contributor has a different take on the same feature. I'll defer to that pull request.

@ofek
Copy link
Contributor

ofek commented Jul 5, 2020

@dholth Link? I can't find it.

@sbidoul
Copy link
Member

sbidoul commented Jul 5, 2020

@ofek see #8212. It's pretty complete on the pip (frontend) side and we are waiting for backend prototypes to be developed.

@ofek
Copy link
Contributor

ofek commented Jul 5, 2020

@sbidoul Thanks!

@pradyunsg
Copy link
Member

Another contributor has a different take on the same feature. I'll defer to that pull request.

Closing based on this.

@pradyunsg pradyunsg closed this Apr 2, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs rebase or merge PR has conflicts with current master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants