Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

file based filter #99

Open
wants to merge 45 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
c13cf11
file based filter
xinzhengzhang Dec 23, 2022
0ea3fd6
Unify --file parsing logic
cpsauer Dec 24, 2022
4244747
file based filter: deal with header lib
xinzhengzhang Mar 18, 2023
0befc09
End loop if header is found & Add max attempts limit
xinzhengzhang Mar 3, 2023
92e83c1
Improve my language for --file docstring
cpsauer Mar 18, 2023
bf0405e
rm dupe itertools import
cpsauer Mar 18, 2023
9d7f9ab
use variable above
xinzhengzhang Mar 27, 2023
9903410
check file if resolved in a smaller scope
xinzhengzhang Mar 27, 2023
d7e1e31
Improve --file editor wrapper message
cpsauer Apr 24, 2023
d6f60cb
Fix no output error message and spacing.
cpsauer Apr 24, 2023
2b69f05
Fix small old typo
cpsauer May 2, 2023
242d601
Prevent template substitution in comment
cpsauer May 2, 2023
e17428f
ws
cpsauer May 2, 2023
5be98b1
Further improve --file editor wrapper message
cpsauer May 2, 2023
2e980ab
Alphabetize threading import
cpsauer May 2, 2023
8d5f2fe
Document interaction of --file and exclude_* template arguments
cpsauer May 2, 2023
e288d49
Entangled cleanups around thread structure
cpsauer May 2, 2023
4a251e5
Reduce unnecessary work with in header finding with --file flag
cpsauer May 2, 2023
0d86038
improve comments in _get_command
cpsauer May 2, 2023
da95ea6
Mark problems, fix typos, fix depleted iterator bug
cpsauer May 2, 2023
2a10207
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] May 2, 2023
f4fa941
Fix aquery inputs() statement not working for headers
cpsauer May 3, 2023
2e10a53
Add a few more WIP notes
cpsauer May 3, 2023
0851bca
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] May 3, 2023
0f120b6
Fix typo
cpsauer May 3, 2023
9d23866
Add note about allpaths issue
cpsauer May 3, 2023
2d09c75
Refine and note discussion points in _filter_through_headers
cpsauer May 4, 2023
0722615
Improve TODOs
cpsauer May 4, 2023
14e707e
Enrich todos and notes after another read through of the bazel [ac]qu…
cpsauer May 4, 2023
bd7bde1
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] May 4, 2023
89f583a
refresh.template.py: Estimate the number of actions that can be run b…
xinzhengzhang May 5, 2023
ef33509
refresh.template.py: quoting target_statement
xinzhengzhang May 5, 2023
f8b046e
refersh.template.py: running a preliminary query to get specified fi…
xinzhengzhang May 5, 2023
9ce1716
refresh.template.py: simplify/unify logic around target_statement_can…
xinzhengzhang May 5, 2023
40b9131
refresh.template.py: changed the _convert_compile_commands to return …
xinzhengzhang May 5, 2023
6a6777a
normalize file path if the --file is a subpath of the cwd
xinzhengzhang May 5, 2023
0453be4
refresh.template.py: for --file, don't continue traversing targets af…
xinzhengzhang May 5, 2023
97644fd
Revert "refresh.template.py: for --file, don't continue traversing ta…
xinzhengzhang May 6, 2023
e8a986b
Revert "normalize file path if the --file is a subpath of the cwd"
xinzhengzhang May 6, 2023
e67c0bd
Revert "refresh.template.py: changed the _convert_compile_commands to…
xinzhengzhang May 6, 2023
b785c3e
Revert "refresh.template.py: simplify/unify logic around target_state…
xinzhengzhang May 6, 2023
a152055
Revert "refersh.template.py: running a preliminary query to get spec…
xinzhengzhang May 6, 2023
1d8f62e
Revert "refresh.template.py: quoting target_statement"
xinzhengzhang May 6, 2023
33b2310
Revert "refresh.template.py: Estimate the number of actions that can …
xinzhengzhang May 6, 2023
84822ba
Add link to deduped aspects cquery/aquery aspects issue
cpsauer May 12, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
file based filter: deal with header lib
  • Loading branch information
