Fix Variables.PackageVariable with bool default string

CHANGES.txt

@@ -16,6 +16,12 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
 
         - Whatever John Doe did.
 
+  From Bill Prendergast:
+    - Fixed SCons.Variables.PackageVariable to correctly test the default
+      setting against both enable & disable strings. (Fixes #4702)
+    - Extended unittests (crudely) to test for correct/expected response
+      when default setting is a boolean string.
+
 
 RELEASE 4.9.1 -  Thu, 27 Mar 2025 11:40:20 -0700
 

RELEASE.txt

@@ -32,7 +32,8 @@ CHANGED/ENHANCED EXISTING FUNCTIONALITY
 FIXES
 -----
 
-- List fixes of outright bugs
+- Fixed SCons.Variables.PackageVariable to correctly test the default
+  setting against both enable & disable strings. (Fixes #4702)
 
 IMPROVEMENTS
 ------------

SCons/Variables/PackageVariable.py

@@ -86,7 +86,7 @@ def _converter(val: str | bool, default: str) -> str | bool:
     if lval in ENABLE_STRINGS:
         # Can't return the default if it is one of the enable/disable strings;
         # test code expects a bool.
-        if default in ENABLE_STRINGS:
+        if default in ENABLE_STRINGS + DISABLE_STRINGS:
             return True
         else:
             return default

SCons/Variables/PackageVariableTests.py

@@ -92,6 +92,34 @@ def test_converter(self) -> None:
             x = o.converter(False)
             assert not x, f"converter returned False for {t!r}"
 
+        # When the variable is created with boolean string make sure the converter
+        # returns the correct result i.e. a bool or a passed path
+        opts = SCons.Variables.Variables()
+        opts.Add(SCons.Variables.PackageVariable('test', 'test build variable help', 'yes'))
+        o = opts.options[0]
+
+        x = o.converter(str(True))
+        assert not isinstance(x, str), "converter with default str(yes) returned a string when given str(True)"
+        assert x, "converter with default str(yes) returned False for str(True)"
+        x = o.converter(str(False))
+        assert not isinstance(x, str), "converter with default str(yes) returned a string when given str(False)"
+        assert not x, "converter with default str(yes) returned True for str(False)"
+        x = o.converter('/explicit/path')
+        assert x == '/explicit/path', "converter with default str(yes) did not return path"
+
+        opts = SCons.Variables.Variables()
+        opts.Add(SCons.Variables.PackageVariable('test', 'test build variable help', 'no'))
+        o = opts.options[0]
+
+        x = o.converter(str(True))
+        assert not isinstance(x, str), "converter with default str(no) returned a string when given str(True)"
+        assert x, "converter with default str(no) returned False for str(True)"
+        x = o.converter(str(False))
+        assert not isinstance(x, str), "converter with default str(no) returned a string when given str(False)"
+        assert not x, "converter with default str(no) returned True for str(False)"
+        x = o.converter('/explicit/path')
+        assert x == '/explicit/path', "converter with default str(no) did not return path"
+
     def test_validator(self) -> None:
         """Test the PackageVariable validator"""
         opts = SCons.Variables.Variables()