Fix saving/restoring PackageOptions to a file. (Chris Burghart)

4 files changed, 25 insertions, 15 deletions

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index 9baeba3..a4e4c60 100644
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -25,6 +25,10 @@ RELEASE 0.95 - XXX
 
   - Fix a problem with the msvc tool with Python versions prior to 2.3.
 
+  From Chris Burghart:
+
+  - Fix the ability to save/restore a PackageOption to a file.
+
   From David M. Cooke:
 
   - Make the Fortran scanner case-insensitive for the INCLUDE string.
diff --git a/src/engine/SCons/Options/PackageOption.py b/src/engine/SCons/Options/PackageOption.py
index 5940974..9ecb42a 100644
--- a/src/engine/SCons/Options/PackageOption.py
+++ b/src/engine/SCons/Options/PackageOption.py
@@ -1,4 +1,4 @@
-"""engine.SCons.Options
+"""engine.SCons.Options.PackageOption
 
 This file defines the option type for SCons implementing 'package
 activation'.
@@ -59,8 +59,8 @@ import string
 from BoolOption import True, False
 import SCons.Errors
 
-__enable_strings  = ('yes', 'true',  'on', 'enable', 'search')
-__disable_strings = ('no',  'false', 'off', 'disable')
+__enable_strings  = (str(True), 'yes', 'true',  'on', 'enable', 'search')
+__disable_strings = (str(False), 'no',  'false', 'off', 'disable')
 
 def _converter(val):
     """
diff --git a/src/engine/SCons/Options/PackageOptionTests.py b/src/engine/SCons/Options/PackageOptionTests.py
index d9fbe7b..7c868e5 100644
--- a/src/engine/SCons/Options/PackageOptionTests.py
+++ b/src/engine/SCons/Options/PackageOptionTests.py
@@ -28,6 +28,7 @@ import unittest
 
 import SCons.Errors
 import SCons.Options
+from SCons.Options.BoolOption import True, False
 
 import TestCmd
 
@@ -76,6 +77,15 @@ class PackageOptionTestCase(unittest.TestCase):
         x = o.converter('/explicit/path')
         assert x == '/explicit/path', x
 
+        # Make sure the converter returns True if we give it str(True) and
+        # False when we give it str(False).  This assures consistent operation
+        # through a cycle of Options.Save(<file>) -> Options(<file>).
+        x = o.converter(str(True))
+        assert x == True, "converter returned a string when given str(True)"
+
+        x = o.converter(str(False))
+        assert x == False, "converter returned a string when given str(False)"
+
     def test_validator(self):
         """Test the PackageOption validator"""
         opts = SCons.Options.Options()
diff --git a/test/OptionsTypes.py b/test/OptionsTypes.py
index 43cbfbe..55cf035 100644
--- a/test/OptionsTypes.py
+++ b/test/OptionsTypes.py
@@ -224,14 +224,9 @@ Default(env.Alias('dummy', None))
 test.run()
 check(['1'])
 test.run(arguments='x11=no'); check(['0'])
+test.run(arguments='x11=0'); check(['0'])
 test.run(arguments='"x11=%s"' % test.workpath()); check([test.workpath()])
 
-test.run(arguments='x11=0',
-         stderr = """
-scons: *** Path does not exist for option x11: 0
-File "SConstruct", line 11, in ?
-""", status=2)
-
 test.run(arguments='x11=/non/existing/path/',
          stderr = """
 scons: *** Path does not exist for option x11: /non/existing/path/
@@ -249,12 +244,12 @@ libpath = os.path.join(workpath, 'lib')
 test.write('SConstruct', """
 from SCons.Options import PathOption
 
-qtdir = '%s'
+qtdir = r'%s'
 
 opts = Options(args=ARGUMENTS)
 opts.AddOptions(
     PathOption('qtdir', 'where the root of Qt is installed', qtdir),
-    PathOption('qt_libraries', 'where the Qt library is installed', '%s'),
+    PathOption('qt_libraries', 'where the Qt library is installed', r'%s'),
     )
 
 env = Environment(options=opts)
@@ -313,7 +308,7 @@ from SCons.Options import BoolOption, EnumOption, ListOption, \
    PackageOption, PathOption
 
 list_of_libs = Split('x11 gl qt ical')
-qtdir = '%(qtdir)s'
+qtdir = r'%(qtdir)s'
 
 opts = Options(args=ARGUMENTS)
 opts.AddOptions(
@@ -337,7 +332,7 @@ opts.AddOptions(
                   'yes'), PathOption('qtdir', 'where the root of Qt is installed', qtdir),
     PathOption('qt_libraries',
                'where the Qt library is installed',
-               '%(libdirvar)s'),
+               r'%(libdirvar)s'),
     )
 
 env = Environment(options=opts)
@@ -390,11 +385,12 @@ qtdir: where the root of Qt is installed ( /path/to/qtdir )
     actual: %(qtdir)s
 
 qt_libraries: where the Qt library is installed ( /path/to/qt_libraries )
-    default: $qtdir/lib
+    default: %(qtdir_lib)s
     actual: %(libdir)s
 
 Use scons -H for help about command-line options.
-""" % {'qtdir': qtpath, 'libdirvar': libdirvar, 'libdir': libpath})
+""" % {'qtdir': qtpath, 'qtdir_lib' : os.path.join('$qtdir', 'lib'),
+       'libdirvar': libdirvar, 'libdir': libpath})
 
 
 test.pass_test()