feat: support source files with duplicate basename

This change removes the duplicate basename checks for macOS and MSVS. The macOS one is no longer necessary and the solution generator for MSVS is updated to support duplicate names by reproducing the source directory structure in the intermediate directory. Closes: nodejs#60 BREAKING CHANGE: The `--no-duplicate-basename-check` option was removed.
targos · Sep 7, 2020 · bba609e · bba609e
1 parent 4815435
commit bba609e
Show file tree

Hide file tree

Showing 5 changed files with 17 additions and 137 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## [Unreleased]
 
+### Added
+- Extended compile_commands_json generator to consider more file extensions than
+  just `c` and `cc`. `cpp` and `cxx` are now supported.
+- Source files with duplicate basenames are now supported.
+
+### Removed
+- The `--no-duplicate-basename-check` option was removed.
+
 ## [0.4.0] - 2020-07-14
 
 ### Added

diff --git a/pylib/gyp/__init__.py b/pylib/gyp/__init__.py
@@ -68,7 +68,6 @@ def Load(
     params=None,
     check=False,
     circular_check=True,
-    duplicate_basename_check=True,
 ):
     """
   Loads one or more specified build files.
@@ -156,7 +155,6 @@ def Load(
         generator_input_info,
         check,
         circular_check,
-        duplicate_basename_check,
         params["parallel"],
         params["root_targets"],
     )
@@ -431,20 +429,6 @@ def gyp_main(args):
         regenerate=False,
         help="don't check for circular relationships between files",
     )
-    # --no-duplicate-basename-check disables the check for duplicate basenames
-    # in a static_library/shared_library project. Visual C++ 2008 generator
-    # doesn't support this configuration. Libtool on Mac also generates warnings
-    # when duplicate basenames are passed into Make generator on Mac.
-    # TODO(yukawa): Remove this option when these legacy generators are
-    # deprecated.
-    parser.add_argument(
-        "--no-duplicate-basename-check",
-        dest="duplicate_basename_check",
-        action="store_false",
-        default=True,
-        regenerate=False,
-        help="don't check for duplicate basenames",
-    )
     parser.add_argument(
         "--no-parallel",
         action="store_true",
@@ -651,7 +635,6 @@ def gyp_main(args):
             params,
             options.check,
             options.circular_check,
-            options.duplicate_basename_check,
         )
 
         # TODO(mark): Pass |data| for now because the generator needs a list of

diff --git a/pylib/gyp/generator/make.py b/pylib/gyp/generator/make.py
@@ -30,7 +30,6 @@
 import gyp.common
 import gyp.xcode_emulation
 from gyp.common import GetEnvironFallback
-from gyp.common import GypError
 
 import hashlib
 
@@ -671,43 +670,6 @@ def SourceifyAndQuoteSpaces(path):
     return QuoteSpaces(Sourceify(path))
 
 
-# TODO: Avoid code duplication with _ValidateSourcesForMSVSProject in msvs.py.
-def _ValidateSourcesForOSX(spec, all_sources):
-    """Makes sure if duplicate basenames are not specified in the source list.
-
-  Arguments:
-    spec: The target dictionary containing the properties of the target.
-  """
-    if spec.get("type", None) != "static_library":
-        return
-
-    basenames = {}
-    for source in all_sources:
-        name, ext = os.path.splitext(source)
-        is_compiled_file = ext in [".c", ".cc", ".cpp", ".cxx", ".m", ".mm", ".s", ".S"]
-        if not is_compiled_file:
-            continue
-        basename = os.path.basename(name)  # Don't include extension.
-        basenames.setdefault(basename, []).append(source)
-
-    error = ""
-    for basename, files in basenames.items():
-        if len(files) > 1:
-            error += "  %s: %s\n" % (basename, " ".join(files))
-
-    if error:
-        print(
-            (
-                "static library %s has several files with the same basename:\n"
-                % spec["target_name"]
-            )
-            + error
-            + "libtool on OS X will generate"
-            + " warnings for them."
-        )
-        raise GypError("Duplicate basenames in sources section, see list above")
-
-
 # Map from qualified target to path to output.
 target_outputs = {}
 # Map from qualified target to any linkable output.  A subset
@@ -866,10 +828,6 @@ def Write(
         # Sources.
         all_sources = spec.get("sources", []) + extra_sources
         if all_sources:
-            if self.flavor == "mac":
-                # libtool on OS X generates warnings for duplicate basenames in the same
-                # target.
-                _ValidateSourcesForOSX(spec, all_sources)
             self.WriteSources(
                 configs,
                 deps,

diff --git a/pylib/gyp/generator/msvs.py b/pylib/gyp/generator/msvs.py
@@ -1030,45 +1030,6 @@ def _GenerateProject(project, options, version, generator_flags, spec):
         return _GenerateMSVSProject(project, options, version, generator_flags)
 
 
-# TODO: Avoid code duplication with _ValidateSourcesForOSX in make.py.
-def _ValidateSourcesForMSVSProject(spec, version):
-    """Makes sure if duplicate basenames are not specified in the source list.
-
-  Arguments:
-    spec: The target dictionary containing the properties of the target.
-    version: The VisualStudioVersion object.
-  """
-    # This validation should not be applied to MSVC2010 and later.
-    assert not version.UsesVcxproj()
-
-    # TODO: Check if MSVC allows this for loadable_module targets.
-    if spec.get("type", None) not in ("static_library", "shared_library"):
-        return
-    sources = spec.get("sources", [])
-    basenames = {}
-    for source in sources:
-        name, ext = os.path.splitext(source)
-        is_compiled_file = ext in [".c", ".cc", ".cpp", ".cxx", ".m", ".mm", ".s", ".S"]
-        if not is_compiled_file:
-            continue
-        basename = os.path.basename(name)  # Don't include extension.
-        basenames.setdefault(basename, []).append(source)
-
-    error = ""
-    for basename, files in basenames.items():
-        if len(files) > 1:
-            error += "  %s: %s\n" % (basename, " ".join(files))
-
-    if error:
-        print(
-            "static library %s has several files with the same basename:\n"
-            % spec["target_name"]
-            + error
-            + "MSVC08 cannot handle that."
-        )
-        raise GypError("Duplicate basenames in sources section, see list above")
-
-
 def _GenerateMSVSProject(project, options, version, generator_flags):
     """Generates a .vcproj file.  It may create .rules and .user files too.
 
