win msvs: make ordering match .gyp order
The order of .obj on the link command line determines the order in the
final binary in the absence of other need to re-order. Make them
consistent across the windows generators by making them match the order
in the .gyp file, and add a test for this behaviour. This was already
ninja's behaviour.
R=thakis@chromium.org
BUG=chromium:326030
Review URL: https://codereview.chromium.org/107293004
git-svn-id: http://gyp.googlecode.com/svn/trunk@1807 78cadc50-ecff-11dd-a971-7dbc132099af
diff --git a/pylib/gyp/generator/msvs.py b/pylib/gyp/generator/msvs.py
index 0287eb1..3a1b610 100644
--- a/pylib/gyp/generator/msvs.py
+++ b/pylib/gyp/generator/msvs.py
@@ -2,6 +2,7 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
+import collections
import copy
import ntpath
import os
@@ -86,6 +87,46 @@
cached_domain = None
+# Based on http://code.activestate.com/recipes/576694/.
+class OrderedSet(collections.MutableSet):
+ def __init__(self, iterable=None):
+ self.end = end = []
+ end += [None, end, end] # sentinel node for doubly linked list
+ self.map = {} # key --> [key, prev, next]
+ if iterable is not None:
+ self |= iterable
+
+ def __len__(self):
+ return len(self.map)
+
+ def discard(self, key):
+ if key in self.map:
+ key, prev, next = self.map.pop(key)
+ prev[2] = next
+ next[1] = prev
+
+ def __contains__(self, key):
+ return key in self.map
+
+ def add(self, key):
+ if key not in self.map:
+ end = self.end
+ curr = end[1]
+ curr[2] = end[1] = self.map[key] = [key, curr, end]
+
+ def update(self, iterable):
+ for i in iterable:
+ if i not in self:
+ self.add(i)
+
+ def __iter__(self):
+ end = self.end
+ curr = end[2]
+ while curr is not end:
+ yield curr[0]
+ curr = curr[2]
+
+
# TODO(gspencer): Switch the os.environ calls to be
# win32api.GetDomainName() and win32api.GetUserName() once the
# python version in depot_tools has been updated to work on Vista
@@ -415,13 +456,13 @@
dicts describing the actions attached to that input file.
"""
for primary_input in actions_dict:
- inputs = set()
- outputs = set()
+ inputs = OrderedSet()
+ outputs = OrderedSet()
descriptions = []
commands = []
for action in actions_dict[primary_input]:
- inputs.update(set(action['inputs']))
- outputs.update(set(action['outputs']))
+ inputs.update(OrderedSet(action['inputs']))
+ outputs.update(OrderedSet(action['outputs']))
descriptions.append(action['description'])
commands.append(action['command'])
# Add the custom build step for one input file.
@@ -477,8 +518,8 @@
"""
raw_inputs = _FixPaths(rule.get('inputs', []))
raw_outputs = _FixPaths(rule.get('outputs', []))
- inputs = set()
- outputs = set()
+ inputs = OrderedSet()
+ outputs = OrderedSet()
inputs.add(trigger_file)
for i in raw_inputs:
inputs.add(_RuleExpandPath(i, trigger_file))
@@ -549,16 +590,16 @@
mk_file.write('OutDirCygwin:=$(shell cygpath -u "$(OutDir)")\n')
mk_file.write('IntDirCygwin:=$(shell cygpath -u "$(IntDir)")\n')
# Gather stuff needed to emit all: target.
- all_inputs = set()
- all_outputs = set()
- all_output_dirs = set()
+ all_inputs = OrderedSet()
+ all_outputs = OrderedSet()
+ all_output_dirs = OrderedSet()
first_outputs = []
for rule in rules:
trigger_files = _FindRuleTriggerFiles(rule, sources)
for tf in trigger_files:
inputs, outputs = _RuleInputsAndOutputs(rule, tf)
- all_inputs.update(set(inputs))
- all_outputs.update(set(outputs))
+ all_inputs.update(OrderedSet(inputs))
+ all_outputs.update(OrderedSet(outputs))
# Only use one target from each rule as the dependency for
# 'all' so we don't try to build each rule multiple times.
first_outputs.append(list(outputs)[0])
@@ -799,8 +840,8 @@
trigger_files = _FindRuleTriggerFiles(rule, sources)
for trigger_file in trigger_files:
inputs, outputs = _RuleInputsAndOutputs(rule, trigger_file)
- inputs = set(_FixPaths(inputs))
- outputs = set(_FixPaths(outputs))
+ inputs = OrderedSet(_FixPaths(inputs))
+ outputs = OrderedSet(_FixPaths(outputs))
inputs.remove(_FixPath(trigger_file))
sources.update(inputs)
if not spec.get('msvs_external_builder'):
@@ -817,7 +858,7 @@
Returns:
excluded_sources with files that have actions attached removed.
"""
- must_keep = set(_FixPaths(actions_to_add.keys()))
+ must_keep = OrderedSet(_FixPaths(actions_to_add.keys()))
return [s for s in excluded_sources if s not in must_keep]
@@ -965,7 +1006,7 @@
The MSVSUserFile object created.
"""
# Gather list of unique platforms.
- platforms = set()
+ platforms = OrderedSet()
for configuration in spec['configurations']:
platforms.add(_ConfigPlatform(spec['configurations'][configuration]))
platforms = list(platforms)
@@ -1152,7 +1193,7 @@
# in libraries that are assumed to be in the default library path).
# Also remove duplicate entries, leaving only the last duplicate, while
# preserving order.
- found = set()
+ found = OrderedSet()
unique_libraries_list = []
for entry in reversed(libraries):
library = re.sub('^\-l', '', entry)
@@ -1331,8 +1372,7 @@
def _AddNormalizedSources(sources_set, sources_array):
- sources = [_NormalizedSource(s) for s in sources_array]
- sources_set.update(set(sources))
+ sources_set.update(_NormalizedSource(s) for s in sources_array)
def _PrepareListOfSources(spec, generator_flags, gyp_file):
@@ -1350,9 +1390,9 @@
A pair of (list of sources, list of excluded sources).
The sources will be relative to the gyp file.
"""
- sources = set()
+ sources = OrderedSet()
_AddNormalizedSources(sources, spec.get('sources', []))
- excluded_sources = set()
+ excluded_sources = OrderedSet()
# Add in the gyp file.
if not generator_flags.get('standalone'):
sources.add(gyp_file)
@@ -1362,7 +1402,7 @@
inputs = a['inputs']
inputs = [_NormalizedSource(i) for i in inputs]
# Add all inputs to sources and excluded sources.
- inputs = set(inputs)
+ inputs = OrderedSet(inputs)
sources.update(inputs)
if not spec.get('msvs_external_builder'):
excluded_sources.update(inputs)
@@ -1391,7 +1431,7 @@
path of excluded IDL file)
"""
# Exclude excluded sources coming into the generator.
- excluded_sources.update(set(spec.get('sources_excluded', [])))
+ excluded_sources.update(OrderedSet(spec.get('sources_excluded', [])))
# Add excluded sources into sources for good measure.
sources.update(excluded_sources)
# Convert to proper windows form.
@@ -1484,7 +1524,7 @@
def _AddToolFilesToMSVS(p, spec):
# Add in tool files (rules).
- tool_files = set()
+ tool_files = OrderedSet()
for _, config in spec['configurations'].iteritems():
for f in config.get('msvs_tool_files', []):
tool_files.add(f)
@@ -3207,16 +3247,16 @@
Returns:
A pair of (action specification, the sources handled by this action).
"""
- sources_handled_by_action = set()
+ sources_handled_by_action = OrderedSet()
actions_spec = []
for primary_input, actions in actions_to_add.iteritems():
- inputs = set()
- outputs = set()
+ inputs = OrderedSet()
+ outputs = OrderedSet()
descriptions = []
commands = []
for action in actions:
- inputs.update(set(action['inputs']))
- outputs.update(set(action['outputs']))
+ inputs.update(OrderedSet(action['inputs']))
+ outputs.update(OrderedSet(action['outputs']))
descriptions.append(action['description'])
cmd = action['command']
# For most actions, add 'call' so that actions that invoke batch files
diff --git a/test/win/gyptest-link-ordering.py b/test/win/gyptest-link-ordering.py
new file mode 100644
index 0000000..5d724ee
--- /dev/null
+++ b/test/win/gyptest-link-ordering.py
@@ -0,0 +1,53 @@
+#!/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 the link order of object files is the same between msvs and ninja.
+"""
+
+import TestGyp
+
+import sys
+
+if sys.platform == 'win32':
+ test = TestGyp.TestGyp(formats=['msvs', 'ninja'])
+
+ CHDIR = 'linker-flags'
+ test.run_gyp('link-ordering.gyp', chdir=CHDIR)
+ test.build('link-ordering.gyp', test.ALL, chdir=CHDIR)
+
+ def GetDisasm(exe):
+ full_path = test.built_file_path(exe, chdir=CHDIR)
+ # Get disassembly and drop int3 padding between functions.
+ return '\n'.join(
+ x for x in test.run_dumpbin('/disasm', full_path).splitlines()
+ if 'CC' not in x)
+
+ # This is the full dump that we expect. The source files in the .gyp match
+ # this order which is what determines the ordering in the binary.
+
+ expected_disasm = '''
+_mainCRTStartup:
+ 00401000: B8 05 00 00 00 mov eax,5
+ 00401005: C3 ret
+?z@@YAHXZ:
+ 00401010: B8 03 00 00 00 mov eax,3
+ 00401015: C3 ret
+?x@@YAHXZ:
+ 00401020: B8 01 00 00 00 mov eax,1
+ 00401025: C3 ret
+?y@@YAHXZ:
+ 00401030: B8 02 00 00 00 mov eax,2
+ 00401035: C3 ret
+_main:
+ 00401040: 33 C0 xor eax,eax
+ 00401042: C3 ret
+'''
+
+ if expected_disasm not in GetDisasm('test_ordering_exe.exe'):
+ print GetDisasm('test_ordering_exe.exe')
+ test.fail_test()
+ test.pass_test()
diff --git a/test/win/linker-flags/link-ordering.gyp b/test/win/linker-flags/link-ordering.gyp
new file mode 100644
index 0000000..47718dd
--- /dev/null
+++ b/test/win/linker-flags/link-ordering.gyp
@@ -0,0 +1,37 @@
+# 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_ordering_exe',
+ 'type': 'executable',
+ # These are so the names of the functions appear in the disassembly.
+ 'msvs_settings': {
+ 'VCCLCompilerTool': {
+ 'DebugInformationFormat': '3',
+ 'Optimization': '2',
+ },
+ 'VCLinkerTool': {
+ 'GenerateDebugInformation': 'true',
+ 'LinkIncremental': '1',
+ 'GenerateManifest': 'false',
+ # Minimize the disassembly to just our code.
+ 'AdditionalOptions': [
+ '/NODEFAULTLIB',
+ ],
+ },
+ },
+ 'sources': [
+ # Explicitly sorted the same way as the disassembly in the test .py.
+ 'main-crt.c',
+ 'z.cc',
+ 'x.cc',
+ 'y.cc',
+ 'hello.cc',
+ ],
+
+ },
+ ]
+}
diff --git a/test/win/linker-flags/main-crt.c b/test/win/linker-flags/main-crt.c
new file mode 100644
index 0000000..bdc80c5
--- /dev/null
+++ b/test/win/linker-flags/main-crt.c
@@ -0,0 +1,8 @@
+// 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.
+
+// Stub so we can link with /NODEFAULTLIB when checking disasm.
+int mainCRTStartup() {
+ return 5;
+}
diff --git a/test/win/linker-flags/x.cc b/test/win/linker-flags/x.cc
new file mode 100644
index 0000000..f5f763b
--- /dev/null
+++ b/test/win/linker-flags/x.cc
@@ -0,0 +1,7 @@
+// 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.
+
+int x() {
+ return 1;
+}
diff --git a/test/win/linker-flags/y.cc b/test/win/linker-flags/y.cc
new file mode 100644
index 0000000..bd88411
--- /dev/null
+++ b/test/win/linker-flags/y.cc
@@ -0,0 +1,7 @@
+// 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.
+
+int y() {
+ return 2;
+}
diff --git a/test/win/linker-flags/z.cc b/test/win/linker-flags/z.cc
new file mode 100644
index 0000000..8a43501
--- /dev/null
+++ b/test/win/linker-flags/z.cc
@@ -0,0 +1,7 @@
+// 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.
+
+int z() {
+ return 3;
+}
