Page MenuHomePhabricator

[analyzer] SATest: Add initial docker infrastructure
ClosedPublic

Authored by vsavchenko on Jun 10 2020, 6:30 AM.

Details

Summary

Static analysis is very sensitive to environment.
OS and libraries installed can affect the results. This fact makes
it extremely hard to have a regression testing system that will
produce stable results.

For this very reason, this commit introduces a new dockerized testing
environment, so that every analyzer developer can check their changes
against previous analysis results.

Diff Detail

Event Timeline

vsavchenko created this revision.Jun 10 2020, 6:30 AM
NoQ added inline comments.Jun 18 2020, 7:17 AM
clang/utils/analyzer/Dockerfile
11

Ouch. I'm pretty bad with docker but is it possible to simply take a newer ubuntu?

16

Maybe put apt-get update on a separate line?

Also i think you mentioned offline that you want to freeze package versions here, can you add a FIXME?

clang/utils/analyzer/entrypoint.py
32

-DLLVM_ENABLE_ASSERTIONS=ON???

36

I think this one's on by default but i guess it wouldn't hurt^^

NoQ added inline comments.Jun 18 2020, 7:27 AM
clang/utils/analyzer/Dockerfile
16

can you add a FIXME?

Wait, you already did that in D81600, nvm.

vsavchenko marked 5 inline comments as done.Jun 18 2020, 7:50 AM
vsavchenko added inline comments.
clang/utils/analyzer/Dockerfile
11

This is the latest LTS version of Ubuntu. And actually that was one of the reasons I migrated to Fedora in my later pre-macOS years, Ubuntu is pretty bad with maintaining newer versions of packages.

16

It is actually one of the good practices for writing Docker files and related to the Docker's build cache.

It is even recommended to do every apt-related stuff on the same line. I went with one decision in between:
https://forums.docker.com/t/dockerfile-run-apt-get-install-all-packages-at-once-or-one-by-one/17191

clang/utils/analyzer/entrypoint.py
32

I was thinking about adding a separate list of options specifically for building. Assertions can significantly affect performance and I don't know if that should be a default.

NoQ accepted this revision.Jun 19 2020, 7:50 AM

Ok then!

clang/utils/analyzer/entrypoint.py
32

I still think assertions should be on by default; it's much worse when somebody accidentally tests a logic-related patch without assertions than when somebody tests a performance-related patch with assertions.

This revision is now accepted and ready to land.Jun 19 2020, 7:50 AM
This revision was automatically updated to reflect the committed changes.