From: Ramsay Jones <ramsay@ramsayjones.plus.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
GIT Mailing-list <git@vger.kernel.org>,
Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Subject: [PATCH v2 0/2] Using sparse in a CI job
Date: Tue, 5 Feb 2019 02:24:16 +0000 [thread overview]
Message-ID: <af361187-4d9d-2c41-0850-bf774d5523b2@ramsayjones.plus.com> (raw)
Changes in v2:
- Add a new patch #1, which removes an obsolete use of SPARSE_FLAGS
- Actually initialise the new SP_EXTRA_FLAGS variable
- Reword the commit message (now #2) to, hopefully, clarify the
usage of SPARSE_FLAGS and SP_EXTRA_FLAGS
A range-diff against v1 is given at the very end.
Original cover-letter content:
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 (2):
config.mak.uname: remove obsolete SPARSE_FLAGS setting
Makefile: improve SPARSE_FLAGS customisation
Makefile | 14 +++++++++-----
config.mak.uname | 1 -
2 files changed, 9 insertions(+), 6 deletions(-)
Range-diff against v1:
-: ---------- > 1: 7b15bd9b31 config.mak.uname: remove obsolete SPARSE_FLAGS setting
1: 889cc64f90 ! 2: c2b6ce71da Makefile: improve SPARSE_FLAGS customisation
@@ -7,10 +7,13 @@
target specific settings. Without using the new variable, setting
the SPARSE_FLAGS on the 'make' command-line would also override the
value set by the target-specific rules in the Makefile (effectively
- making them useless). In addition, we initialise the SPARSE_FLAGS
- to the default (empty) value using a conditional assignment (?=).
- This allows the SPARSE_FLAGS to be set from the environment as
- well as from the command-line.
+ making them useless). Also, this enables the SP_EXTRA_FLAGS to be
+ used in the future for any other internal customisations, such as
+ for some platform specific values.
+
+ In addition, we initialise the SPARSE_FLAGS to the default (empty)
+ value using a conditional assignment (?=). This allows SPARSE_FLAGS
+ to be set from the environment as well as from the command-line.
diff --git a/Makefile b/Makefile
--- a/Makefile
@@ -20,7 +23,11 @@
export TCL_PATH TCLTK_PATH
-SPARSE_FLAGS =
++# user customisation variable for 'sparse' target
+SPARSE_FLAGS ?=
++# internal/platform customisation variable for 'sparse'
++SP_EXTRA_FLAGS =
++
SPATCH_FLAGS = --all-includes --patch .
--
2.20.0
next reply other threads:[~2019-02-05 2:24 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-05 2:24 Ramsay Jones [this message]
2019-02-05 18:08 ` [PATCH v2 0/2] Using sparse in a CI job Junio C Hamano
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=af361187-4d9d-2c41-0850-bf774d5523b2@ramsayjones.plus.com \
--to=ramsay@ramsayjones.plus.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=luc.vanoostenryck@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).