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

Go: update make and CI to use bazel #16398

Merged
merged 7 commits into from
May 13, 2024
Merged

Go: update make and CI to use bazel #16398

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
bab2dd4
Go: bring back `make` and CI changes
redsun82 May 2, 2024
c123513
Go: bring back test action
redsun82 May 6, 2024
e71d038
Go: specify Go runtime version in actions
redsun82 May 6, 2024
bbdc275
Merge branch 'main' into redsun82/go-make
redsun82 May 6, 2024
5b1b30a
Apply suggestions from code review
redsun82 May 8, 2024
12c28f3
Go: reorganize CI steps
redsun82 May 8, 2024
e359744
Go: bring back qhelp in action
redsun82 May 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
65 changes: 4 additions & 61 deletions .github/workflows/go-tests-other-os.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ on:
- .github/workflows/go-tests-other-os.yml
- .github/actions/**
- codeql-workspace.yml
env:
GO_VERSION: '~1.22.0'

permissions:
contents: read
Expand All @@ -18,72 +16,17 @@ jobs:
name: Test MacOS
runs-on: macos-latest
steps:
- name: Set up Go ${{ env.GO_VERSION }}
uses: actions/setup-go@v5
with:
go-version: ${{ env.GO_VERSION }}
cache: false
id: go

- name: Check out code
uses: actions/checkout@v4

- name: Set up CodeQL CLI
uses: ./.github/actions/fetch-codeql

- name: Enable problem matchers in repository
shell: bash
run: 'find .github/problem-matchers -name \*.json -exec echo "::add-matcher::{}" \;'

- name: Build
run: |
cd go
make

- name: Cache compilation cache
id: query-cache
uses: ./.github/actions/cache-query-compilation
with:
key: go-qltest
- name: Test
run: |
cd go
make test cache="${{ steps.query-cache.outputs.cache-dir }}"
- name: Run tests
uses: ./go/actions/test

test-win:
if: github.repository_owner == 'github'
name: Test Windows
runs-on: windows-latest-xl
steps:
- name: Set up Go ${{ env.GO_VERSION }}
uses: actions/setup-go@v5
with:
go-version: ${{ env.GO_VERSION }}
cache: false
id: go

- name: Check out code
uses: actions/checkout@v4

- name: Set up CodeQL CLI
uses: ./.github/actions/fetch-codeql

- name: Enable problem matchers in repository
shell: bash
run: 'find .github/problem-matchers -name \*.json -exec echo "::add-matcher::{}" \;'

- name: Build
run: |
cd go
make

- name: Cache compilation cache
id: query-cache
uses: ./.github/actions/cache-query-compilation
with:
key: go-qltest

- name: Test
run: |
cd go
make test cache="${{ steps.query-cache.outputs.cache-dir }}"
- name: Run tests
uses: ./go/actions/test
51 changes: 3 additions & 48 deletions .github/workflows/go-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@ on:
- .github/actions/**
- codeql-workspace.yml

env:
GO_VERSION: '~1.22.0'

permissions:
contents: read

Expand All @@ -28,51 +25,9 @@ jobs:
name: Test Linux (Ubuntu)
runs-on: ubuntu-latest-xl
steps:
- name: Set up Go ${{ env.GO_VERSION }}
uses: actions/setup-go@v5
with:
go-version: ${{ env.GO_VERSION }}
cache: false
id: go

- name: Check out code
uses: actions/checkout@v4

- name: Set up CodeQL CLI
uses: ./.github/actions/fetch-codeql

- name: Enable problem matchers in repository
shell: bash
run: 'find .github/problem-matchers -name \*.json -exec echo "::add-matcher::{}" \;'

- name: Build
run: |
cd go
make

- name: Check that all Go code is autoformatted
run: |
cd go
make check-formatting

- name: Compile qhelp files to markdown
run: |
cd go
env QHELP_OUT_DIR=qhelp-out make qhelp-to-markdown

- name: Upload qhelp markdown
uses: actions/upload-artifact@v3
- name: Run tests
uses: ./go/actions/test
with:
name: qhelp-markdown
path: go/qhelp-out/**/*.md

- name: Cache compilation cache
id: query-cache
uses: ./.github/actions/cache-query-compilation
with:
key: go-qltest

- name: Test
run: |
cd go
make test cache="${{ steps.query-cache.outputs.cache-dir }}"
run-code-checks: true
89 changes: 7 additions & 82 deletions go/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,30 +1,10 @@
all: extractor ql/lib/go.dbscheme

ifeq ($(OS),Windows_NT)
EXE = .exe
CODEQL_PLATFORM = win64
else
EXE =
UNAME_S := $(shell uname -s)
ifeq ($(UNAME_S),Linux)
CODEQL_PLATFORM = linux64
endif
ifeq ($(UNAME_S),Darwin)
CODEQL_PLATFORM = osx64
endif
endif