@@ -1095,11 +1056,6 @@ def _GenerateMSVSProject(project, options, version, generator_flags):
     for config_name, config in spec["configurations"].items():
         _AddConfigurationToMSVSProject(p, spec, config_type, config_name, config)
 
-    # MSVC08 and prior version cannot handle duplicate basenames in the same
-    # target.
-    # TODO: Take excluded sources into consideration if possible.
-    _ValidateSourcesForMSVSProject(spec, version)
-
     # Prepare list of sources and excluded sources.
     gyp_file = os.path.split(project.build_file)[1]
     sources, excluded_sources = _PrepareListOfSources(spec, generator_flags, gyp_file)
@@ -3659,6 +3615,15 @@ def _AddSources2(
                     extension_to_rule_name,
                     _GetUniquePlatforms(spec),
                 )
+                if group == "compile":
+                    # Always add an <ObjectFileName> value to support duplicate
+                    # source file basenames.
+                    file_name = source
+                    if (file_name.startswith("..\\")):
+                        file_name = re.sub(r"^(\.\.\\)+", "", file_name)
+                    elif (file_name.startswith("$(")):
+                        file_name = re.sub(r"^\$\([^)]+\)\\", "", file_name)
+                    detail.append(["ObjectFileName", "$(IntDir)\\" + file_name])
                 grouped_sources[group].append([element, {"Include": source}] + detail)
 
 

diff --git a/pylib/gyp/input.py b/pylib/gyp/input.py
@@ -2750,36 +2750,6 @@ def ValidateTargetType(target, target_dict):
         )
 
 
-def ValidateSourcesInTarget(target, target_dict, build_file, duplicate_basename_check):
-    if not duplicate_basename_check:
-        return
-    if target_dict.get("type", None) != "static_library":
-        return
-    sources = target_dict.get("sources", [])
-    basenames = {}
-    for source in sources:
-        name, ext = os.path.splitext(source)
-        is_compiled_file = ext in [".c", ".cc", ".cpp", ".cxx", ".m", ".mm", ".s", ".S"]
-        if not is_compiled_file:
-            continue
-        basename = os.path.basename(name)  # Don't include extension.
-        basenames.setdefault(basename, []).append(source)
-
-    error = ""
-    for basename, files in basenames.items():
-        if len(files) > 1:
-            error += "  %s: %s\n" % (basename, " ".join(files))
-
-    if error:
-        print(
-            "static library %s has several files with the same basename:\n" % target
-            + error
-            + "libtool on Mac cannot handle that. Use "
-            "--no-duplicate-basename-check to disable this validation."
-        )
-        raise GypError("Duplicate basenames in sources section, see list above")
-
-
 def ValidateRulesInTarget(target, target_dict, extra_sources_for_rules):
     """Ensures that the rules sections in target_dict are valid and consistent,
   and determines which sources they apply to.
@@ -3021,7 +2991,6 @@ def Load(
     generator_input_info,
     check,
     circular_check,
-    duplicate_basename_check,
     parallel,
     root_targets,
 ):
@@ -3167,9 +3136,6 @@ def Load(
         target_dict = targets[target]
         build_file = gyp.common.BuildFile(target)
         ValidateTargetType(target, target_dict)
-        ValidateSourcesInTarget(
-            target, target_dict, build_file, duplicate_basename_check
-        )
         ValidateRulesInTarget(target, target_dict, extra_sources_for_rules)
         ValidateRunAsInTarget(target, target_dict, build_file)
         ValidateActionsInTarget(target, target_dict, build_file)