Correct the order in which OutputDirectory and IntermediateDirectory are defined.

pylib/gyp/generator/msvs.py

 # Copyright (c) 2012 Google Inc. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 import copy
 import ntpath
 import os
 import posixpath
 import re
 import subprocess
@@ skipping 13 matching lines @@
 # contains lowercase hex letters, MSVS will be fine. However,
 # IncrediBuild BuildConsole will parse the solution file, but then
 # silently skip building the target causing hard to track down errors.
 # Note that this only happens with the BuildConsole, and does not occur
 # if IncrediBuild is executed from inside Visual Studio.  This regex
 # validates that the string looks like a GUID with all uppercase hex
 # letters.
 VALID_MSVS_GUID_CHARS = re.compile('^[A-F0-9\-]+$')
 
 
+# Regex for msvs variable references.
+MSVS_VARIABLE_REFERENCE = re.compile('\$\(([a-zA-Z_][a-zA-Z0-9_]*)\)')
+
+
 generator_default_variables = {
     'EXECUTABLE_PREFIX': '',
     'EXECUTABLE_SUFFIX': '.exe',
     'STATIC_LIB_PREFIX': '',
     'SHARED_LIB_PREFIX': '',
     'STATIC_LIB_SUFFIX': '.lib',
     'SHARED_LIB_SUFFIX': '.dll',
     'INTERMEDIATE_DIR': '$(IntDir)',
     'SHARED_INTERMEDIATE_DIR': '$(OutDir)/obj/global_intermediate',
     'OS': 'win',
@@ skipping 2598 matching lines @@
     spec: The target project dict.
     label: An optional label for the PropertyGroup.
     properties: The dictionary to be converted.  The key is the name of the
         property.  The value is itself a dictionary; its key is the value and
         the value a list of condition for which this value is true.
   """
   group = ['PropertyGroup']
   if label:
     group.append({'Label': label})
   num_configurations = len(spec['configurations'])
-  for name, values in sorted(properties.iteritems()):
+  def GetEdges(node):
+    edges = set()
+    for value in sorted(properties[node].keys()):
+      edges.update(set([v for v in MSVS_VARIABLE_REFERENCE.findall(value)
+                        if v in properties and v != node]))

[gab, 2012/05/10 13:57:07]

It took me a little while to figure out how this works, i.e. that the edges of a
node (i.e. a property) are defined to be the properties used as variables in the
value of the given property.

This is really neat, but I think a comment would help :)!

I would also consider moving the MSVS_VARIABLE_REFERENCE regex closer to the
body of this function given it's only used locally and I had to look up and down
the file to understand what it does. I don't know much about python style though
so it's your call.

Also, is it even possible that "v == node"?! That would mean that a variable is
defined with a reference to itself... shouldn't this throw an exception rather
than simply not put an edge, it would be a nice to way to detect
self-references:
e.g. myvar = hello$(myvar)
is wrong imo

[bradn, 2012/05/10 20:17:56]

Done
I see nothing in the style guide to preclude moving it closer at module scope,
so I've done that. Putting it in the function would cause it to be regenerated
each time, so I left it outside the function.
I've added a comment.
There are some cases where variables are defined in terms of themselves as a way
to "append", things like PATH=$(PATH);more_paths

+    return edges
+  properties_ordered = gyp.common.TopologicallySorted(
+      properties.keys(), GetEdges)
+  properties_ordered.reverse()

[Nico, 2012/05/10 19:20:23]

Here too you're reversing twice

[bradn, 2012/05/10 20:17:56]

So I've moved to normal topological order everywhere called Topological (just
for sanity). But I believe I've eliminated all the double reversing.

+  for name in properties_ordered:
+    values = properties[name]
     for value, conditions in sorted(values.iteritems()):
       if len(conditions) == num_configurations:
         # If the value is the same all configurations,
         # just add one unconditional entry.
         group.append([name, value])
       else:
         for condition in conditions:
           group.append([name, {'Condition': condition}, value])
   return [group]
 
@@ skipping 401 matching lines @@
   action_spec.extend(
       # TODO(jeanluc) 'Document' for all or just if as_sources?
       [['FileType', 'Document'],
        ['Command', command],
        ['Message', description],
        ['Outputs', outputs]
       ])
   if additional_inputs:
     action_spec.append(['AdditionalInputs', additional_inputs])
   actions_spec.append(action_spec)