-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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.
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
[tools] add working codeql build #11590
Conversation
Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: Asra Ali <asraa@google.com>
@jhutchings1 The results of the CodeQL build display on my fork https://github.com/asraa/envoy/runs/781027478?check_suite_focus=true Question for review, just the http core takes about 3 hours for the analysis. I would say to only trigger on post-submit if that's possible but that defeats using it for CI. Using our own runners would speed this up and free resources up, but I have been headed about opening code execution via that pathway. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, will wait for @jhutchings1 and you to give the go ahead for merge.
/wait
.github/workflows/codeql-common.yml
Outdated
sudo apt-get update && sudo apt-get install libtool cmake automake autoconf make ninja-build curl unzip virtualenv openjdk-11-jdk build-essential libc++1 | ||
mkdir -p bin/clang9 | ||
cd bin/clang9 | ||
wget https://releases.llvm.org/9.0.0/clang+llvm-9.0.0-x86_64-linux-gnu-ubuntu-18.04.tar.xz |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to bump to Clang 10 given we're now there with CI, as of today?
@asraa Yes, that's by design since you're using the fork. Once you merge, the results will pop in the main repo here. Eek, 3 hours is definitely long. For that kind of runtime, you're probably best off removing the |
@asraa could we have two builds? One that is descoped to some small, interesting target, e.g. router or codecs, whatever is safety critical and has high impact for time spent, and another massive build that runs only on master? Whatever you think is best; I'm just keen to see this awesome go live :-D |
Signed-off-by: Asra Ali <asraa@google.com>
Done! I did a daily schedule of http code, and put the codecs on push schedule. |
Signed-off-by: Asra Ali <asraa@google.com>
LGTM, good to merge once @jhutchings1 OKs. |
@@ -0,0 +1,55 @@ | |||
on: | |||
push: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you need to filter this further so it only runs when there are changes on the components you're building on push, you could do so with this syntax: https://help.github.com/en/actions/reference/workflow-syntax-for-github-actions#onpushpull_requestpaths. Otherwise, LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the link! That's great. In a follow-up I'll start making push workflows for different sub-components when there are changes there. So far the build runs when there's any changes in the core.
Signed-off-by: Asra Ali <asraa@google.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Commit Message: Add working CodeQL database build for http core.
Risk level: low
Testing: Build succeeds.
The results show on my fork for some reason https://github.com/asraa/envoy/runs/781027478?check_suite_focus=true
Signed-off-by: Asra Ali asraa@google.com