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

Support build system requires and enable use of custom executable for build script execution #58

Closed
wants to merge 2 commits into from

Conversation

abn
Copy link
Member

@abn abn commented Aug 10, 2020

This change adds build-system.requires requirements other than poetry and poetry-core as a dependency for the package (marked with build category). This is required in order to allow for poetry to effectively manage development environments, for example when the project is installed as an editable package during development. In scenarios where a build script is executed, and it requires cython (as an example), today this will fail unless cython is listed as a dev-dependency explicitly. This change allows for this to be detected from build-system configuration table and handled transparently.

Additionally, in order to allow for build script execution under development environment, we allow for the python executable used to be configurable.

Relates-to: python-poetry/poetry#2789

@abn abn requested a review from sdispater August 10, 2020 11:50
@abn
Copy link
Member Author

abn commented Aug 10, 2020

@sdispater any issues with this approach? The change in poetry would look something like this python-poetry/poetry@6a73f4e.

@abn abn force-pushed the support-build-system-requires branch 2 times, most recently from 66bd42d to 0658ad0 Compare August 10, 2020 12:47
@abn abn requested a review from a team August 10, 2020 23:39
@abn abn force-pushed the support-build-system-requires branch from 0658ad0 to cb72178 Compare August 15, 2020 18:32
@abn abn force-pushed the support-build-system-requires branch from cb72178 to ba8fc3e Compare August 27, 2020 22:25
@sdispater
Copy link
Member

@abn I don't think we should consider the requires of the build-system as dependencies of the project. They are only dependencies of the build system.

Any extra logic should only be handled by builders (editable or wheel). I am not sure how we can approach this yet, though.

@abn
Copy link
Member Author

abn commented Aug 30, 2020

@sdispater not sure if I fully agree there. To me, non-poetry build requirements are part of the development envionnment just as much as any other development dependency of the project as without it we cannot develop the package. Ofcourse, we can add it explicitly to the dev-dependencies but then it feels like a redundant declaration of a dependency. The change here is to add this to the lock file so we can have a "resolved" environment during development for editable installer etc.

Unless we start having a "poetry" venv per project isolated from the main envrionemnt for tasks like executing build scripts and even running poetry build command, I think we will need to have this depenendency in the project venv. Alternatively, we can use a pep517 isolated build when execuring poetry build; while this makes it a bit slower it does ensure that we are building the right artifacts, but this leads to issues with an editable install.

I guess another approach might to revisit this once we have tagging and have reserved tags likedev and build. Etiher way we need to handle this somewhere. However, since these delendencies need to get managed by the PEP 517 frontend, we cannot declare them only outside of the build-systemsection either.

@abn
Copy link
Member Author

abn commented Aug 30, 2020

@sdispater while we put this on ice for now, I have broken out some enhancements in this PR into individual ones so we can handle build-system dependency logic in editable installs a bit more cleanly.

See: #72 #73

@abn abn marked this pull request as draft August 30, 2020 21:42
@abn
Copy link
Member Author

abn commented Sep 2, 2020

@sdispater as an additional follow up to this one. Would you say that if a package's build script requires an external package, it should be perhaps defined not in the build-system but rather in the project's tool.poetry.build section? As an example;

[tool.poetry.build]
script = "build.py"

[tool.poetry.build.dependencies]
cpython = "*"

This would lend itself well to the use of PEP 517 optional hooks. It, however, will make this a "poetry" config. I am unsure which is the better option here. I like this approach because it is more explicit and can allow us to manage this via the cli cleanly.

@abn
Copy link
Member Author

abn commented Mar 20, 2021

@sdispater any furhter thoughts on this?

@abn
Copy link
Member Author

abn commented Apr 2, 2022

Closing this in favour of python-poetry/poetry#5401 for now.

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