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.

Sign up for GitHub

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

Jump to bottom

Bazel: add an install shortcut and an experimental attribute to codeql_pack #18023

Merged
merged 4 commits into from
Nov 19, 2024
Merged

Bazel: add an install shortcut and an experimental attribute to codeql_pack #18023

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6e33f97
Bazel: add an `installer` shortcut to `codeql_pack`
Nov 19, 2024
40b8406
Bazel: use installer shortcut in docs and workflows
Nov 19, 2024
e8677b4
Bazel: rename shortcut to `install`
Nov 19, 2024
918b0bf
Bazel: add `experimental` to `codeql_pack`
Nov 19, 2024
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
2 changes: 1 addition & 1 deletion .github/workflows/cpp-swift-analysis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ jobs:
- name: "Build Swift extractor using Bazel"
run: |
bazel clean --expunge
bazel run //swift:create-extractor-pack --nouse_action_cache --noremote_accept_cached --noremote_upload_local_results --spawn_strategy=local
bazel run //swift:install --nouse_action_cache --noremote_accept_cached --noremote_upload_local_results --spawn_strategy=local
bazel shutdown

- name: Perform CodeQL Analysis
Expand Down
2 changes: 1 addition & 1 deletion actions/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ package(default_visibility = ["//visibility:public"])
pack_prefix = "/".join(parts),
)
for parts in (
["actions"],
[
"experimental",
"actions",
],
["actions"],
)
]
7 changes: 5 additions & 2 deletions misc/bazel/pkg.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,9 +448,10 @@ def codeql_pack(
contain the `{CODEQL_PLATFORM}` marker.
All files in the pack will be prefixed with `name`, unless `pack_prefix` is set, then is used instead.

This rule also provides a convenient installer target, with a path governed by `install_dest`.
This rule also provides a convenient installer target named `<name>-installer`, with a path governed by `install_dest`.
This installer is used for installing this pack into the source-tree, relative to the directory where the rule is used.
See `codeql_pack_install` for more details.
See `codeql_pack_install` for more details. The first `codeql_pack` defined in a bazel package also aliases this
installer target with the `install` name as a shortcut.

This function does not accept `visibility`, as packs are always public to make it easy to define pack groups.
"""
Expand All @@ -474,6 +475,8 @@ def codeql_pack(
visibility = ["//visibility:public"],
)
_codeql_pack_install(internal("installer"), [name], install_dest = install_dest, apply_pack_prefix = False)
if not native.existing_rule("install"):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this implementation - per https://bazel.build/rules/lib/toplevel/native#existing_rule, this API will be deprecated and removed at some point:

If possible, use this function only in implementation functions of rule finalizer symbolic macros. Use of this function in other contexts is not recommened, and will be disabled in a future Bazel release; it makes BUILD files brittle and order-dependent.

Given the fairly small impact of this, I'm fine merging this now, but we will have to remove this feature again once bazel follows through with the removal of this function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point! I wasn't even aware of that (or of the new macro definition framework)

I agree it's better to avoid that, and have changed the code to do that, and added an experimental flag to aid the definition of experimental packs. Alas, I still needed existing_rule because the internal repo defines two codeql_packs in a bazel package (the java single and multi kotlin variants). So for backward compatibility I've left that. Once the internal repo is updated (probably we can put install_dest = None on all codeql_packs there), we can remove TODOs here with a third PR.

native.alias(name = "install", actual = internal("installer"))

strip_prefix = _strip_prefix

Expand Down
2 changes: 1 addition & 1 deletion rust/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,10 @@ codeql_pkg_files(
pack_prefix = "/".join(parts),
)
for parts in (
["rust"],
[
"experimental",
"rust",
],
["rust"],
)
]
2 changes: 1 addition & 1 deletion rust/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ If you don't have the `semmle-code` repo you may need to install Bazel manually,

This approach uses a released `codeql` version and is simpler to use for QL development. From your `semmle-code` directory run:
```bash
bazel run @codeql//rust:rust-installer
bazel run @codeql//rust:install
```
You now need to create a [per-user CodeQL configuration file](https://docs.github.com/en/code-security/codeql-cli/using-the-advanced-functionality-of-the-codeql-cli/specifying-command-options-in-a-codeql-configuration-file#using-a-codeql-configuration-file) and specify the option:
```
Expand Down
2 changes: 1 addition & 1 deletion swift/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ brew install bazelisk
then from the `ql` directory run

```bash
bazel run //swift:create-extractor-pack
bazel run //swift:install
```

If you are running on macOS and you encounter errors mentioning `XXX is unavailable: introduced in macOS YY.ZZ`,
Expand Down
2 changes: 1 addition & 1 deletion swift/actions/build-and-test/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ runs:
- name: Build Swift extractor
shell: bash
run: |
bazel run //swift:create-extractor-pack
bazel run //swift:install
- name: Run codegen tests
if : ${{ github.event_name == 'pull_request' }}
shell: bash
Expand Down