Skip to content

Commit 4244747

Browse files
committed
file based filter: deal with header lib
1 parent 0ea3fd6 commit 4244747

File tree

1 file changed

+98
-74
lines changed

1 file changed

+98
-74
lines changed

refresh.template.py

Lines changed: 98 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -832,7 +832,75 @@ def _convert_compile_commands(aquery_output):
832832

833833

834834
def _get_commands(target: str, flags: str):
835-
"""Yields compile_commands.json entries for a given target and flags, gracefully tolerating errors."""
835+
"""Return compile_commands.json entries for a given target and flags, gracefully tolerating errors."""
836+
def _get_commands(target_statment):
837+
aquery_args = [
838+
'bazel',
839+
'aquery',
840+
# Aquery docs if you need em: https://docs.bazel.build/versions/master/aquery.html
841+
# Aquery output proto reference: https://github.com/bazelbuild/bazel/blob/master/src/main/protobuf/analysis_v2.proto
842+
# One bummer, not described in the docs, is that aquery filters over *all* actions for a given target, rather than just those that would be run by a build to produce a given output. This mostly isn't a problem, but can sometimes surface extra, unnecessary, misconfigured actions. Chris has emailed the authors to discuss and filed an issue so anyone reading this could track it: https://github.com/bazelbuild/bazel/issues/14156.
843+
f"mnemonic('(Objc|Cpp)Compile', {target_statment})",
844+
# We switched to jsonproto instead of proto because of https://github.com/bazelbuild/bazel/issues/13404. We could change back when fixed--reverting most of the commit that added this line and tweaking the build file to depend on the target in that issue. That said, it's kinda nice to be free of the dependency, unless (OPTIMNOTE) jsonproto becomes a performance bottleneck compated to binary protos.
845+
'--output=jsonproto',
846+
# We'll disable artifact output for efficiency, since it's large and we don't use them. Small win timewise, but dramatically less json output from aquery.
847+
'--include_artifacts=false',
848+
# Shush logging. Just for readability.
849+
'--ui_event_filters=-info',
850+
'--noshow_progress',
851+
# Disable param files, which would obscure compile actions
852+
# Mostly, people enable param files on Windows to avoid the relatively short command length limit.
853+
# For more, see compiler_param_file in https://bazel.build/docs/windows
854+
# They are, however, technically supported on other platforms/compilers.
855+
# That's all well and good, but param files would prevent us from seeing compile actions before the param files had been generated by compilation.
856+
# Since clangd has no such length limit, we'll disable param files for our aquery run.
857+
'--features=-compiler_param_file',
858+
# Disable layering_check during, because it causes large-scale dependence on generated module map files that prevent header extraction before their generation
859+
# For more context, see https://github.com/hedronvision/bazel-compile-commands-extractor/issues/83
860+
# If https://github.com/clangd/clangd/issues/123 is resolved and we're not doing header extraction, we could try removing this, checking that there aren't erroneous red squigglies squigglies before the module maps are generated.
861+
# If Bazel starts supporting modules (https://github.com/bazelbuild/bazel/issues/4005), we'll probably need to make changes that subsume this.
862+
'--features=-layering_check',
863+
] + additional_flags
864+
865+
aquery_process = subprocess.run(
866+
aquery_args,
867+
# MIN_PY=3.7: Replace PIPEs with capture_output.
868+
stdout=subprocess.PIPE,
869+
stderr=subprocess.PIPE,
870+
encoding=locale.getpreferredencoding(),
871+
check=False, # We explicitly ignore errors from `bazel aquery` and carry on.
872+
)
873+
874+
875+
# Filter aquery error messages to just those the user should care about.
876+
# Shush known warnings about missing graph targets.
877+
# The missing graph targets are not things we want to introspect anyway.
878+
# Tracking issue https://github.com/bazelbuild/bazel/issues/13007
879+
missing_targets_warning: typing.Pattern[str] = re.compile(r'(\(\d+:\d+:\d+\) )?(\033\[[\d;]+m)?WARNING: (\033\[[\d;]+m)?Targets were missing from graph:') # Regex handles --show_timestamps and --color=yes. Could use "in" if we ever need more flexibility.
880+
aquery_process.stderr = '\n'.join(line for line in aquery_process.stderr.splitlines() if not missing_targets_warning.match(line))
881+
if aquery_process.stderr: print(aquery_process.stderr, file=sys.stderr)
882+
883+
# Parse proto output from aquery
884+
try:
885+
# object_hook -> SimpleNamespace allows object.member syntax, like a proto, while avoiding the protobuf dependency
886+
parsed_aquery_output = json.loads(aquery_process.stdout, object_hook=lambda d: types.SimpleNamespace(**d))
887+
except json.JSONDecodeError:
888+
print("Bazel aquery failed. Command:", aquery_args, file=sys.stderr)
889+
log_warning(f">>> Failed extracting commands for {target}\n Continuing gracefully...")
890+
return []
891+
892+
if not getattr(parsed_aquery_output, 'actions', None): # Unifies cases: No actions (or actions list is empty)
893+
if aquery_process.stderr:
894+
log_warning(f""">>> Bazel lists no applicable compile commands for {target}, probably because of errors in your BUILD files, printed above.
895+
Continuing gracefully...""")
896+
else:
897+
log_warning(f""">>> Bazel lists no applicable compile commands for {target}
898+
If this is a header-only library, please instead specify a test or binary target that compiles it (search "header-only" in README.md).
899+
Continuing gracefully...""")
900+
return []
901+
902+
return _convert_compile_commands(parsed_aquery_output)
903+
836904
# Log clear completion messages
837905
log_info(f">>> Analyzing commands used in {target}")
838906

