From 67badd93c886bcd0d6852e1349ddc26f5be1c3ff Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Fri, 20 Sep 2024 15:44:37 -0400 Subject: [PATCH] fix: support negative envvar values correctly Signed-off-by: Henry Schreiner --- docs/overrides.md | 5 ++-- .../settings/skbuild_overrides.py | 12 ++++---- tests/test_settings_overrides.py | 30 +++++++++++++++++++ 3 files changed, 40 insertions(+), 7 deletions(-) diff --git a/docs/overrides.md b/docs/overrides.md index a313ffac..cafc6617 100644 --- a/docs/overrides.md +++ b/docs/overrides.md @@ -11,8 +11,9 @@ you can set any value `tool.scikit-build` supports, and it will override if the There are three types of conditions. Booleans, strings, and version numbers. Booleans take a bool; if the boolean matches the bool you give, the override matches. If the value is a string (such as an environment variable), it will -match truth-like values. Strings take a regex which will try to match. Version -numbers take a specifier set, like `>=1.0`. +match non false-like values, and if the variable is unset or empty, that counts +as false. Strings take a regex which will try to match. Version numbers take +a specifier set, like `>=1.0`. If multiple conditions are given, they all must be true. Use `if.any` (below) if you would rather matching on any one of multiple conditions being true. diff --git a/src/scikit_build_core/settings/skbuild_overrides.py b/src/scikit_build_core/settings/skbuild_overrides.py index 53152108..2147aa4e 100644 --- a/src/scikit_build_core/settings/skbuild_overrides.py +++ b/src/scikit_build_core/settings/skbuild_overrides.py @@ -35,10 +35,12 @@ def strtobool(value: str) -> bool: """ Converts a environment variable string into a boolean value. """ + if not value: + return False value = value.lower() if value.isdigit(): return bool(int(value)) - return value in {"y", "yes", "on", "true", "t"} + return value not in {"n", "no", "off", "false", "f"} def version_match(value: str, match: str, name: str) -> str: @@ -216,13 +218,13 @@ def override_match( if env: for key, value in env.items(): - if key not in current_env: - failed_set.add(f"env.{key}") - elif isinstance(value, bool): - if strtobool(current_env[key]) == value: + if isinstance(value, bool): + if strtobool(current_env.get(key, "")) == value: passed_dict[f"env.{key}"] = f"env {key} is {value}" else: failed_set.add(f"env.{key}") + elif key not in current_env: + failed_set.add(f"env.{key}") else: current_value = current_env[key] match_msg = regex_match(current_value, value) diff --git a/tests/test_settings_overrides.py b/tests/test_settings_overrides.py index 39f484a0..9b824814 100644 --- a/tests/test_settings_overrides.py +++ b/tests/test_settings_overrides.py @@ -414,6 +414,36 @@ def test_skbuild_env_bool( assert not settings.sdist.cmake +@pytest.mark.parametrize("envvar", ["random", "FalSE", "", "0", None]) +def test_skbuild_env_negative_bool( + envvar: str | None, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +): + if envvar is None: + monkeypatch.delenv("FOO", raising=False) + else: + monkeypatch.setenv("FOO", envvar) + + pyproject_toml = tmp_path / "pyproject.toml" + pyproject_toml.write_text( + dedent( + """\ + [[tool.scikit-build.overrides]] + if.env.FOO = false + sdist.cmake = true + """ + ), + encoding="utf-8", + ) + + settings_reader = SettingsReader.from_file(pyproject_toml) + settings = settings_reader.settings + + if envvar in {"random"}: + assert not settings.sdist.cmake + else: + assert settings.sdist.cmake + + @pytest.mark.parametrize("foo", ["true", "false"]) @pytest.mark.parametrize("bar", ["true", "false"]) @pytest.mark.parametrize("any", [True, False])