git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH 0/1] Using sparse in a CI job
@ 2019-02-01 21:01 Ramsay Jones
  2019-02-01 22:40 ` Luc Van Oostenryck
  2019-02-02  0:41 ` SZEDER Gábor
  0 siblings, 2 replies; 7+ messages in thread
From: Ramsay Jones @ 2019-02-01 21:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, GIT Mailing-list, Luc Van Oostenryck


I suspect that the Makefile sparse target is not easy to use in a CI
job, since the 'sparse' program (via cgcc -no-compile) does not exit
with a non-zero value, even when issuing errors and warnings.

The way I use sparse, this is not an issue. I run sparse over the
master branch, check that the output file has the no/expected
errors and warnings, then just diff the output files from the
next and pu branches. something like:

  $ make sparse >sp-out 2>&1 # on master branch
  $ vim sp-out # check for errors/warnings
  ...
  $ make sparse >nsp-out 2>&1 # on next branch
  $ diff sp-out nsp-out # any new errors/warnings?
  ...
  $ make sparse >psp-out 2>&1 # on pu branch
  $ diff nsp-out psp-out # any new errors/warnings?

At the moment, on Linux, the sp-out file is free from any sparse errors
or warnings. So are next and pu:

  $ grep error sp-out
  $ grep warning sp-out
  $ diff sp-out nsp-out
  391a392
  >     SP fuzz-commit-graph.c
  $ diff nsp-out psp-out
  170a171,180
  >     SP trace2.c
  >     SP trace2/tr2_cfg.c
  >     SP trace2/tr2_dst.c
  >     SP trace2/tr2_sid.c
  >     SP trace2/tr2_tbuf.c
  >     SP trace2/tr2_tgt_event.c
  >     SP trace2/tr2_tgt_normal.c
  >     SP trace2/tr2_tgt_perf.c
  >     SP trace2/tr2_tls.c
  >     SP trace2/tr2_verb.c
  298a309
  >     SP builtin/stash.c
  375a387
  >     SP t/helper/test-trace2.c
  376a389
  >     SP t/helper/test-xml-encode.c
  $ 

However, if we go back a few days, then the pu branch was not clean:

  $ diff nsp-out psp-out
  18a20,24
  >     SP change-table.c
  > change-table.h:53:24: error: dubious one-bit signed bitfield
  > change-table.h:54:25: error: dubious one-bit signed bitfield
  > change-table.h:55:25: error: dubious one-bit signed bitfield
  > change-table.h:56:26: error: dubious one-bit signed bitfield
  69a76
  >     GEN command-list.h
  93a101,106
  >     SP metacommit.c
  > change-table.h:53:24: error: dubious one-bit signed bitfield
  > change-table.h:54:25: error: dubious one-bit signed bitfield
  > change-table.h:55:25: error: dubious one-bit signed bitfield
  > change-table.h:56:26: error: dubious one-bit signed bitfield
  >     SP metacommit-parser.c
  170a184,193
  >     SP trace2.c
  >     SP trace2/tr2_cfg.c
  >     SP trace2/tr2_dst.c
  >     SP trace2/tr2_sid.c
  >     SP trace2/tr2_tbuf.c
  >     SP trace2/tr2_tgt_event.c
  >     SP trace2/tr2_tgt_normal.c
  >     SP trace2/tr2_tgt_perf.c
  >     SP trace2/tr2_tls.c
  >     SP trace2/tr2_verb.c
  213a237
  >     SP builtin/change.c
  298a323
  >     SP builtin/stash.c
  375a401
  >     SP t/helper/test-trace2.c
  376a403,405
  >     SP t/helper/test-xml-encode.c
  > t/helper/test-xml-encode.c:29:40: warning: Using plain integer as NULL pointer
  > t/helper/test-xml-encode.c:37:40: warning: Using plain integer as NULL pointer
  $ 
 
Note that 'make' does not exit at the first 'error', since the command exit
code is zero (or not non-zero! :) ):
 
  $ make change-table.sp
      SP change-table.c
  change-table.h:53:24: error: dubious one-bit signed bitfield
  change-table.h:54:25: error: dubious one-bit signed bitfield
  change-table.h:55:25: error: dubious one-bit signed bitfield
  change-table.h:56:26: error: dubious one-bit signed bitfield
  $ echo $?
  0
  $ 