@@ -862,90 +930,46 @@ def _get_commands(target: str, flags: str):
862930
Try adding them as flags in your refresh_compile_commands rather than targets.
863931
In a moment, Bazel will likely fail to parse.""")
864932

933+
compile_commands = []
865934
# First, query Bazel's C-family compile actions for that configured target
866935
target_statment = f'deps({target})'
867-
if {exclude_external_sources}:
868-
# For efficiency, have bazel filter out external targets (and therefore actions) before they even get turned into actions or serialized and sent to us. Note: this is a different mechanism than is used for excluding just external headers.
869-
target_statment = f"filter('^(//|@//)',{target_statment})"
936+
870937
if file_flags:
871938
file_path = file_flags[0]
872-
if file_path.endswith(_get_files.source_extensions):
873-
target_statment = f"inputs('{re.escape(file_path)}', {target_statment})"
939+
found = False
940+
target_statment_canidates = []
941+
if file_flags[0].endswith(_get_files.source_extensions):
942+
target_statment_canidates.append(f"inputs('{re.escape(file_path)}', {target_statment})")
874943
else:
875-
# For header files we try to find from hdrs and srcs to get the targets
876-
# Since attr function can't query with full path, get the file name to query
877944
fname = os.path.basename(file_path)
878-
target_statment = f"let v = {target_statment} in attr(hdrs, '{fname}', $v) + attr(srcs, '{fname}', $v)"
879-
aquery_args = [
880-
'bazel',
881-
'aquery',
882-
# Aquery docs if you need em: https://docs.bazel.build/versions/master/aquery.html
883-
# Aquery output proto reference: https://github.com/bazelbuild/bazel/blob/master/src/main/protobuf/analysis_v2.proto
884-
# One bummer, not described in the docs, is that aquery filters over *all* actions for a given target, rather than just those that would be run by a build to produce a given output. This mostly isn't a problem, but can sometimes surface extra, unnecessary, misconfigured actions. Chris has emailed the authors to discuss and filed an issue so anyone reading this could track it: https://github.com/bazelbuild/bazel/issues/14156.
885-
f"mnemonic('(Objc|Cpp)Compile', {target_statment})",
886-
# We switched to jsonproto instead of proto because of https://github.com/bazelbuild/bazel/issues/13404. We could change back when fixed--reverting most of the commit that added this line and tweaking the build file to depend on the target in that issue. That said, it's kinda nice to be free of the dependency, unless (OPTIMNOTE) jsonproto becomes a performance bottleneck compated to binary protos.
887-
'--output=jsonproto',
888-
# We'll disable artifact output for efficiency, since it's large and we don't use them. Small win timewise, but dramatically less json output from aquery.
889-
'--include_artifacts=false',
890-
# Shush logging. Just for readability.
891-
'--ui_event_filters=-info',
892-
'--noshow_progress',
893-
# Disable param files, which would obscure compile actions
894-
# Mostly, people enable param files on Windows to avoid the relatively short command length limit.
895-
# For more, see compiler_param_file in https://bazel.build/docs/windows
896-
# They are, however, technically supported on other platforms/compilers.
897-
# That's all well and good, but param files would prevent us from seeing compile actions before the param files had been generated by compilation.
898-
# Since clangd has no such length limit, we'll disable param files for our aquery run.
899-
'--features=-compiler_param_file',
900-
# Disable layering_check during, because it causes large-scale dependence on generated module map files that prevent header extraction before their generation
901-
# For more context, see https://github.com/hedronvision/bazel-compile-commands-extractor/issues/83
902-
# If https://github.com/clangd/clangd/issues/123 is resolved and we're not doing header extraction, we could try removing this, checking that there aren't erroneous red squigglies squigglies before the module maps are generated.
903-
# If Bazel starts supporting modules (https://github.com/bazelbuild/bazel/issues/4005), we'll probably need to make changes that subsume this.
904-
'--features=-layering_check',
905-
] + additional_flags
906-
907-
aquery_process = subprocess.run(
908-
aquery_args,
909-
# MIN_PY=3.7: Replace PIPEs with capture_output.
910-
stdout=subprocess.PIPE,
911-
stderr=subprocess.PIPE,
912-
encoding=locale.getpreferredencoding(),
913-
check=False, # We explicitly ignore errors from `bazel aquery` and carry on.
914-
)
915-
916-
917-
# Filter aquery error messages to just those the user should care about.
918-
# Shush known warnings about missing graph targets.
919-
# The missing graph targets are not things we want to introspect anyway.
920-
# Tracking issue https://github.com/bazelbuild/bazel/issues/13007
921-
missing_targets_warning: typing.Pattern[str] = re.compile(r'(\(\d+:\d+:\d+\) )?(\033\[[\d;]+m)?WARNING: (\033\[[\d;]+m)?Targets were missing from graph:') # Regex handles --show_timestamps and --color=yes. Could use "in" if we ever need more flexibility.
922-
aquery_process.stderr = '\n'.join(line for line in aquery_process.stderr.splitlines() if not missing_targets_warning.match(line))
923-
if aquery_process.stderr: print(aquery_process.stderr, file=sys.stderr)
924-
925-
# Parse proto output from aquery
926-
try:
927-
# object_hook -> SimpleNamespace allows object.member syntax, like a proto, while avoiding the protobuf dependency
928-
parsed_aquery_output = json.loads(aquery_process.stdout, object_hook=lambda d: types.SimpleNamespace(**d))
929-
except json.JSONDecodeError:
930-
print("Bazel aquery failed. Command:", aquery_args, file=sys.stderr)
931-
log_warning(f">>> Failed extracting commands for {target}\n Continuing gracefully...")
932-
return
933-
934-
if not getattr(parsed_aquery_output, 'actions', None): # Unifies cases: No actions (or actions list is empty)
935-
if aquery_process.stderr:
936-
log_warning(f""">>> Bazel lists no applicable compile commands for {target}, probably because of errors in your BUILD files, printed above.
937-
Continuing gracefully...""")
938-
else:
945+
target_statment_canidates.extend([
946+
f"let v = {target_statment} in attr(hdrs, '{fname}', $v) + attr(srcs, '{fname}', $v)",
947+
f"inputs('{re.escape(file_path)}', {target_statment})",
948+
f'deps({target})',
949+
])
950+
951+
for target_statment in target_statment_canidates:
952+
compile_commands.extend( _get_commands(target_statment))
953+
if any(command['file'].endswith(file_path) for command in reversed(compile_commands)):
954+
found = True
955+
break
956+
if not found:
957+
log_warning(f""">>> Bazel lists no applicable compile commands for {file_path} in {target}.
958+
Continuing gracefully...""")
959+
else:
960+
if {exclude_external_sources}:
961+
# For efficiency, have bazel filter out external targets (and therefore actions) before they even get turned into actions or serialized and sent to us. Note: this is a different mechanism than is used for excluding just external headers.
962+
target_statment = f"filter('^(//|@//)',{target_statment})"
963+
compile_commands.extend(_get_commands(target_statment))
964+
if len(compile_commands) == 0:
939965
log_warning(f""">>> Bazel lists no applicable compile commands for {target}
940-
If this is a header-only library, please instead specify a test or binary target that compiles it (search "header-only" in README.md).
941-
Continuing gracefully...""")
942-
return
943-
944-
yield from _convert_compile_commands(parsed_aquery_output)
966+
If this is a header-only library, please instead specify a test or binary target that compiles it (search "header-only" in README.md).
967+
Continuing gracefully...""")
945968

946969

947970
# Log clear completion messages
948971
log_success(f">>> Finished extracting commands for {target}")
972+
return compile_commands
949973

950974

951975
def _ensure_external_workspaces_link_exists():

0 commit comments

Comments
 (0)