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

feat(config): installer.build-config-settings.pkg #10129

Merged
merged 2 commits into from
Feb 1, 2025

Conversation

abn
Copy link
Member

@abn abn commented Jan 30, 2025

This change introduces the installer.build-config-settings.<pkg> configuration option to allow for PEP 517 build config settings to be passed to the respective build backends when a dependency is built during installation.

This feature was chosen not to be exposed via an addition to the dependency specification schema as these configurations can differ between environments.

Resolves: #845 #8909
Closes: python-poetry/poetry-core#715

Examples

$ # Empty values
$ poetry config installer.build-config-settings.pip 
No build config settings configured for pip.
$ poetry config installer.build-config-settings
No packages configured with build config settings.
$
$ # Configuration via env var
$ POETRY_INSTALLER_BUILD_CONFIG_SETTINGS_POD='{"baz" : ["ONE"]}' poetry config installer.build-config-settings
pod.baz = "[ONE]"
$
$ # Configuration via cli
$ poetry config installer.build-config-settings.pip '{"CC": "gcc", "--build-option": ["--build-option1", "--build-option2"]}'
$
$ # Reading env var and toml
$ POETRY_INSTALLER_BUILD_CONFIG_SETTINGS_POD='{"baz" : ["ONE"]}' poetry config installer.build-config-settings
pip.--build-option = "[--build-option1, --build-option2]"
pip.CC = "gcc"
pod.baz = "[ONE]"
$
$ # Env var takes precedence
$ POETRY_INSTALLER_BUILD_CONFIG_SETTINGS_PIP='{"baz" : ["ONE"]}' poetry config installer.build-config-settings
pip.baz = "[ONE]"
$
$ # Show single value
$ poetry config installer.build-config-settings.pip 
{"CC": "gcc", "--build-option": ["--build-option1", "--build-option2"]}

Testing

We can verify that the settings are passed to the builder using the following snippet.

podman run --rm -i --entrypoint bash docker.io/python:latest <<EOF
set -xe
python -m pip install --disable-pip-version-check -q git+https://github.com/python-poetry/poetry.git@refs/pull/10129/head
poetry new foobar
pushd foobar
cat > poetry.toml <<TOML
[installer.build-config-settings.pygraphviz]
--global-option = ["build_ext", "-I/bad/path/graphviz/include/", "-L/bad/path/lib/"]
TOML
poetry add pygraphviz
EOF

The relevant output that shows the correct passing of the config settings are the following.

gcc -fno-strict-overflow -Wsign-compare -DNDEBUG -g -O3 -Wall -fPIC -DSWIG_PYTHON_STRICT_BYTE_CHAR -I/bad/path/graphviz/include/ -I/tmp/tmpy02y2dmo/.venv/include -I/usr/local/include/python3.13 -c pygraphviz/graphviz_wrap.c -o build/temp.linux-x86_64-cpython-313/pygraphviz/graphviz_wrap.o
You can verify this by running pip wheel --no-cache-dir --use-pep517 --config-settings='--global-option=build_ext' --config-settings='--global-option=-I/bad/path/graphviz/include/' --config-settings='--global-option=-L/bad/path/lib/' "pygraphviz (==1.14)".
console.output

Creating virtualenv foobar-lWDpn5M1-py3.13 in /root/.cache/pypoetry/virtualenvs
Using version ^1.14 for pygraphviz

Updating dependencies
Resolving dependencies...

Package operations: 1 install, 0 updates, 0 removals

  - Installing pygraphviz (1.14)

PEP517 build of a dependency failed