We can change that by passing '-Wsparse-error' to 'sparse':

  $ make SPARSE_FLAGS=-Wsparse-error change-table.sp
      SP change-table.c
  change-table.h:53:24: error: dubious one-bit signed bitfield
  change-table.h:54:25: error: dubious one-bit signed bitfield
  change-table.h:55:25: error: dubious one-bit signed bitfield
  change-table.h:56:26: error: dubious one-bit signed bitfield
  Makefile:2729: recipe for target 'change-table.sp' failed
  make: *** [change-table.sp] Error 1
  $ echo $?
  2
  $ 

Note that '-Wsparse-error' not only returns a non-zero exit code (1), but
it also changes a 'warning' into an 'error' (see above):
  
  $ make SPARSE_FLAGS=-Wsparse-error t/helper/test-xml-encode.sp
      SP t/helper/test-xml-encode.c
  t/helper/test-xml-encode.c:29:40: error: Using plain integer as NULL pointer
  t/helper/test-xml-encode.c:37:40: error: Using plain integer as NULL pointer
  Makefile:2729: recipe for target 't/helper/test-xml-encode.sp' failed
  make: *** [t/helper/test-xml-encode.sp] Error 1
  $ echo $?
  2
  $ 

Unfortunately, using SPARSE_FLAGS from the 'make' command-line is not
ideal, since it was not really intended to be used that way. This will
cause problems for those files which have target specific settings for
SPARSE_FLAGS. For example:

  $ make pack-revindex.sp
      SP pack-revindex.c
  $ make SPARSE_FLAGS=-Wsparse-error pack-revindex.sp
      SP pack-revindex.c
  pack-revindex.c:65:23: error: memset with byte count of 262144
  Makefile:2729: recipe for target 'pack-revindex.sp' failed
  make: *** [pack-revindex.sp] Error 1
  $ echo $?
  2
  $ 

So, passing SPARSE_FLAGS on the command-line, effectively nullifies the
target specific settings (making them useless).

This patch splits the duties of SPARSE_FLAGS, by introducing a separate
SP_EXTRA_FLAGS variable, for use with the target specific settings. In
addition, we use a conditional assignment (?=) of the default (empty)
value of SPARSE_FLAGS, to allow setting the value of this variable from
the environment as well as the command-line. So, after this patch:

  $ make SPARSE_FLAGS=-Wsparse-error pack-revindex.sp
      SP pack-revindex.c
  $ echo $?
  0
  $ 

  $ SPARSE_FLAGS=-Wsparse-error make pack-revindex.sp
      SP pack-revindex.c
  $ echo $?
  0
  $ 

Now, we should be able to run the sparse Makefile target in a CI job, and
still find all sparse errors and warnings (now marked as errors also),
using something like this:

  $ SPARSE_FLAGS=-Wsparse-error make -k sparse >sp-out 2>&1

Note that the '-k' argument to 'make' is now required. ;-)


ATB,
Ramsay Jones 

Ramsay Jones (1):
  Makefile: improve SPARSE_FLAGS customisation

 Makefile | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

-- 
2.20.0

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/1] Using sparse in a CI job
  2019-02-01 21:01 [PATCH 0/1] Using sparse in a CI job Ramsay Jones
@ 2019-02-01 22:40 ` Luc Van Oostenryck
  2019-02-03 15:19   ` Ramsay Jones
  2019-02-02  0:41 ` SZEDER Gábor
  1 sibling, 1 reply; 7+ messages in thread
From: Luc Van Oostenryck @ 2019-02-01 22:40 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: Junio C Hamano, Johannes Schindelin, GIT Mailing-list

On Fri, Feb 01, 2019 at 09:01:20PM +0000, Ramsay Jones wrote:
> 
> I suspect that the Makefile sparse target is not easy to use in a CI
> job, since the 'sparse' program (via cgcc -no-compile) does not exit
> with a non-zero value, even when issuing errors and warnings.

...
 
> We can change that by passing '-Wsparse-error' to 'sparse':
> 
>   $ make SPARSE_FLAGS=-Wsparse-error change-table.sp
>       SP change-table.c
>   change-table.h:53:24: error: dubious one-bit signed bitfield
>   change-table.h:54:25: error: dubious one-bit signed bitfield
>   change-table.h:55:25: error: dubious one-bit signed bitfield
>   change-table.h:56:26: error: dubious one-bit signed bitfield
>   Makefile:2729: recipe for target 'change-table.sp' failed
>   make: *** [change-table.sp] Error 1
>   $ echo $?
>   2
>   $ 
> 
> Note that '-Wsparse-error' not only returns a non-zero exit code (1), but
> it also changes a 'warning' into an 'error' (see above):

Yes, I know :(
The fact that, by default, sparse doesn't fail on errors is wanted
(otherwise it would break the kernel compile). But that the only way
to return an error is to use -Wsparse-error (which is supposed to
replace GCC's -Werror) is a real problem.

-- Luc

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/1] Using sparse in a CI job
  2019-02-01 21:01 [PATCH 0/1] Using sparse in a CI job Ramsay Jones
  2019-02-01 22:40 ` Luc Van Oostenryck
