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

Add PEP517 check suite and support disabling setup.py for sdists #26

Merged
merged 6 commits into from
Apr 23, 2020

Conversation

abn
Copy link
Member

@abn abn commented Apr 17, 2020

  1. Adds scaffolding and a few integration tests for PEP517 tests.
  2. Support opt-in disabling of setup.py in sdists.
  3. Support "tool.poetry.build" section to support "generate-setup-file"
[tool.poetry.build]
generate-setup-file = true

This change will also mean that tool.poetry.build = "build.py" usage is now deprecated in favour of tool.poetry.build.script. The older cofiguration will continue to work as expected.

I have also added some additional converage for PEP-517 concerns in a new github actions workflow.

Integration Test Suite Notes

To run pytest integration cases locally, you can do so using one of the following.

  1. Use tox. This runs only the integration tests placed in tests/integration.
tox -e integration
  1. Using pytest. Using the --integration option will enable all tests with the integration marker. You can restrict running only these tests by specifying tests/integration directory as an argument.
pytest --integration

If developing/adding tests, you can simply mark a test as integration, and it will only be executed when the integration suite is executed.

@abn abn requested a review from a team April 17, 2020 23:44
@abn
Copy link
Member Author

abn commented Apr 18, 2020

PEP-517 tests seem to pass okay locally on all versions, but CI does not like them. Need to investigate a bit further.

EDIT: This was resolved.

@abn abn force-pushed the disable-setyp-py branch 7 times, most recently from 1b7bd43 to 0b20938 Compare April 18, 2020 16:12
@abn abn self-assigned this Apr 18, 2020
@abn
Copy link
Member Author

abn commented Apr 19, 2020

@sdispater any thought on the naming for the configuration section? The idea is to reuse this section for project specific configuration. Ideally would like to evolve this further later to provide project specific overrides etc.

@sdispater
Copy link
Member

The table to store the setting should be tool.poetry.build.config (or even a simpler tool.poetry.build where it would be possible to put build-related settings) instead of tool.poetry.config.

The reason for that is I don't want people to start thinking that they can put any setting in this section (we already have poetry.toml for that). So having a build section would be better. However this will break the projects of people currently using the build property to specify a build script.

I don't know how many users use this feature, and I don't mind breaking it this it was never documented and it needs improvements anyway but that's something to have in mind and we need to find a way to make it work.

@abn
Copy link
Member Author

abn commented Apr 23, 2020

@sdispater I fear quite a few people are using this feature (based on incoming issues) as this is the only way today the extensions work. I am reluctant to recommend removing it as it is a useful feature.

Also, tool.poetry.toml is not really documented either as far as I can tell.

@sdispater
Copy link
Member

@abn When I said poetry.toml, I meant the poetry.toml file that's used for the --local option of the config command (see https://python-poetry.org/docs/configuration/#local-configuration).

If we are not comfortable in removing it, we will need to make it backward compatible, meaning we will need to support both (build as a string pointing to a build script and build as a table representing a build configuration).

So basically people would need to migrate from:

build = "my_script.py"

to

[tool.poetry.build.config]
script = "my_script.py"
generate-setup-file = false

@abn
Copy link
Member Author

abn commented Apr 23, 2020

@sdispater the change has been applied.

Copy link
Member

@sdispater sdispater left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

