[DTrace-devel] [PATCH] build: install sdt*.h in /usr/lib64/dtrace/include/sys

[Nick Alcock]

On 23 May 2024, Kris Van Hees spake thusly:

> On Thu, May 23, 2024 at 07:49:26PM +0100, Nick Alcock wrote:
>> On 23 May 2024, Kris Van Hees verbalised:
>> 
>> > Can you split the move of sdt.h and sdt_internal.h into its own patch
>> > please?  And I guess the pkg-config stuff in a follw-up patch?  Still
>> > need to look into that a bit more because I have reservations.  But
>> > the moving of the header files is definitely something that should be
>> > in its own patch.
>> 
>> I put them in the same commit for a reason: doing otherwise would break
>> the installed testsuite in the intermediate commit, and I don't want to
>> implement *two distinct* mechanisms for locating the headers when we
>> already have one that works perfectly well (pkg-config).
>
> Do the first patch with the moving of the sdt header files, adding an
> explicit -I/usr/lib64/dtrace/include to the CFLAGS, and the testsuite
> should work perfectly fine with the file move.  That is hardly another
> mechanism.

It doesn't :( We need to adjust a bunch of test files too, since almost
all that do sdt.h compilations define their *own* CFLAGS -- and if we do
that and hardwire a location, we'll just have to do it *all over again*
when we introduce the, may I reiterate, *entirely standard* pkg-config
method of finding headers in unusual locations *just like this one*.

Oh, also, we can't use any fixed location, because the testsuite
actually needs to look in *different locations* when installed versus
when uninstalled. pkg-config already has a mechanism to handle this (the
next round of this patch will use it). Hardwiring some sort of "are we
installed? use this, otherwise this" into a dozen distinct test files
seems... unsatisfactory to me.


Another reason to do it now -- when we move the headers like this, we're
requiring all our sdt.h-using clients (if any exist) to change their
build systems. If we introduce a pkg-config-based header-location system
now, they only need to change their build systems once, and they'll
automatically work with whatever future DTrace finally allows relocation
of /usr/lib64/dtrace elsewhere (and that's not far off -- after all, the
reason we're doing this at all is that we have had requests from distros
for the ability to do just that.)

(Obviously this applies only if such clients exist. But if no such
clients exist... why do we care about this change at all? Why not just
*not ship* sdt.h for a while?)


I have *absolutely no idea* why you're choosing this hill to die on.
At this point *not* using pkg-config is far more unusual than using it,
in any project that has unusual CFLAGS requirements at all. All the
other methods (-config shell scripts, mostly) have thankfully died out.
The only one remaining is cmake scripts, and even those are now
deprecated in favour of... pkg-config!

>> (Also, splitting them up runs the risk of your integrating one without
>> the other, and they really do go together.)
>
> They are two different things - I mentioned that from the very beginning
> of the discussions to deal with the sys/sdt.h conflict,  Please split it
> into two patches.  The pkg-config support is bigger than just this sdt
> header file move.

Is it? Why? I have no idea what you're driving at. You have to start
somewhere, and starting with a tiny four-line pkg-config script is about
the least impactful and least-likely-to-be-damaging way possible.

>> > Introducing pkg-config as a mechanism that people can use is a separate
>> > thing (and as far as I can see, could use some extra work anyway because
>> > e.g. runtest.sh still has other absolute paths in it that cause failures
>> > when the DTrace build is configured to install things in non-standard
>> > locations).
>> 
>> Well, yes, I didn't try to make the *entire build* relocatable in this
>> patch. That really *would* have been too much for one commit.
>
> Yet the purpose of introducing pkg-config is to deal with installs in
> non-standard locations.

Well, yes, that's one purpose indeed -- and... /usr/lib64/dtrace/include
is a *distinctly* non-standard location.

-- 
NULL && (void)