@ 2019-02-02  0:41 ` SZEDER Gábor
  2019-02-03  1:49   ` Ramsay Jones
  1 sibling, 1 reply; 7+ messages in thread
From: SZEDER Gábor @ 2019-02-02  0:41 UTC (permalink / raw)
  To: Ramsay Jones
  Cc: Junio C Hamano, Johannes Schindelin, GIT Mailing-list,
	Luc Van Oostenryck

On Fri, Feb 01, 2019 at 09:01:20PM +0000, Ramsay Jones wrote:
> I suspect that the Makefile sparse target is not easy to use in a CI
> job, since the 'sparse' program (via cgcc -no-compile) does not exit
> with a non-zero value, even when issuing errors and warnings.

We had the same issue with Coccinelle in the static analysis build job
on Travis CI, which was happily succeeding even when Coccinelle
noticed undesired code patterns entering the codebase.  We've dealt
with that by looking at the patch files generated by 'make
coccicheck': if they were empty, then all is well, if they are not,
then dump their contents into the log and 'exit 1'.  See 0860a7641b
(travis-ci: fail if Coccinelle static analysis found something to
transform, 2018-07-23).

I think we could do something like that with sparse as well.

> At the moment, on Linux, the sp-out file is free from any sparse errors
> or warnings. So are next and pu:
> 
>   $ grep error sp-out
>   $ grep warning sp-out

On 'master' I get:

  $ grep error sp-out 
  $ grep warning sp-out 
  connect.c:652:40: warning: incorrect type in argument 2 (invalid types)
  pack-revindex.c:65:23: warning: memset with byte count of 262144
  unix-socket.c:83:26: warning: incorrect type in argument 2 (invalid types)
  unix-socket.c:108:23: warning: incorrect type in argument 2 (invalid types)
  daemon.c:1041:36: warning: incorrect type in argument 2 (invalid types)
  daemon.c:1184:67: warning: incorrect type in argument 2 (invalid types)
  imap-send.c:1022:42: warning: incorrect type in argument 2 (invalid types)
  credential-cache--daemon.c:180:37: warning: incorrect type in argument 2 (invalid types)
  $ sparse --version
  v0.5.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/1] Using sparse in a CI job
  2019-02-02  0:41 ` SZEDER Gábor
@ 2019-02-03  1:49   ` Ramsay Jones
  2019-02-03 12:12     ` SZEDER Gábor
  0 siblings, 1 reply; 7+ messages in thread
From: Ramsay Jones @ 2019-02-03  1:49 UTC (permalink / raw)
  To: SZEDER Gábor
  Cc: Junio C Hamano, Johannes Schindelin, GIT Mailing-list,
	Luc Van Oostenryck



On 02/02/2019 00:41, SZEDER Gábor wrote:
> On Fri, Feb 01, 2019 at 09:01:20PM +0000, Ramsay Jones wrote:
[snip]

 
>> At the moment, on Linux, the sp-out file is free from any sparse errors
>> or warnings. So are next and pu:
>>
>>   $ grep error sp-out
>>   $ grep warning sp-out
> 
> On 'master' I get:
> 
>   $ grep error sp-out 
>   $ grep warning sp-out 
>   connect.c:652:40: warning: incorrect type in argument 2 (invalid types)
>   pack-revindex.c:65:23: warning: memset with byte count of 262144
>   unix-socket.c:83:26: warning: incorrect type in argument 2 (invalid types)
>   unix-socket.c:108:23: warning: incorrect type in argument 2 (invalid types)
>   daemon.c:1041:36: warning: incorrect type in argument 2 (invalid types)
>   daemon.c:1184:67: warning: incorrect type in argument 2 (invalid types)
>   imap-send.c:1022:42: warning: incorrect type in argument 2 (invalid types)
>   credential-cache--daemon.c:180:37: warning: incorrect type in argument 2 (invalid types)
>   $ sparse --version
>   v0.5.0

Yeah, that version of sparse is a bit too old.

If memory serves (it may not), all of the 'argument 2 (invalid types)'
errors are caused by the glibc headers using a 'transparent union' to
define the 'struct sockaddr' type. sparse could not handle that until
commit 7698bae699 (aka. v0.5.0-5-g7698bae). The only remaining warning
was addressed by commit bcfe020ed9 (aka. v0.5.1-rc1-22-gbcfe020) in
sparse and commit 54360a1956 in git.

So, it seems you need at least v0.5.2 of sparse on your Linux system
(which can't be too recent, or you would need v0.6.0).

ATB,
Ramsay Jones


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/1] Using sparse in a CI job
  2019-02-03  1:49   ` Ramsay Jones
