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.

Sign up for GitHub

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

Jump to bottom

fix: pip config options were broken #1430

Merged
merged 2 commits into from
Mar 11, 2023
Merged

fix: pip config options were broken #1430

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c681860
fix: pip config options were broken
henryiii Mar 10, 2023
2e48667
refactor: use frontend instead of plural
henryiii Mar 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion cibuildwheel/linux.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,9 @@ def build_in_container(
container.call(["mkdir", "-p", built_wheel_dir])

verbosity_flags = get_build_verbosity_extra_flags(build_options.build_verbosity)
extra_flags = split_config_settings(build_options.config_settings)
extra_flags = split_config_settings(
build_options.config_settings, plural=build_options.build_frontend == "pip"
)

if build_options.build_frontend == "pip":
extra_flags += verbosity_flags
Expand Down
4 changes: 3 additions & 1 deletion cibuildwheel/macos.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,9 @@ def build(options: Options, tmp_path: Path) -> None:
built_wheel_dir.mkdir()

verbosity_flags = get_build_verbosity_extra_flags(build_options.build_verbosity)
extra_flags = split_config_settings(build_options.config_settings)
extra_flags = split_config_settings(
build_options.config_settings, plural=build_options.build_frontend == "pip"
)

if build_options.build_frontend == "pip":
extra_flags += verbosity_flags
Expand Down
5 changes: 3 additions & 2 deletions cibuildwheel/util.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,9 +208,10 @@ def get_build_verbosity_extra_flags(level: int) -> list[str]:
return []


def split_config_settings(config_settings: str) -> list[str]:
def split_config_settings(config_settings: str, *, plural: bool) -> list[str]:
Copy link
Contributor

Choose a reason for hiding this comment

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

the 'plural' argument here is a little odd... Do you think it would be better to pass in the build backend instead (build or pip) and use that to decide the option name?

Copy link
Contributor Author

@henryiii henryiii Mar 10, 2023 •
edited

Choose a reason for hiding this comment

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

I started that way, but then went this way as I thought it was a little nicer than passing a string around. But I don't have a strong preference. If a third backend was added someday, I guess mypy would force handling the extra literal. I'd hope this goes away in the future when we can rely on the (not yet released) next version of pip, which supports -C, like build.

Edit: I think either way is fine, so I changed it.

config_settings_list = shlex.split(config_settings)
return [f"--config-setting={setting}" for setting in config_settings_list]
s = "s" if plural else ""
return [f"--config-setting{s}={setting}" for setting in config_settings_list]


def read_python_configs(config: PlatformName) -> list[dict[str, str]]:
Expand Down
4 changes: 3 additions & 1 deletion cibuildwheel/windows.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,9 @@ def build(options: Options, tmp_path: Path) -> None:
built_wheel_dir.mkdir()

verbosity_flags = get_build_verbosity_extra_flags(build_options.build_verbosity)
extra_flags = split_config_settings(build_options.config_settings)
extra_flags = split_config_settings(
build_options.config_settings, plural=build_options.build_frontend == "pip"
)

if build_options.build_frontend == "pip":
extra_flags += verbosity_flags
Expand Down
8 changes: 7 additions & 1 deletion unit_test/main_tests/main_options_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,12 +283,18 @@ def test_config_settings(platform_specific, platform, intercepted_build_args, mo

assert build_options.config_settings == config_settings

assert split_config_settings(config_settings) == [
assert split_config_settings(config_settings, plural=False) == [
"--config-setting=setting=value",
"--config-setting=setting=value2",
"--config-setting=other=something else",
]

assert split_config_settings(config_settings, plural=True) == [
"--config-settings=setting=value",
"--config-settings=setting=value2",
"--config-settings=other=something else",
]


@pytest.mark.parametrize(
"selector",
Expand Down