Skip to content

Commit

Permalink
feat: support source files with duplicate basename
Browse files Browse the repository at this point in the history
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 is removed.
  • Loading branch information
targos committed Sep 7, 2020
1 parent 4815435 commit 5e4f448
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 137 deletions.
17 changes: 0 additions & 17 deletions pylib/gyp/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ def Load(
params=None,
check=False,
circular_check=True,
duplicate_basename_check=True,
):
"""
Loads one or more specified build files.
Expand Down Expand Up @@ -156,7 +155,6 @@ def Load(
generator_input_info,
check,
circular_check,
duplicate_basename_check,
params["parallel"],
params["root_targets"],
)
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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
Expand Down
41 changes: 0 additions & 41 deletions pylib/gyp/generator/make.py
Original file line number Diff line number Diff line change
Expand Up @@ -671,43 +671,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
Expand Down Expand Up @@ -866,10 +829,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,
Expand Down
57 changes: 12 additions & 45 deletions pylib/gyp/generator/msvs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
Expand Down Expand Up @@ -3659,7 +3615,18 @@ def _AddSources2(
extension_to_rule_name,
_GetUniquePlatforms(spec),
)
grouped_sources[group].append([element, {"Include": source}] + detail)
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)
else:
grouped_sources[group].append([element, {"Include": source}] + detail)


def _GetMSBuildProjectReferences(project):
Expand Down
34 changes: 0 additions & 34 deletions pylib/gyp/input.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -3021,7 +2991,6 @@ def Load(
generator_input_info,
check,
circular_check,
duplicate_basename_check,
parallel,
root_targets,
):
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 5e4f448

Please sign in to comment.