@ 2019-02-03 12:12     ` SZEDER Gábor
  2019-02-03 15:43       ` Ramsay Jones
  0 siblings, 1 reply; 7+ messages in thread
From: SZEDER Gábor @ 2019-02-03 12:12 UTC (permalink / raw)
  To: Ramsay Jones
  Cc: Junio C Hamano, Johannes Schindelin, GIT Mailing-list,
	Luc Van Oostenryck

On Sun, Feb 03, 2019 at 01:49:37AM +0000, Ramsay Jones wrote:
> On 02/02/2019 00:41, SZEDER Gábor wrote:
> > On Fri, Feb 01, 2019 at 09:01:20PM +0000, Ramsay Jones wrote:
> >> At the moment, on Linux, the sp-out file is free from any sparse errors
> >> or warnings. So are next and pu:
> >>
> >>   $ grep error sp-out
> >>   $ grep warning sp-out
> > 
> > On 'master' I get:
> > 
> >   $ grep error sp-out 
> >   $ grep warning sp-out 
> >   connect.c:652:40: warning: incorrect type in argument 2 (invalid types)
> >   pack-revindex.c:65:23: warning: memset with byte count of 262144
> >   unix-socket.c:83:26: warning: incorrect type in argument 2 (invalid types)
> >   unix-socket.c:108:23: warning: incorrect type in argument 2 (invalid types)
> >   daemon.c:1041:36: warning: incorrect type in argument 2 (invalid types)
> >   daemon.c:1184:67: warning: incorrect type in argument 2 (invalid types)
> >   imap-send.c:1022:42: warning: incorrect type in argument 2 (invalid types)
> >   credential-cache--daemon.c:180:37: warning: incorrect type in argument 2 (invalid types)
> >   $ sparse --version
> >   v0.5.0
> 
> Yeah, that version of sparse is a bit too old.
> 
> If memory serves (it may not), all of the 'argument 2 (invalid types)'
> errors are caused by the glibc headers using a 'transparent union' to
> define the 'struct sockaddr' type. sparse could not handle that until
> commit 7698bae699 (aka. v0.5.0-5-g7698bae). The only remaining warning
> was addressed by commit bcfe020ed9 (aka. v0.5.1-rc1-22-gbcfe020) in
> sparse and commit 54360a1956 in git.
> 
> So, it seems you need at least v0.5.2 of sparse on your Linux system
> (which can't be too recent, or you would need v0.6.0).

The latest Linux image available on Travis CI is based on Ubuntu 16.04
LTS, which contains sparse 0.5.0, while their default image is based
on 14.04, with sparse 0.4.5-rc1.  Even the latest Ubuntu LTS is only
at 0.5.1.

  https://travis-ci.org/szeder/git/jobs/488113660#L487
  https://travis-ci.org/szeder/git/jobs/488113661#L208


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/1] Using sparse in a CI job
  2019-02-01 22:40 ` Luc Van Oostenryck
@ 2019-02-03 15:19   ` Ramsay Jones
  0 siblings, 0 replies; 7+ messages in thread
From: Ramsay Jones @ 2019-02-03 15:19 UTC (permalink / raw)
  To: Luc Van Oostenryck; +Cc: Junio C Hamano, Johannes Schindelin, GIT Mailing-list



