Merge branch 'main' into feature/file-based-filter

.gitignore

@@ -1,12 +1,5 @@
 # Ignore macOS folder attributes.
 .DS_Store
 
-### This tool (`bazel-compile-commands-extractor`) automatically ensures the following entries are present in any WORKSPACE from which it is invoked:
-# Ignore the `external` link (that is added by `bazel-compile-commands-extractor`). The link differs between macOS/Linux and Windows, so it shouldn't be checked in. The pattern must not end with a trailing `/` because it's a symlink on macOS/Linux.
-/external
 # Ignore links to Bazel's output. The pattern needs the `*` because people can change the name of the directory into which the repository is cloned (changing the `bazel-<workspace_name>` symlink), and must not end with a trailing `/` because it's a symlink on macOS/Linux.
 /bazel-*
-# Ignore generated output. Although valuable (after all, the primary purpose of `bazel-compile-commands-extractor` is to produce `compile_commands.json`!), it should not be checked in.
-/compile_commands.json
-# Ignore the directory in which `clangd` stores its local index.
-/.cache/

BUILD

@@ -13,4 +13,4 @@ refresh_compile_commands(
 # Implementation:
 # If you are looking into the implementation, start with the overview in ImplementationReadme.md.
 
-exports_files(["refresh.template.py"]) # For implicit use by refresh_compile_commands.
+exports_files(["refresh.template.py"])  # For implicit use by refresh_compile_commands.

README.md

@@ -197,7 +197,6 @@ There should be a `compile_commands.json` file in the root of your workspace, en
 
 Behind the scenes, that `compile_commands.json` file contains entries describing all the commands used to build every source file in your project. And, for now, there's also one entry per header, describing one way it is compiled. (This gets you great autocomplete in header files, too, so you don't have to think about [`clangd`'s biggest rough edge](https://github.com/clangd/clangd/issues/123)). Crucially, all these commands have been sufficiently de-Bazeled for clang tooling (or you!) to understand them.
 
-
 ### Here's what you should be expecting, based on our experience:
 
 We use this tool every day to develop a cross-platform library for iOS and Android on macOS. Expect Android completion in Android source, macOS in macOS, iOS in iOS, etc. People use it on Linux/Ubuntu and Windows, too.

refresh.template.py

@@ -787,17 +787,11 @@ def _convert_compile_commands(aquery_output):
     # Tag actions as external if we're going to need to know that later.
     if {exclude_headers} == "external" and not {exclude_external_sources}:
         targets_by_id = {target.id : target.label for target in aquery_output.targets}
-
-        def _amend_action_as_external(action):
-            """Tag action as external if it's created by an external target"""
+        for action in aquery_output.actions:
+            # Tag action as external if it's created by an external target
             target = targets_by_id[action.targetId] # Should always be present. KeyError as implicit assert.
-            assert not target.startswith("@//"), f"Expecting local targets to start with // in aquery output. Found @// for action {action}, target {target}"
-            assert not target.startswith("//external"), f"Expecting external targets will start with @. Found //external for action {action}, target {target}"
-
-            action.is_external = target.startswith("@")
-            return action
-
-        aquery_output.actions = (_amend_action_as_external(action) for action in aquery_output.actions)
+            assert not target.startswith('//external'), f"Expecting external targets will start with @. Found //external for action {action}, target {target}"
+            action.is_external = target.startswith('@') and not target.startswith('@//')
 
     # Process each action from Bazelisms -> file paths and their clang commands
     # Threads instead of processes because most of the execution time is farmed out to subprocesses. No need to sidestep the GIL. Might change after https://github.com/clangd/clangd/issues/123 resolved
@@ -865,7 +859,7 @@ def _get_commands(target: str, flags: str):
     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})"
+        target_statment = f"filter('^(//|@//)',{target_statment})"
     if file_flags:
         file_path = file_flags[0]
         if file_path.endswith(_get_files.source_extensions):
@@ -996,28 +990,36 @@ def _ensure_external_workspaces_link_exists():
 
 
 def _ensure_gitignore_entries_exist():