Backend subprocess exited when trying to invoke build_wheel

    | Command '['/tmp/tmpy02y2dmo/.venv/bin/python', '/usr/local/lib/python3.13/site-packages/pyproject_hooks/_in_process/_in_process.py', 'build_wheel', '/tmp/tmpokr672bw']' returned non-zero exit status 1.
    | 
    | running build_ext
    | building 'pygraphviz._graphviz' extension
    | creating build/temp.linux-x86_64-cpython-313/pygraphviz
    | gcc -fno-strict-overflow -Wsign-compare -DNDEBUG -g -O3 -Wall -fPIC -DSWIG_PYTHON_STRICT_BYTE_CHAR -I/bad/path/graphviz/include/ -I/tmp/tmpy02y2dmo/.venv/include -I/usr/local/include/python3.13 -c pygraphviz/graphviz_wrap.c -o build/temp.linux-x86_64-cpython-313/pygraphviz/graphviz_wrap.o
    | pygraphviz/graphviz_wrap.c:9: warning: "SWIG_PYTHON_STRICT_BYTE_CHAR" redefined
    |     9 | #define SWIG_PYTHON_STRICT_BYTE_CHAR
    |       | 
    | <command-line>: note: this is the location of the previous definition
    | pygraphviz/graphviz_wrap.c:3023:10: fatal error: graphviz/cgraph.h: No such file or directory
    |  3023 | #include "graphviz/cgraph.h"
    |       |          ^~~~~~~~~~~~~~~~~~~
    | compilation terminated.
    | error: command '/usr/bin/gcc' failed with exit code 1

Note: This error originates from the build backend, and is likely not a problem with poetry but one of the following issues with pygraphviz (1.14)

  - not supporting PEP 517 builds
  - not specifying PEP 517 build requirements correctly
  - the build requirements are incompatible with your operating system or Python version
  - the build requirements are missing system dependencies (eg: compilers, libraries, headers).

You can verify this by running pip wheel --no-cache-dir --use-pep517 --config-settings='--global-option=build_ext' --config-settings='--global-option=-I/bad/path/graphviz/include/' --config-settings='--global-option=-L/bad/path/lib/' "pygraphviz (==1.14)".

Summary by Sourcery

Add support for PEP 517 build config settings during package installation. This allows specifying build configuration settings for dependencies that need to be built from source.

New Features:

  • Added a new configuration option installer.build-config-settings.<pkg> to allow passing PEP 517 build configuration settings to build backends when building dependencies during installation.

Tests:

  • Added tests to verify the new build config settings functionality.

Summary by Sourcery

Add support for PEP 517 build config settings during dependency installation. This allows users to specify build configuration settings for dependencies that need to be built from source using the installer.build-config-settings.<pkg> config option.

New Features:

  • Added the installer.build-config-settings.<pkg> configuration option to allow passing PEP 517 build configuration settings to build backends when building dependencies during installation.

Tests:

  • Added tests to verify that build config settings are passed correctly to the builder.

@abn abn added area/installer Related to the dependency installer area/config Related to configuration management impact/docs Contains or requires documentation changes labels Jan 30, 2025
@abn abn requested a review from a team January 30, 2025 23:46
Copy link

sourcery-ai bot commented Jan 30, 2025

Reviewer's Guide by Sourcery

This pull request introduces the installer.build-config-settings.<pkg> configuration option to allow for PEP 517 build config settings to be passed to the respective build backends when a dependency is built during installation. This feature was chosen not to be exposed via an addition to the dependency specification schema as these configurations can differ between environments.

Sequence diagram for package installation with build config settings

sequenceDiagram
    participant User
    participant Poetry
    participant Chef
    participant BuildBackend

    User->>Poetry: Install package with build config settings
    Note over Poetry: Read build config settings from config
    Poetry->>Chef: prepare(archive, config_settings)
    Chef->>BuildBackend: build(distribution, config_settings)
    BuildBackend-->>Chef: Built package
    Chef-->>Poetry: Prepared package
    Poetry-->>User: Package installed
Loading

Class diagram for config settings implementation

classDiagram
    class Config {
        +dict _config
        +get(setting_name: str, default: Any)
        +_get_environment_build_config_settings()
        +_get_normalizer(name: str)
    }

    class Chef {
        +prepare(archive: Path, config_settings: Mapping)
        -_prepare(directory: Path, config_settings: Mapping)
        -_prepare_sdist(archive: Path, config_settings: Mapping)
    }

    class Executor {
        -dict _build_config_settings
        -_execute_operation(operation: Operation)
        -_prepare_archive(operation: Operation)
    }

    Config -- Executor: provides config
    Executor -- Chef: uses
Loading