On 01/02/2019 22:40, Luc Van Oostenryck wrote:
> On Fri, Feb 01, 2019 at 09:01:20PM +0000, Ramsay Jones wrote:
>>
>> I suspect that the Makefile sparse target is not easy to use in a CI
>> job, since the 'sparse' program (via cgcc -no-compile) does not exit
>> with a non-zero value, even when issuing errors and warnings.
> 
> ...
>  
>> We can change that by passing '-Wsparse-error' to 'sparse':
>>
>>   $ make SPARSE_FLAGS=-Wsparse-error change-table.sp
>>       SP change-table.c
>>   change-table.h:53:24: error: dubious one-bit signed bitfield
>>   change-table.h:54:25: error: dubious one-bit signed bitfield
>>   change-table.h:55:25: error: dubious one-bit signed bitfield
>>   change-table.h:56:26: error: dubious one-bit signed bitfield
>>   Makefile:2729: recipe for target 'change-table.sp' failed
>>   make: *** [change-table.sp] Error 1
>>   $ echo $?
>>   2
>>   $ 
>>
>> Note that '-Wsparse-error' not only returns a non-zero exit code (1), but
>> it also changes a 'warning' into an 'error' (see above):
> 
> Yes, I know :(
> The fact that, by default, sparse doesn't fail on errors is wanted
> (otherwise it would break the kernel compile). But that the only way
> to return an error is to use -Wsparse-error (which is supposed to
> replace GCC's -Werror) is a real problem.

Given that I only use sparse as a checker, I actually don't mind
the current behaviour. That would be different if I was using
sparsec/sparsei etc., as a compiler, of course! ;-)

[If I were to suggest any change at all to -Wsparse-error it would
be: do not change the 'warning' to an 'error' (yes, the actual text
label of the message), exit(1) if any errors or warnings, but *if*
only warnings have been issued, then print an "treating warnings
as errors [-Wsparse-error]" message.]

ATB,
Ramsay Jones


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/1] Using sparse in a CI job
  2019-02-03 12:12     ` SZEDER Gábor
@ 2019-02-03 15:43       ` Ramsay Jones
  0 siblings, 0 replies; 7+ messages in thread
From: Ramsay Jones @ 2019-02-03 15:43 UTC (permalink / raw)
  To: SZEDER Gábor
  Cc: Junio C Hamano, Johannes Schindelin, GIT Mailing-list,
	Luc Van Oostenryck



On 03/02/2019 12:12, SZEDER Gábor wrote:
> On Sun, Feb 03, 2019 at 01:49:37AM +0000, Ramsay Jones wrote:
>> On 02/02/2019 00:41, SZEDER Gábor wrote:
>>> On Fri, Feb 01, 2019 at 09:01:20PM +0000, Ramsay Jones wrote:
>>>> At the moment, on Linux, the sp-out file is free from any sparse errors
>>>> or warnings. So are next and pu:
>>>>
>>>>   $ grep error sp-out
>>>>   $ grep warning sp-out
>>>
>>> On 'master' I get:
>>>
>>>   $ grep error sp-out 
>>>   $ grep warning sp-out 
>>>   connect.c:652:40: warning: incorrect type in argument 2 (invalid types)
>>>   pack-revindex.c:65:23: warning: memset with byte count of 262144
>>>   unix-socket.c:83:26: warning: incorrect type in argument 2 (invalid types)
>>>   unix-socket.c:108:23: warning: incorrect type in argument 2 (invalid types)
>>>   daemon.c:1041:36: warning: incorrect type in argument 2 (invalid types)
>>>   daemon.c:1184:67: warning: incorrect type in argument 2 (invalid types)
>>>   imap-send.c:1022:42: warning: incorrect type in argument 2 (invalid types)
>>>   credential-cache--daemon.c:180:37: warning: incorrect type in argument 2 (invalid types)
>>>   $ sparse --version
>>>   v0.5.0
>>
>> Yeah, that version of sparse is a bit too old.
>>
>> If memory serves (it may not), all of the 'argument 2 (invalid types)'
>> errors are caused by the glibc headers using a 'transparent union' to
>> define the 'struct sockaddr' type. sparse could not handle that until
>> commit 7698bae699 (aka. v0.5.0-5-g7698bae). The only remaining warning
>> was addressed by commit bcfe020ed9 (aka. v0.5.1-rc1-22-gbcfe020) in
>> sparse and commit 54360a1956 in git.
>>
>> So, it seems you need at least v0.5.2 of sparse on your Linux system
>> (which can't be too recent, or you would need v0.6.0).
> 
> The latest Linux image available on Travis CI is based on Ubuntu 16.04
> LTS, which contains sparse 0.5.0, while their default image is based
> on 14.04, with sparse 0.4.5-rc1.  Even the latest Ubuntu LTS is only
> at 0.5.1.
> 
>   https://travis-ci.org/szeder/git/jobs/488113660#L487
>   https://travis-ci.org/szeder/git/jobs/488113661#L208
> 

Indeed.

[This is why I build sparse from source! :-D ]

With a bit of luck, v0.6.1 will be available soon.

ATB,
Ramsay Jones



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-02-03 15:43 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-01 21:01 [PATCH 0/1] Using sparse in a CI job Ramsay Jones
2019-02-01 22:40 ` Luc Van Oostenryck
2019-02-03 15:19   ` Ramsay Jones
2019-02-02  0:41 ` SZEDER Gábor
2019-02-03  1:49   ` Ramsay Jones
2019-02-03 12:12     ` SZEDER Gábor
2019-02-03 15:43       ` Ramsay Jones

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).