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

Cristian

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); \

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); \