Re: [PATCH 2/2] selftests/damon: make selftests executable

[SeongJae Park <sj@kernel.org>]

On Fri, 18 Feb 2022 09:01:11 +0100 Greg KH <gregkh@linuxfoundation.org> wrote:

> On Fri, Feb 18, 2022 at 07:52:54AM +0000, SeongJae Park wrote:
> > Hello Yuanchu,
> > 
> > Thank you for this patch!
> > 
> > On Fri, 18 Feb 2022 00:10:17 +0000 Yuanchu Xie <yuanchu@google.com> wrote:
> > 
> > > The damon selftests do not have the executable bit on. We fix that by
> > > setting the x bits on the .sh files similar to other existing shell
> > > selftests.
> > > 
> > > Fixes: 9ab3b0c8ef62 ("selftests/damon: split test cases")
> > > Signed-off-by: Yuanchu Xie <yuanchu@google.com>
> > 
> > Reviewed-by: SeongJae Park <sj@kernel.org>
> 
> This type of change does not work outside of git, so why not just make
> the tool that calls these scripts not care about the executable bit like
> we do for other scripts?

Actually, we made kselftest receives scripts having no executable bit[1],
though it still prints warning.  I guess Yuanchu wants to remove the warning?

To remove the warning, simply making kselftest (runner.sh) stop printing the
warning message might make more sense.  Nevertheless, it's also true that
letting some scripts have executable bits while others not looks inconsistent
to me.  That's why I left the warning message there.  Should we remove the
warning from kselftest and remove executable bits from other selftest test
scripts?  Or, let the inconsistency be?  I have no real opinion here, so just
wanted to hear others' opinion if possible.

BTW, similar issue is also in Kconfig's 'shell' macro.  For example,
'runst-version.sh' on -mm tree bothered me before[2], and now
'pahole-version.sh'.  I might make change in 'scrips/Kconfig/preprocess.c'
later.

[1] https://lore.kernel.org/linux-kselftest/20210810164534.25902-1-sj38.park@gmail.com/
[2] https://lore.kernel.org/all/20220110085952.6137-1-sj@kernel.org/


Thanks,
SJ

> 
> thanks,
> 
> greg k-h