Dear Cristian,
On Mon, Oct 21, 2019 at 4:15 PM Cristian Marussi
<cristian.marussi@xxxxxxx> 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@xxxxxxx
>
Thanks for Reviewing.
I have to send v2 patch with author mail id fix. I will keep your Reviewed-by.
--prabhakar (pk)
>
> > Signed-off-by: Prabhakar Kushwaha <pkushwaha@xxxxxxxxxxx>
> > Signed-off-by: Prabhakar Kushwaha <prabhakar.pkin@xxxxxxxxx>
> > CC: Cristian Marussi <cristian.marussi@xxxxxxx>
> > ---
> > 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); \
> >
>
