win ninja: Refactor manifest generate and embed to be 1-pass
The 'normal' way to do manifests is to have link generate a manifest
based on gathering dependencies from the object files, then merge that
manifest with other manifests supplied as sources, convert the merged
manifest to a resource, and then *relink*, including the compiled
version of the manifest resource. This breaks incremental linking, and
is generally overly complicated.
Instead, we merge all the manifests provided (along with one that
includes what would normally be in the linker-generated one, see
msvs_emulation.py), and include that into the first and only link.
We still tell link to generate a manifest, but we only use that to
assert that our simpler process did not miss anything.
R=thakis@chromium.org, yukawa@chromium.org
BUG=chromium:324863
Review URL: https://codereview.chromium.org/111373002
git-svn-id: http://gyp.googlecode.com/svn/trunk@1813 78cadc50-ecff-11dd-a971-7dbc132099af
diff --git a/pylib/gyp/common.py b/pylib/gyp/common.py
index b9d2abe..f9c6c6f 100644
--- a/pylib/gyp/common.py
+++ b/pylib/gyp/common.py
@@ -391,6 +391,14 @@
return Writer()
+def EnsureDirExists(path):
+ """Make sure the directory for |path| exists."""
+ try:
+ os.makedirs(os.path.dirname(path))
+ except OSError:
+ pass
+
+
def GetFlavor(params):
"""Returns |params.flavor| if it's set, the system's default flavor else."""
flavors = {
diff --git a/pylib/gyp/generator/ninja.py b/pylib/gyp/generator/ninja.py
index a40c7fe..d3db2c8 100644
--- a/pylib/gyp/generator/ninja.py
+++ b/pylib/gyp/generator/ninja.py
@@ -1039,13 +1039,18 @@
elif self.flavor == 'win':
manifest_name = self.GypPathToUniqueOutput(
self.ComputeOutputFileName(spec))
- ldflags, manifest_files = self.msvs_settings.GetLdflags(config_name,
- self.GypPathToNinja, self.ExpandSpecial, manifest_name, is_executable)
+ ldflags, intermediate_manifest, manifest_files = \
+ self.msvs_settings.GetLdflags(config_name, self.GypPathToNinja,
+ self.ExpandSpecial, manifest_name,
+ is_executable, self.toplevel_build)
ldflags = env_ldflags + ldflags
self.WriteVariableList(ninja_file, 'manifests', manifest_files)
+ implicit_deps = implicit_deps.union(manifest_files)
+ if intermediate_manifest:
+ self.WriteVariableList(
+ ninja_file, 'intermediatemanifest', [intermediate_manifest])
command_suffix = _GetWinLinkRuleNameSuffix(
- self.msvs_settings.IsEmbedManifest(config_name),
- self.msvs_settings.IsLinkIncremental(config_name))
+ self.msvs_settings.IsEmbedManifest(config_name))
def_file = self.msvs_settings.GetDefFile(self.GypPathToNinja)
if def_file:
implicit_deps.add(def_file)
@@ -1505,10 +1510,7 @@
def OpenOutput(path, mode='w'):
"""Open |path| for writing, creating directories if necessary."""
- try:
- os.makedirs(os.path.dirname(path))
- except OSError:
- pass
+ gyp.common.EnsureDirExists(path)
return open(path, mode)
@@ -1567,63 +1569,28 @@
return 1
-def _GetWinLinkRuleNameSuffix(embed_manifest, link_incremental):
+def _GetWinLinkRuleNameSuffix(embed_manifest):
"""Returns the suffix used to select an appropriate linking rule depending on
- whether the manifest embedding and/or incremental linking is enabled."""
- suffix = ''
- if embed_manifest:
- suffix += '_embed'
- if link_incremental:
- suffix += '_inc'
- return suffix
+ whether the manifest embedding is enabled."""
+ return '_embed' if embed_manifest else ''
-def _AddWinLinkRules(master_ninja, embed_manifest, link_incremental):
+def _AddWinLinkRules(master_ninja, embed_manifest):
"""Adds link rules for Windows platform to |master_ninja|."""
def FullLinkCommand(ldcmd, out, binary_type):
- """Returns a one-liner written for cmd.exe to handle multiphase linker
- operations including manifest file generation. The command will be
- structured as follows:
- cmd /c (linkcmd1 a b) && (linkcmd2 x y) && ... &&
- if not "$manifests"=="" ((manifestcmd1 a b) && (manifestcmd2 x y) && ... )
- Note that $manifests becomes empty when no manifest file is generated."""
- link_commands = ['%(ldcmd)s',
- 'if exist %(out)s.manifest del %(out)s.manifest']
- mt_cmd = ('%(python)s gyp-win-tool manifest-wrapper'
- ' $arch $mt -nologo -manifest $manifests')
- if embed_manifest and not link_incremental:
- # Embed manifest into a binary. If incremental linking is enabled,
- # embedding is postponed to the re-linking stage (see below).
- mt_cmd += ' -outputresource:%(out)s;%(resname)s'
- else:
- # Save manifest as an external file.
- mt_cmd += ' -out:%(out)s.manifest'
- manifest_commands = [mt_cmd]
- if link_incremental:
- # There is no point in generating separate rule for the case when
- # incremental linking is enabled, but manifest embedding is disabled.
- # In that case the basic rule should be used (e.g. 'link').
- # See also implementation of _GetWinLinkRuleNameSuffix().
- assert embed_manifest
- # Make .rc file out of manifest, compile it to .res file and re-link.
- manifest_commands += [
- ('%(python)s gyp-win-tool manifest-to-rc $arch %(out)s.manifest'
- ' %(out)s.manifest.rc %(resname)s'),
- '%(python)s gyp-win-tool rc-wrapper $arch $rc %(out)s.manifest.rc',
- '%(ldcmd)s %(out)s.manifest.res']
- cmd = 'cmd /c %s && if not "$manifests"=="" (%s)' % (
- ' && '.join(['(%s)' % c for c in link_commands]),
- ' && '.join(['(%s)' % c for c in manifest_commands]))
resource_name = {
'exe': '1',
'dll': '2',
}[binary_type]
- return cmd % {'python': sys.executable,
- 'out': out,
- 'ldcmd': ldcmd,
- 'resname': resource_name}
-
- rule_name_suffix = _GetWinLinkRuleNameSuffix(embed_manifest, link_incremental)
+ return '%(python)s gyp-win-tool link-with-manifests $arch %(embed)s ' \
+ '%(out)s "%(ldcmd)s" %(resname)s $mt $rc "$intermediatemanifest" ' \
+ '$manifests' % {
+ 'python': sys.executable,
+ 'out': out,
+ 'ldcmd': ldcmd,
+ 'resname': resource_name,
+ 'embed': embed_manifest }
+ rule_name_suffix = _GetWinLinkRuleNameSuffix(embed_manifest)
dlldesc = 'LINK%s(DLL) $dll' % rule_name_suffix.upper()
dllcmd = ('%s gyp-win-tool link-wrapper $arch '
'$ld /nologo $implibflag /DLL /OUT:$dll '
@@ -1915,12 +1882,8 @@
sys.executable),
rspfile='$out.rsp',
rspfile_content='$in_newline $libflags')
- _AddWinLinkRules(master_ninja, embed_manifest=True, link_incremental=True)
- _AddWinLinkRules(master_ninja, embed_manifest=True, link_incremental=False)
- _AddWinLinkRules(master_ninja, embed_manifest=False, link_incremental=False)
- # Do not generate rules for embed_manifest=False and link_incremental=True
- # because in that case rules for (False, False) should be used (see
- # implementation of _GetWinLinkRuleNameSuffix()).
+ _AddWinLinkRules(master_ninja, embed_manifest=True)
+ _AddWinLinkRules(master_ninja, embed_manifest=False)
else:
master_ninja.rule(
'objc',
diff --git a/pylib/gyp/msvs_emulation.py b/pylib/gyp/msvs_emulation.py
index 723201e..a9f65a1 100644
--- a/pylib/gyp/msvs_emulation.py
+++ b/pylib/gyp/msvs_emulation.py
@@ -454,7 +454,7 @@
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):
"""Returns the flags that need to be added to link commands, and the
manifest files."""
config = self._TargetConfig(config)
@@ -531,17 +531,21 @@
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)
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):
+ """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 +553,7 @@
# 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 +561,25 @@
'/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 passes 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 = []
+ generated_manifest_outer = \
+"<?xml version='1.0' encoding='UTF-8' standalone='yes'?>" \
+"<assembly xmlns='urn:schemas-microsoft-com:asm.v1' manifestVersion='1.0'>%s" \
+"</assembly>"
if enable_uac == 'true':
execution_level = self._Setting(('VCLinkerTool', 'UACExecutionLevel'),
config, default='0')
@@ -571,18 +591,38 @@
ui_access = self._Setting(('VCLinkerTool', 'UACUIAccess'), config,
default='false')
- flags.append('''/MANIFESTUAC:"level='%s' uiAccess='%s'"''' %
- (execution_level_map[execution_level], ui_access))
+
+ inner = '''
+<trustInfo xmlns="urn:schemas-microsoft-com:asm.v3">
+ <security>
+ <requestedPrivileges>
+ <requestedExecutionLevel level='%s' uiAccess='%s' />
+ </requestedPrivileges>
+ </security>
+</trustInfo>''' % (execution_level_map[execution_level], ui_access)
else:
- flags.append('/MANIFESTUAC:NO')
+ inner = ''
+
+ generated_manifest_contents = generated_manifest_outer % inner
+ 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. We only write the file if the contents
+ # have changed so that simply regenerating the project files doesn't
+ # cause a relink.
+ build_dir_generated_name = os.path.join(build_dir, generated_name)
+ gyp.common.EnsureDirExists(build_dir_generated_name)
+ f = gyp.common.WriteOnDiff(build_dir_generated_name)
+ f.write(generated_manifest_contents)
+ f.close()
+ 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
diff --git a/pylib/gyp/win_tool.py b/pylib/gyp/win_tool.py
index 7f3b0a5..1634ff9 100755
--- a/pylib/gyp/win_tool.py
+++ b/pylib/gyp/win_tool.py
@@ -13,6 +13,7 @@
import re
import shutil
import subprocess
+import string
import sys
BASE_DIR = os.path.dirname(os.path.abspath(__file__))
@@ -116,6 +117,82 @@
print line
return link.returncode
+ def ExecLinkWithManifests(self, arch, embed_manifest, out, ldcmd, resname,
+ mt, rc, intermediate_manifest, *manifests):
+ """A wrapper for handling creating a manifest resource and then executing
+ a link command."""
+ # The 'normal' way to do manifests is to have link generate a manifest
+ # based on gathering dependencies from the object files, then merge that
+ # manifest with other manifests supplied as sources, convert the merged
+ # manifest to a resource, and then *relink*, including the compiled
+ # version of the manifest resource. This breaks incremental linking, and
+ # is generally overly complicated. Instead, we merge all the manifests
+ # provided (along with one that includes what would normally be in the
+ # linker-generated one, see msvs_emulation.py), and include that into the
+ # first and only link. We still tell link to generate a manifest, but we
+ # only use that to assert that our simpler process did not miss anything.
+ variables = {
+ 'python': sys.executable,
+ 'arch': arch,
+ 'out': out,
+ 'ldcmd': ldcmd,
+ 'resname': resname,
+ 'mt': mt,
+ 'rc': rc,
+ 'intermediate_manifest': intermediate_manifest,
+ 'manifests': ' '.join(manifests),
+ }
+ add_to_ld = ''
+ if manifests:
+ subprocess.check_call(
+ '%(python)s gyp-win-tool manifest-wrapper %(arch)s %(mt)s -nologo '
+ '-manifest %(manifests)s -out:%(out)s.manifest' % variables)
+ if embed_manifest == 'True':
+ subprocess.check_call(
+ '%(python)s gyp-win-tool manifest-to-rc %(arch)s %(out)s.manifest'
+ ' %(out)s.manifest.rc %(resname)s' % variables)
+ subprocess.check_call(
+ '%(python)s gyp-win-tool rc-wrapper %(arch)s %(rc)s '
+ '%(out)s.manifest.rc' % variables)
+ add_to_ld = ' %(out)s.manifest.res' % variables
+ subprocess.check_call(ldcmd + add_to_ld)
+
+ # Run mt.exe on the theoretically complete manifest we generated, merging
+ # it with the one the linker generated to confirm that the linker
+ # generated one does not add anything. This is strictly unnecessary for
+ # correctness, it's only to verify that e.g. /MANIFESTDEPENDENCY was not
+ # used in a #pragma comment.
+ if manifests:
+ # Merge the intermediate one with ours to .assert.manifest, then check
+ # that .assert.manifest is identical to ours.
+ subprocess.check_call(
+ '%(python)s gyp-win-tool manifest-wrapper %(arch)s %(mt)s -nologo '
+ '-manifest %(out)s.manifest %(intermediate_manifest)s '
+ '-out:%(out)s.assert.manifest' % variables)
+ assert_manifest = '%(out)s.assert.manifest' % variables
+ our_manifest = '%(out)s.manifest' % variables
+ # Load and normalize the manifests. mt.exe sometimes removes whitespace,
+ # and sometimes doesn't unfortunately.
+ with open(our_manifest, 'rb') as our_f:
+ with open(assert_manifest, 'rb') as assert_f:
+ our_data = our_f.read().translate(None, string.whitespace)
+ assert_data = assert_f.read().translate(None, string.whitespace)
+ if our_data != assert_data:
+ os.unlink(out)
+ def dump(filename):
+ sys.stderr.write('%s\n-----\n' % filename)
+ with open(filename, 'rb') as f:
+ sys.stderr.write(f.read() + '\n-----\n')
+ dump(intermediate_manifest)
+ dump(our_manifest)
+ dump(assert_manifest)
+ sys.stderr.write(
+ 'Linker generated manifest "%s" added to final manifest "%s" '
+ '(result in "%s"). '
+ 'Were /MANIFEST switches used in #pragma statements? ' % (
+ intermediate_manifest, our_manifest, assert_manifest))
+ return 1
+
def ExecManifestWrapper(self, arch, *args):
"""Run manifest tool with environment set. Strip out undesirable warning
(some XML blocks are recognized by the OS loader, but not the manifest
diff --git a/test/win/gyptest-link-generate-manifest.py b/test/win/gyptest-link-generate-manifest.py
index ff03afd..3210b64 100644
--- a/test/win/gyptest-link-generate-manifest.py
+++ b/test/win/gyptest-link-generate-manifest.py
@@ -52,6 +52,10 @@
test.run_gyp('generate-manifest.gyp', chdir=CHDIR)
test.build('generate-manifest.gyp', test.ALL, chdir=CHDIR)
+ # Make sure that generation of .generated.manifest does not cause a relink.
+ test.run_gyp('generate-manifest.gyp', chdir=CHDIR)
+ test.up_to_date('generate-manifest.gyp', test.ALL, chdir=CHDIR)
+
def test_manifest(filename, generate_manifest, embedded_manifest,
extra_manifest):
exe_file = test.built_file_path(filename, chdir=CHDIR)
diff --git a/test/win/gyptest-link-unsupported-manifest.py b/test/win/gyptest-link-unsupported-manifest.py
new file mode 100644
index 0000000..8f7e12b
--- /dev/null
+++ b/test/win/gyptest-link-unsupported-manifest.py
@@ -0,0 +1,27 @@
+#!/usr/bin/env python
+
+# Copyright (c) 2013 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.
+
+"""
+Make sure we error out if #pragma comments are used to modify manifests.
+"""
+
+import TestGyp
+
+import sys
+
+if sys.platform == 'win32':
+ # This assertion only applies to the ninja build.
+ test = TestGyp.TestGyp(formats=['ninja'])
+
+ CHDIR = 'linker-flags'
+ test.run_gyp('unsupported-manifest.gyp', chdir=CHDIR)
+
+ # Just needs to fail to build.
+ test.build('unsupported-manifest.gyp',
+ 'test_unsupported', chdir=CHDIR, status=1)
+ test.must_not_exist(test.built_file_path('test_unsupported.exe', chdir=CHDIR))
+
+ test.pass_test()
diff --git a/test/win/gyptest-link-update-manifest.py b/test/win/gyptest-link-update-manifest.py
new file mode 100644
index 0000000..4f8b2b9
--- /dev/null
+++ b/test/win/gyptest-link-update-manifest.py
@@ -0,0 +1,103 @@
+#!/usr/bin/env python
+
+# Copyright (c) 2013 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.
+
+"""
+Make sure binary is relinked when manifest settings are changed.
+"""
+
+import TestGyp
+
+import os
+import sys
+
+if sys.platform == 'win32':
+ import pywintypes
+ import win32api
+ import winerror
+
+ RT_MANIFEST = 24
+
+ class LoadLibrary(object):
+ """Context manager for loading and releasing binaries in Windows.
+ Yields the handle of the binary loaded."""
+ def __init__(self, path):
+ self._path = path
+ self._handle = None
+
+ def __enter__(self):
+ self._handle = win32api.LoadLibrary(self._path)
+ return self._handle
+
+ def __exit__(self, type, value, traceback):
+ win32api.FreeLibrary(self._handle)
+
+ def extract_manifest(path, resource_name):
+ """Reads manifest from |path| and returns it as a string.
+ Returns None is there is no such manifest."""
+ with LoadLibrary(path) as handle:
+ try:
+ return win32api.LoadResource(handle, RT_MANIFEST, resource_name)
+ except pywintypes.error as error:
+ if error.args[0] == winerror.ERROR_RESOURCE_DATA_NOT_FOUND:
+ return None
+ else:
+ raise
+
+ test = TestGyp.TestGyp(formats=['msvs', 'ninja'])
+
+ CHDIR = 'linker-flags'
+
+ gyp_template = '''
+{
+ 'targets': [
+ {
+ 'target_name': 'test_update_manifest',
+ 'type': 'executable',
+ 'sources': ['hello.cc'],
+ 'msvs_settings': {
+ 'VCLinkerTool': {
+ 'EnableUAC': 'true',
+ 'UACExecutionLevel': '%(uac_execution_level)d',
+ },
+ 'VCManifestTool': {
+ 'EmbedManifest': 'true',
+ 'AdditionalManifestFiles': '%(additional_manifest_files)s',
+ },
+ },
+ },
+ ],
+}
+'''
+
+ gypfile = 'update-manifest.gyp'
+
+ def WriteAndUpdate(uac_execution_level, additional_manifest_files, do_build):
+ with open(os.path.join(CHDIR, gypfile), 'wb') as f:
+ f.write(gyp_template % {
+ 'uac_execution_level': uac_execution_level,
+ 'additional_manifest_files': additional_manifest_files,
+ })
+ test.run_gyp(gypfile, chdir=CHDIR)
+ if do_build:
+ test.build(gypfile, chdir=CHDIR)
+ exe_file = test.built_file_path('test_update_manifest.exe', chdir=CHDIR)
+ return extract_manifest(exe_file, 1)
+
+ manifest = WriteAndUpdate(0, '', True)
+ test.fail_test('asInvoker' not in manifest)
+ test.fail_test('35138b9a-5d96-4fbd-8e2d-a2440225f93a' in manifest)
+
+ # Make sure that updating .gyp and regenerating doesn't cause a rebuild.
+ WriteAndUpdate(0, '', False)
+ test.up_to_date(gypfile, test.ALL, chdir=CHDIR)
+
+ # But make sure that changing a manifest property does cause a relink.
+ manifest = WriteAndUpdate(2, '', True)
+ test.fail_test('requireAdministrator' not in manifest)
+
+ # Adding a manifest causes a rebuild.
+ manifest = WriteAndUpdate(2, 'extra.manifest', True)
+ test.fail_test('35138b9a-5d96-4fbd-8e2d-a2440225f93a' not in manifest)
diff --git a/test/win/linker-flags/manifest-in-comment.cc b/test/win/linker-flags/manifest-in-comment.cc
new file mode 100644
index 0000000..ae54ae5
--- /dev/null
+++ b/test/win/linker-flags/manifest-in-comment.cc
@@ -0,0 +1,13 @@
+// Copyright 2013 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.
+
+#pragma comment(linker, \
+ "\"/manifestdependency:type='Win32' " \
+ "name='Test.Research.SampleAssembly' version='6.0.0.0' " \
+ "processorArchitecture='X86' " \
+ "publicKeyToken='0000000000000000' language='*'\"")
+
+int main() {
+ return 0;
+}
diff --git a/test/win/linker-flags/unsupported-manifest.gyp b/test/win/linker-flags/unsupported-manifest.gyp
new file mode 100644
index 0000000..5549e7c
--- /dev/null
+++ b/test/win/linker-flags/unsupported-manifest.gyp
@@ -0,0 +1,13 @@
+# Copyright (c) 2013 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.
+
+{
+ 'targets': [
+ {
+ 'target_name': 'test_unsupported',
+ 'type': 'executable',
+ 'sources': ['manifest-in-comment.cc'],
+ },
+ ],
+}
