From: Cristian Marussi <cristian.marussi@arm.com>
To: Prabhakar Kushwaha <prabhakar.pkin@gmail.com>
Cc: linux-kselftest@vger.kernel.org,
Shuah Khan <skhan@linuxfoundation.org>,
Prabhakar Kushwaha <pkushwaha@marvell.com>
Subject: Re: [PATCH] kselftest: Fix NULL INSTALL_PATH for TARGETS runlist
Date: Tue, 22 Oct 2019 10:49:49 +0100 [thread overview]
Message-ID: <9900c764-5e3f-26c3-d0e6-6fa444724f75@arm.com> (raw)
In-Reply-To: <CAJ2QiJ+PORAhUQHnbnG3mKKU6WAF6rxrWZhLrQ2_1fYeOxv-9A@mail.gmail.com>
Hi
On 22/10/2019 04:52, Prabhakar Kushwaha wrote:
> Dear Cristian,
>
> On Mon, Oct 21, 2019 at 4:15 PM Cristian Marussi
> <cristian.marussi@arm.com> wrote:
>>
>> Hi
>>
>> On 20/10/2019 13:24, Prabhakar Kushwaha wrote:
>>> As per commit 131b30c94fbc ("kselftest: exclude failed TARGETS from
>>> runlist") failed targets were excluded from the runlist. But value
>>> $$INSTALL_PATH is always NULL. It should be $INSTALL_PATH instead
>>> $$INSTALL_PATH.
>>>
>>> So, fix Makefile to use $INSTALL_PATH.
>>>
>>
>> I was a bit puzzled at first since I never saw the NULLified value while testing
>> the original patch. Looking at it closely today, I realized that I used to test it
>> like:
>>
>> $ rm -rf $HOME/KSFT_TEST && make -C tools/testing/selftests/ INSTALL_PATH=$HOME/KSFT_TEST install
>>
>> which in fact causes INSTALL_PATH to be exported down to the subshell in the recipe, so that even
>> referring it as $$INSTALL_PATH from the recipe line make it work fine.
>>
>> Instead, using the default Makefile provided value (unexported) by invoking like:
>>
>> $ rm -rf $HOME/KSFT_TEST && make -C tools/testing/selftests/ install
>>
>> exposes the error you mentioned, being INSTALL_PATH not accessible form the subshell and so NULL.
>> Moreover it's anyway certainly better to refer with $(INSTALL_PATH) being it a strict Makefile var.
>> So it's fine for me, thanks to have spotted this.
>>
>> Reviewed-by: cristian.marussi@arm.com
>>
>
> Thanks for Reviewing.
>
> I have to send v2 patch with author mail id fix. I will keep your Reviewed-by.
Thanks to you.
I forgot to say that maybe it's worth also adding a Fixes: tag too like:
Fixes: 131b30c94fbc ("kselftest: exclude failed TARGETS from runlist")
given that I've spotted the original patch being already picked up for
some stable queues like in:
https://lore.kernel.org/lkml/20191018220324.8165-22-sashal@kernel.org/
Thanks
Cristian
>
> --prabhakar (pk)
>
>
>>
>>> Signed-off-by: Prabhakar Kushwaha <pkushwaha@marvell.com>
>>> Signed-off-by: Prabhakar Kushwaha <prabhakar.pkin@gmail.com>
>>> CC: Cristian Marussi <cristian.marussi@arm.com>
>>> ---
>>> tools/testing/selftests/Makefile | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
>>> index 4cdbae6f4e61..612f6757015d 100644
>>> --- a/tools/testing/selftests/Makefile
>>> +++ b/tools/testing/selftests/Makefile
>>> @@ -213,7 +213,7 @@ ifdef INSTALL_PATH
>>> @# included in the generated runlist.
>>> for TARGET in $(TARGETS); do \
>>> BUILD_TARGET=$$BUILD/$$TARGET; \
>>> - [ ! -d $$INSTALL_PATH/$$TARGET ] && echo "Skipping non-existent dir: $$TARGET" && continue; \
>>> + [ ! -d $(INSTALL_PATH)/$$TARGET ] && echo "Skipping non-existent dir: $$TARGET" && continue; \
>>> echo "[ -w /dev/kmsg ] && echo \"kselftest: Running tests in $$TARGET\" >> /dev/kmsg" >> $(ALL_SCRIPT); \
>>> echo "cd $$TARGET" >> $(ALL_SCRIPT); \
>>> echo -n "run_many" >> $(ALL_SCRIPT); \
>>>
>>
prev parent reply other threads:[~2019-10-22 9:50 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-20 12:24 [PATCH] kselftest: Fix NULL INSTALL_PATH for TARGETS runlist Prabhakar Kushwaha
2019-10-21 10:45 ` Cristian Marussi
2019-10-22 3:52 ` Prabhakar Kushwaha
2019-10-22 9:49 ` Cristian Marussi [this message]
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9900c764-5e3f-26c3-d0e6-6fa444724f75@arm.com \
--to=cristian.marussi@arm.com \
--cc=linux-kselftest@vger.kernel.org \
--cc=pkushwaha@marvell.com \
--cc=prabhakar.pkin@gmail.com \
--cc=skhan@linuxfoundation.org \
/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.
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 NNTP newsgroup(s).