CODEQL_TOOLS = $(addprefix codeql-tools/,autobuild.cmd autobuild.sh baseline-config-empty.json baseline-config-vendor.json configure-baseline.cmd configure-baseline.sh identify-environment.cmd identify-environment.sh index.cmd index.sh pre-finalize.cmd pre-finalize.sh tracing-config.lua)
all: gen extractor

EXTRACTOR_PACK_OUT = build/codeql-extractor-go

BINARIES = go-extractor go-tokenizer go-autobuilder go-build-runner go-bootstrap go-gen-dbscheme

.PHONY: tools tools-codeql tools-codeql-full clean autoformat \
tools-linux64 tools-osx64 tools-win64 check-formatting
.PHONY: extractor gen clean autoformat check-formatting

clean:
rm -rf tools/bin tools/linux64 tools/osx64 tools/win64 tools/net tools/opencsv
rm -rf $(EXTRACTOR_PACK_OUT) build/stats build/testdb

autoformat:
Expand All @@ -47,66 +27,11 @@ endif
qhelp-to-markdown:
scripts/qhelp-to-markdown.sh ql/src "$(QHELP_OUT_DIR)"

tools: tools-codeql tools/tokenizer.jar

.PHONY: $(addsuffix $(EXE),$(addprefix tools/bin/,$(BINARIES)))
$(addsuffix $(EXE),$(addprefix tools/bin/,$(BINARIES))):
go build -C extractor -mod=vendor -o ../$@ ./cli/$(basename $(@F))

tools-codeql: tools-$(CODEQL_PLATFORM)

tools-codeql-full: tools-linux64 tools-osx64 tools-win64

tools-linux64: $(addprefix tools/linux64/,$(BINARIES))

.PHONY: $(addprefix tools/linux64/,$(BINARIES))
$(addprefix tools/linux64/,$(BINARIES)):
GOOS=linux GOARCH=amd64 go build -C extractor -mod=vendor -o ../$@ ./cli/$(@F)

tools-osx64: $(addprefix tools/osx64/,$(BINARIES))

.PHONY: $(addprefix tools/osx64/,$(BINARIES))
$(addprefix tools/osx64/,$(BINARIES)):
GOOS=darwin GOARCH=amd64 go build -C extractor -mod=vendor -o ../$@.amd64 ./cli/$(@F)
GOOS=darwin GOARCH=arm64 go build -C extractor -mod=vendor -o ../$@.arm64 ./cli/$(@F)
lipo -create $@.amd64 $@.arm64 -output $@
rm $@.amd64 $@.arm64

tools-win64: $(addsuffix .exe,$(addprefix tools/win64/,$(BINARIES)))

.PHONY: $(addsuffix .exe,$(addprefix tools/win64/,$(BINARIES)))
$(addsuffix .exe,$(addprefix tools/win64/,$(BINARIES))):
env GOOS=windows GOARCH=amd64 go build -C extractor -mod=vendor -o ../$@ ./cli/$(basename $(@F))

.PHONY: extractor-common extractor extractor-full
extractor-common: codeql-extractor.yml LICENSE ql/lib/go.dbscheme \
tools/tokenizer.jar $(CODEQL_TOOLS)
rm -rf $(EXTRACTOR_PACK_OUT)
mkdir -p $(EXTRACTOR_PACK_OUT)
cp codeql-extractor.yml LICENSE ql/lib/go.dbscheme ql/lib/go.dbscheme.stats $(EXTRACTOR_PACK_OUT)
mkdir $(EXTRACTOR_PACK_OUT)/tools
cp -r tools/tokenizer.jar $(CODEQL_TOOLS) $(EXTRACTOR_PACK_OUT)/tools
cp -r downgrades $(EXTRACTOR_PACK_OUT)

extractor: extractor-common tools-codeql
cp -r tools/$(CODEQL_PLATFORM) $(EXTRACTOR_PACK_OUT)/tools

extractor-full: extractor-common tools-codeql-full
cp -r $(addprefix tools/,linux64 osx64 win64) $(EXTRACTOR_PACK_OUT)/tools

tools/tokenizer.jar: tools/net/sourceforge/pmd/cpd/GoLanguage.class
jar cf $@ -C tools net
jar uf $@ -C tools opencsv

tools/net/sourceforge/pmd/cpd/GoLanguage.class: extractor/net/sourceforge/pmd/cpd/GoLanguage.java
javac -cp extractor -d tools $<
rm tools/net/sourceforge/pmd/cpd/AbstractLanguage.class
rm tools/net/sourceforge/pmd/cpd/SourceCode.class
rm tools/net/sourceforge/pmd/cpd/TokenEntry.class
rm tools/net/sourceforge/pmd/cpd/Tokenizer.class
extractor:
bazel run :create-extractor-pack

