Update chromium_src include logic to allow <> includes (#29652)
[chromium_src] Allow overrides to reference original files with #include <...> This change updates the include path handling for brave/chromium_src overrides: - Adds support for referencing original Chromium files using #include <...> in overrides. - Enables this by replacing -I../../brave/chromium_src with -iquote../../brave/chromium_src, so the path is only used for #include "..." directives. With this, other files in the build tree can reference brave/chromium_src overrides using #include "...", while the overrides themselves can reference original Chromium files using #include <...>. Since Chromium uses #include "..." for all in-tree files, we can leverage this convention and eventually drop support for #include "src/" by removing -I../../.. and making rbe_exec_root modification obsolete.
This commit is contained in:
+1
-1
@@ -11,7 +11,7 @@ vendor/omaha/
|
||||
.gclient_*
|
||||
.tags*
|
||||
.cipd
|
||||
/.clang-format
|
||||
/chromium_src/.clang-format
|
||||
/.idea/
|
||||
/components/brave_new_tab_ui/data/LICENSE
|
||||
/components/brave_wallet/browser/zcash/rust/librustzcash/src/
|
||||
|
||||
@@ -158,10 +158,20 @@ hooks = [
|
||||
'action': ['python3', 'build/apple/download_swift_format.py', '510.1.0', '0ddbb486640cde862fa311dc0f7387e6c5171bdcc0ee0c89bc9a1f8a75e8bfaf']
|
||||
},
|
||||
{
|
||||
# Generate .clang-format.
|
||||
'name': 'generate_clang_format',
|
||||
# Chromium_src files require custom formatting to correctly sort includes
|
||||
# that reference original files.
|
||||
'name': 'generate_chromium_src_clang_format',
|
||||
'pattern': '.',
|
||||
'action': ['vpython3', 'build/util/generate_clang_format.py', '../.clang-format', '.clang-format']
|
||||
'action': ['vpython3', 'tools/chromium_src/generate_clang_format.py',
|
||||
'../.clang-format', 'chromium_src/.clang-format'],
|
||||
},
|
||||
{
|
||||
# We only need a custom .clang-format in chromium_src. It was previously
|
||||
# generated in the root of brave/, so we remove it now. This hook can be
|
||||
# removed after 08/2025.
|
||||
'name': 'remove_stale_clang_format',
|
||||
'pattern': '.',
|
||||
'action': ['python3', '../tools/remove_stale_files.py', '.clang-format']
|
||||
},
|
||||
{
|
||||
'name': 'update_midl_files',
|
||||
|
||||
+11
-6
@@ -6,11 +6,16 @@
|
||||
import("//brave/build/config.gni")
|
||||
|
||||
config("brave_chromium_src_support") {
|
||||
# Add max priority "//brave/chromium_src" include search path to be able to
|
||||
# redirect Chromium #include into our file if it exists.
|
||||
# For example, to override //base/macros.h, the overriden file must be
|
||||
# placed at //brave/chromium_src/base/macros.h.
|
||||
include_dirs = [ "//brave/chromium_src" ]
|
||||
# Add "//brave/chromium_src" to the quoted include search path using -iquote.
|
||||
# This allows all source files to point to Brave overrides via #include "...",
|
||||
# while the overrides themselves can reference original Chromium files with
|
||||
# #include <...>.
|
||||
cflags = []
|
||||
if (is_win) {
|
||||
# Clang-cl doesn't know -iquote. Prepend -Xclang to make it work.
|
||||
cflags += [ "-Xclang" ]
|
||||
}
|
||||
cflags += [ "-iquote" + rebase_path("//brave/chromium_src", root_build_dir) ]
|
||||
|
||||
# Add lowest priority "//.." include search path to be able to include original
|
||||
# Chromium files using "src/" prefix.
|
||||
@@ -18,7 +23,7 @@ config("brave_chromium_src_support") {
|
||||
# because it should have the lowest priority to not break compile steps when a
|
||||
# path clash is possible: //base/macros.h vs //third_party/v8/src/base/macros.h.
|
||||
relative_root_dir = rebase_path("//..", root_build_dir)
|
||||
cflags = [ "-I${relative_root_dir}" ]
|
||||
cflags += [ "-I${relative_root_dir}" ]
|
||||
}
|
||||
|
||||
# Config to support //base build without redirect_cc to build redirect_cc itself.
|
||||
|
||||
@@ -3,6 +3,8 @@
|
||||
# License, v. 2.0. If a copy of the MPL was not distributed with this file,
|
||||
# You can obtain one at https://mozilla.org/MPL/2.0/.
|
||||
|
||||
import os
|
||||
|
||||
import brave_chromium_utils
|
||||
import chromium_presubmit_overrides
|
||||
|
||||
@@ -24,14 +26,18 @@ def CheckOverriddenHeadersDeclareIWYUExport(input_api, output_api):
|
||||
file_filter = lambda f: input_api.FilterSourceFile(
|
||||
f, files_to_check=files_to_check, files_to_skip=files_to_skip)
|
||||
|
||||
include_prefixes = ('#include "src/', '#include "../gen/')
|
||||
include_prefixes = ('#include <', '#include "src/', '#include "../gen/')
|
||||
nolint = 'NOLINT'
|
||||
expected_suffix = '// IWYU pragma: export'
|
||||
|
||||
items = []
|
||||
for f in input_api.AffectedSourceFiles(file_filter):
|
||||
overridden_file_include_prefixes = tuple(
|
||||
f'{prefix}{f.UnixLocalPath().replace("chromium_src/", "")}'
|
||||
for prefix in include_prefixes)
|
||||
for lineno, line in enumerate(f.NewContents(), 1):
|
||||
if not line.startswith(include_prefixes) or nolint in line:
|
||||
if not line.startswith(
|
||||
overridden_file_include_prefixes) or nolint in line:
|
||||
continue
|
||||
if line.endswith(expected_suffix):
|
||||
continue
|
||||
@@ -42,7 +48,40 @@ def CheckOverriddenHeadersDeclareIWYUExport(input_api, output_api):
|
||||
|
||||
return [
|
||||
output_api.PresubmitError(
|
||||
f'#include "src/**/*.h" should end with {expected_suffix}', items)
|
||||
f'Overridden file include should end with {expected_suffix}',
|
||||
items)
|
||||
]
|
||||
|
||||
|
||||
# Ensure overridden sources include original headers only via "" syntax.
|
||||
def CheckOverriddenSourceIncludeOriginalHeaderOnlyViaQuotes(
|
||||
input_api, output_api):
|
||||
files_to_check = (r'.+\.(c|cc|cpp|m|mm)$', )
|
||||
files_to_skip = ()
|
||||
|
||||
file_filter = lambda f: input_api.FilterSourceFile(
|
||||
f, files_to_check=files_to_check, files_to_skip=files_to_skip)
|
||||
|
||||
include_template = '#include <{}>'
|
||||
nolint = 'NOLINT'
|
||||
|
||||
items = []
|
||||
for f in input_api.AffectedSourceFiles(file_filter):
|
||||
include_with_path = include_template.format(
|
||||
os.path.splitext(f.UnixLocalPath().replace("chromium_src/", ""))[0]
|
||||
+ '.h')
|
||||
for lineno, line in enumerate(f.NewContents(), 1):
|
||||
if not line.startswith(include_with_path) or nolint in line:
|
||||
continue
|
||||
items.append(f'{f.LocalPath()}:{lineno}')
|
||||
|
||||
if not items:
|
||||
return []
|
||||
|
||||
return [
|
||||
output_api.PresubmitError(
|
||||
'In source files, headers for the overridden files should be '
|
||||
'included via "" syntax, not <>', items)
|
||||
]
|
||||
|
||||
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
|
||||
#include "base/check_is_test.h"
|
||||
|
||||
#include "src/base/check_is_test.cc"
|
||||
#include <base/check_is_test.cc>
|
||||
|
||||
namespace {
|
||||
bool g_this_is_a_brave_test = false;
|
||||
|
||||
@@ -6,9 +6,9 @@
|
||||
#ifndef BRAVE_CHROMIUM_SRC_BASE_CHECK_IS_TEST_H_
|
||||
#define BRAVE_CHROMIUM_SRC_BASE_CHECK_IS_TEST_H_
|
||||
|
||||
#include "base/gtest_prod_util.h"
|
||||
#include <base/check_is_test.h> // IWYU pragma: export
|
||||
|
||||
#include "src/base/check_is_test.h" // IWYU pragma: export
|
||||
#include "base/gtest_prod_util.h"
|
||||
|
||||
namespace variations {
|
||||
class PublicKeyWrapper;
|
||||
|
||||
@@ -6,14 +6,14 @@
|
||||
#ifndef BRAVE_CHROMIUM_SRC_BASE_DEBUG_ALIAS_H_
|
||||
#define BRAVE_CHROMIUM_SRC_BASE_DEBUG_ALIAS_H_
|
||||
|
||||
#include <base/debug/alias.h> // IWYU pragma: export
|
||||
|
||||
#include <algorithm>
|
||||
|
||||
#include "base/containers/span.h"
|
||||
#include "base/memory/raw_ptr_exclusion.h"
|
||||
#include "base/memory/stack_allocated.h"
|
||||
|
||||
#include "src/base/debug/alias.h" // IWYU pragma: export
|
||||
|
||||
namespace base::debug {
|
||||
|
||||
// StackObjectCopy creates a byte-for-byte copy of an object on the stack,
|
||||
|
||||
@@ -137,7 +137,7 @@ FeatureState FeatureList::GetCompileTimeFeatureState(const Feature& feature) {
|
||||
#define IsFeatureOverridden IsFeatureOverridden_ChromiumImpl
|
||||
#define GetStateIfOverridden GetStateIfOverridden_ChromiumImpl
|
||||
|
||||
#include "src/base/feature_list.cc"
|
||||
#include <base/feature_list.cc>
|
||||
|
||||
#undef GetStateIfOverridden
|
||||
#undef IsFeatureOverridden
|
||||
|
||||
@@ -15,7 +15,7 @@
|
||||
GetStateIfOverridden_ChromiumImpl(const Feature& feature); \
|
||||
static std::optional<bool> GetStateIfOverridden
|
||||
|
||||
#include "src/base/feature_list.h" // IWYU pragma: export
|
||||
#include <base/feature_list.h> // IWYU pragma: export
|
||||
|
||||
#undef IsFeatureOverridden
|
||||
#undef GetStateIfOverridden
|
||||
|
||||
@@ -3,10 +3,10 @@
|
||||
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
|
||||
* You can obtain one at https://mozilla.org/MPL/2.0/. */
|
||||
|
||||
#include "src/base/features.cc"
|
||||
|
||||
#include "base/feature_override.h"
|
||||
|
||||
#include <base/features.cc>
|
||||
|
||||
namespace base::features {
|
||||
|
||||
OVERRIDE_FEATURE_DEFAULT_STATES({{
|
||||
|
||||
@@ -8,7 +8,7 @@
|
||||
#include "base/logging.h"
|
||||
|
||||
#define print_rust_log print_rust_log_chromium_impl
|
||||
#include "src/base/logging/rust_log_integration.cc"
|
||||
#include <base/logging/rust_log_integration.cc>
|
||||
#undef print_rust_log
|
||||
|
||||
namespace logging {
|
||||
|
||||
@@ -15,7 +15,7 @@
|
||||
#define BRAVE_HISTOGRAM_FUNCTIONS_UMA_HISTOGRAM_ENUMERATION \
|
||||
if (static_cast<intmax_t>(sample) >= 0)
|
||||
|
||||
#include "src/base/metrics/histogram_functions.h" // IWYU pragma: export
|
||||
#include <base/metrics/histogram_functions.h> // IWYU pragma: export
|
||||
|
||||
#undef BRAVE_HISTOGRAM_FUNCTIONS_UMA_HISTOGRAM_ENUMERATION
|
||||
|
||||
|
||||
@@ -10,7 +10,7 @@
|
||||
#define TestLauncher TestLauncher_ChromiumImpl
|
||||
#define AddTestResult(...) AddTestResult(OnTestResult(__VA_ARGS__));
|
||||
|
||||
#include "src/base/test/launcher/test_launcher.cc"
|
||||
#include <base/test/launcher/test_launcher.cc>
|
||||
|
||||
#undef TestLauncher
|
||||
#undef AddTestResult
|
||||
|
||||
@@ -23,7 +23,7 @@ using TestLauncher_BraveImpl = TestLauncher;
|
||||
virtual void OnTestFinished
|
||||
#define MaybeSaveSummaryAsJSON virtual MaybeSaveSummaryAsJSON
|
||||
|
||||
#include "src/base/test/launcher/test_launcher.h" // IWYU pragma: export
|
||||
#include <base/test/launcher/test_launcher.h> // IWYU pragma: export
|
||||
|
||||
#undef TestLauncher
|
||||
#undef OnTestFinished
|
||||
|
||||
@@ -13,6 +13,6 @@
|
||||
testsuite_name.c_str(), result.GetTestName().c_str(), \
|
||||
result.output_snippet.c_str());
|
||||
|
||||
#include "src/base/test/launcher/test_results_tracker.cc"
|
||||
#include <base/test/launcher/test_results_tracker.cc>
|
||||
|
||||
#undef TEST_RESULTS_TRACKER_ADD_FAILURE_DETAILS
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
|
||||
#include "base/test/scoped_feature_list.h"
|
||||
|
||||
#include "src/base/test/scoped_feature_list.cc"
|
||||
#include <base/test/scoped_feature_list.cc>
|
||||
|
||||
namespace base::test {
|
||||
|
||||
|
||||
@@ -11,7 +11,7 @@
|
||||
__VA_ARGS__); \
|
||||
void InitWithFeatures(__VA_ARGS__)
|
||||
|
||||
#include "src/base/test/scoped_feature_list.h" // IWYU pragma: export
|
||||
#include <base/test/scoped_feature_list.h> // IWYU pragma: export
|
||||
|
||||
#undef InitWithFeatures
|
||||
|
||||
|
||||
@@ -15,7 +15,7 @@ class ProcessLauncher;
|
||||
friend class ::BraveBrowsingDataRemoverDelegate; \
|
||||
friend class brave::ProcessLauncher;
|
||||
|
||||
#include "src/base/threading/thread_restrictions.h" // IWYU pragma: export
|
||||
#include <base/threading/thread_restrictions.h> // IWYU pragma: export
|
||||
|
||||
#undef BRAVE_SCOPED_ALLOW_BASE_SYNC_PRIMITIVES_H
|
||||
|
||||
|
||||
@@ -19,7 +19,7 @@
|
||||
perfetto::Category("brave"), perfetto::Category("brave.adblock"), \
|
||||
perfetto::Category("brave.ads"),
|
||||
|
||||
#include "src/base/trace_event/builtin_categories.h" // IWYU pragma: export
|
||||
#include <base/trace_event/builtin_categories.h> // IWYU pragma: export
|
||||
|
||||
#undef BRAVE_INTERNAL_TRACE_LIST_BUILTIN_CATEGORIES
|
||||
|
||||
|
||||
@@ -7,7 +7,7 @@
|
||||
|
||||
#define IsMemoryAllocatorDumpNameInAllowlist \
|
||||
IsMemoryAllocatorDumpNameInAllowlist_ChromiumImpl
|
||||
#include "src/base/trace_event/memory_infra_background_allowlist.cc"
|
||||
#include <base/trace_event/memory_infra_background_allowlist.cc>
|
||||
#undef IsMemoryAllocatorDumpNameInAllowlist
|
||||
|
||||
namespace base::trace_event {
|
||||
|
||||
@@ -10,7 +10,7 @@
|
||||
|
||||
#define GetChannelString GetChannelString_ChromiumImpl
|
||||
|
||||
#include "src/base/version_info/channel.h" // IWYU pragma: export
|
||||
#include <base/version_info/channel.h> // IWYU pragma: export
|
||||
#undef GetChannelString
|
||||
|
||||
namespace version_info {
|
||||
|
||||
@@ -48,8 +48,8 @@ def ProcessCompileDatabase(original_function,
|
||||
def _FilterFlags(original_function, command, additional_filtered_flags):
|
||||
flags = original_function(command, additional_filtered_flags)
|
||||
flags_to_restore = [
|
||||
# Clangd 15+ is required, VSCode extension includes it.
|
||||
' -Xclang -fexperimental-max-bitint-width=256',
|
||||
' -Xclang -iquote../../brave/chromium_src',
|
||||
]
|
||||
|
||||
for flag_to_restore in flags_to_restore:
|
||||
|
||||
@@ -165,7 +165,7 @@ class ChromiumSrcOverridesChecker:
|
||||
|
||||
def do_check_includes(self, override_filepath, original_is_in_gen):
|
||||
"""
|
||||
Checks if |override_filepath| uses relative includes and also checks
|
||||
Checks if |override_filepath| uses relative includes and also checks <>,
|
||||
src/ and ../gen-prefixed includes for naming consistency between the
|
||||
original and the override.
|
||||
"""
|
||||
@@ -175,29 +175,42 @@ class ChromiumSrcOverridesChecker:
|
||||
display_override_filepath = os.path.join('chromium_src',
|
||||
override_filepath)
|
||||
override_filename = os.path.basename(override_filepath)
|
||||
regexp = r'^#include "src/(.*)"'
|
||||
regexp = r"""
|
||||
^\#include\s
|
||||
(?:
|
||||
# 1. Double quoted include starting with src/
|
||||
"src/(.*)"
|
||||
# 2. Angle brackets include <...> for source files
|
||||
|<(.*\.(?:c|cc|cpp|m|mm))>
|
||||
# 3. Angle brackets include <...> for header files followed
|
||||
# by `// IWYU pragma: export`
|
||||
|<(.*\.h)>\s*//\ IWYU\ pragma:\ export
|
||||
)
|
||||
"""
|
||||
gen_regexp = r'^#include "\.\./gen/(.*)"'
|
||||
rel_regexp = rf'^#include "(\.\./.*{override_filename})"'
|
||||
|
||||
for line in override_file:
|
||||
# Check src/-prefixed includes
|
||||
line_match = re.search(regexp, line)
|
||||
line_match = re.search(regexp, line, re.VERBOSE)
|
||||
if line_match:
|
||||
line_match_path = line_match.group(1) or line_match.group(
|
||||
2) or line_match.group(3)
|
||||
if original_is_in_gen:
|
||||
self.AddError(
|
||||
f" {display_override_filepath} overrides a " +
|
||||
"generated source file, but uses a src/-prefixed " +
|
||||
"include.\n A ../gen/-prefixed include should " +
|
||||
"be used instead.")
|
||||
elif line_match.group(1) != normalized_override_filepath:
|
||||
"generated source file, but does not use a " +
|
||||
"../gen/-prefixed include.\n A ../gen/-prefixed "
|
||||
+ "include should be used instead.")
|
||||
elif line_match_path != normalized_override_filepath:
|
||||
# Check for v8 overrides, they can have includes
|
||||
# starting with src.
|
||||
if normalized_override_filepath.startswith("v8/src"):
|
||||
continue
|
||||
self.AddError(
|
||||
f" {display_override_filepath} uses a " +
|
||||
"src/-prefixed include that doesn't point to " +
|
||||
"the expected file:\n" + f" Include: {line}" +
|
||||
"src/-prefixed or <> include that doesn't point " +
|
||||
"to the expected file:\n" + f" Include: {line}" +
|
||||
" Expected include target: src/" +
|
||||
f"{normalized_override_filepath}")
|
||||
continue
|
||||
|
||||
@@ -42,6 +42,19 @@ def add_chromium_src_include_categories_rule(data):
|
||||
# wildcard rule.
|
||||
include_categories.insert(wildcard_rule_idx, chromium_src_rule)
|
||||
|
||||
# Add a new category for original source file #include statements. This
|
||||
# category will put those includes after all other includes by default.
|
||||
# However it still be possible to rearrange them by having a #define or a
|
||||
# comment or anything else before the #include statement.
|
||||
lowest_rule_priority = max(rule['Priority'] for rule in include_categories)
|
||||
chromium_src_source_rule = {
|
||||
'Regex': r'^<.*\.(c|cc|cpp|m|mm)>',
|
||||
'Priority': lowest_rule_priority + 1,
|
||||
}
|
||||
# The rule is placed at the beginning of the list to match interesting files
|
||||
# first. Otherwise, the wildcard rule will match them.
|
||||
include_categories.insert(0, chromium_src_source_rule)
|
||||
|
||||
|
||||
def load_clang_format(path):
|
||||
with open(path, 'r') as file:
|
||||
@@ -27,7 +27,8 @@
|
||||
#include "base/environment.h"
|
||||
#endif // defined(REDIRECT_CC_AS_REWRAPPER)
|
||||
|
||||
const base::FilePath::StringViewType kIncludeFlag = FILE_PATH_LITERAL("-I");
|
||||
const base::FilePath::StringViewType kIncludeQuotedFlag =
|
||||
FILE_PATH_LITERAL("-iquote");
|
||||
const base::FilePath::StringViewType kBraveChromiumSrc =
|
||||
FILE_PATH_LITERAL("brave/chromium_src");
|
||||
const base::FilePath::StringViewType kGen = FILE_PATH_LITERAL("gen");
|
||||
@@ -93,9 +94,9 @@ class RedirectCC {
|
||||
// Find directories to work with first.
|
||||
for (const auto* arg : args_.subspan(first_compiler_arg_idx)) {
|
||||
base::FilePath::StringViewType arg_piece(arg);
|
||||
if (arg_piece.starts_with(kIncludeFlag) &&
|
||||
if (arg_piece.starts_with(kIncludeQuotedFlag) &&
|
||||
arg_piece.ends_with(kBraveChromiumSrc)) {
|
||||
arg_piece.remove_prefix(kIncludeFlag.size());
|
||||
arg_piece.remove_prefix(kIncludeQuotedFlag.size());
|
||||
brave_chromium_src_dir = base::FilePath::StringType(arg_piece);
|
||||
arg_piece.remove_suffix(kBraveChromiumSrc.size());
|
||||
chromium_src_dir_with_slash = base::FilePath::StringType(arg_piece);
|
||||
|
||||
Reference in New Issue
Block a user