Skip to content

Commit

Permalink
feat: Use Bazel's Packaged Java (#24)
Browse files Browse the repository at this point in the history
Add wrappers around Java and Bazel. `_java` now references bazel's
installed Java, and `_bazel` now adds startup options.

We suspect setting up Java adds additional startup flags, which incurs a
Bazel server start latency.

Future work may include allow passing in specific Bazel / Java / etc.
versions, which we can address if need be (config file? lots of input
variables? respecting .bazelrc-esque files?)

Suggested by @pat.
  • Loading branch information
nikhilbirmiwal authored Sep 5, 2023
1 parent 4ed951f commit 0cfd2ca
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 18 deletions.
8 changes: 1 addition & 7 deletions .github/workflows/compute_impacted_tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,6 @@ jobs:
- name: Checkout
uses: actions/checkout@v3

- name: Setup Java
uses: actions/setup-java@v3
with:
distribution: oracle
java-version: "17"

- name: Setup Bazel
# trunk-ignore(semgrep): Trust third-party `bazelbuild` GH Action
uses: bazelbuild/setup-bazelisk@v2
Expand All @@ -30,7 +24,7 @@ jobs:
PR_BRANCH: do_not_delete/stable_test_branch
VERBOSE: 1
WORKSPACE_PATH: ./tests/simple_bazel_workspace
BAZEL_STARTUP_OPTIONS: --host_jvm_args=-Xmx12G,--block_for_lock
BAZEL_STARTUP_OPTIONS: --host_jvm_args=-Xmx12G,--block_for_lock,--client_debug

- name: Validate Impacted Targets Computation
shell: bash
Expand Down
6 changes: 0 additions & 6 deletions action.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,6 @@ inputs:
runs:
using: composite
steps:
- name: Setup Java
uses: actions/setup-java@v3
with:
distribution: oracle
java-version: "17"

- name: Setup Bazel
# trunk-ignore(semgrep): Trust third-party `bazelbuild` GH Action
uses: bazelbuild/setup-bazelisk@v2
Expand Down
18 changes: 13 additions & 5 deletions src/scripts/compute_impacted_targets.sh
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#!/usr/bin/env bash

set -euo pipefail
shopt -s expand_aliases

if [[ (-z ${MERGE_INSTANCE_BRANCH}) || (-z ${PR_BRANCH}) ]]; then
echo "Missing branch"
Expand Down Expand Up @@ -30,11 +31,19 @@ if [[ -n ${BAZEL_STARTUP_OPTIONS} ]]; then
fi
logIfVerbose "Bazel startup options" "${bazel_startup_options}"

_bazel() {
# trunk-ignore(shellcheck)
bazel ${bazel_startup_options} "$@"
}

# trunk-ignore(shellcheck)
alias _java=$(_bazel info java-home)/bin/java

bazelDiff() {
if [[ -n ${VERBOSE} ]]; then
java -jar bazel-diff.jar "$@" --verbose
_java -jar bazel-diff.jar "$@" --verbose
else
java -jar bazel-diff.jar "$@"
_java -jar bazel-diff.jar "$@"
fi
}

Expand Down Expand Up @@ -81,9 +90,8 @@ fi

# Install the bazel-diff JAR. Avoid cloning the repo, as there will be conflicting WORKSPACES.
curl --retry 5 -Lo bazel-diff.jar https://github.com/Tinder/bazel-diff/releases/latest/download/bazel-diff_deploy.jar
java -jar bazel-diff.jar -V
# trunk-ignore(shellcheck/SC2086)
bazel ${bazel_startup_options} version
_java -jar bazel-diff.jar -V
bazel version # Does not require running with startup options.

# Output Files
merge_instance_branch_out=./${merge_instance_branch_head_sha}
Expand Down

1 comment on commit 0cfd2ca

@pat
Copy link

@pat pat commented on 0cfd2ca Sep 5, 2023

Choose a reason for hiding this comment

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

Appreciate the credit, but I had nothing to do with this 😅

(I wouldn't mind so much, but I think this is the second reference to 'me' in this project lately)

Please sign in to comment.