-    """Ensure `/compile_commands.json`, `/external`, and other useful entries are `.gitignore`'d if it looks like git is used."""
-
-    # Do nothing if we aren't within a git repository and there is no `.gitignore` file. We still add to the .gitignore file if it exists, even if we aren't in a git repository (perhaps because git was temporarily uninstalled).
-    if not os.path.isfile('.gitignore'): # Check .gitignore first as a fast path
-        # Silently check if we're (nested) within a git repository. It isn't sufficient to check for the presence of a `.git` directory, in case the bazel workspace lives underneath the top-level git repository.
-        # See https://stackoverflow.com/questions/2180270/check-if-current-directory-is-a-git-repository for a few ways to test.
-        is_git_repository = subprocess.run('git rev-parse --git-dir',
-            shell=True,  # Ensure this will still fail with a nonzero error code even if `git` isn't installed, unifying error cases.
-            stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL
-        ).returncode == 0  # A nonzero error code indicates that we are not (nested) within a git repository.
-        if not is_git_repository: return
+    """Ensure `//compile_commands.json`, `//external`, and other useful entries are `.gitignore`'d if in a git repo."""
+    # Silently check if we're (nested) within a git repository. It isn't sufficient to check for the presence of a `.git` directory, in case the bazel workspace is nested inside the git repository.
+    git_dir_process = subprocess.run('git rev-parse --git-dir',
+        shell=True,  # Ensure this will still fail with a nonzero error code even if `git` isn't installed, unifying error cases.
+        stdout=subprocess.PIPE, stderr=subprocess.DEVNULL,
+        encoding=locale.getpreferredencoding(),
+    )
+    # A nonzero error code indicates that we are not (nested) within a git repository.
+    if git_dir_process.returncode: return
+
+    # Write into the gitignore hidden inside the .git directory
+    # This makes ignoring work automagically for people, while minimizing the code changes they have to think about or check in. https://github.com/hedronvision/bazel-compile-commands-extractor/pull/100 and https://github.com/hedronvision/bazel-compile-commands-extractor/issues/59 are exampels of use cases that this simplifies. It also marginally simplifies the case where people can't commit use of this tool to the repo they're working on.
+    # IMO tools should to do this more broadly, especially now that git is so dominant.
+    # Hidden gitignore documented in https://git-scm.com/docs/gitignore
+    git_dir = pathlib.Path(git_dir_process.stdout.rstrip())
+    hidden_gitignore_path = git_dir / 'info' / 'exclude'
+    pattern_prefix = str(pathlib.Path.cwd().relative_to(git_dir.parent.absolute()))
+    if pattern_prefix == '.': pattern_prefix = ''
+    elif pattern_prefix: pattern_prefix += '/'
 
     # Each (pattern, explanation) will be added to the `.gitignore` file if the pattern isn't present.
     needed_entries = [
-        ('/external', "# Ignore the `external` link (that is added by `bazel-compile-commands-extractor`). The link differs between macOS/Linux and Windows, so it shouldn't be checked in. The pattern must not end with a trailing `/` because it's a symlink on macOS/Linux."),
-        ('/bazel-*', "# Ignore links to Bazel's output. The pattern needs the `*` because people can change the name of the directory into which your repository is cloned (changing the `bazel-<workspace_name>` symlink), and must not end with a trailing `/` because it's a symlink on macOS/Linux."),
-        ('/compile_commands.json', "# Ignore generated output. Although valuable (after all, the primary purpose of `bazel-compile-commands-extractor` is to produce `compile_commands.json`!), it should not be checked in."),
-        ('/.cache/', "# Ignore the directory in which `clangd` stores its local index."),
+        (f'/{pattern_prefix}external', "# Ignore the `external` link (that is added by `bazel-compile-commands-extractor`). The link differs between macOS/Linux and Windows, so it shouldn't be checked in. The pattern must not end with a trailing `/` because it's a symlink on macOS/Linux."),
+        (f'/{pattern_prefix}bazel-*', "# Ignore links to Bazel's output. The pattern needs the `*` because people can change the name of the directory into which your repository is cloned (changing the `bazel-<workspace_name>` symlink), and must not end with a trailing `/` because it's a symlink on macOS/Linux. This ignore pattern should almost certainly be checked into a .gitignore in your workspace root, too, for folks who don't use this tool."),
+        (f'/{pattern_prefix}compile_commands.json', "# Ignore generated output. Although valuable (after all, the primary purpose of `bazel-compile-commands-extractor` is to produce `compile_commands.json`!), it should not be checked in."),
+        ('.cache/', "# Ignore the directory in which `clangd` stores its local index."),
     ]
 
     # Create `.gitignore` if it doesn't exist (and don't truncate if it does) and open it for appending/updating.
