{"thread":{"id":"50380","subject":"[PATCH 0/1] Using sparse in a CI job","startedAt":"2019-02-01T21:01:29Z","lastAt":"2019-02-03T15:43:48Z","messageCount":7,"participants":["Ramsay Jones","Luc Van Oostenryck","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"368371","messageId":"78f85f0b-176f-0b54-2ec8-4c13fd386733@ramsayjones.plus.com","threadId":"50380","inReplyTo":null,"subject":"[PATCH 0/1] Using sparse in a CI job","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2019-02-01T21:01:20Z","receivedAt":"2019-02-01T21:01:29Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\nI suspect that the Makefile sparse target is not easy to use in a CI\njob, since the 'sparse' program (via cgcc -no-compile) does not exit\nwith a non-zero value, even when issuing errors and warnings.\n\nThe way I use sparse, this is not an issue. I run sparse over the\nmaster branch, check that the output file has the no/expected\nerrors and warnings, then just diff the output files from the\nnext and pu branches. something like:\n\n  $ make sparse >sp-out 2>&1 # on master branch\n  $ vim sp-out # check for errors/warnings\n  ...\n  $ make sparse >nsp-out 2>&1 # on next branch\n  $ diff sp-out nsp-out # any new errors/warnings?\n  ...\n  $ make sparse >psp-out 2>&1 # on pu branch\n  $ diff nsp-out psp-out # any new errors/warnings?\n\nAt the moment, on Linux, the sp-out file is free from any sparse errors\nor warnings. So are next and pu:\n\n  $ grep error sp-out\n  $ grep warning sp-out\n  $ diff sp-out nsp-out\n  391a392\n  >     SP fuzz-commit-graph.c\n  $ diff nsp-out psp-out\n  170a171,180\n  >     SP trace2.c\n  >     SP trace2/tr2_cfg.c\n  >     SP trace2/tr2_dst.c\n  >     SP trace2/tr2_sid.c\n  >     SP trace2/tr2_tbuf.c\n  >     SP trace2/tr2_tgt_event.c\n  >     SP trace2/tr2_tgt_normal.c\n  >     SP trace2/tr2_tgt_perf.c\n  >     SP trace2/tr2_tls.c\n  >     SP trace2/tr2_verb.c\n  298a309\n  >     SP builtin/stash.c\n  375a387\n  >     SP t/helper/test-trace2.c\n  376a389\n  >     SP t/helper/test-xml-encode.c\n  $ \n\nHowever, if we go back a few days, then the pu branch was not clean:\n\n  $ diff nsp-out psp-out\n  18a20,24\n  >     SP change-table.c\n  > change-table.h:53:24: error: dubious one-bit signed bitfield\n  > change-table.h:54:25: error: dubious one-bit signed bitfield\n  > change-table.h:55:25: error: dubious one-bit signed bitfield\n  > change-table.h:56:26: error: dubious one-bit signed bitfield\n  69a76\n  >     GEN command-list.h\n  93a101,106\n  >     SP metacommit.c\n  > change-table.h:53:24: error: dubious one-bit signed bitfield\n  > change-table.h:54:25: error: dubious one-bit signed bitfield\n  > change-table.h:55:25: error: dubious one-bit signed bitfield\n  > change-table.h:56:26: error: dubious one-bit signed bitfield\n  >     SP metacommit-parser.c\n  170a184,193\n  >     SP trace2.c\n  >     SP trace2/tr2_cfg.c\n  >     SP trace2/tr2_dst.c\n  >     SP trace2/tr2_sid.c\n  >     SP trace2/tr2_tbuf.c\n  >     SP trace2/tr2_tgt_event.c\n  >     SP trace2/tr2_tgt_normal.c\n  >     SP trace2/tr2_tgt_perf.c\n  >     SP trace2/tr2_tls.c\n  >     SP trace2/tr2_verb.c\n  213a237\n  >     SP builtin/change.c\n  298a323\n  >     SP builtin/stash.c\n  375a401\n  >     SP t/helper/test-trace2.c\n  376a403,405\n  >     SP t/helper/test-xml-encode.c\n  > t/helper/test-xml-encode.c:29:40: warning: Using plain integer as NULL pointer\n  > t/helper/test-xml-encode.c:37:40: warning: Using plain integer as NULL pointer\n  $ \n \nNote that 'make' does not exit at the first 'error', since the command exit\ncode is zero (or not non-zero! :) ):\n \n  $ make change-table.sp\n      SP change-table.c\n  change-table.h:53:24: error: dubious one-bit signed bitfield\n  change-table.h:54:25: error: dubious one-bit signed bitfield\n  change-table.h:55:25: error: dubious one-bit signed bitfield\n  change-table.h:56:26: error: dubious one-bit signed bitfield\n  $ echo $?\n  0\n  $ \n\nWe can change that by passing '-Wsparse-error' to 'sparse':\n\n  $ make SPARSE_FLAGS=-Wsparse-error change-table.sp\n      SP change-table.c\n  change-table.h:53:24: error: dubious one-bit signed bitfield\n  change-table.h:54:25: error: dubious one-bit signed bitfield\n  change-table.h:55:25: error: dubious one-bit signed bitfield\n  change-table.h:56:26: error: dubious one-bit signed bitfield\n  Makefile:2729: recipe for target 'change-table.sp' failed\n  make: *** [change-table.sp] Error 1\n  $ echo $?\n  2\n  $ \n\nNote that '-Wsparse-error' not only returns a non-zero exit code (1), but\nit also changes a 'warning' into an 'error' (see above):\n  \n  $ make SPARSE_FLAGS=-Wsparse-error t/helper/test-xml-encode.sp\n      SP t/helper/test-xml-encode.c\n  t/helper/test-xml-encode.c:29:40: error: Using plain integer as NULL pointer\n  t/helper/test-xml-encode.c:37:40: error: Using plain integer as NULL pointer\n  Makefile:2729: recipe for target 't/helper/test-xml-encode.sp' failed\n  make: *** [t/helper/test-xml-encode.sp] Error 1\n  $ echo $?\n  2\n  $ \n\nUnfortunately, using SPARSE_FLAGS from the 'make' command-line is not\nideal, since it was not really intended to be used that way. This will\ncause problems for those files which have target specific settings for\nSPARSE_FLAGS. For example:\n\n  $ make pack-revindex.sp\n      SP pack-revindex.c\n  $ make SPARSE_FLAGS=-Wsparse-error pack-revindex.sp\n      SP pack-revindex.c\n  pack-revindex.c:65:23: error: memset with byte count of 262144\n  Makefile:2729: recipe for target 'pack-revindex.sp' failed\n  make: *** [pack-revindex.sp] Error 1\n  $ echo $?\n  2\n  $ \n\nSo, passing SPARSE_FLAGS on the command-line, effectively nullifies the\ntarget specific settings (making them useless).\n\nThis patch splits the duties of SPARSE_FLAGS, by introducing a separate\nSP_EXTRA_FLAGS variable, for use with the target specific settings. In\naddition, we use a conditional assignment (?=) of the default (empty)\nvalue of SPARSE_FLAGS, to allow setting the value of this variable from\nthe environment as well as the command-line. So, after this patch:\n\n  $ make SPARSE_FLAGS=-Wsparse-error pack-revindex.sp\n      SP pack-revindex.c\n  $ echo $?\n  0\n  $ \n\n  $ SPARSE_FLAGS=-Wsparse-error make pack-revindex.sp\n      SP pack-revindex.c\n  $ echo $?\n  0\n  $ \n\nNow, we should be able to run the sparse Makefile target in a CI job, and\nstill find all sparse errors and warnings (now marked as errors also),\nusing something like this:\n\n  $ SPARSE_FLAGS=-Wsparse-error make -k sparse >sp-out 2>&1\n\nNote that the '-k' argument to 'make' is now required. ;-)\n\n\nATB,\nRamsay Jones \n\nRamsay Jones (1):\n  Makefile: improve SPARSE_FLAGS customisation\n\n Makefile | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\n-- \n2.20.0\n"},{"id":"368383","messageId":"20190201223959.zmlyg4buwolw2b3p@ltop.local","threadId":"50380","inReplyTo":"78f85f0b-176f-0b54-2ec8-4c13fd386733@ramsayjones.plus.com","subject":"Re: [PATCH 0/1] Using sparse in a CI job","fromName":"Luc Van Oostenryck","fromEmail":"luc.vanoostenryck@gmail.com","sentAt":"2019-02-01T22:40:00Z","receivedAt":"2019-02-01T22:40:07Z","isPatch":true,"sender":{"key":"luc.vanoostenryck@gmail.com","avatar":"https://avatars.githubusercontent.com/u/16168334?v=4"},"body":"On Fri, Feb 01, 2019 at 09:01:20PM +0000, Ramsay Jones wrote:\n> \n> I suspect that the Makefile sparse target is not easy to use in a CI\n> job, since the 'sparse' program (via cgcc -no-compile) does not exit\n> with a non-zero value, even when issuing errors and warnings.\n\n...\n \n> We can change that by passing '-Wsparse-error' to 'sparse':\n> \n>   $ make SPARSE_FLAGS=-Wsparse-error change-table.sp\n>       SP change-table.c\n>   change-table.h:53:24: error: dubious one-bit signed bitfield\n>   change-table.h:54:25: error: dubious one-bit signed bitfield\n>   change-table.h:55:25: error: dubious one-bit signed bitfield\n>   change-table.h:56:26: error: dubious one-bit signed bitfield\n>   Makefile:2729: recipe for target 'change-table.sp' failed\n>   make: *** [change-table.sp] Error 1\n>   $ echo $?\n>   2\n>   $ \n> \n> Note that '-Wsparse-error' not only returns a non-zero exit code (1), but\n> it also changes a 'warning' into an 'error' (see above):\n\nYes, I know :(\nThe fact that, by default, sparse doesn't fail on errors is wanted\n(otherwise it would break the kernel compile). But that the only way\nto return an error is to use -Wsparse-error (which is supposed to\nreplace GCC's -Werror) is a real problem.\n\n-- Luc\n"},{"id":"368395","messageId":"20190202004145.GE10587@szeder.dev","threadId":"50380","inReplyTo":"78f85f0b-176f-0b54-2ec8-4c13fd386733@ramsayjones.plus.com","subject":"Re: [PATCH 0/1] Using sparse in a CI job","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-02-02T00:41:45Z","receivedAt":"2019-02-02T00:41:53Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Feb 01, 2019 at 09:01:20PM +0000, Ramsay Jones wrote:\n> I suspect that the Makefile sparse target is not easy to use in a CI\n> job, since the 'sparse' program (via cgcc -no-compile) does not exit\n> with a non-zero value, even when issuing errors and warnings.\n\nWe had the same issue with Coccinelle in the static analysis build job\non Travis CI, which was happily succeeding even when Coccinelle\nnoticed undesired code patterns entering the codebase.  We've dealt\nwith that by looking at the patch files generated by 'make\ncoccicheck': if they were empty, then all is well, if they are not,\nthen dump their contents into the log and 'exit 1'.  See 0860a7641b\n(travis-ci: fail if Coccinelle static analysis found something to\ntransform, 2018-07-23).\n\nI think we could do something like that with sparse as well.\n\n> At the moment, on Linux, the sp-out file is free from any sparse errors\n> or warnings. So are next and pu:\n> \n>   $ grep error sp-out\n>   $ grep warning sp-out\n\nOn 'master' I get:\n\n  $ grep error sp-out \n  $ grep warning sp-out \n  connect.c:652:40: warning: incorrect type in argument 2 (invalid types)\n  pack-revindex.c:65:23: warning: memset with byte count of 262144\n  unix-socket.c:83:26: warning: incorrect type in argument 2 (invalid types)\n  unix-socket.c:108:23: warning: incorrect type in argument 2 (invalid types)\n  daemon.c:1041:36: warning: incorrect type in argument 2 (invalid types)\n  daemon.c:1184:67: warning: incorrect type in argument 2 (invalid types)\n  imap-send.c:1022:42: warning: incorrect type in argument 2 (invalid types)\n  credential-cache--daemon.c:180:37: warning: incorrect type in argument 2 (invalid types)\n  $ sparse --version\n  v0.5.0\n\n"},{"id":"368422","messageId":"719468c8-3c62-96d6-57de-b0629fab0be3@ramsayjones.plus.com","threadId":"50380","inReplyTo":"20190202004145.GE10587@szeder.dev","subject":"Re: [PATCH 0/1] Using sparse in a CI job","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2019-02-03T01:49:37Z","receivedAt":"2019-02-03T01:49:42Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 02/02/2019 00:41, SZEDER Gábor wrote:\n> On Fri, Feb 01, 2019 at 09:01:20PM +0000, Ramsay Jones wrote:\n[snip]\n\n \n>> At the moment, on Linux, the sp-out file is free from any sparse errors\n>> or warnings. So are next and pu:\n>>\n>>   $ grep error sp-out\n>>   $ grep warning sp-out\n> \n> On 'master' I get:\n> \n>   $ grep error sp-out \n>   $ grep warning sp-out \n>   connect.c:652:40: warning: incorrect type in argument 2 (invalid types)\n>   pack-revindex.c:65:23: warning: memset with byte count of 262144\n>   unix-socket.c:83:26: warning: incorrect type in argument 2 (invalid types)\n>   unix-socket.c:108:23: warning: incorrect type in argument 2 (invalid types)\n>   daemon.c:1041:36: warning: incorrect type in argument 2 (invalid types)\n>   daemon.c:1184:67: warning: incorrect type in argument 2 (invalid types)\n>   imap-send.c:1022:42: warning: incorrect type in argument 2 (invalid types)\n>   credential-cache--daemon.c:180:37: warning: incorrect type in argument 2 (invalid types)\n>   $ sparse --version\n>   v0.5.0\n\nYeah, that version of sparse is a bit too old.\n\nIf memory serves (it may not), all of the 'argument 2 (invalid types)'\nerrors are caused by the glibc headers using a 'transparent union' to\ndefine the 'struct sockaddr' type. sparse could not handle that until\ncommit 7698bae699 (aka. v0.5.0-5-g7698bae). The only remaining warning\nwas addressed by commit bcfe020ed9 (aka. v0.5.1-rc1-22-gbcfe020) in\nsparse and commit 54360a1956 in git.\n\nSo, it seems you need at least v0.5.2 of sparse on your Linux system\n(which can't be too recent, or you would need v0.6.0).\n\nATB,\nRamsay Jones\n\n"},{"id":"368436","messageId":"20190203121207.GF10587@szeder.dev","threadId":"50380","inReplyTo":"719468c8-3c62-96d6-57de-b0629fab0be3@ramsayjones.plus.com","subject":"Re: [PATCH 0/1] Using sparse in a CI job","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-02-03T12:12:07Z","receivedAt":"2019-02-03T12:12:13Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Sun, Feb 03, 2019 at 01:49:37AM +0000, Ramsay Jones wrote:\n> On 02/02/2019 00:41, SZEDER Gábor wrote:\n> > On Fri, Feb 01, 2019 at 09:01:20PM +0000, Ramsay Jones wrote:\n> >> At the moment, on Linux, the sp-out file is free from any sparse errors\n> >> or warnings. So are next and pu:\n> >>\n> >>   $ grep error sp-out\n> >>   $ grep warning sp-out\n> > \n> > On 'master' I get:\n> > \n> >   $ grep error sp-out \n> >   $ grep warning sp-out \n> >   connect.c:652:40: warning: incorrect type in argument 2 (invalid types)\n> >   pack-revindex.c:65:23: warning: memset with byte count of 262144\n> >   unix-socket.c:83:26: warning: incorrect type in argument 2 (invalid types)\n> >   unix-socket.c:108:23: warning: incorrect type in argument 2 (invalid types)\n> >   daemon.c:1041:36: warning: incorrect type in argument 2 (invalid types)\n> >   daemon.c:1184:67: warning: incorrect type in argument 2 (invalid types)\n> >   imap-send.c:1022:42: warning: incorrect type in argument 2 (invalid types)\n> >   credential-cache--daemon.c:180:37: warning: incorrect type in argument 2 (invalid types)\n> >   $ sparse --version\n> >   v0.5.0\n> \n> Yeah, that version of sparse is a bit too old.\n> \n> If memory serves (it may not), all of the 'argument 2 (invalid types)'\n> errors are caused by the glibc headers using a 'transparent union' to\n> define the 'struct sockaddr' type. sparse could not handle that until\n> commit 7698bae699 (aka. v0.5.0-5-g7698bae). The only remaining warning\n> was addressed by commit bcfe020ed9 (aka. v0.5.1-rc1-22-gbcfe020) in\n> sparse and commit 54360a1956 in git.\n> \n> So, it seems you need at least v0.5.2 of sparse on your Linux system\n> (which can't be too recent, or you would need v0.6.0).\n\nThe latest Linux image available on Travis CI is based on Ubuntu 16.04\nLTS, which contains sparse 0.5.0, while their default image is based\non 14.04, with sparse 0.4.5-rc1.  Even the latest Ubuntu LTS is only\nat 0.5.1.\n\n  https://travis-ci.org/szeder/git/jobs/488113660#L487\n  https://travis-ci.org/szeder/git/jobs/488113661#L208\n\n"},{"id":"368437","messageId":"29138259-51cb-dec5-6f99-865868a00366@ramsayjones.plus.com","threadId":"50380","inReplyTo":"20190201223959.zmlyg4buwolw2b3p@ltop.local","subject":"Re: [PATCH 0/1] Using sparse in a CI job","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2019-02-03T15:19:18Z","receivedAt":"2019-02-03T15:19:24Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 01/02/2019 22:40, Luc Van Oostenryck wrote:\n> On Fri, Feb 01, 2019 at 09:01:20PM +0000, Ramsay Jones wrote:\n>>\n>> I suspect that the Makefile sparse target is not easy to use in a CI\n>> job, since the 'sparse' program (via cgcc -no-compile) does not exit\n>> with a non-zero value, even when issuing errors and warnings.\n> \n> ...\n>  \n>> We can change that by passing '-Wsparse-error' to 'sparse':\n>>\n>>   $ make SPARSE_FLAGS=-Wsparse-error change-table.sp\n>>       SP change-table.c\n>>   change-table.h:53:24: error: dubious one-bit signed bitfield\n>>   change-table.h:54:25: error: dubious one-bit signed bitfield\n>>   change-table.h:55:25: error: dubious one-bit signed bitfield\n>>   change-table.h:56:26: error: dubious one-bit signed bitfield\n>>   Makefile:2729: recipe for target 'change-table.sp' failed\n>>   make: *** [change-table.sp] Error 1\n>>   $ echo $?\n>>   2\n>>   $ \n>>\n>> Note that '-Wsparse-error' not only returns a non-zero exit code (1), but\n>> it also changes a 'warning' into an 'error' (see above):\n> \n> Yes, I know :(\n> The fact that, by default, sparse doesn't fail on errors is wanted\n> (otherwise it would break the kernel compile). But that the only way\n> to return an error is to use -Wsparse-error (which is supposed to\n> replace GCC's -Werror) is a real problem.\n\nGiven that I only use sparse as a checker, I actually don't mind\nthe current behaviour. That would be different if I was using\nsparsec/sparsei etc., as a compiler, of course! ;-)\n\n[If I were to suggest any change at all to -Wsparse-error it would\nbe: do not change the 'warning' to an 'error' (yes, the actual text\nlabel of the message), exit(1) if any errors or warnings, but *if*\nonly warnings have been issued, then print an \"treating warnings\nas errors [-Wsparse-error]\" message.]\n\nATB,\nRamsay Jones\n\n"},{"id":"368438","messageId":"9892484b-93f4-b99b-a600-fdcc8fbff806@ramsayjones.plus.com","threadId":"50380","inReplyTo":"20190203121207.GF10587@szeder.dev","subject":"Re: [PATCH 0/1] Using sparse in a CI job","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2019-02-03T15:43:43Z","receivedAt":"2019-02-03T15:43:48Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 03/02/2019 12:12, SZEDER Gábor wrote:\n> On Sun, Feb 03, 2019 at 01:49:37AM +0000, Ramsay Jones wrote:\n>> On 02/02/2019 00:41, SZEDER Gábor wrote:\n>>> On Fri, Feb 01, 2019 at 09:01:20PM +0000, Ramsay Jones wrote:\n>>>> At the moment, on Linux, the sp-out file is free from any sparse errors\n>>>> or warnings. So are next and pu:\n>>>>\n>>>>   $ grep error sp-out\n>>>>   $ grep warning sp-out\n>>>\n>>> On 'master' I get:\n>>>\n>>>   $ grep error sp-out \n>>>   $ grep warning sp-out \n>>>   connect.c:652:40: warning: incorrect type in argument 2 (invalid types)\n>>>   pack-revindex.c:65:23: warning: memset with byte count of 262144\n>>>   unix-socket.c:83:26: warning: incorrect type in argument 2 (invalid types)\n>>>   unix-socket.c:108:23: warning: incorrect type in argument 2 (invalid types)\n>>>   daemon.c:1041:36: warning: incorrect type in argument 2 (invalid types)\n>>>   daemon.c:1184:67: warning: incorrect type in argument 2 (invalid types)\n>>>   imap-send.c:1022:42: warning: incorrect type in argument 2 (invalid types)\n>>>   credential-cache--daemon.c:180:37: warning: incorrect type in argument 2 (invalid types)\n>>>   $ sparse --version\n>>>   v0.5.0\n>>\n>> Yeah, that version of sparse is a bit too old.\n>>\n>> If memory serves (it may not), all of the 'argument 2 (invalid types)'\n>> errors are caused by the glibc headers using a 'transparent union' to\n>> define the 'struct sockaddr' type. sparse could not handle that until\n>> commit 7698bae699 (aka. v0.5.0-5-g7698bae). The only remaining warning\n>> was addressed by commit bcfe020ed9 (aka. v0.5.1-rc1-22-gbcfe020) in\n>> sparse and commit 54360a1956 in git.\n>>\n>> So, it seems you need at least v0.5.2 of sparse on your Linux system\n>> (which can't be too recent, or you would need v0.6.0).\n> \n> The latest Linux image available on Travis CI is based on Ubuntu 16.04\n> LTS, which contains sparse 0.5.0, while their default image is based\n> on 14.04, with sparse 0.4.5-rc1.  Even the latest Ubuntu LTS is only\n> at 0.5.1.\n> \n>   https://travis-ci.org/szeder/git/jobs/488113660#L487\n>   https://travis-ci.org/szeder/git/jobs/488113661#L208\n> \n\nIndeed.\n\n[This is why I build sparse from source! :-D ]\n\nWith a bit of luck, v0.6.1 will be available soon.\n\nATB,\nRamsay Jones\n\n\n"}]}