abn added 5 commits April 23, 2020 23:24
This change also improves tox configuration to use isolated builds
and also adds integration environment
This change adds coverage for the following.
- pip wheel build
- pep517 compliance checks
- pep517 builds for sdists and wheels
- pip install with --no-binary :all:
@abn abn merged commit 127a32c into python-poetry:master Apr 23, 2020
@abn abn deleted the disable-setyp-py branch April 23, 2020 21:31
@sdispater sdispater mentioned this pull request Apr 24, 2020
abn added a commit to abn/poetry-core that referenced this pull request Apr 1, 2022
This is the first step in removing setup file generation from Poetry
projects. With this change, projects that require a setup.py to be
generated when a build script is used needs to explicitly set
`tool.poetry.build.generate-setup-file`, introduced in python-poetry#26, to `true`
in `pyproject.toml`.
abn added a commit to abn/poetry-core that referenced this pull request Apr 1, 2022
This is the first step in removing setup file generation from Poetry
projects. With this change, projects that require a setup.py to be
generated when a build script is used needs to explicitly set
`tool.poetry.build.generate-setup-file`, introduced in python-poetry#26, to `true`
in `pyproject.toml`.
finswimmer pushed a commit to abn/poetry-core that referenced this pull request Oct 7, 2022
This is the first step in removing setup file generation from Poetry
projects. With this change, projects that require a setup.py to be
generated when a build script is used needs to explicitly set
`tool.poetry.build.generate-setup-file`, introduced in python-poetry#26, to `true`
in `pyproject.toml`.
finswimmer pushed a commit to abn/poetry-core that referenced this pull request Oct 8, 2022
This is the first step in removing setup file generation from Poetry
projects. With this change, projects that require a setup.py to be
generated when a build script is used needs to explicitly set
`tool.poetry.build.generate-setup-file`, introduced in python-poetry#26, to `true`
in `pyproject.toml`.
neersighted pushed a commit to abn/poetry-core that referenced this pull request Oct 11, 2022
This is the first step in removing setup file generation from Poetry
projects. With this change, projects that require a setup.py to be
generated when a build script is used needs to explicitly set
`tool.poetry.build.generate-setup-file`, introduced in python-poetry#26, to `true`
in `pyproject.toml`.

Co-authored-by: Bartosz Sokorski <[email protected]>
finswimmer pushed a commit to abn/poetry-core that referenced this pull request Nov 9, 2022
This is the first step in removing setup file generation from Poetry
projects. With this change, projects that require a setup.py to be
generated when a build script is used needs to explicitly set
`tool.poetry.build.generate-setup-file`, introduced in python-poetry#26, to `true`
in `pyproject.toml`.

Co-authored-by: Bartosz Sokorski <[email protected]>
finswimmer pushed a commit to abn/poetry-core that referenced this pull request Jan 1, 2023
This is the first step in removing setup file generation from Poetry
projects. With this change, projects that require a setup.py to be
generated when a build script is used needs to explicitly set
`tool.poetry.build.generate-setup-file`, introduced in python-poetry#26, to `true`
in `pyproject.toml`.

Co-authored-by: Bartosz Sokorski <[email protected]>
finswimmer pushed a commit to abn/poetry-core that referenced this pull request Jan 21, 2023
This is the first step in removing setup file generation from Poetry
projects. With this change, projects that require a setup.py to be
generated when a build script is used needs to explicitly set
`tool.poetry.build.generate-setup-file`, introduced in python-poetry#26, to `true`
in `pyproject.toml`.

Co-authored-by: Bartosz Sokorski <[email protected]>
@ghost
Copy link

ghost commented Feb 28, 2023

Hello 2023 people!

Once again we have a great poetry point release breaking existing features.

This previous statement is no longer true:

we will need to make it backward compatible, meaning we will need to support both (build as a string pointing to a build script and build as a table representing a build configuration).

The previous syntax is now gone and your builds are breaking until you use the new syntax (which i couldn't find/search in the docs anywhere? it only exists here?) of:

[tool.poetry.build.config]
script = "my_script.py"
generate-setup-file = true

@phillipuniverse
Copy link

phillipuniverse commented Mar 7, 2023

@mattpeople that config doesn't work, here's what I have:

  [tool.poetry.build.config]
  generate-setup-file = true

and I get this error:

The Poetry configuration is invalid:
  - [build] {'config': {'generate-setup-file': True}} is not valid under any of the given schemas

The one in the description of this PR does work:

  [tool.poetry.build]
  generate-setup-file = true

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.

5 participants