Skip to content

Go: Fix makefile to use bash to look up bazel path. #17821

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

Merged
merged 2 commits into from
Oct 22, 2024
Merged

Go: Fix makefile to use bash to look up bazel path. #17821

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ad1ef65
Go: Fix makefile to use bash to look up bazel path.
criemen Oct 22, 2024
1ccadbc
Add comment.
criemen Oct 22, 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
13 changes: 10 additions & 3 deletions go/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
# On Windows, make's path resolution algorithm is incorrect. It picks up a bazel.exe in PATH that's
# after a bazel binary. In particular, on actions, the non-exe binary is a bazelisk instance, whereas
# bazel.exe is a bazel installation.
# This means we pick up the wrong bazel version, and if the differences between the bazel we want
# and that we actually get are too big, the build fails on CI.
BAZEL := $(shell bash -c "which bazel")
Copy link
Contributor

Choose a reason for hiding this comment

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

probably worth a small comment pointing out the windows make binary resolution bug, or someone in the future might revert this thinking "why not use bazel directly?"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

argh I thought I did that, I wanted to do that, and apparently all I did was writing the commit explanation. I'm clearly too tired.


all: gen extractor

EXTRACTOR_PACK_OUT = extractor-pack
Expand Down Expand Up @@ -28,10 +35,10 @@ qhelp-to-markdown:
scripts/qhelp-to-markdown.sh ql/src "$(QHELP_OUT_DIR)"

extractor:
bazel run :go-installer
$(BAZEL) run :go-installer

gen:
bazel run :gen
$(BAZEL) run :gen

build/stats/src.stamp:
mkdir -p $(@D)/src
Expand All @@ -48,7 +55,7 @@ test: all build/testdb/check-upgrade-path
codeql test run -j0 ql/test --search-path .. --consistency-queries ql/test/consistency --compilation-cache=$(cache)
# use GOOS=linux because GOOS=darwin GOARCH=386 is no longer supported
env GOOS=linux GOARCH=386 codeql$(EXE) test run -j0 ql/test/query-tests/Security/CWE-681 --search-path .. --consistency-queries ql/test/consistency --compilation-cache=$(cache)
cd extractor; bazel test ...
cd extractor; $(BAZEL) test ...
bash extractor-smoke-test/test.sh || (echo "Extractor smoke test FAILED"; exit 1)

.PHONY: build/testdb/check-upgrade-path
Expand Down
Loading