{"thread":{"id":"47980","subject":"[GSoC] [PATCH] travis-ci: added clang static analysis","startedAt":"2018-03-05T20:05:10Z","lastAt":"2018-04-02T20:08:21Z","messageCount":9,"participants":["SiddharthaMishra","Junio C Hamano","Siddhartha Mishra","Johannes Schindelin","Lars Schneider"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"341056","messageId":"20180305200400.3769-1-sidm1999@gmail.com","threadId":"47980","inReplyTo":null,"subject":"[GSoC] [PATCH] travis-ci: added clang static analysis","fromName":"SiddharthaMishra","fromEmail":"sidm1999@gmail.com","sentAt":"2018-03-05T20:04:00Z","receivedAt":"2018-03-05T20:05:10Z","isPatch":true,"sender":{"key":"sidm1999@gmail.com","avatar":null},"body":"Added a job to run clang static code analysis on the master and maint branch\n\nSigned-off-by: SiddharthaMishra \n---\n .travis.yml | 17 ++++++++++++++++-\n ci/run-static-analysis.sh | 9 ++++++++-\n 2 files changed, 24 insertions(+), 2 deletions(-)\n\ndiff --git a/.travis.yml b/.travis.yml\nindex 4684b3f4f..9b891d182 100644\n--- a/.travis.yml\n+++ b/.travis.yml\n@@ -48,7 +48,7 @@ matrix:\n before_install:\n before_script:\n script: ci/run-linux32-docker.sh\n- - env: jobname=StaticAnalysis\n+ - env: jobname=CocciStaticAnalysis\n os: linux\n compiler:\n addons:\n@@ -59,6 +59,21 @@ matrix:\n before_script:\n script: ci/run-static-analysis.sh\n after_failure:\n+ - if: branch IN (master, maint)\n+ env: jobname=ClangStaticAnalysis\n+ os: linux\n+ compiler:\n+ add_ons:\n+ apt:\n+ sources:\n+ - ubuntu-toolchain-r-test\n+ - llvm-toolchain-trusty\n+ packages:\n+ - clang\n+ before_install:\n+ before_script:\n+ script: ci/run-static-analysis.sh\n+ after_failure:\n - env: jobname=Documentation\n os: linux\n compiler:\ndiff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh\nindex fe4ee4e06..6ae032f54 100755\n--- a/ci/run-static-analysis.sh\n+++ b/ci/run-static-analysis.sh\n@@ -5,6 +5,13 @@\n \n . ${0%/*}/lib-travisci.sh\n \n-make coccicheck\n+case \"$jobname\" in\n+ClangStaticAnalysis)\n+\tscan-build -analyze-headers --status-bugs make\n+\t;;\n+CocciStaticAnalysis)\n+\tmake coccicheck\n+\t;;\n+esac\n \n save_good_tree\n-- \n2.16.2.248.ge2408a6f7.dirty\n\n"},{"id":"341087","messageId":"xmqqpo4i6lj0.fsf@gitster-ct.c.googlers.com","threadId":"47980","inReplyTo":"20180305200400.3769-1-sidm1999@gmail.com","subject":"Re: [GSoC] [PATCH] travis-ci: added clang static analysis","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-06T01:27:31Z","receivedAt":"2018-03-06T01:27:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SiddharthaMishra writes:\n\n> Added a job to run clang static code analysis on the master and maint branch\n>\n> Signed-off-by: SiddharthaMishra \n> ---\n\nWhy on 'master' and 'maint' and not others? Quite frankly, I find\nthis choice of branches rather odd, as these two branches are not\nwhere the real development happens. If we do not want to increase\nthe number of jobs and limit the test only to a single branch, I\nwould probably pick 'next', and if we can afford two, probably\n'pu' and 'next'.\n\n"},{"id":"341107","messageId":"CAMOtk6BafDOX6PkOs=KVPp5rf1wF3Zq9ZWMAiKgw+EN2KTsndg@mail.gmail.com","threadId":"47980","inReplyTo":"xmqqpo4i6lj0.fsf@gitster-ct.c.googlers.com","subject":"Re: [GSoC] [PATCH] travis-ci: added clang static analysis","fromName":"Siddhartha Mishra","fromEmail":"sidm1999@gmail.com","sentAt":"2018-03-06T08:34:28Z","receivedAt":"2018-03-06T08:34:34Z","isPatch":true,"sender":{"key":"sidm1999@gmail.com","avatar":null},"body":"On Tue, Mar 6, 2018 at 6:57 AM, Junio C Hamano wrote:\n> SiddharthaMishra writes:\n>\n>> Added a job to run clang static code analysis on the master and maint branch\n>>\n>> Signed-off-by: SiddharthaMishra \n>> ---\n>\n> Why on 'master' and 'maint' and not others? Quite frankly, I find\n> this choice of branches rather odd, as these two branches are not\n> where the real development happens. If we do not want to increase\n> the number of jobs and limit the test only to a single branch, I\n> would probably pick 'next', and if we can afford two, probably\n> 'pu' and 'next'.\n\nI might have been misinterpreting it, but I did so because that's what\nit said in the microproject description. Thinking about it, I guess it\ndoes makes more sense to work on the other two branches instead. Are\nthere any other glaring issues you see in the code?\n"},{"id":"341200","messageId":"nycvar.QRO.7.76.6.1803071416350.20700@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"47980","inReplyTo":"CAMOtk6BafDOX6PkOs=KVPp5rf1wF3Zq9ZWMAiKgw+EN2KTsndg@mail.gmail.com","subject":"Re: [GSoC] [PATCH] travis-ci: added clang static analysis","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-03-07T13:16:57Z","receivedAt":"2018-03-07T13:17:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 6 Mar 2018, Siddhartha Mishra wrote:\n\n> Are there any other glaring issues you see in the code?\n\nThe rest looks good to me!\n\nCiao,\nJohannes\n"},{"id":"341471","messageId":"89AEA176-2D3F-4271-958E-1C6BCC944842@gmail.com","threadId":"47980","inReplyTo":"20180305200400.3769-1-sidm1999@gmail.com","subject":"Re: [GSoC] [PATCH] travis-ci: added clang static analysis","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2018-03-12T10:19:35Z","receivedAt":"2018-03-12T10:19:44Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"Hi,\n\nThat looks interesting but I agree with Dscho that we should not limit\nthis to master/maint.\n\nI assume you did run this on TravisCI already? Can you share a link?\nI assume you did find errors? Can we fix them or are there too many?\nIf there are existing errors, how do we define a \"successful\" build?\n\nThanks for working on this,\nLars\n\n> On 05 Mar 2018, at 21:04, SiddharthaMishra wrote:\n> \n> Added a job to run clang static code analysis on the master and maint branch\n> \n> Signed-off-by: SiddharthaMishra \n> ---\n> .travis.yml | 17 ++++++++++++++++-\n> ci/run-static-analysis.sh | 9 ++++++++-\n> 2 files changed, 24 insertions(+), 2 deletions(-)\n> \n> diff --git a/.travis.yml b/.travis.yml\n> index 4684b3f4f..9b891d182 100644\n> --- a/.travis.yml\n> +++ b/.travis.yml\n> @@ -48,7 +48,7 @@ matrix:\n> before_install:\n> before_script:\n> script: ci/run-linux32-docker.sh\n> - - env: jobname=StaticAnalysis\n> + - env: jobname=CocciStaticAnalysis\n> os: linux\n> compiler:\n> addons:\n> @@ -59,6 +59,21 @@ matrix:\n> before_script:\n> script: ci/run-static-analysis.sh\n> after_failure:\n> + - if: branch IN (master, maint)\n> + env: jobname=ClangStaticAnalysis\n> + os: linux\n> + compiler:\n> + add_ons:\n> + apt:\n> + sources:\n> + - ubuntu-toolchain-r-test\n> + - llvm-toolchain-trusty\n> + packages:\n> + - clang\n> + before_install:\n> + before_script:\n> + script: ci/run-static-analysis.sh\n> + after_failure:\n> - env: jobname=Documentation\n> os: linux\n> compiler:\n> diff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh\n> index fe4ee4e06..6ae032f54 100755\n> --- a/ci/run-static-analysis.sh\n> +++ b/ci/run-static-analysis.sh\n> @@ -5,6 +5,13 @@\n> \n> . ${0%/*}/lib-travisci.sh\n> \n> -make coccicheck\n> +case \"$jobname\" in\n> +ClangStaticAnalysis)\n> +\tscan-build -analyze-headers --status-bugs make\n> +\t;;\n> +CocciStaticAnalysis)\n> +\tmake coccicheck\n> +\t;;\n> +esac\n> \n> save_good_tree\n> -- \n> 2.16.2.248.ge2408a6f7.dirty\n> \n\n"},{"id":"341556","messageId":"CAMOtk6Df_7Jb9bX=8PfB9e4-5rRi8QSmXEK7=DmZui6LNBMSLQ@mail.gmail.com","threadId":"47980","inReplyTo":"89AEA176-2D3F-4271-958E-1C6BCC944842@gmail.com","subject":"Re: [GSoC] [PATCH] travis-ci: added clang static analysis","fromName":"Siddhartha Mishra","fromEmail":"sidm1999@gmail.com","sentAt":"2018-03-13T17:45:07Z","receivedAt":"2018-03-13T17:45:20Z","isPatch":true,"sender":{"key":"sidm1999@gmail.com","avatar":null},"body":"On Mon, Mar 12, 2018 at 3:49 PM, Lars Schneider\n wrote:\n> Hi,\n>\n> That looks interesting but I agree with Dscho that we should not limit\n> this to master/maint.\n>\n> I assume you did run this on TravisCI already? Can you share a link?\n> I assume you did find errors? Can we fix them or are there too many?\n> If there are existing errors, how do we define a \"successful\" build?\n>\n> Thanks for working on this,\n> Lars\n>\n\nThanks for the reply,\n\nI assume there will be false positives in the code which we can't fix\nby making small modifications to the code as recommended in the FAQ\n(https://clang-analyzer.llvm.org/faq.html). According to the FAQ,\nthere is no solid mechanism for suppressing a specific warning, so are\noptions are limited. Some of the things which might help reduce the\nnoise are:\n\n1) To add specific tags in our source code to tell the analyzer to\nignore the code. This is probably a bad idea since it is intrusive and\nforces changes to the actual source code which only affect one task.\n\n2) Count the number of bugs in the previous pushed build and fail the\nbuild if the number of bugs increases. It doesn't help remove the\nnoise from the error log but it does tell you if you've added more\nbugs. However if you add a bug and remove one, it'll pass the job and\nmight mislead you into thinking that the code is correct.\n\n3) Write a script to check the diff of the error log from that of the\nprevious pushed build(ignoring the line numbers). I haven't thought\nabout how exactly it would be implemented so I'm not commenting on it.\n\nIs there a better solution that I'm missing or should I try coming up\nwith a script to come up the diff?\n\nThanks for the time,\nSiddhartha\n\nOn Mon, Mar 12, 2018 at 3:49 PM, Lars Schneider\n wrote:\n> Hi,\n>\n> That looks interesting but I agree with Dscho that we should not limit\n> this to master/maint.\n>\n> I assume you did run this on TravisCI already? Can you share a link?\n> I assume you did find errors? Can we fix them or are there too many?\n> If there are existing errors, how do we define a \"successful\" build?\n>\n> Thanks for working on this,\n> Lars\n>\n>> On 05 Mar 2018, at 21:04, SiddharthaMishra wrote:\n>>\n>> Added a job to run clang static code analysis on the master and maint branch\n>>\n>> Signed-off-by: SiddharthaMishra \n>> ---\n>> .travis.yml | 17 ++++++++++++++++-\n>> ci/run-static-analysis.sh | 9 ++++++++-\n>> 2 files changed, 24 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/.travis.yml b/.travis.yml\n>> index 4684b3f4f..9b891d182 100644\n>> --- a/.travis.yml\n>> +++ b/.travis.yml\n>> @@ -48,7 +48,7 @@ matrix:\n>> before_install:\n>> before_script:\n>> script: ci/run-linux32-docker.sh\n>> - - env: jobname=StaticAnalysis\n>> + - env: jobname=CocciStaticAnalysis\n>> os: linux\n>> compiler:\n>> addons:\n>> @@ -59,6 +59,21 @@ matrix:\n>> before_script:\n>> script: ci/run-static-analysis.sh\n>> after_failure:\n>> + - if: branch IN (master, maint)\n>> + env: jobname=ClangStaticAnalysis\n>> + os: linux\n>> + compiler:\n>> + add_ons:\n>> + apt:\n>> + sources:\n>> + - ubuntu-toolchain-r-test\n>> + - llvm-toolchain-trusty\n>> + packages:\n>> + - clang\n>> + before_install:\n>> + before_script:\n>> + script: ci/run-static-analysis.sh\n>> + after_failure:\n>> - env: jobname=Documentation\n>> os: linux\n>> compiler:\n>> diff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh\n>> index fe4ee4e06..6ae032f54 100755\n>> --- a/ci/run-static-analysis.sh\n>> +++ b/ci/run-static-analysis.sh\n>> @@ -5,6 +5,13 @@\n>>\n>> . ${0%/*}/lib-travisci.sh\n>>\n>> -make coccicheck\n>> +case \"$jobname\" in\n>> +ClangStaticAnalysis)\n>> + scan-build -analyze-headers --status-bugs make\n>> + ;;\n>> +CocciStaticAnalysis)\n>> + make coccicheck\n>> + ;;\n>> +esac\n>>\n>> save_good_tree\n>> --\n>> 2.16.2.248.ge2408a6f7.dirty\n>>\n>\n"},{"id":"341557","messageId":"CAMOtk6AYFi7XeqqhUT4wjoUZsfy8ZouJYwqWE7h3_zCtgXzaHQ@mail.gmail.com","threadId":"47980","inReplyTo":"CAMOtk6Df_7Jb9bX=8PfB9e4-5rRi8QSmXEK7=DmZui6LNBMSLQ@mail.gmail.com","subject":"Re: [GSoC] [PATCH] travis-ci: added clang static analysis","fromName":"Siddhartha Mishra","fromEmail":"sidm1999@gmail.com","sentAt":"2018-03-13T17:52:59Z","receivedAt":"2018-03-13T17:53:06Z","isPatch":true,"sender":{"key":"sidm1999@gmail.com","avatar":null},"body":"I'm extremely sorry, I forgot to attach my travis-ci job build with my\nprevious message. Here it is :\nhttps://travis-ci.org/SiddharthaMishra/git/jobs/349478746#L1204.\n\nI also apologize for not trimming the email properly in the previous message.\n\nSorry for the inconvenience,\nSiddhartha\n"},{"id":"343556","messageId":"74BBE628-B8FC-4699-9A23-F774D4908011@gmail.com","threadId":"47980","inReplyTo":"CAMOtk6Df_7Jb9bX=8PfB9e4-5rRi8QSmXEK7=DmZui6LNBMSLQ@mail.gmail.com","subject":"Re: [GSoC] [PATCH] travis-ci: added clang static analysis","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2018-04-01T14:39:52Z","receivedAt":"2018-04-01T14:40:06Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 13 Mar 2018, at 18:45, Siddhartha Mishra wrote:\n> \n> On Mon, Mar 12, 2018 at 3:49 PM, Lars Schneider\n> wrote:\n>> Hi,\n>> \n>> That looks interesting but I agree with Dscho that we should not limit\n>> this to master/maint.\n>> \n>> I assume you did run this on TravisCI already? Can you share a link?\n>> I assume you did find errors? Can we fix them or are there too many?\n>> If there are existing errors, how do we define a \"successful\" build?\n>> \n>> Thanks for working on this,\n>> Lars\n>> \n> \n> Thanks for the reply,\n> \n> I assume there will be false positives in the code which we can't fix\n> by making small modifications to the code as recommended in the FAQ\n> (https://clang-analyzer.llvm.org/faq.html). According to the FAQ,\n> there is no solid mechanism for suppressing a specific warning, so are\n> options are limited. Some of the things which might help reduce the\n> noise are:\n> \n> 1) To add specific tags in our source code to tell the analyzer to\n> ignore the code. This is probably a bad idea since it is intrusive and\n> forces changes to the actual source code which only affect one task.\n> \n> 2) Count the number of bugs in the previous pushed build and fail the\n> build if the number of bugs increases. It doesn't help remove the\n> noise from the error log but it does tell you if you've added more\n> bugs. However if you add a bug and remove one, it'll pass the job and\n> might mislead you into thinking that the code is correct.\n> \n> 3) Write a script to check the diff of the error log from that of the\n> previous pushed build(ignoring the line numbers). I haven't thought\n> about how exactly it would be implemented so I'm not commenting on it.\n> \n> Is there a better solution that I'm missing or should I try coming up\n> with a script to come up the diff?\n\nThat's a good summary and I don't see a better solution. While (3)\nsounds nice, I think (2) is the fastest/most pragmatic solution.\n\nWe already use the Travis cache [1]. You could use that mechanism to \nstore a file with the latest number of bugs in the cache directory \n$HOME/travis-cache\n\nIf the \"number of bugs\" file does not exist, then create it and don't\ncomplain. If the file exists and the previous number of bugs is higher\nor equal, then don't complain either. If the file exists and the previous\nnumber of bugs is lower, then let the build fail.\n\nDo you think that could work?\n\nCheers,\nLars \n\n[1] https://docs.travis-ci.com/user/caching/\n\n\n> \n> Thanks for the time,\n> Siddhartha\n> \n> On Mon, Mar 12, 2018 at 3:49 PM, Lars Schneider\n> wrote:\n>> Hi,\n>> \n>> That looks interesting but I agree with Dscho that we should not limit\n>> this to master/maint.\n>> \n>> I assume you did run this on TravisCI already? Can you share a link?\n>> I assume you did find errors? Can we fix them or are there too many?\n>> If there are existing errors, how do we define a \"successful\" build?\n>> \n>> Thanks for working on this,\n>> Lars\n>> \n>>> On 05 Mar 2018, at 21:04, SiddharthaMishra wrote:\n>>> \n>>> Added a job to run clang static code analysis on the master and maint branch\n>>> \n>>> Signed-off-by: SiddharthaMishra \n>>> ---\n>>> .travis.yml | 17 ++++++++++++++++-\n>>> ci/run-static-analysis.sh | 9 ++++++++-\n>>> 2 files changed, 24 insertions(+), 2 deletions(-)\n>>> \n>>> diff --git a/.travis.yml b/.travis.yml\n>>> index 4684b3f4f..9b891d182 100644\n>>> --- a/.travis.yml\n>>> +++ b/.travis.yml\n>>> @@ -48,7 +48,7 @@ matrix:\n>>> before_install:\n>>> before_script:\n>>> script: ci/run-linux32-docker.sh\n>>> - - env: jobname=StaticAnalysis\n>>> + - env: jobname=CocciStaticAnalysis\n>>> os: linux\n>>> compiler:\n>>> addons:\n>>> @@ -59,6 +59,21 @@ matrix:\n>>> before_script:\n>>> script: ci/run-static-analysis.sh\n>>> after_failure:\n>>> + - if: branch IN (master, maint)\n>>> + env: jobname=ClangStaticAnalysis\n>>> + os: linux\n>>> + compiler:\n>>> + add_ons:\n>>> + apt:\n>>> + sources:\n>>> + - ubuntu-toolchain-r-test\n>>> + - llvm-toolchain-trusty\n>>> + packages:\n>>> + - clang\n>>> + before_install:\n>>> + before_script:\n>>> + script: ci/run-static-analysis.sh\n>>> + after_failure:\n>>> - env: jobname=Documentation\n>>> os: linux\n>>> compiler:\n>>> diff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh\n>>> index fe4ee4e06..6ae032f54 100755\n>>> --- a/ci/run-static-analysis.sh\n>>> +++ b/ci/run-static-analysis.sh\n>>> @@ -5,6 +5,13 @@\n>>> \n>>> . ${0%/*}/lib-travisci.sh\n>>> \n>>> -make coccicheck\n>>> +case \"$jobname\" in\n>>> +ClangStaticAnalysis)\n>>> + scan-build -analyze-headers --status-bugs make\n>>> + ;;\n>>> +CocciStaticAnalysis)\n>>> + make coccicheck\n>>> + ;;\n>>> +esac\n>>> \n>>> save_good_tree\n>>> --\n>>> 2.16.2.248.ge2408a6f7.dirty\n>>> \n>> \n\n"},{"id":"343604","messageId":"CAMOtk6D9AtzNpufnv7hbCUAVYVgTiL-O_Ni=R0DB4LOHUNV=7A@mail.gmail.com","threadId":"47980","inReplyTo":"74BBE628-B8FC-4699-9A23-F774D4908011@gmail.com","subject":"Re: [GSoC] [PATCH] travis-ci: added clang static analysis","fromName":"Siddhartha Mishra","fromEmail":"sidm1999@gmail.com","sentAt":"2018-04-02T20:08:16Z","receivedAt":"2018-04-02T20:08:21Z","isPatch":true,"sender":{"key":"sidm1999@gmail.com","avatar":null},"body":"On Sun, Apr 1, 2018, 8:10 PM Lars Schneider wrote:\n\n>\n> That's a good summary and I don't see a better solution. While (3)\n> sounds nice, I think (2) is the fastest/most pragmatic solution.\n>\n> We already use the Travis cache [1]. You could use that mechanism to\n> store a file with the latest number of bugs in the cache directory\n> $HOME/travis-cache\n>\n> If the \"number of bugs\" file does not exist, then create it and don't\n> complain. If the file exists and the previous number of bugs is higher\n> or equal, then don't complain either. If the file exists and the previous\n> number of bugs is lower, then let the build fail.\n>\n> Do you think that could work?\n\nApologies for having sent this message twice, the last one wasn't in\nplain text and were therefore not properly forwarded to the mail\ngroup.\n\nIf we only consider the number of bugs in the last pushed commit, a\nsituation might happen that a commit creates a lot of bugs. If a\nsubsequent commit then fixes only a few of the previously created bugs\nthe build would still pass since the number of bugs from the last\ncommit has decreased.\n\nShould this be the intended behaviour or should adding the number of\nallowed bugs be a more deliberate process?\n\nThanks,\nSiddhartha\n"}]}