From e29ff7f24438d966461d13e20349580e4887b2c8 Mon Sep 17 00:00:00 2001 From: Googler Date: Mon, 30 Jan 2023 06:40:18 -0800 Subject: [PATCH] Fix a bug where if the first argument of _get_relative was an empty string the second path was not properly normalized before returning. Redirect get_relative call to paths.bzl Delete unused methods. PiperOrigin-RevId: 505677440 Change-Id: Ic8cf80b112c53805b36149f01b187b3a3eed4fd7 --- .../builtins_bzl/common/cc/cc_helper.bzl | 137 ++---------------- 1 file changed, 9 insertions(+), 128 deletions(-) diff --git a/src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl b/src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl index 2150b8498691c8..31eb6f47bef1e9 100644 --- a/src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl +++ b/src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl @@ -15,6 +15,7 @@ """Utility functions for C++ rules.""" load(":common/objc/semantics.bzl", objc_semantics = "semantics") +load(":common/paths.bzl", "paths") CcInfo = _builtins.toplevel.CcInfo cc_common = _builtins.toplevel.cc_common @@ -420,25 +421,6 @@ extensions = struct( DISALLOWED_HDRS_FILES = DISALLOWED_HDRS_FILES, # Also includes VERSIONED_SHARED_LIBRARY files. ) -def _collect_header_tokens( - ctx, - cpp_configuration, - compilation_outputs, - process_hdrs, - add_self_tokens): - header_tokens_transitive = [] - for dep in ctx.attr.deps: - if "_hidden_header_tokens_INTERNAL_" in dep[OutputGroupInfo]: - header_tokens_transitive.append(dep[OutputGroupInfo]["_hidden_header_tokens_INTERNAL_"]) - else: - header_tokens_transitive.append(depset([])) - - header_tokens_direct = [] - if add_self_tokens and process_hdrs: - header_tokens_direct.extend(compilation_outputs.header_tokens()) - - return depset(direct = header_tokens_direct, transitive = header_tokens_transitive) - def _collect_library_hidden_top_level_artifacts( ctx, files_to_compile): @@ -1052,107 +1034,6 @@ def _report_invalid_options(cc_toolchain, cpp_config): def _is_repository_main(repository): return repository == "" -def _get_drive_str_length(path): - if len(path) == 0: - return 0 - if path[0] == "/": - return 1 - return 0 - -def _needs_to_normalize(path): - dot_count = 0 - prev_char = "" - for i in range(len(path)): - c = path[i] - if c == "\\": - return True - if c == "/": - if prev_char == "/": - return True - if dot_count == 1 or dot_count == 2: - return True - if c == ".": - dot_count += 1 - else: - dot_count = 0 - prev_char = c - if prev_char == "/" or dot_count == 1 or dot_count == 2: - return True - return False - -# Normalizes any '.' and '..' in-place in the segment list by shifting other segments to the -# front. Returns the remaining number of items. -def _remove_relative_paths(segments, is_absolute, start_index = 0): - segment_count = 0 - shift = start_index - for i in range(start_index, len(segments)): - segment = segments[i] - if segment == ".": - shift += 1 - continue - if segment == "..": - if segment_count > 0 and segments[segment_count - 1] != "..": - # Remove the last segment, if there is one and it is not "..". This - # means that the resulting path can still contain ".." - # segments at the beginning. - segment_count -= 1 - shift += 2 - continue - elif is_absolute: - # If this is absolute, then just pop it the ".." off and remain at root - shift += 1 - continue - segment_count += 1 - if shift > 0: - segments[i - shift] = segments[i] - return segment_count - -def _normalize(path): - if len(path) == 0: - return path - is_absolute = path[0] == "/" - result = [] - if is_absolute: - result.append("/") - segments = path.split("/") - segment_count = _remove_relative_paths(segments, is_absolute) - - # segment_count might not be the same as len(segments) - for i in range(segment_count): - result.append(segments[i]) - result.append("/") - - # Remove trailing "/". - if segment_count > 0: - result = result[:-1] - return "".join(result) - -def _get_relative(original, other): - if len(original) == 0: - return other - if len(other) == 0: - return original - - other_drive_str_length = _get_drive_str_length(other) - needs_to_normalize = _needs_to_normalize(other) - - # This is an absolute path, simply return it. - if other_drive_str_length > 0: - normalized_path = other - if needs_to_normalize: - normalized_path = _normalize(other) - return normalized_path - new_path = "" - if original.endswith("/"): - original = original[:-1] - if other.endswith("/"): - other = other[:-1] - - new_path = original + "/" + other - if needs_to_normalize: - return _normalize(new_path) - return new_path - def _repository_exec_path(ctx, sibling_repository_layout): repository = ctx.label.workspace_name if _is_repository_main(repository): @@ -1162,10 +1043,10 @@ def _repository_exec_path(ctx, sibling_repository_layout): prefix = ".." if repository.startswith("@"): repository = repository[1:] - return _get_relative(prefix, repository) + return paths.get_relative(prefix, repository) def _package_exec_path(ctx, package, sibling_repository_layout): - return _get_relative(_repository_exec_path(ctx, sibling_repository_layout), package) + return paths.get_relative(_repository_exec_path(ctx, sibling_repository_layout), package) def _package_source_root(ctx, package, sibling_repository_layout): repository = ctx.label.workspace_name @@ -1173,7 +1054,7 @@ def _package_source_root(ctx, package, sibling_repository_layout): return package if repository.startswith("@"): repository = repository[1:] - return _get_relative(_get_relative("external", repository), package) + return paths.get_relative(paths.get_relative("external", repository), package) def _contains_up_level_references(path): return path.startswith("..") and (len(path) == 2 or path[2] == "/") @@ -1188,11 +1069,11 @@ def _system_include_dirs(ctx, additional_make_variable_substitutions): includes_attr = _expand(ctx, include, additional_make_variable_substitutions) if includes_attr.startswith("/"): continue - includes_path = _get_relative(package_exec_path, includes_attr) + includes_path = paths.get_relative(package_exec_path, includes_attr) if not sibling_repository_layout and _contains_up_level_references(includes_path): fail("Path references a path above the execution root.", attr = "includes") - if len(includes_path) == 0: + if includes_path == ".": fail("'" + includes_attr + "' resolves to the workspace root, which would allow this rule and all of its " + "transitive dependents to include any file in your workspace. Please include only" + " what you need", attr = "includes") @@ -1200,10 +1081,10 @@ def _system_include_dirs(ctx, additional_make_variable_substitutions): # We don't need to perform the above checks against out_includes_path again since any errors # must have manifested in includesPath already. - out_includes_path = _get_relative(package_source_root, includes_attr) + out_includes_path = paths.get_relative(package_source_root, includes_attr) if (ctx.configuration.has_separate_genfiles_directory()): - result.append(_get_relative(ctx.genfiles_dir.path, out_includes_path)) - result.append(_get_relative(ctx.bin_dir.path, out_includes_path)) + result.append(paths.get_relative(ctx.genfiles_dir.path, out_includes_path)) + result.append(paths.get_relative(ctx.bin_dir.path, out_includes_path)) return result def _get_coverage_environment(ctx, cc_config, cc_toolchain):