Flow diagram for build config settings resolution

flowchart TD
    A[Start] --> B{Check env vars}
    B -->|Yes| C[Get settings from env]
    B -->|No| D{Check config file}
    D -->|Yes| E[Get settings from config]
    D -->|No| F[Use default empty settings]
    C --> G[Merge settings]
    E --> G
    F --> G
    G --> H[Apply to build process]
    H --> I[End]
Loading

File-Level Changes

Change Details Files
Added support for build config settings when installing packages.
  • Added a new configuration option installer.build-config-settings.<pkg>.
  • The configuration can be set via environment variables.
  • The configuration can be set via the CLI.
  • The configuration is passed to the build backend when building a dependency.
  • Added tests to verify the new build config settings functionality.
tests/installation/test_executor.py
src/poetry/config/config.py
tests/console/commands/test_config.py
src/poetry/console/commands/config.py
docs/configuration.md
src/poetry/installation/chef.py
tests/config/test_config.py
src/poetry/installation/executor.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!
  • Generate a plan of action for an issue: Comment @sourcery-ai plan on
    an issue to generate a plan of action for it.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

github-actions bot commented Jan 30, 2025

Deploy preview for website ready!

✅ Preview
https://website-44nypm0th-python-poetry.vercel.app

Built with commit 366ebad.
This pull request is being automatically deployed with vercel-action

@abn abn force-pushed the feat/build-config-settings branch 2 times, most recently from 4d484a2 to 96e2063 Compare January 31, 2025 00:15
@abn abn force-pushed the feat/build-config-settings branch 5 times, most recently from 5ef0deb to df3f641 Compare January 31, 2025 16:06
@abn abn marked this pull request as ready for review January 31, 2025 16:09
Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @abn - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟡 Testing: 1 issue found
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

tests/installation/test_executor.py Show resolved Hide resolved
Copy link
Member

@radoering radoering left a comment

Choose a reason for hiding this comment

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

You probably have to add the new setting in

def unique_config_values(self) -> dict[str, tuple[Any, Any]]:

src/poetry/console/commands/config.py Outdated Show resolved Hide resolved
src/poetry/config/config.py Show resolved Hide resolved
@abn
Copy link
Member Author

abn commented Feb 1, 2025

Hmm, don't think we need to add it there as this is not a single value setting.

@radoering
Copy link
Member

Hmm, don't think we need to add it there as this is not a single value setting.

You are probably right. I had always thought that all settings have to be added there and never took a closer look.

@abn abn force-pushed the feat/build-config-settings branch from df3f641 to 49adacf Compare February 1, 2025 10:34
@abn
Copy link
Member Author

abn commented Feb 1, 2025

To be fair, the whole config handling stuff needs a re-write. Right now it works, but doing anything in that area of the code is a headache.

@abn abn requested a review from radoering February 1, 2025 10:35
@abn abn force-pushed the feat/build-config-settings branch from 49adacf to 97a1518 Compare February 1, 2025 10:35
tests/config/test_config.py Outdated Show resolved Hide resolved
abn and others added 2 commits February 1, 2025 12:16
This change introduces the `installer.build-config-settings.<pkg>`
configuration option to allow for PEP 517 build config settings to be
passed to the respective build backends when a dependency is built
during installation.

This feature was chosen not to be exposed via an addition to the
dependency specification schema as these configurations can differ
between environments.

Resolves: python-poetry#845
@abn abn force-pushed the feat/build-config-settings branch from 198eee7 to 366ebad Compare February 1, 2025 11:16
@abn abn enabled auto-merge (squash) February 1, 2025 11:17
@abn abn requested a review from radoering February 1, 2025 11:17
@abn abn dismissed radoering’s stale review February 1, 2025 11:22

Review comments implemented.

@abn abn merged commit 28ad7d3 into python-poetry:main Feb 1, 2025
54 checks passed
@abn abn deleted the feat/build-config-settings branch February 1, 2025 11:58
@abn abn mentioned this pull request Feb 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/config Related to configuration management area/installer Related to the dependency installer impact/docs Contains or requires documentation changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for pip install --build-options / --global-options / --install-options
2 participants