xinzhengzhang committed Mar 18, 2023
commit 4244747c788fea5dd99a012aa3e5dcda584bb7b1
172 changes: 98 additions & 74 deletions refresh.template.py
Original file line number Diff line number Diff line change
Expand Up @@ -832,7 +832,75 @@ def _convert_compile_commands(aquery_output):


def _get_commands(target: str, flags: str):
"""Yields compile_commands.json entries for a given target and flags, gracefully tolerating errors."""
"""Return compile_commands.json entries for a given target and flags, gracefully tolerating errors."""
def _get_commands(target_statment):
aquery_args = [
'bazel',
'aquery',
# Aquery docs if you need em: https://docs.bazel.build/versions/master/aquery.html
# Aquery output proto reference: https://github.com/bazelbuild/bazel/blob/master/src/main/protobuf/analysis_v2.proto
# 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.
f"mnemonic('(Objc|Cpp)Compile', {target_statment})",
# 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.
'--output=jsonproto',
# 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.
'--include_artifacts=false',
# Shush logging. Just for readability.
'--ui_event_filters=-info',
'--noshow_progress',
# Disable param files, which would obscure compile actions
# Mostly, people enable param files on Windows to avoid the relatively short command length limit.
# For more, see compiler_param_file in https://bazel.build/docs/windows
# They are, however, technically supported on other platforms/compilers.
# 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.
# Since clangd has no such length limit, we'll disable param files for our aquery run.
'--features=-compiler_param_file',
# Disable layering_check during, because it causes large-scale dependence on generated module map files that prevent header extraction before their generation
# For more context, see https://github.com/hedronvision/bazel-compile-commands-extractor/issues/83
# 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.
# If Bazel starts supporting modules (https://github.com/bazelbuild/bazel/issues/4005), we'll probably need to make changes that subsume this.
'--features=-layering_check',
] + additional_flags

aquery_process = subprocess.run(
aquery_args,
# MIN_PY=3.7: Replace PIPEs with capture_output.
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
encoding=locale.getpreferredencoding(),
check=False, # We explicitly ignore errors from `bazel aquery` and carry on.
)


# Filter aquery error messages to just those the user should care about.
# Shush known warnings about missing graph targets.
# The missing graph targets are not things we want to introspect anyway.
# Tracking issue https://github.com/bazelbuild/bazel/issues/13007
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.
aquery_process.stderr = '\n'.join(line for line in aquery_process.stderr.splitlines() if not missing_targets_warning.match(line))
if aquery_process.stderr: print(aquery_process.stderr, file=sys.stderr)

# Parse proto output from aquery
try:
# object_hook -> SimpleNamespace allows object.member syntax, like a proto, while avoiding the protobuf dependency
parsed_aquery_output = json.loads(aquery_process.stdout, object_hook=lambda d: types.SimpleNamespace(**d))
except json.JSONDecodeError:
print("Bazel aquery failed. Command:", aquery_args, file=sys.stderr)
log_warning(f">>> Failed extracting commands for {target}\n Continuing gracefully...")
return []

if not getattr(parsed_aquery_output, 'actions', None): # Unifies cases: No actions (or actions list is empty)
if aquery_process.stderr:
log_warning(f""">>> Bazel lists no applicable compile commands for {target}, probably because of errors in your BUILD files, printed above.
Continuing gracefully...""")
else:
log_warning(f""">>> Bazel lists no applicable compile commands for {target}
If this is a header-only library, please instead specify a test or binary target that compiles it (search "header-only" in README.md).
Continuing gracefully...""")
return []

return _convert_compile_commands(parsed_aquery_output)

# Log clear completion messages
log_info(f">>> Analyzing commands used in {target}")

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

compile_commands = []
# First, query Bazel's C-family compile actions for that configured target
target_statment = f'deps({target})'
if {exclude_external_sources}:
# 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.
target_statment = f"filter('^(//|@//)',{target_statment})"