-    with open('.gitignore', 'a+') as gitignore:
+    with open(hidden_gitignore_path, 'a+') as gitignore:
         gitignore.seek(0)  # Files opened in `a` mode seek to the end, so we reset to the beginning so we can read.
         # Recall that trailing spaces, when escaped with `\`, are meaningful to git. However, none of the entries for which we're searching end with literal spaces, so we can safely trim all trailing whitespace. That said, we can't rewrite these stripped lines to the file, in case an existing entry is e.g. `/foo\ `, matching the file "foo " (with a trailing space), whereas the entry `/foo\` does not match the file `"foo "`.
         lines = [l.rstrip() for l in gitignore]
@@ -1034,7 +1036,7 @@ def _ensure_gitignore_entries_exist():
         for pattern, comment in missing:
             print(comment, file=gitignore)
             print(pattern, file=gitignore)
-    log_success(">>> Automatically added entries to .gitignore to avoid problems.")
+    log_success(">>> Automatically added entries to .git/info/exclude to gitignore generated output.")
 
 
 def _ensure_cwd_is_workspace_root():

refresh_compile_commands.bzl

@@ -70,7 +70,7 @@ def refresh_compile_commands(
     # In Python, `type(x) == y` is an antipattern, but [Starlark doesn't support inheritance](https://bazel.build/rules/language), so `isinstance` doesn't exist, and this is the correct way to switch on type.
     if not targets:  # Default to all targets in main workspace
         targets = {"@//...": ""}
-    elif type(targets) == "select": # Allow select: https://bazel.build/reference/be/functions#select
+    elif type(targets) == "select":  # Allow select: https://bazel.build/reference/be/functions#select
         # Pass select() to _expand_template to make it work
         # see https://bazel.build/docs/configurable-attributes#faq-select-macro
         pass
@@ -95,8 +95,8 @@ def _expand_template_impl(ctx):
             # Note, don't delete whitespace. Correctly doing multiline indenting.
             "        {target_flag_pairs}": "\n".join(["        {},".format(pair) for pair in ctx.attr.labels_to_flags.items()]),
             "        {windows_default_include_paths}": "\n".join(["        %r," % path for path in find_cpp_toolchain(ctx).built_in_include_directories]),  # find_cpp_toolchain is from https://docs.bazel.build/versions/main/integrating-with-rules-cc.html
-            "{exclude_headers}": '"' + str(ctx.attr.exclude_headers) + '"',
-            "{exclude_external_sources}": str(ctx.attr.exclude_external_sources),
+            "{exclude_headers}": repr(ctx.attr.exclude_headers),
+            "{exclude_external_sources}": repr(ctx.attr.exclude_external_sources),
         },
     )
     return DefaultInfo(files = depset([script]))
@@ -105,9 +105,11 @@ _expand_template = rule(
     attrs = {
         "labels_to_flags": attr.string_dict(mandatory = True),  # string keys instead of label_keyed because Bazel doesn't support parsing wildcard target patterns (..., *, :all) in BUILD attributes.
         "exclude_external_sources": attr.bool(default = False),
-        "exclude_headers": attr.string(values = ["all", "external", ""]), # "" needed only for compatibility with Bazel < 3.6.0
+        "exclude_headers": attr.string(values = ["all", "external", ""]),  # "" needed only for compatibility with Bazel < 3.6.0
         "_script_template": attr.label(allow_single_file = True, default = "refresh.template.py"),
-        "_cc_toolchain": attr.label(default = "@bazel_tools//tools/cpp:current_cc_toolchain"),  # For Windows INCLUDE. If this were eliminated, for example by the resolution of https://github.com/clangd/clangd/issues/123, we'd be able to just use a macro and skylib's expand_template rule: https://github.com/bazelbuild/bazel-skylib/pull/330
+        # For Windows INCLUDE. If this were eliminated, for example by the resolution of https://github.com/clangd/clangd/issues/123, we'd be able to just use a macro and skylib's expand_template rule: https://github.com/bazelbuild/bazel-skylib/pull/330
+        # Once https://github.com/bazelbuild/bazel/pull/17108 is widely released, we should be able to eliminate this and get INCLUDE directly. Perhaps for 7.0? Should be released in the sucessor to 6.0
+        "_cc_toolchain": attr.label(default = "@bazel_tools//tools/cpp:current_cc_toolchain"),
     },
     toolchains = ["@bazel_tools//tools/cpp:toolchain_type"],  # Needed for find_cpp_toolchain with --incompatible_enable_cc_toolchain_resolution
     implementation = _expand_template_impl,