ql/lib/go.dbscheme: tools/$(CODEQL_PLATFORM)/go-gen-dbscheme$(EXE)
$< $@
gen:
bazel run :gen

build/stats/src.stamp:
mkdir -p $(@D)/src
Expand All @@ -123,7 +48,7 @@ test: all build/testdb/check-upgrade-path
codeql test run -j0 ql/test --search-path build/codeql-extractor-go --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 build/codeql-extractor-go --consistency-queries ql/test/consistency --compilation-cache=$(cache)
cd extractor; go test -mod=vendor ./...
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
76 changes: 76 additions & 0 deletions go/actions/test/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
name: Test go extractor
redsun82 marked this conversation as resolved.
Show resolved Hide resolved
description: Run build, QL tests and optionally basic code sanity checks (formatting and generation)
redsun82 marked this conversation as resolved.
Show resolved Hide resolved
inputs:
go-version:
redsun82 marked this conversation as resolved.
Show resolved Hide resolved
description: Which Go version to use for running the tests
required: false
default: ~1.22.0
run-code-checks:
description: Whether to run formatting, code and qhelp generation checks
required: false
default: false
runs:
using: composite
steps:
- name: Set up Go
redsun82 marked this conversation as resolved.
Show resolved Hide resolved
uses: actions/setup-go@v5
with:
go-version: ${{ inputs.go-version }}
redsun82 marked this conversation as resolved.
Show resolved Hide resolved
cache: false
id: go

- name: Set up CodeQL CLI
uses: ./.github/actions/fetch-codeql

- name: Enable problem matchers in repository
shell: bash
run: 'find .github/problem-matchers -name \*.json -exec echo "::add-matcher::{}" \;'

- name: Build
shell: bash
run: |
bazel run go:create-extractor-pack
Copy link
Member

Choose a reason for hiding this comment

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

It may be good to add a comment here noting that this explicitly does not run make (and therefore no bazel run :gen) because we want generated files to be checked-in. This uses whatever is checked-in for the extractor pack and the later check then complains if the generated files aren't up-to-date.

Related question: Might we want the "Check checked-in generated code" step to take place before "Build" to fail the build if the generated files are not up-to-date, rather than performing this step with whatever is checked-in?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm, yeah, it makes sense to do that I think, thanks!


- name: Check that all Go code is autoformatted
if: inputs.run-code-checks == 'true'
shell: bash
run: |
cd go
make check-formatting

- name: Check checked-in generated code
if: inputs.run-code-checks == 'true'
shell: bash
run: |
bazel run go:gen
git add .
git diff --exit-code HEAD || (
echo "please run bazel run //go:gen"
exit 1
)
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to change the conditions to inputs.run-code-checks == 'true' && always() so that they run, even if a previous step failed? That way the checks would be performed, even if the build / a previous check fails and save us from potentially needing multiple CI runs / incremental fixes. (Would need to check that the proposed conditions work as intended, since always() is sort of magical.)

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 suggestion, but I guess that this shouldn't be required if we move this check up as you suggested

Copy link
Member

Choose a reason for hiding this comment

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

I think it would probably still make sense to have for the "Check checked-in generated code" step, since it would be nice to run it even if "Check that all Go code is autoformatted" fails so that potentially two issues can become apparent from one CI run.

Copy link
Contributor Author

@redsun82 redsun82 May 8, 2024 •
edited

Choose a reason for hiding this comment

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

Indeed. As far as I know the correct magic github action incantation to get that is !cancelled() (as otherwise always() would run even when the workflow got cancelled, which is not ideal, and cancellation is forcibly done any way after a short timeout)


- name: Compile qhelp files to markdown
if: inputs.run-code-checks == 'true'
Copy link
Member

Choose a reason for hiding this comment

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

Same as above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, this on the other hand might make sense to render independent... but maybe we could do that with a separate parallel job?

Copy link
Member

Choose a reason for hiding this comment

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

A separate parallel job would work, too. The downside to doing that may be that it requires the overhead of checking out the repo on a separate runner. It's a question of whether we care about that overhead or not. I have no strong opinion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ended up keeping it here, as there was also the go setup being required, which I prefer having shared here with one place where the go version should be updated.

shell: bash
run: |
cd go
env QHELP_OUT_DIR=qhelp-out make qhelp-to-markdown

- name: Upload qhelp markdown
if: inputs.run-code-checks == 'true'
Copy link
Member

Choose a reason for hiding this comment

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

Same as above

uses: actions/upload-artifact@v3
with:
name: qhelp-markdown
path: go/qhelp-out/**/*.md

- name: Cache compilation cache
id: query-cache
uses: ./.github/actions/cache-query-compilation
with:
key: go-qltest

- name: Test
shell: bash
run: |
cd go
make test cache="${{ steps.query-cache.outputs.cache-dir }}"