Fix msvs-ninja OutputDirectory path.

pylib/gyp/generator/msvs.py

Index: pylib/gyp/generator/msvs.py
===================================================================
--- pylib/gyp/generator/msvs.py	(revision 1924)
+++ pylib/gyp/generator/msvs.py	(working copy)
@@ -12,6 +12,7 @@
 
 import gyp.common
 import gyp.easy_xml as easy_xml
+import gyp.generator.ninja as ninja_generator
 import gyp.MSVSNew as MSVSNew
 import gyp.MSVSProject as MSVSProject
 import gyp.MSVSSettings as MSVSSettings
@@ -1787,7 +1788,7 @@
   return projects
 
 
-def _InitNinjaFlavor(options, target_list, target_dicts):
+def _InitNinjaFlavor(params, target_list, target_dicts):
   """Initialize targets for the ninja flavor.
 
   This sets up the necessary variables in the targets to generate msvs projects
@@ -1795,7 +1796,7 @@
   if they have not been set. This allows individual specs to override the
   default values initialized here.
   Arguments:
-    options: Options provided to the generator.
+    params: Params provided to the generator.
     target_list: List of target pairs: 'base/base.gyp:base'.
     target_dicts: Dict of target properties keyed on target pair.
   """
@@ -1810,7 +1811,7 @@
     spec['msvs_external_builder'] = 'ninja'
     if not spec.get('msvs_external_builder_out_dir'):
       spec['msvs_external_builder_out_dir'] = \
-        options.depth + '/out/$(Configuration)'
+          ninja_generator.ComputeOutputDir(params) + '\\$(Configuration)'

[Shezan Baig (Bloomberg), 2014/05/31 05:09:55]

I ran into another issue with this when I was trying it out.  I think we need to
join this with options.toplevel_dir.  Something like:

os.path.join(
    options.toplevel_dir,
    ninja_generator.ComputeOutputDir(params)) + '\\$(Configuration)'

This simulates the code in ninja_generator.GenerateOutputForConfig where it
computes 'toplevel_build'.

[bungeman-chromium, 2014/06/02 14:55:47]

I see now. msvs_external_builder_out_dir is always defined in the target spec
and is therefore relative to the gyp file which contains it. When running
msvs-ninja, msvs_exernal_builder_out_dir (if not already set) needs to be set to
the path from the gyp file in which the target is contained to the desired
output directory (in this case, where ninja put its output directory plus
$(Configuration)). ComputeOutputParams returns a path from the toplevel_dir to
the ninja build output directory. So this needs to be more like

spec['msvs_external_builder_out_dir'] = os.path.join(gyp_to_toplevel,
ninja_generator.ComputeOutputDir(params)) + '\\$(Configuration)'

In order to be consistent with how msvs_external_builder_out_dir would have been
defined in the target directly.

     if not spec.get('msvs_external_builder_build_cmd'):
       spec['msvs_external_builder_build_cmd'] = [
         path_to_ninja,
@@ -1905,7 +1906,7 @@
 
   # Optionally configure each spec to use ninja as the external builder.
   if params.get('flavor') == 'ninja':
-    _InitNinjaFlavor(options, target_list, target_dicts)
+    _InitNinjaFlavor(params, target_list, target_dicts)
 
   # Prepare the set of configurations.
   configs = set()
@@ -2560,10 +2561,11 @@
   ]
 
 
-def _GetMSBuildConfigurationDetails(spec, build_file):
+def _GetMSBuildConfigurationDetails(spec, build_file, project_path):
   properties = {}
   for name, settings in spec['configurations'].iteritems():
-    msbuild_attributes = _GetMSBuildAttributes(spec, settings, build_file)
+    msbuild_attributes = _GetMSBuildAttributes(spec, settings,
+                                               build_file, project_path)
     condition = _GetConfigurationCondition(name, settings)
     character_set = msbuild_attributes.get('CharacterSet')
     _AddConditionalProperty(properties, condition, 'ConfigurationType',
@@ -2670,7 +2672,7 @@
   return config_type
 
 
-def _GetMSBuildAttributes(spec, config, build_file):
+def _GetMSBuildAttributes(spec, config, build_file, project_path):
   if 'msbuild_configuration_attributes' not in config:
     msbuild_attributes = _ConvertMSVSBuildAttributes(spec, config, build_file)
 
@@ -2696,7 +2698,9 @@
 
   if spec.get('msvs_external_builder'):
     external_out_dir = spec.get('msvs_external_builder_out_dir', '.')
-    msbuild_attributes['OutputDirectory'] = _FixPath(external_out_dir) + '\\'
+    project_dir = os.path.dirname(project_path)
+    msbuild_attributes['OutputDirectory'] = \
+        gyp.common.RelativePath(external_out_dir, project_dir) + '\\'

[bungeman-chromium, 2014/06/02 14:55:47]

The external_out_dir is relative to the gyp file in which the target was
defined. The OutputDirectory needs to be the same path, but relative to the
msbuild project file. As a result, using _FixPath here was actually correct. The
issue was just that msvs-ninja wasn't setting the path so that this would work.

 
   # Make sure that 'TargetPath' matches 'Lib.OutputFile' or 'Link.OutputFile'
   # (depending on the tool used) to avoid MSB8012 warning.
@@ -2719,7 +2723,8 @@
   return msbuild_attributes
 
 
-def _GetMSBuildConfigurationGlobalProperties(spec, configurations, build_file):
+def _GetMSBuildConfigurationGlobalProperties(spec, configurations,
+                                             build_file, project_path):
   # TODO(jeanluc) We could optimize out the following and do it only if
   # there are actions.
   # TODO(jeanluc) Handle the equivalent of setting 'CYGWIN=nontsec'.
@@ -2738,7 +2743,8 @@
   properties = {}
   for (name, configuration) in sorted(configurations.iteritems()):
     condition = _GetConfigurationCondition(name, configuration)
-    attributes = _GetMSBuildAttributes(spec, configuration, build_file)
+    attributes = _GetMSBuildAttributes(spec, configuration,
+                                       build_file, project_path)
     msbuild_settings = configuration['finalized_msbuild_settings']
     _AddConditionalProperty(properties, condition, 'IntDir',
                             attributes['IntermediateDirectory'])
@@ -3168,14 +3174,16 @@
   content += _GetMSBuildProjectConfigurations(configurations)
   content += _GetMSBuildGlobalProperties(spec, project.guid, project_file_name)
   content += import_default_section
-  content += _GetMSBuildConfigurationDetails(spec, project.build_file)
+  content += _GetMSBuildConfigurationDetails(spec,
+                                             project.build_file, project.path)
   content += _GetMSBuildLocalProperties(project.msbuild_toolset)
   content += import_cpp_props_section
   content += _GetMSBuildExtensions(props_files_of_rules)
   content += _GetMSBuildPropertySheets(configurations)
   content += macro_section
   content += _GetMSBuildConfigurationGlobalProperties(spec, configurations,
-                                                      project.build_file)
+                                                      project.build_file,
+                                                      project.path)
   content += _GetMSBuildToolSettingsSections(spec, configurations)
   content += _GetMSBuildSources(
       spec, sources, exclusions, extension_to_rule_name, actions_spec,