if file_flags:
file_path = file_flags[0]
if file_path.endswith(_get_files.source_extensions):
target_statment = f"inputs('{re.escape(file_path)}', {target_statment})"
found = False
target_statment_canidates = []
if file_flags[0].endswith(_get_files.source_extensions):
target_statment_canidates.append(f"inputs('{re.escape(file_path)}', {target_statment})")
else:
# For header files we try to find from hdrs and srcs to get the targets
# Since attr function can't query with full path, get the file name to query
fname = os.path.basename(file_path)
target_statment = f"let v = {target_statment} in attr(hdrs, '{fname}', $v) + attr(srcs, '{fname}', $v)"
aquery_args = [
'bazel',
'aquery',
# Aquery docs if you need em: https://docs.bazel.build/versions/master/aquery.html
# Aquery output proto reference: https://github.com/bazelbuild/bazel/blob/master/src/main/protobuf/analysis_v2.proto
# 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.
f"mnemonic('(Objc|Cpp)Compile', {target_statment})",
# 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.
'--output=jsonproto',
# 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.
'--include_artifacts=false',
# Shush logging. Just for readability.
'--ui_event_filters=-info',
'--noshow_progress',
# Disable param files, which would obscure compile actions
# Mostly, people enable param files on Windows to avoid the relatively short command length limit.
# For more, see compiler_param_file in https://bazel.build/docs/windows
# They are, however, technically supported on other platforms/compilers.
# 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.
# Since clangd has no such length limit, we'll disable param files for our aquery run.
'--features=-compiler_param_file',
# Disable layering_check during, because it causes large-scale dependence on generated module map files that prevent header extraction before their generation
# For more context, see https://github.com/hedronvision/bazel-compile-commands-extractor/issues/83
# 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.
# If Bazel starts supporting modules (https://github.com/bazelbuild/bazel/issues/4005), we'll probably need to make changes that subsume this.
'--features=-layering_check',
] + additional_flags

aquery_process = subprocess.run(
aquery_args,
# MIN_PY=3.7: Replace PIPEs with capture_output.
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
encoding=locale.getpreferredencoding(),
check=False, # We explicitly ignore errors from `bazel aquery` and carry on.
)


# Filter aquery error messages to just those the user should care about.
# Shush known warnings about missing graph targets.
# The missing graph targets are not things we want to introspect anyway.
# Tracking issue https://github.com/bazelbuild/bazel/issues/13007
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.
aquery_process.stderr = '\n'.join(line for line in aquery_process.stderr.splitlines() if not missing_targets_warning.match(line))
if aquery_process.stderr: print(aquery_process.stderr, file=sys.stderr)

# Parse proto output from aquery
try:
# object_hook -> SimpleNamespace allows object.member syntax, like a proto, while avoiding the protobuf dependency
parsed_aquery_output = json.loads(aquery_process.stdout, object_hook=lambda d: types.SimpleNamespace(**d))
except json.JSONDecodeError:
print("Bazel aquery failed. Command:", aquery_args, file=sys.stderr)
log_warning(f">>> Failed extracting commands for {target}\n Continuing gracefully...")
return

if not getattr(parsed_aquery_output, 'actions', None): # Unifies cases: No actions (or actions list is empty)
if aquery_process.stderr:
log_warning(f""">>> Bazel lists no applicable compile commands for {target}, probably because of errors in your BUILD files, printed above.
Continuing gracefully...""")
else:
target_statment_canidates.extend([
f"let v = {target_statment} in attr(hdrs, '{fname}', $v) + attr(srcs, '{fname}', $v)",
f"inputs('{re.escape(file_path)}', {target_statment})",
f'deps({target})',
])

for target_statment in target_statment_canidates:
compile_commands.extend( _get_commands(target_statment))
if any(command['file'].endswith(file_path) for command in reversed(compile_commands)):
found = True
break
if not found:
log_warning(f""">>> Bazel lists no applicable compile commands for {file_path} in {target}.
Continuing gracefully...""")
else:
if {exclude_external_sources}:
# 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.
target_statment = f"filter('^(//|@//)',{target_statment})"
compile_commands.extend(_get_commands(target_statment))
if len(compile_commands) == 0:
log_warning(f""">>> Bazel lists no applicable compile commands for {target}
If this is a header-only library, please instead specify a test or binary target that compiles it (search "header-only" in README.md).
Continuing gracefully...""")
return

yield from _convert_compile_commands(parsed_aquery_output)
If this is a header-only library, please instead specify a test or binary target that compiles it (search "header-only" in README.md).
Continuing gracefully...""")


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


def _ensure_external_workspaces_link_exists():
Expand Down