Skip to content

build_wheel_for_editable#1

Open
sbidoul wants to merge 1 commit intomasterfrom
build_wheel_for_editable-sbi
Open

build_wheel_for_editable#1
sbidoul wants to merge 1 commit intomasterfrom
build_wheel_for_editable-sbi

Conversation

@sbidoul
Copy link
Owner

@sbidoul sbidoul commented Apr 2, 2021

No description provided.

@sbidoul
Copy link
Owner Author

sbidoul commented Apr 2, 2021

@dholth I added some content to your draft, in particular in 9c5d969. Also took the liberty to add myself as co-author. I think I covered all comments from the discuss thread. Do you like to have a look before we formally publish the draft to discuss.python.org in a new thread ?

@sbidoul
Copy link
Owner Author

sbidoul commented Apr 2, 2021

And we need to decide what to do about scheme. I'm not sure I want to fight over that one, but I'm ok to leave it in the draft too.

@dholth
Copy link

dholth commented Apr 2, 2021

Forget about scheme, what about deleting metadata_directory? I'm not sure we need it... would pip care, or does it always have to call the equivalent of setup.py egg_info before installing things?

@sbidoul
Copy link
Owner Author

sbidoul commented Apr 2, 2021

Ok, I dropped scheme.

About metadata_directory: pip calls prepare_metadata_for_build_wheel during dependency resolution.

Interestingly, since prepare_metadata_for_build_wheel does not know about editables, this means that the editable metadata must be the same as regular metadata, which means same dependencies too. I can live with that for now, I'd just make a provision that build_wheel_for_editable is allowed to add a local version identifier but otherwise not change any other metadata.

And I'd keep metadata_directory for symmetry with build_wheel, which should make it easier for frontends.

@dholth
Copy link

dholth commented Apr 2, 2021

Ok, I dropped scheme.

About metadata_directory: pip calls prepare_metadata_for_build_wheel during dependency resolution.

Interestingly, since prepare_metadata_for_build_wheel does not know about editables, this means that the editable metadata must be the same as regular metadata, which means same dependencies too. I can live with that for now, I'd just make a provision that build_wheel_for_editable is allowed to add a local version identifier but otherwise not change any other metadata.

And I'd keep metadata_directory for symmetry with build_wheel, which should make it easier for frontends.

I didn't mean "drop scheme". I meant that the metadata_directory argument has been ignored by build backends in practice, so why keep it here?

@sbidoul
Copy link
Owner Author

sbidoul commented Apr 2, 2021

I didn't mean "drop scheme".

Ah sorry, I misinterpreted what you meant with "forget about scheme".

the metadata_directory argument has been ignored by build backends in practice, so why keep it here?

For symmetry and easier implementation by frontends. I'd say if metadata_directory turns out to be useless in practice, that should be clarified in an amendment to PEP 517 that addresses both variants of build_wheel.

@dholth
Copy link

dholth commented Apr 2, 2021

This argument could go in circles. It's already asymmetrical because there isn't a prepare_metadata_for_editable, but, it would be very unsatisfying to have a prepare_metadata_for_editable hook.

@sbidoul
Copy link
Owner Author

sbidoul commented Apr 2, 2021

Ok, lets drop metadata_directory and see what people say.

@sbidoul
Copy link
Owner Author

sbidoul commented Apr 3, 2021

We could also say that frontends must not call prepare_metadata_for_build_wheel in case of editables, and rely only on the metadata produced by build_wheel_for_editable in such case. Not sure.

@dholth
Copy link

dholth commented Apr 3, 2021

Are you supposed to install the requirements before build or are we doing those in pyproject.toml these days?

We would do the same steps here. The mechanism for making sure the metadata is (mostly) the same is to have deterministic metadata generation, not to copy it from a 📂

@sbidoul
Copy link
Owner Author

sbidoul commented Apr 3, 2021

@dholth sorry I don't understand your last comment.

@dholth
Copy link

dholth commented Apr 3, 2021

Nothing changes by removing the metadata directory argument. It only means we have to do what backends do anyway, which is to regenerate the metadata.

I suppose there will be details about exactly when dependencies are installed, lack of build isolation etc.

@sbidoul
Copy link
Owner Author

sbidoul commented Apr 3, 2021

Ok, perhaps I overthink this. So if this is in good shape for you, I guess the next step is to summon @pfmoore ;) Paul, as sponsor, would you like to have a look and tell us if this seems good enough to open a draft PEP or if you see some changes to do before that ?

@sbidoul sbidoul force-pushed the build_wheel_for_editable-sbi branch from e69ce33 to f57d22c Compare April 3, 2021 16:59
@sbidoul sbidoul force-pushed the build_wheel_for_editable-sbi branch from 3cf3e1b to 11f5323 Compare April 25, 2021 14:34
@sbidoul
Copy link
Owner Author

sbidoul commented Apr 25, 2021

@dholth I added 3 commits. Please let me know what you think. If ok for you I think this is ready to go for public discussion.

@sbidoul sbidoul force-pushed the build_wheel_for_editable-sbi branch from 11f5323 to 2b0a164 Compare April 25, 2021 16:45
@dholth
Copy link

dholth commented Apr 25, 2021

For example if we don't provide a separate build editables dependency hook then those dependencies can't differ...

@sbidoul
Copy link
Owner Author

sbidoul commented Apr 25, 2021

Yeah but I think this is not the place nor the time to question the philosophy of pep 517. If get_requires_for_build_* is in the spec, there must be use cases for it, and it makes sens to have a third one for editables.

In the case of prepare_metadata_for_build_* I am now convinced it is different because, as an optimization to help the resolver, it is not necessary, since the frontend knows beforehand that the editable will have to be installed, so there is no need to prepare discardable metadata.

@dholth
Copy link

dholth commented Apr 25, 2021 via email

@sbidoul
Copy link
Owner Author

sbidoul commented Apr 25, 2021

Ok thanks. I'll post it soon. Let's see if this thing flies.

@sbidoul sbidoul force-pushed the build_wheel_for_editable-sbi branch 5 times, most recently from 7d0488d to f0927bc Compare May 9, 2021 10:11
@sbidoul sbidoul force-pushed the build_wheel_for_editable-sbi branch from 13cb33d to 7ffadee Compare May 20, 2021 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants