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.

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

Merged
merged 8 commits into from
Jun 26, 2020

Conversation

asraa
Copy link
Contributor

@asraa asraa commented Jun 15, 2020

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

Signed-off-by: Asra Ali <asraa@google.com>
@htuch htuch self-assigned this Jun 15, 2020
@htuch htuch self-requested a review June 15, 2020 15:57
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>
@asraa
Copy link
Contributor Author

asraa commented Jun 18, 2020

@jhutchings1 The results of the CodeQL build display on my fork https://github.com/asraa/envoy/runs/781027478?check_suite_focus=true
Is that expected since the PR is from my fork?

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.

Copy link
Member

@htuch htuch left a 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

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
Copy link
Member

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?

@jhutchings1
Copy link

@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 push and pull_request events and just moving to a daily schedule. That'll keep the insights fresh, but you don't get the really nice PR analysis

@htuch
Copy link
Member

htuch commented Jun 19, 2020

@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>
@asraa
Copy link
Contributor Author

asraa commented Jun 22, 2020

Done! I did a daily schedule of http code, and put the codecs on push schedule.

Signed-off-by: Asra Ali <asraa@google.com>
@htuch
Copy link
Member

htuch commented Jun 22, 2020

LGTM, good to merge once @jhutchings1 OKs.

jhutchings1
jhutchings1 previously approved these changes Jun 22, 2020
@@ -0,0 +1,55 @@
on:
push:

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!

Copy link
Contributor Author

@asraa asraa Jun 22, 2020

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>
Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@asraa asraa changed the title [draft] add working codeql build [tools] add working codeql build Jun 24, 2020
@htuch htuch merged commit e959577 into envoyproxy:master Jun 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants