Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210520231821.12272-1-maciej.falkowski9@gmail.com>
 <20210520231821.12272-2-maciej.falkowski9@gmail.com> <CAKwvOd=32_yNvAWRjyczOoEjsWje9SfaB=S1s9kS1bRFZhuU8g@mail.gmail.com>
In-Reply-To: <CAKwvOd=32_yNvAWRjyczOoEjsWje9SfaB=S1s9kS1bRFZhuU8g@mail.gmail.com>
From:   Masahiro Yamada <masahiroy@kernel.org>
Date:   Sat, 22 May 2021 12:03:20 +0900
Message-ID: <CAK7LNAQtnX6x1AWUU8+7MtUo4OkCrhKjoP=w0-cbH08u__wGWg@mail.gmail.com>
Subject: Re: [PATCH 2/2] Makefile: clang-tools: Omit printing stack trace when
 KeyboardInterrupt is raised
To:     Nick Desaulniers <ndesaulniers@google.com>
Cc:     Maciej Falkowski <maciej.falkowski9@gmail.com>,
        Nathan Chancellor <natechancellor@gmail.com>,
        clang-built-linux <clang-built-linux@googlegroups.com>,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sat, May 22, 2021 at 2:18 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Thu, May 20, 2021 at 4:18 PM Maciej Falkowski
> <maciej.falkowski9@gmail.com> wrote:
> >
> > When user terminates the script (also implicitly through for example
> > `make clang-analyzer`) by using
> > Ctrl+C (or sending SIGINT more generally) the KeyboardInterrupt
> > is raised printing stack trace of the execution as shown below:
> >
> > $ ./scripts/clang-tools/run-clang-tools.py clang-tidy ./compile_commands.json
> > ^CTraceback (most recent call last):
> >   File "./scripts/clang-tools/run-clang-tools.py", line 74, in <module>
> >     main()
> >   File "./scripts/clang-tools/run-clang-tools.py", line 70, in main
> >     pool.map(run_analysis, datastore)
> >   File "/usr/lib64/python3.8/multiprocessing/pool.py", line 364, in map
> >     return self._map_async(func, iterable, mapstar, chunksize).get()
> >   File "/usr/lib64/python3.8/multiprocessing/pool.py", line 765, in get
> >     self.wait(timeout)
> >   File "/usr/lib64/python3.8/multiprocessing/pool.py", line 762, in wait
> >     self._event.wait(timeout)
> >   File "/usr/lib64/python3.8/threading.py", line 558, in wait
> > Process ForkPoolWorker-6:
> > Process ForkPoolWorker-1:
> > Process ForkPoolWorker-5:
> > Process ForkPoolWorker-7:
> > Process ForkPoolWorker-2:
> > Process ForkPoolWorker-3:
> > Process ForkPoolWorker-4:
> > Process ForkPoolWorker-8:
> >     signaled = self._cond.wait(timeout)
> >   File "/usr/lib64/python3.8/threading.py", line 302, in wait
> >     waiter.acquire()
> > KeyboardInterrupt
>
> With this applied,
> $ make LLVM=1 LLVM_IAS=1 -j72 clang-analyzer
> ^C
> Process ForkPoolWorker-5:
> make: *** [Makefile:1902: clang-analyzer] Error 130
>
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> Tested-by: Nick Desaulniers <ndesaulniers@google.com>
>
> Thanks for the patch!


I am not a python expert, but is
"Let's suppress back-trace when a user presses an interrupt"
the common coding style?

If really so, do we need to do something similar in all python scripts?
I do not know what is special about run-clang-tools.py.

For example, if I press Ctrl-C while building Clang
by using tc-build, I see a back-trace.
I have never thought back-tracing was annoying.

The exit code is 130 regardless of this patch.


BTW, I prefer not having "Makefile:" in the patch subject
since this is not touching Makefile at all.

I rather like "clang-tools:" or "scripts/clang-tools:".


> > The patch handles the raise of the KeyboardInterrupt and exits when occurred
> > with code 130 as documented in: https://tldp.org/LDP/abs/html/exitcodes.html
> >
> > Signed-off-by: Maciej Falkowski <maciej.falkowski9@gmail.com>
> > ---
> >  scripts/clang-tools/run-clang-tools.py | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/scripts/clang-tools/run-clang-tools.py b/scripts/clang-tools/run-clang-tools.py
> > index 38fc311d2e03..eb0e0ecfce24 100755
> > --- a/scripts/clang-tools/run-clang-tools.py
> > +++ b/scripts/clang-tools/run-clang-tools.py
> > @@ -77,4 +77,7 @@ def main():
> >
> >
> >  if __name__ == "__main__":
> > -    main()
> > +    try:
> > +        main()
> > +    except KeyboardInterrupt:
> > +        sys.exit(130)
> > --
> > 2.26.3
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers


--
Best Regards
Masahiro Yamada