win ninja: Refactor manifest generate and embed to be 1-pass

pylib/gyp/msvs_emulation.py

Index: pylib/gyp/msvs_emulation.py
diff --git a/pylib/gyp/msvs_emulation.py b/pylib/gyp/msvs_emulation.py
index 723201eb6a1ea0071672059c828cfe01aa312b4d..6a7d67ed23e42ec79b031363dcfb4e9210679b46 100644
--- a/pylib/gyp/msvs_emulation.py
+++ b/pylib/gyp/msvs_emulation.py
@@ -454,7 +454,7 @@ class MsvsSettings(object):
     return output_file
 
   def GetLdflags(self, config, gyp_to_build_path, expand_special,
-                 manifest_base_name, is_executable):
+                 manifest_base_name, is_executable, build_dir, open_output):
     """Returns the flags that need to be added to link commands, and the
     manifest files."""
     config = self._TargetConfig(config)
@@ -531,17 +531,22 @@ class MsvsSettings(object):
       ldflags.append('/NXCOMPAT')
 
     have_def_file = filter(lambda x: x.startswith('/DEF:'), ldflags)
-    manifest_flags, manifest_files = self._GetLdManifestFlags(
-        config, manifest_base_name, gyp_to_build_path,
-        is_executable and not have_def_file)
+    manifest_flags, intermediate_manifest, manifest_files = \
+        self._GetLdManifestFlags(config, manifest_base_name, gyp_to_build_path,
+                                 is_executable and not have_def_file, build_dir,
+                                 open_output)
     ldflags.extend(manifest_flags)
-    return ldflags, manifest_files
+    return ldflags, intermediate_manifest, manifest_files
 
   def _GetLdManifestFlags(self, config, name, gyp_to_build_path,
-                          allow_isolation):
-    """Returns the set of flags that need to be added to the link to generate
-    a default manifest, as well as the list of all the manifest files to be
-    merged by the manifest tool."""
+                          allow_isolation, build_dir, open_output):
+    """Returns a 3-tuple:
+    - the set of flags that need to be added to the link to generate
+      a default manifest
+    - the intermediate manifest that the linker will generate that should be
+      used to assert it doesn't add anything to the merged one.
+    - the list of all the manifest files to be merged by the manifest tool and
+      included into the link."""
     generate_manifest = self._Setting(('VCLinkerTool', 'GenerateManifest'),
                                       config,
                                       default='true')
@@ -549,7 +554,7 @@ class MsvsSettings(object):
       # This means not only that the linker should not generate the intermediate
       # manifest but also that the manifest tool should do nothing even when
       # additional manifests are specified.
-      return ['/MANIFEST:NO'], []
+      return ['/MANIFEST:NO'], [], []
 
     output_name = name + '.intermediate.manifest'
     flags = [
@@ -557,9 +562,21 @@ class MsvsSettings(object):
       '/ManifestFile:' + output_name,
     ]
 
+    # Instead of using the MANIFESTUAC flags, we generate a .manifest to
+    # include into the list of manifests. This allows us to avoid the need to
+    # do two passing during linking. The /MANIFEST flag and /ManifestFile are
+    # still used, and the intermediate manifest is used to assert that the
+    # final manifest we get from merging all the additional manifest files
+    # (plus the one we generate here) isn't modified by merging the
+    # intermediate into it.
+
+    # Always NO, because we generate a manifest file that has what we want.
+    flags.append('/MANIFESTUAC:NO')
+
     config = self._TargetConfig(config)
     enable_uac = self._Setting(('VCLinkerTool', 'EnableUAC'), config,
                                default='true')
+    manifest_files = []
     if enable_uac == 'true':
       execution_level = self._Setting(('VCLinkerTool', 'UACExecutionLevel'),
                                       config, default='0')
@@ -571,18 +588,32 @@ class MsvsSettings(object):
 
       ui_access = self._Setting(('VCLinkerTool', 'UACUIAccess'), config,
                                 default='false')
-      flags.append('''/MANIFESTUAC:"level='%s' uiAccess='%s'"''' %
-          (execution_level_map[execution_level], ui_access))
-    else:
-      flags.append('/MANIFESTUAC:NO')
+
+      generated_manifest_contents = '''
+<?xml version='1.0' encoding='UTF-8' standalone='yes'?>
+<assembly xmlns='urn:schemas-microsoft-com:asm.v1' manifestVersion='1.0'>
+  <trustInfo xmlns="urn:schemas-microsoft-com:asm.v3">
+    <security>
+      <requestedPrivileges>
+        <requestedExecutionLevel level='%s' uiAccess='%s' />
+      </requestedPrivileges>
+    </security>
+  </trustInfo>
+</assembly>''' % (execution_level_map[execution_level], ui_access)
+      generated_name = name + '.generated.manifest'
+      # Need to join with the build_dir here as we're writing it during
+      # generation time, but we return the un-joined version because the build
+      # will occur in that directory.
+      with open_output(os.path.join(build_dir, generated_name), 'w') as f:

[yukawa, 2013/12/10 11:18:30]

If the time stamp of |os.path.join(build_dir, generated_name)| is unnecessary
updated here, does Ninja think that all the exe/dll targets are out-of-date?

Does this make sense if we leave the original |os.path.join(build_dir,
generated_name)| as it is when it already exists and its content is exactly the
same to |generated_manifest_contents|?

[scottmg, 2013/12/10 19:27:21]

Good question! I believe it will not cause unnecessary rebuilds because
.generated.manifest is not mentioned as a ninja input. I added a test for this
to gyptest-link-generate-manifest.py.

However, I think this also points to a bug, because now if e.g. the 'EnableUAC'
setting is modified it will not be enough to trigger a relink. I will address
this and add a test for it.
Once the above is fixed, I will need to do this also.

+        f.write(generated_manifest_contents)
+      manifest_files = [generated_name]
 
     if allow_isolation:
       flags.append('/ALLOWISOLATION')
 
-    manifest_files = [output_name]
     manifest_files += self._GetAdditionalManifestFiles(config,
                                                        gyp_to_build_path)
-    return flags, manifest_files
+    return flags, output_name, manifest_files
 
   def _GetAdditionalManifestFiles(self, config, gyp_to_build_path):
     """Gets additional manifest files that are added to the default one