From: Lars Schneider <larsxschneider@gmail.com>
To: Samuel Lijin <sxlijin@gmail.com>
Cc: "git@vger.kernel.org" <git@vger.kernel.org>, Jeff King <peff@peff.net>
Subject: Re: [PATCH] travis-ci: run scan-build every time
Date: Sat, 25 Feb 2017 22:48:52 +0100 [thread overview]
Message-ID: <BAB1EE0E-B258-4108-AE24-110172086DE4@gmail.com> (raw)
In-Reply-To: <CAJZjrdXP3n5fOLx4rEEkbJT7JBMPUqk4Qdutm6KpvMVUMwCSPQ@mail.gmail.com>
> On 24 Feb 2017, at 18:29, Samuel Lijin <sxlijin@gmail.com> wrote:
>
> Introduces the scan-build static code analysis tool from the Clang
> project to all Travis CI builds. Installs clang (since scan-build
> needs clang as a dependency) to make this possible (on macOS, also
> updates PATH to allow scan-build to be invoked without referencing the
> full path).
This is a pretty neat idea. However, I think this should become a
dedicated job in a TravisCI build (similar to the Documentation job [1])
because:
a) We don't want to build and test a scan-build version of Git (AFAIK
scan-build kind of proxies the compiler to do its job - I don't if
this has any side effects)
b) We don't want to slow down the other builds
c) It should be enough to run scan-build once on Linux per build
I ran scan-build on the current master and it detected 72 potential bugs [2].
I looked through a few of them and they seem to be legitimate. If the list agrees
that running scan-build is a useful thing and that these problems should be fixed
then we could:
(1) Add scan-build check to Travis CI but only print errors as warning
(2) Fix the 72 existing bugs over time
(3) Turn scan-build warnings into errors
[1] https://github.com/git/git/blob/e7e07d5a4fcc2a203d9873968ad3e6bd4d7419d7/.travis.yml#L42-L53
[2] https://larsxschneider.github.io/git-scan/
> ---
>
> A build with this patch can be found at [1]. Note that if reports *are*
> generated, this doesn't allow us to access them, and if we dumped
> the reports as build artifacts, I'm not sure where we would want to
> dump them to.
We could upload the results to a Git repo and then use GitHub pages to serve
it. I did that with my run here: https://larsxschneider.github.io/git-scan/
> It's worth noting that there seems to be a weird issue with scan-build
> where it *will* generate a report for something locally, but won't do it
> on Travis. See [2] for an example where I have a C program with a
> very obvious memory leak but scan-build on Travis doesn't generate
> a report (despite complaining about it in stdout), even though it does
> on my local machine.
>
> [1] https://travis-ci.org/sxlijin/git/builds/204853233
> [2] https://travis-ci.org/sxlijin/travis-testing/jobs/205025319#L331-L342
Scan-build stores the report in some temp folder. I assume you can't access
this folder on TravisCI. Try the scan-build option "-o scan-build-results"
to store the report in the local directory.
>
> .travis.yml | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/.travis.yml b/.travis.yml
> index 9c63c8c3f..1038b1b3d 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -20,6 +20,7 @@ addons:
> - language-pack-is
> - git-svn
> - apache2
> + - clang
>
> env:
> global:
> @@ -78,9 +79,8 @@ before_install:
> brew update --quiet
> # Uncomment this if you want to run perf tests:
> # brew install gnu-time
> - brew install git-lfs gettext
> - brew link --force gettext
> - brew install caskroom/cask/perforce
> + brew install git-lfs gettext caskroom/cask/perforce llvm
> + brew link --force gettext llvm
This wouldn't be necessary if we only scan on Linux.
> ;;
> esac;
> echo "$(tput setaf 6)Perforce Server Version$(tput sgr0)";
> @@ -92,9 +92,9 @@ before_install:
> mkdir -p $HOME/travis-cache;
> ln -s $HOME/travis-cache/.prove t/.prove;
>
> -before_script: make --jobs=2
> +before_script: scan-build make --jobs=2
I think we should run it like this:
scan-build -analyze-headers --status-bugs --keep-going --force-analyze-debug-code make --jobs=2
This way TravisCI would be notified via the return code if scan-build detected
errors I think.
> -script: make --quiet test
> +script: scan-build make --quiet test
Why do you want to scan the tests?
Cheers,
Lars
next prev parent reply other threads:[~2017-02-25 21:56 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-24 17:29 [PATCH] travis-ci: run scan-build every time Samuel Lijin
2017-02-25 21:48 ` Lars Schneider [this message]
2017-02-25 22:31 ` Jeff King
2017-02-25 23:02 ` Lars Schneider
2017-02-26 2:09 ` Samuel Lijin
2017-02-26 14:12 ` Lars Schneider
2017-02-27 0:34 ` Samuel Lijin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=BAB1EE0E-B258-4108-AE24-110172086DE4@gmail.com \
--to=larsxschneider@gmail.com \
--cc=git@vger.kernel.org \
--cc=peff@peff.net \
--cc=sxlijin@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).