fix: support negative envvar values correctly (#907)

Noticed in rapidfuzz/RapidFuzz#397.

Signed-off-by: Henry Schreiner <[email protected]>

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.

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)

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])