Fix issue #2091 by adding new delete_existing flags to Util.{Ap,Pre}pendPath and env.{Ap,Pre}pendENVPath.

5 files changed, 216 insertions, 35 deletions

diff --git a/doc/man/scons.1 b/doc/man/scons.1
index eaf18e6..1d5142a 100644
--- a/doc/man/scons.1
+++ b/doc/man/scons.1
@@ -2682,7 +2682,7 @@ env.Append(CCFLAGS = ' -g', FOO = ['foo.yyy'])
 
 '\"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
 .TP
-.RI env.AppendENVPath( name ", " newpath ", [" envname ", " sep ])
+.RI env.AppendENVPath( name ", " newpath ", [" envname ", " sep ", " preserve_old_paths ])
 This appends new path elements to the given path in the
 specified external environment
 .RB ( ENV
@@ -2699,6 +2699,11 @@ This can also handle the
 case where the given old path variable is a list instead of a
 string, in which case a list will be returned instead of a string.
 
+If 
+.I preserve_old_paths
+is 1, then adding a path that already exists
+will not move it to the end; it will stay where it is in the list.
+
 Example:
 
 .ES
@@ -4321,11 +4326,50 @@ exists in a repository.
 Returns a list of the target Node or Nodes.
 
 '\"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
+\" .TP
+\" .RI env.MergeShellPaths( arg ", [" prepend ])
+\" Merges the elements of the specified
+\" .IR arg ,
+\" which must be a dictionary, to the construction
+\" environment's copy of the shell environment
+\" in env['ENV'].
+\" (This is the environment which is passed
+\" to subshells spawned by SCons.)
+\" Note that
+\" .I arg
+\" must be a single value,
+\" so multiple strings must
+\" be passed in as a list,
+\" not as separate arguments to
+\" .BR env.MergeShellPaths ().
+
+\" New values are prepended to the environment variable by default,
+\" unless prepend=0 is specified.  
+\" Duplicate values are always eliminated, 
+\" since this function calls
+\" .B AppendENVPath
+\" or
+\" .B PrependENVPath
+\" depending on the
+\" .I prepend
+\" argument.  See those functions for more details.
+
+\" Examples:
+
+\" .ES
+\" # Prepend a path to the shell PATH.
+\" env.MergeShellPaths({'PATH':'/usr/local/bin'} )
+\" # Append two dirs to the shell INCLUDE.
+\" env.MergeShellPaths({'INCLUDE':['c:/inc1', 'c:/inc2']}, prepend=0 )
+
+.EE
+
+'\"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
 .TP
 .RI env.MergeFlags( arg ", [" unique ])
 Merges the specified
 .I arg
-values to the construction envrionment's construction variables.
+values to the construction environment's construction variables.
 If the
 .I arg
 argument is not a dictionary,
@@ -4842,7 +4886,7 @@ env.Prepend(CCFLAGS = '-g ', FOO = ['foo.yyy'])
 
 '\"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
 .TP
-.RI env.PrependENVPath( name ", " newpath ", [" envname ", " sep ])
+.RI env.PrependENVPath( name ", " newpath ", [" envname ", " sep ", " preserve_old_paths ])
 This appends new path elements to the given path in the
 specified external environment
 .RB ( ENV
@@ -4859,6 +4903,12 @@ This can also handle the
 case where the given old path variable is a list instead of a
 string, in which case a list will be returned instead of a string.
 
+If
+.I preserve_old_paths
+is 1, then adding a path that already exists
+will not move it to the beginning;
+it will stay where it is in the list.
+
 Example:
 
 .ES
diff --git a/src/engine/SCons/Environment.py b/src/engine/SCons/Environment.py
index 40c92c7..b16a1ef 100644
--- a/src/engine/SCons/Environment.py
+++ b/src/engine/SCons/Environment.py
@@ -747,13 +747,16 @@ class SubstitutionEnvironment:
             do_parse(arg, do_parse)
         return dict
 
-    def MergeFlags(self, args, unique=1):
+    def MergeFlags(self, args, unique=1, dict=None):
         """
-        Merge the dict in args into the construction variables.  If args
-        is not a dict, it is converted into a dict using ParseFlags.
-        If unique is not set, the flags are appended rather than merged.
+        Merge the dict in args into the construction variables of this
+        env, or the passed-in dict.  If args is not a dict, it is
+        converted into a dict using ParseFlags.  If unique is not set,
+        the flags are appended rather than merged.
         """
 
+        if dict is None:
+            dict = self
         if not SCons.Util.is_Dict(args):
             args = self.ParseFlags(args)
         if not unique:
@@ -801,6 +804,26 @@ class SubstitutionEnvironment:
             self[key] = t
         return self
 
+#     def MergeShellPaths(self, args, prepend=1):
+#         """
+#         Merge the dict in args into the shell environment in env['ENV'].  
+#         Shell path elements are appended or prepended according to prepend.
+
+#         Uses Pre/AppendENVPath, so it always appends or prepends uniquely.
+
+#         Example: env.MergeShellPaths({'LIBPATH': '/usr/local/lib'})
+#         prepends /usr/local/lib to env['ENV']['LIBPATH'].
+#         """
+
+#         for pathname, pathval in args.items():
+#             if not pathval:
+#                 continue
+#             if prepend:
+#                 apply(self.PrependENVPath, (pathname, pathval))
+#             else:
+#                 apply(self.AppendENVPath, (pathname, pathval))
+
+
 # Used by the FindSourceFiles() method, below.
 # Stuck here for support of pre-2.2 Python versions.
 def build_source(ss, result):
@@ -1139,19 +1162,23 @@ class Base(SubstitutionEnvironment):
                                 orig[val] = None
         self.scanner_map_delete(kw)
 
-    def AppendENVPath(self, name, newpath, envname = 'ENV', sep = os.pathsep):
+    def AppendENVPath(self, name, newpath, envname = 'ENV', 
+                      sep = os.pathsep, delete_existing=1):
         """Append path elements to the path 'name' in the 'ENV'
         dictionary for this environment.  Will only add any particular
         path once, and will normpath and normcase all paths to help
         assure this.  This can also handle the case where the env
         variable is a list instead of a string.
+
+        If delete_existing is 0, a newpath which is already in the path
+        will not be moved to the end (it will be left where it is).
         """
 
         orig = ''
         if self._dict.has_key(envname) and self._dict[envname].has_key(name):
             orig = self._dict[envname][name]
 
-        nv = SCons.Util.AppendPath(orig, newpath, sep)
+        nv = SCons.Util.AppendPath(orig, newpath, sep, delete_existing)
 
         if not self._dict.has_key(envname):
             self._dict[envname] = {}
@@ -1477,19 +1504,23 @@ class Base(SubstitutionEnvironment):
                                 orig[val] = None
         self.scanner_map_delete(kw)
 
-    def PrependENVPath(self, name, newpath, envname = 'ENV', sep = os.pathsep):
+    def PrependENVPath(self, name, newpath, envname = 'ENV', sep = os.pathsep,
+                       delete_existing=1):
         """Prepend path elements to the path 'name' in the 'ENV'
         dictionary for this environment.  Will only add any particular
         path once, and will normpath and normcase all paths to help
         assure this.  This can also handle the case where the env
         variable is a list instead of a string.
+
+        If delete_existing is 0, a newpath which is already in the path
+        will not be moved to the front (it will be left where it is).
         """
 
         orig = ''
         if self._dict.has_key(envname) and self._dict[envname].has_key(name):
             orig = self._dict[envname][name]
 
-        nv = SCons.Util.PrependPath(orig, newpath, sep)
+        nv = SCons.Util.PrependPath(orig, newpath, sep, delete_existing)
 
         if not self._dict.has_key(envname):
             self._dict[envname] = {}
diff --git a/src/engine/SCons/EnvironmentTests.py b/src/engine/SCons/EnvironmentTests.py
index ecc7252..fe9ddc8 100644
--- a/src/engine/SCons/EnvironmentTests.py
+++ b/src/engine/SCons/EnvironmentTests.py
@@ -840,6 +840,30 @@ sys.exit(0)
         assert env['A'] == ['aaa'], env['A']
         assert env['B'] == ['bbb'], env['B']
 
+#     def test_MergeShellPaths(self):
+#         """Test the MergeShellPaths() method
+#         """
+#         env = Environment()
+#         env.MergeShellPaths({})
+#         assert not env['ENV'].has_key('INCLUDE'), env['INCLUDE']
+#         env.MergeShellPaths({'INCLUDE': r'c:\Program Files\Stuff'})
+#         assert env['ENV']['INCLUDE'] == r'c:\Program Files\Stuff', env['ENV']['INCLUDE']
+#         env.MergeShellPaths({'INCLUDE': r'c:\Program Files\Stuff'})
+#         assert env['ENV']['INCLUDE'] == r'c:\Program Files\Stuff', env['ENV']['INCLUDE']
+#         env.MergeShellPaths({'INCLUDE': r'xyz'})
+#         assert env['ENV']['INCLUDE'] == r'xyz%sc:\Program Files\Stuff'%os.pathsep, env['ENV']['INCLUDE']
+
+#         env = Environment()
+#         env['ENV']['INCLUDE'] = 'xyz'
+#         env.MergeShellPaths({'INCLUDE':['c:/inc1', 'c:/inc2']} )
+#         assert env['ENV']['INCLUDE'] == r'c:/inc1%sc:/inc2%sxyz'%(os.pathsep, os.pathsep), env['ENV']['INCLUDE']
+
+#         # test prepend=0
+#         env = Environment()
+#         env.MergeShellPaths({'INCLUDE': r'c:\Program Files\Stuff'}, prepend=0)
+#         assert env['ENV']['INCLUDE'] == r'c:\Program Files\Stuff', env['ENV']['INCLUDE']
+#         env.MergeShellPaths({'INCLUDE': r'xyz'}, prepend=0)
+#         assert env['ENV']['INCLUDE'] == r'c:\Program Files\Stuff%sxyz'%os.pathsep, env['ENV']['INCLUDE']
 
 
 class BaseTestCase(unittest.TestCase,TestEnvironmentFixture):
@@ -1531,6 +1555,8 @@ def exists(env):
         env1.AppendENVPath('PATH',r'C:\dir\num\three', sep = ';')
         env1.AppendENVPath('MYPATH',r'C:\mydir\num\three','MYENV', sep = ';')
         env1.AppendENVPath('MYPATH',r'C:\mydir\num\one','MYENV', sep = ';')
+        # this should do nothing since delete_existing is 0
+        env1.AppendENVPath('MYPATH',r'C:\mydir\num\three','MYENV', sep = ';', delete_existing=0)
         assert(env1['ENV']['PATH'] == r'C:\dir\num\one;C:\dir\num\two;C:\dir\num\three')
         assert(env1['MYENV']['MYPATH'] == r'C:\mydir\num\two;C:\mydir\num\three;C:\mydir\num\one')
 
@@ -2165,6 +2191,8 @@ f5: \
         env1.PrependENVPath('PATH',r'C:\dir\num\three',sep = ';')
         env1.PrependENVPath('MYPATH',r'C:\mydir\num\three','MYENV',sep = ';')
         env1.PrependENVPath('MYPATH',r'C:\mydir\num\one','MYENV',sep = ';')
+        # this should do nothing since delete_existing is 0
+        env1.PrependENVPath('MYPATH',r'C:\mydir\num\three','MYENV', sep = ';', delete_existing=0)
         assert(env1['ENV']['PATH'] == r'C:\dir\num\three;C:\dir\num\two;C:\dir\num\one')
         assert(env1['MYENV']['MYPATH'] == r'C:\mydir\num\one;C:\mydir\num\three;C:\mydir\num\two')
 
diff --git a/src/engine/SCons/Util.py b/src/engine/SCons/Util.py
index 44f615b..3fdc14c 100644
--- a/src/engine/SCons/Util.py
+++ b/src/engine/SCons/Util.py
@@ -844,7 +844,7 @@ else:
                     continue
         return None
 
-def PrependPath(oldpath, newpath, sep = os.pathsep):
+def PrependPath(oldpath, newpath, sep = os.pathsep, delete_existing=1):
     """This prepends newpath elements to the given oldpath.  Will only
     add any particular path once (leaving the first one it encounters
     and ignoring the rest, to preserve path order), and will
@@ -857,6 +857,10 @@ def PrependPath(oldpath, newpath, sep = os.pathsep):
       Old Path: "/foo/bar:/foo"
       New Path: "/biz/boom:/foo"
       Result:   "/biz/boom:/foo:/foo/bar"
+
+    If delete_existing is 0, then adding a path that exists will
+    not move it to the beginning; it will stay where it is in the
+    list.
     """
 
     orig = oldpath
@@ -871,23 +875,49 @@ def PrependPath(oldpath, newpath, sep = os.pathsep):
     else:
         newpaths = string.split(newpath, sep)
 
-    newpaths = newpaths + paths # prepend new paths
+    if not delete_existing:
+        # First uniquify the old paths, making sure to 
+        # preserve the first instance (in Unix/Linux,
+        # the first one wins), and remembering them in normpaths.
+        # Then insert the new paths at the head of the list
+        # if they're not already in the normpaths list.
+        result = []
+        normpaths = []
+        for path in paths:
+            if not path:
+                continue
+            normpath = os.path.normpath(os.path.normcase(path))
+            if normpath not in normpaths:
+                result.append(path)
+                normpaths.append(normpath)
+        newpaths.reverse()      # since we're inserting at the head
+        for path in newpaths:
+            if not path:
+                continue
+            normpath = os.path.normpath(os.path.normcase(path))
+            if normpath not in normpaths:
+                result.insert(0, path)
+                normpaths.append(normpath)
+        paths = result
 
-    normpaths = []
-    paths = []
-    # now we add them only if they are unique
-    for path in newpaths:
-        normpath = os.path.normpath(os.path.normcase(path))
-        if path and not normpath in normpaths:
-            paths.append(path)
-            normpaths.append(normpath)
+    else:
+        newpaths = newpaths + paths # prepend new paths
+
+        normpaths = []
+        paths = []
+        # now we add them only if they are unique
+        for path in newpaths:
+            normpath = os.path.normpath(os.path.normcase(path))
+            if path and not normpath in normpaths:
+                paths.append(path)
+                normpaths.append(normpath)
 
     if is_list:
         return paths
     else:
         return string.join(paths, sep)
 
-def AppendPath(oldpath, newpath, sep = os.pathsep):
+def AppendPath(oldpath, newpath, sep = os.pathsep, delete_existing=1):
     """This appends new path elements to the given old path.  Will
     only add any particular path once (leaving the last one it
     encounters and ignoring the rest, to preserve path order), and
@@ -900,6 +930,9 @@ def AppendPath(oldpath, newpath, sep = os.pathsep):
       Old Path: "/foo/bar:/foo"
       New Path: "/biz/boom:/foo"
       Result:   "/foo/bar:/biz/boom:/foo"
+
+    If delete_existing is 0, then adding a path that exists
+    will not move it to the end; it will stay where it is in the list.
     """
 
     orig = oldpath
@@ -914,19 +947,42 @@ def AppendPath(oldpath, newpath, sep = os.pathsep):
     else:
         newpaths = string.split(newpath, sep)
 
-    newpaths = paths + newpaths # append new paths
-    newpaths.reverse()
-
-    normpaths = []
-    paths = []
-    # now we add them only of they are unique
-    for path in newpaths:
-        normpath = os.path.normpath(os.path.normcase(path))
-        if path and not normpath in normpaths:
-            paths.append(path)
-            normpaths.append(normpath)
-
-    paths.reverse()
+    if not delete_existing:
+        # add old paths to result, then
+        # add new paths if not already present
+        # (I thought about using a dict for normpaths for speed,
+        # but it's not clear hashing the strings would be faster
+        # than linear searching these typically short lists.)
+        result = []
+        normpaths = []
+        for path in paths:
+            if not path:
+                continue
+            result.append(path)
+            normpaths.append(os.path.normpath(os.path.normcase(path)))
+        for path in newpaths:
+            if not path:
+                continue
+            normpath = os.path.normpath(os.path.normcase(path))
+            if normpath not in normpaths:
+                result.append(path)
+                normpaths.append(normpath)
+        paths = result
+    else:
+        # start w/ new paths, add old ones if not present,
+        # then reverse.
+        newpaths = paths + newpaths # append new paths
+        newpaths.reverse()
+
+        normpaths = []
+        paths = []
+        # now we add them only if they are unique
+        for path in newpaths:
+            normpath = os.path.normpath(os.path.normcase(path))
+            if path and not normpath in normpaths:
+                paths.append(path)
+                normpaths.append(normpath)
+        paths.reverse()
 
     if is_list:
         return paths
diff --git a/src/engine/SCons/UtilTests.py b/src/engine/SCons/UtilTests.py
index 8a24ef1..a27f8c3 100644
--- a/src/engine/SCons/UtilTests.py
+++ b/src/engine/SCons/UtilTests.py
@@ -478,6 +478,22 @@ class UtilTestCase(unittest.TestCase):
         assert(p1 == r'C:\dir\num\one;C:\dir\num\two;C:\dir\num\three')
         assert(p2 == r'C:\mydir\num\two;C:\mydir\num\three;C:\mydir\num\one')
 
+    def test_PrependPathPreserveOld(self):
+        """Test prepending to a path while preserving old paths"""
+        p1 = r'C:\dir\num\one;C:\dir\num\two'
+        # have to include the pathsep here so that the test will work on UNIX too.
+        p1 = PrependPath(p1,r'C:\dir\num\two',sep = ';', delete_existing=0)
+        p1 = PrependPath(p1,r'C:\dir\num\three',sep = ';')
+        assert(p1 == r'C:\dir\num\three;C:\dir\num\one;C:\dir\num\two')
+
+    def test_AppendPathPreserveOld(self):
+        """Test appending to a path while preserving old paths"""
+        p1 = r'C:\dir\num\one;C:\dir\num\two'
+        # have to include the pathsep here so that the test will work on UNIX too.
+        p1 = AppendPath(p1,r'C:\dir\num\one',sep = ';', delete_existing=0)
+        p1 = AppendPath(p1,r'C:\dir\num\three',sep = ';')
+        assert(p1 == r'C:\dir\num\one;C:\dir\num\two;C:\dir\num\three')
+
     def test_NodeList(self):
         """Test NodeList class"""
         class TestClass: