| Message ID | CA+r1Zhj295pmVTmh_TVMuy37qRQ7Q6Mxipk7vhyAVkuXFBU8uw@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 2017-11-20 at 17:00 -0700, Jim Davis wrote: > On Mon, Nov 20, 2017 at 2:22 PM, Luc Van Oostenryck > <luc.vanoostenryck@gmail.com> wrote: > > > Should it be possible to somehow keep the distinction between > > the flags coming from KBUILD_CFLAGS and the pure CHECKFLAGS? > > Well, the practical problem seems to be that $(CHECK) is called in > scripts/Makefile.build with both $(CHECKFLAGS) and $(c_flags) as > arguments, regardless of what $(CHECK) is. That can be hacked around > with something inelegant like > > diff --git a/scripts/Makefile.build b/scripts/Makefile.build > index bb831d49bcfd..102194f9afb9 100644 > --- a/scripts/Makefile.build > +++ b/scripts/Makefile.build > @@ -98,14 +98,20 @@ __build: $(if $(KBUILD_BUILTIN),$(builtin-target) > $(lib-target) $(extra-y)) \ > $(subdir-ym) $(always) > @: > > -# Linus' kernel sanity checking tool > +# Linus' kernel sanity checking tool, or $CHECK program of choice > +ifneq ($(KBUILD_CHECKSRC),) > + add_to_checkflags = > + ifeq ($(CHECK),sparse) > + add_to_checkflags = $(c_flags) > + endif > +endif > ifneq ($(KBUILD_CHECKSRC),0) > ifeq ($(KBUILD_CHECKSRC),2) > quiet_cmd_force_checksrc = CHECK $< > - cmd_force_checksrc = $(CHECK) $(CHECKFLAGS) $(c_flags) $< ; > + cmd_force_checksrc = $(CHECK) $(CHECKFLAGS) $(add_to_checkflags) $< > ; > else > quiet_cmd_checksrc = CHECK $< > - cmd_checksrc = $(CHECK) $(CHECKFLAGS) $(c_flags) $< ; > + cmd_checksrc = $(CHECK) $(CHECKFLAGS) $(add_to_checkflags) $< > ; > endif > endif > > and then if I run scripts/checkpatch.pl as $CHECK and pass --quiet > --file as before it works -- until checkpatch returns with a non-zero > exit code, which causes the Makefile to bail at that point. yes, in an earlier version, I added a --no-errors flag to checkpatch to handle that, but based on feedback this version promotes make -k instead. It seems however that that only holds to the next linker step. It is useful to be able to select whether checkpatch should cause make to stop or not. While fixing issues, failure is a better strategy while to assess the state or generate a report, a return 0 behavior is better. > So maybe a wrapper script, with an exit 0 fixup to keep on keeping on > in case of checkpatch warnings or errors, would be better after all. I can prepare a v2 based on the wrapper script I have already. In that version, all added functionality was implemented in the wrapper (including the configuration file parsing) Would you like to keep the checkpatch changes in some form, or would you rather see everything happening in the wrapper? A 3rd possibility that strikes me is that maybe what's needed is a general checker runner that can also take care of running other checks, running multiple checks, and maybe also handling temporary or decided suppression of sparse errors in a similar fashion as I implemented for checkpatch errors. But maybe that can be left to a later change set.. Thanks, Knut -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Nov 21, 2017 at 1:10 AM, Knut Omang <knut.omang@oracle.com> wrote: > Would you like to keep the checkpatch changes in some form, or would you rather > see everything happening in the wrapper? I don't have a strong preference one way or another, but keeping everything in a wrapper script might be easier if only because you wouldn't need to get signoffs from whoever maintains checkpatch too.
On Tue, 2017-11-21 at 12:48 -0700, Jim Davis wrote: > On Tue, Nov 21, 2017 at 1:10 AM, Knut Omang <knut.omang@oracle.com> wrote: > > > Would you like to keep the checkpatch changes in some form, or would you rather > > see everything happening in the wrapper? > > I don't have a strong preference one way or another, but keeping > everything in a wrapper script might be easier if only because you > wouldn't need to get signoffs from whoever maintains checkpatch too. Keeping everything in a wrapper script would also allow any arbitrary checker to be run with minimal changes to the Makefile/build subsystem. -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/scripts/Makefile.build b/scripts/Makefile.build index bb831d49bcfd..102194f9afb9 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -98,14 +98,20 @@ __build: $(if $(KBUILD_BUILTIN),$(builtin-target) $(lib-target) $(extra-y)) \ $(subdir-ym) $(always) @: -# Linus' kernel sanity checking tool +# Linus' kernel sanity checking tool, or $CHECK program of choice +ifneq ($(KBUILD_CHECKSRC),) + add_to_checkflags = + ifeq ($(CHECK),sparse) + add_to_checkflags = $(c_flags) + endif +endif ifneq ($(KBUILD_CHECKSRC),0) ifeq ($(KBUILD_CHECKSRC),2) quiet_cmd_force_checksrc = CHECK $< - cmd_force_checksrc = $(CHECK) $(CHECKFLAGS) $(c_flags) $< ; + cmd_force_checksrc = $(CHECK) $(CHECKFLAGS) $(add_to_checkflags) $< ; else quiet_cmd_checksrc = CHECK $< - cmd_checksrc = $(CHECK) $(CHECKFLAGS) $(c_flags) $< ; + cmd_checksrc = $(CHECK) $(CHECKFLAGS) $(add_to_checkflags) $< ; endif endif