Re: [PATCH] selftests/alsa/Makefile: fix relative rpath usage

[Eugene Syromiatnikov <esyromia@xxxxxxxxxx>]

On Fri, 9 Aug 2024 at 19:01, Shuah Khan <skhan@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On 8/8/24 19:00, Eugene Syromiatnikov wrote:
> > On Thu, Aug 08, 2024 at 02:20:21PM -0600, Shuah Khan wrote:
> >> Wouldn't make sense to fix fix this in selftests main Makefile
> >> instead of changing the all the test makefiles
> >
> > As of now, the usage of rpath is localised, so it is relatively easy
> > to evaluate the effect/prudence of such a change;  I am not so confident
> > in imposing rpath on all of the selftests (and, if doing so, I would
> > rather opt for runpath, to leave out an ability to override the search
> > path via LD_LIBRARY_PATH, if such need arises);  in that case it is possibly
> > also worth to add -L$(OUTPUT) to the CFLAGS as well, as the compile-time
> > counterpart.  But, again, I was trying to avoid the task of evaluating
> > the possible side effects of such a change, considering the variability
> > in environments and setups selftests are run.
>
> Okay.
>
> >
> >> Same comment on all other files.
> >
> >> It would be easier to send these as series
> >
> > I hesitated to do so due to the fact that different selftests are seemingly
> > maintained by different people.
>
> You can cc everybody on the cover-letter explaining the change
> and the individual patches can be sent selectively.
>
> This is a kind of change it would be good to go as a series so
> it will be easier for reviewers.

I see, thank you for the explanation.

Right now I am working on the variant of the patch that consolidates
the -L/-rpath flags in lib.mk, do you think it will be of use to have
some opt-in/opt-out mechanism, or just impose them unconditionally,
similarly to -D_GNU_SOURCE? So far I don't see any issues with either
building or running the tests, but I can imagine it might be necessary
to avoid such flags in some cases.

> I had to comment on all 3 patches you sent - instead I could have
> sent one reply to the cover letter. It makes it so much easier for
> people to follow the discussion and add to it.

My apologies.

> >> please mentioned the tests run as well after this change.
> >
> > I have checked the ldd output after the change remained the same (and that ldd
> > is able to find the libraries used when run outside the directory the tests
> > reside in) and did a cursory check of the results of the run of the affected
> > tests
>
> Please mention that then in the change log.
>
> I applied this patch and ran alsa test without any issues. You
> could do the same with:
>
> make kselftest TARGETS=alsa

Thanks, will do.

> (but not so sure about the BPF selftests, as they don't compile as-is
> > due to numerous "incompatible pointer types" warnings that are forced
> > into errors by -Werror and the fact that it hanged the machine I tried
> > to run them on).
> >
>
> I see a bpf patch from you in the inbox - if you mention the issues bpf
> people might be able to help you.

Right, I am in the process of condensing my issues into patches or at
least useful bug reports.

> I am not replying to your other patches. Take these as comments on others
> as well.
>
> thanks,
> -- Shuah