Linux DTrace development list
 help / color / mirror / Atom feed
* [PATCH] build: install sdt*.h in /usr/lib64/dtrace/include/sys
@ 2024-05-23 17:56 Nick Alcock
  2024-05-23 18:41 ` Kris Van Hees
  0 siblings, 1 reply; 5+ messages in thread
From: Nick Alcock @ 2024-05-23 17:56 UTC (permalink / raw)
  To: dtrace, dtrace-devel

This moves it out of the way of SystemTap's rather different sdt.h,
and lets us remove the barely-working kludgery in the specfile
to work around it.  We point at the new location with a pkg-config
file, and can immediately use it in make check in the installed
testsuite, compensating for any install-time changes in the
location of the dtrace libdir.

With this change, we no longer confict with any systemtap packages!

Signed-off-by: Nick Alcock <nick.alcock@oracle.com>
---
 GNUmakefile          |  4 ++++
 configure            |  2 ++
 dtrace.spec          | 19 +------------------
 runtest.sh           |  2 +-
 uts/Build            | 26 +++++++++++++++++++++++---
 uts/dtrace_sdt.pc.in |  6 ++++++
 6 files changed, 37 insertions(+), 22 deletions(-)
 create mode 100644 uts/dtrace_sdt.pc.in

This is a provisional patch so people can see what I'm up to --
tests are still running.  Don't commit yet.

diff --git a/GNUmakefile b/GNUmakefile
index 7d1af5641f54a..0390db77b4780 100644
--- a/GNUmakefile
+++ b/GNUmakefile
@@ -86,6 +86,8 @@ BINDIR = $(prefix)/bin
 INSTBINDIR = $(DESTDIR)$(BINDIR)
 INCLUDEDIR = $(prefix)/include
 INSTINCLUDEDIR = $(DESTDIR)$(INCLUDEDIR)
+SDTINCLUDEDIR = $(LIBDIR)/dtrace/include
+INSTSDTINCLUDEDIR = $(DESTDIR)$(SDTINCLUDEDIR)
 SBINDIR = $(prefix)/sbin
 INSTSBINDIR = $(DESTDIR)$(SBINDIR)
 UDEVDIR = $(prefix)/lib/udev/rules.d
@@ -98,6 +100,8 @@ DOCDIR = $(prefix)/share/doc/dtrace-$(VERSION)
 INSTDOCDIR = $(DESTDIR)$(DOCDIR)
 MANDIR = $(prefix)/share/man/man8
 INSTMANDIR = $(DESTDIR)$(MANDIR)
+PKGCONFIGDIR = $(prefix)/share/pkgconfig
+INSTPKGCONFIGDIR = $(DESTDIR)$(PKGCONFIGDIR)
 TESTDIR = $(prefix)/lib$(BITNESS)/dtrace/testsuite
 INSTTESTDIR = $(DESTDIR)$(TESTDIR)
 TARGETS =
diff --git a/configure b/configure
index 40b870f81d6ff..bc42c4f49a3fa 100755
--- a/configure
+++ b/configure
@@ -60,6 +60,7 @@ Installation paths:
 --sbindir=PREFIX/sbin				Alias for --bindir
 --includedir=PREFIX/include			#include directory
 --mandir=PREFIX/share/man/man8			Manpage directory
+--pkg-config-dir=PREFIX/share/pkgconfig		Pkg-config directory
 --udevdir=PREFIX/lib/udev/rules.d		udev rules directory
 --systemd-unit-dir=PREFIX/lib/systemd/system	systemd unit directory
 --docdir=PREFIX/share/doc/dtrace		Documentation directory
@@ -130,6 +131,7 @@ for option in "$@"; do
         --systemd-unit-dir=*) write_make_var SYSTEMDUNITDIR "$option";;
         --docdir=*) write_make_var DOCDIR "$option";;
         --mandir=*) write_make_var MANDIR "$option";;
+        --mandir=*) write_make_var PKGCONFIGDIR "$option";;
         --testdir=*) write_make_var TESTDIR "$option";;
         CC=*) write_make_var CC "$option";;
         CPP=*) write_make_var PREPROCESS "$option";;
diff --git a/dtrace.spec b/dtrace.spec
index 42ea9bc339586..7823b13d981cf 100644
--- a/dtrace.spec
+++ b/dtrace.spec
@@ -86,8 +86,6 @@ BuildRequires: binutils-devel >= 2.30-58.0.8
 Requires:     libdtrace-ctf >= 1.1.0
 BuildRequires: libdtrace-ctf-devel >= 1.1.0
 %endif
-Conflicts:    systemtap-sdt-devel
-Provides:     systemtap-sdt-devel
 Summary:      DTrace user interface.
 Version:      2.0.1
 Release:      0%{?dist}
@@ -194,11 +192,6 @@ make DESTDIR=$RPM_BUILD_ROOT VERSION=%{version} \
 sed -i '/^ProtectSystem=/d; /^ProtectControlGroups=/d; /^RuntimeDirectory/d;' $RPM_BUILD_ROOT/usr/lib/systemd/system/dtprobed.service
 %endif
 
-# Because systemtap creates a sdt.h header file we have to rename
-# ours and then shift theirs out of the way.
-mv $RPM_BUILD_ROOT/usr/include/sys/sdt.h \
-   $RPM_BUILD_ROOT/usr/include/sys/sdt-dtrace.h
-
 %clean
 [ "$RPM_BUILD_ROOT" != "/" ] && rm -rf "$RPM_BUILD_ROOT"
 rm -rf $RPM_BUILD_DIR/%{name}-%{version}
@@ -222,15 +215,6 @@ if [ $1 -ge 2 ] ; then
     %systemd_postun_with_restart dtprobed.service
 fi
 
-# if sdt-systemtap.h doesn't exist then we can move the existing dtrace sdt.h
-SYSINCDIR=/usr/include/sys
-if [ -e $SYSINCDIR/sdt.h -a ! -e $SYSINCDIR/sdt-systemtap.h ]; then
-    mv $SYSINCDIR/sdt.h $SYSINCDIR/sdt-systemtap.h
-    ln -s $SYSINCDIR/sdt-dtrace.h $SYSINCDIR/sdt.h
-elif [ ! -e $SYSINCDIR/sdt.h ]; then
-    ln -s $SYSINCDIR/sdt-dtrace.h $SYSINCDIR/sdt.h
-fi
-
 %preun
 %systemd_preun dtprobed.service
 
@@ -252,9 +236,8 @@ systemctl start dtprobed || :
 %{_libdir}/libdtrace.so.*
 %{_sbindir}/dtrace
 %{_sbindir}/dtprobed
+%{_datadir}/pkgconfig/dtrace_sdt.pc
 %{_mandir}/man8/dtrace.8.gz
-%{_includedir}/sys/sdt-dtrace.h
-%{_includedir}/sys/sdt_internal.h
 %doc %{_docdir}/dtrace-%{version}/*
 %{_unitdir}/dtprobed.service
 %{_unitdir}/dtrace-usdt.target
diff --git a/runtest.sh b/runtest.sh
index 35d5006978469..78f47c23897c8 100755
--- a/runtest.sh
+++ b/runtest.sh
@@ -579,7 +579,7 @@ else
     dtrace="/usr/sbin/dtrace"
     test_libdir="installed"
     test_ldflags=""
-    test_incflags="-DARCH_$arch"
+    test_incflags="-DARCH_$arch $(pkg-config --cflags dtrace_sdt)"
 
     if [[ ! -x $dtrace ]]; then
         echo "$dtrace not available." >&2
diff --git a/uts/Build b/uts/Build
index f445115d683db..99bdf19638a50 100644
--- a/uts/Build
+++ b/uts/Build
@@ -10,11 +10,31 @@ uts_DIR := $(current-dir)
 # provide the userspace versions of types defined distincty by the kernel.
 
 SYS_HEADERS_INSTALL := common/sys/dtrace.h \
-                       common/sys/dtrace_types.h \
-                       common/sys/sdt.h \
-                       common/sys/sdt_internal.h
+                       common/sys/dtrace_types.h
+
+# sdt*.h are used by programs that contain USDT probes that want to define
+# probes by hand rather than using dtrace -h.  SystemTap has another header
+# with the same name, so we install these out of the way and provide a
+# pkg-config file to pull them in.
+
+PROBE_HEADERS_INSTALL := common/sys/sdt.h \
+                         common/sys/sdt_internal.h
+
+# The pkg-config files undergo a few translations with sed before installation.
+
+SHARE_PKG_CONFIG_INSTALL := dtrace_sdt.pc
 
 install::
 	mkdir -p $(INSTINCLUDEDIR)/sys
 	$(call describe-install-target,$(INSTINCLUDEDIR)/sys,$(notdir $(SYS_HEADERS_INSTALL)))
 	cd $(uts_DIR) && install -m 644 $(SYS_HEADERS_INSTALL) $(INSTINCLUDEDIR)/sys
+	mkdir -p $(INSTSDTINCLUDEDIR)/sys
+	$(call describe-install-target,$(INSTSDTINCLUDEDIR)/sys,$(notdir $(PROBE_HEADERS_INSTALL)))
+	cd $(uts_DIR) && install -m 644 $(PROBE_HEADERS_INSTALL) $(INSTSDTINCLUDEDIR)/sys
+	mkdir -p $(INSTPKGCONFIGDIR)
+	$(call describe-install-target,$(INSTPKGCONFIGDIR),$(SHARE_PKG_CONFIG_INSTALL))
+	for name in $(SHARE_PKG_CONFIG_INSTALL); do \
+		cd $(uts_DIR) && \
+		    sed 's,@SDTINCLUDEDIR@,$(SDTINCLUDEDIR),g; s,@VERSION@,$(VERSION),g' < \
+			$${name}.in > $(INSTPKGCONFIGDIR)/$$name; \
+	done
diff --git a/uts/dtrace_sdt.pc.in b/uts/dtrace_sdt.pc.in
new file mode 100644
index 0000000000000..c5506f9355d92
--- /dev/null
+++ b/uts/dtrace_sdt.pc.in
@@ -0,0 +1,6 @@
+sdtincludedir = @SDTINCLUDEDIR@
+
+Name: DTrace SDT
+Description: DTrace raw SDT headers
+Version: @VERSION@
+Cflags: -I${sdtincludedir}

base-commit: 0f499300c150a3a8878361e31f3ba167f7d5b851
-- 
2.45.1.275.g567cb0950c


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] build: install sdt*.h in /usr/lib64/dtrace/include/sys
  2024-05-23 17:56 [PATCH] build: install sdt*.h in /usr/lib64/dtrace/include/sys Nick Alcock
@ 2024-05-23 18:41 ` Kris Van Hees
  2024-05-23 18:49   ` Nick Alcock
  0 siblings, 1 reply; 5+ messages in thread
From: Kris Van Hees @ 2024-05-23 18:41 UTC (permalink / raw)
  To: Nick Alcock; +Cc: dtrace, dtrace-devel

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.

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).

On Thu, May 23, 2024 at 06:56:54PM +0100, Nick Alcock wrote:
> This moves it out of the way of SystemTap's rather different sdt.h,
> and lets us remove the barely-working kludgery in the specfile
> to work around it.  We point at the new location with a pkg-config
> file, and can immediately use it in make check in the installed
> testsuite, compensating for any install-time changes in the
> location of the dtrace libdir.
> 
> With this change, we no longer confict with any systemtap packages!
> 
> Signed-off-by: Nick Alcock <nick.alcock@oracle.com>
> ---
>  GNUmakefile          |  4 ++++
>  configure            |  2 ++
>  dtrace.spec          | 19 +------------------
>  runtest.sh           |  2 +-
>  uts/Build            | 26 +++++++++++++++++++++++---
>  uts/dtrace_sdt.pc.in |  6 ++++++
>  6 files changed, 37 insertions(+), 22 deletions(-)
>  create mode 100644 uts/dtrace_sdt.pc.in
> 
> This is a provisional patch so people can see what I'm up to --
> tests are still running.  Don't commit yet.
> 
> diff --git a/GNUmakefile b/GNUmakefile
> index 7d1af5641f54a..0390db77b4780 100644
> --- a/GNUmakefile
> +++ b/GNUmakefile
> @@ -86,6 +86,8 @@ BINDIR = $(prefix)/bin
>  INSTBINDIR = $(DESTDIR)$(BINDIR)
>  INCLUDEDIR = $(prefix)/include
>  INSTINCLUDEDIR = $(DESTDIR)$(INCLUDEDIR)
> +SDTINCLUDEDIR = $(LIBDIR)/dtrace/include
> +INSTSDTINCLUDEDIR = $(DESTDIR)$(SDTINCLUDEDIR)
>  SBINDIR = $(prefix)/sbin
>  INSTSBINDIR = $(DESTDIR)$(SBINDIR)
>  UDEVDIR = $(prefix)/lib/udev/rules.d
> @@ -98,6 +100,8 @@ DOCDIR = $(prefix)/share/doc/dtrace-$(VERSION)
>  INSTDOCDIR = $(DESTDIR)$(DOCDIR)
>  MANDIR = $(prefix)/share/man/man8
>  INSTMANDIR = $(DESTDIR)$(MANDIR)
> +PKGCONFIGDIR = $(prefix)/share/pkgconfig
> +INSTPKGCONFIGDIR = $(DESTDIR)$(PKGCONFIGDIR)
>  TESTDIR = $(prefix)/lib$(BITNESS)/dtrace/testsuite
>  INSTTESTDIR = $(DESTDIR)$(TESTDIR)
>  TARGETS =
> diff --git a/configure b/configure
> index 40b870f81d6ff..bc42c4f49a3fa 100755
> --- a/configure
> +++ b/configure
> @@ -60,6 +60,7 @@ Installation paths:
>  --sbindir=PREFIX/sbin				Alias for --bindir
>  --includedir=PREFIX/include			#include directory
>  --mandir=PREFIX/share/man/man8			Manpage directory
> +--pkg-config-dir=PREFIX/share/pkgconfig		Pkg-config directory
>  --udevdir=PREFIX/lib/udev/rules.d		udev rules directory
>  --systemd-unit-dir=PREFIX/lib/systemd/system	systemd unit directory
>  --docdir=PREFIX/share/doc/dtrace		Documentation directory
> @@ -130,6 +131,7 @@ for option in "$@"; do
>          --systemd-unit-dir=*) write_make_var SYSTEMDUNITDIR "$option";;
>          --docdir=*) write_make_var DOCDIR "$option";;
>          --mandir=*) write_make_var MANDIR "$option";;
> +        --mandir=*) write_make_var PKGCONFIGDIR "$option";;
>          --testdir=*) write_make_var TESTDIR "$option";;
>          CC=*) write_make_var CC "$option";;
>          CPP=*) write_make_var PREPROCESS "$option";;
> diff --git a/dtrace.spec b/dtrace.spec
> index 42ea9bc339586..7823b13d981cf 100644
> --- a/dtrace.spec
> +++ b/dtrace.spec
> @@ -86,8 +86,6 @@ BuildRequires: binutils-devel >= 2.30-58.0.8
>  Requires:     libdtrace-ctf >= 1.1.0
>  BuildRequires: libdtrace-ctf-devel >= 1.1.0
>  %endif
> -Conflicts:    systemtap-sdt-devel
> -Provides:     systemtap-sdt-devel
>  Summary:      DTrace user interface.
>  Version:      2.0.1
>  Release:      0%{?dist}
> @@ -194,11 +192,6 @@ make DESTDIR=$RPM_BUILD_ROOT VERSION=%{version} \
>  sed -i '/^ProtectSystem=/d; /^ProtectControlGroups=/d; /^RuntimeDirectory/d;' $RPM_BUILD_ROOT/usr/lib/systemd/system/dtprobed.service
>  %endif
>  
> -# Because systemtap creates a sdt.h header file we have to rename
> -# ours and then shift theirs out of the way.
> -mv $RPM_BUILD_ROOT/usr/include/sys/sdt.h \
> -   $RPM_BUILD_ROOT/usr/include/sys/sdt-dtrace.h
> -
>  %clean
>  [ "$RPM_BUILD_ROOT" != "/" ] && rm -rf "$RPM_BUILD_ROOT"
>  rm -rf $RPM_BUILD_DIR/%{name}-%{version}
> @@ -222,15 +215,6 @@ if [ $1 -ge 2 ] ; then
>      %systemd_postun_with_restart dtprobed.service
>  fi
>  
> -# if sdt-systemtap.h doesn't exist then we can move the existing dtrace sdt.h
> -SYSINCDIR=/usr/include/sys
> -if [ -e $SYSINCDIR/sdt.h -a ! -e $SYSINCDIR/sdt-systemtap.h ]; then
> -    mv $SYSINCDIR/sdt.h $SYSINCDIR/sdt-systemtap.h
> -    ln -s $SYSINCDIR/sdt-dtrace.h $SYSINCDIR/sdt.h
> -elif [ ! -e $SYSINCDIR/sdt.h ]; then
> -    ln -s $SYSINCDIR/sdt-dtrace.h $SYSINCDIR/sdt.h
> -fi
> -
>  %preun
>  %systemd_preun dtprobed.service
>  
> @@ -252,9 +236,8 @@ systemctl start dtprobed || :
>  %{_libdir}/libdtrace.so.*
>  %{_sbindir}/dtrace
>  %{_sbindir}/dtprobed
> +%{_datadir}/pkgconfig/dtrace_sdt.pc
>  %{_mandir}/man8/dtrace.8.gz
> -%{_includedir}/sys/sdt-dtrace.h
> -%{_includedir}/sys/sdt_internal.h
>  %doc %{_docdir}/dtrace-%{version}/*
>  %{_unitdir}/dtprobed.service
>  %{_unitdir}/dtrace-usdt.target
> diff --git a/runtest.sh b/runtest.sh
> index 35d5006978469..78f47c23897c8 100755
> --- a/runtest.sh
> +++ b/runtest.sh
> @@ -579,7 +579,7 @@ else
>      dtrace="/usr/sbin/dtrace"
>      test_libdir="installed"
>      test_ldflags=""
> -    test_incflags="-DARCH_$arch"
> +    test_incflags="-DARCH_$arch $(pkg-config --cflags dtrace_sdt)"
>  
>      if [[ ! -x $dtrace ]]; then
>          echo "$dtrace not available." >&2
> diff --git a/uts/Build b/uts/Build
> index f445115d683db..99bdf19638a50 100644
> --- a/uts/Build
> +++ b/uts/Build
> @@ -10,11 +10,31 @@ uts_DIR := $(current-dir)
>  # provide the userspace versions of types defined distincty by the kernel.
>  
>  SYS_HEADERS_INSTALL := common/sys/dtrace.h \
> -                       common/sys/dtrace_types.h \
> -                       common/sys/sdt.h \
> -                       common/sys/sdt_internal.h
> +                       common/sys/dtrace_types.h
> +
> +# sdt*.h are used by programs that contain USDT probes that want to define
> +# probes by hand rather than using dtrace -h.  SystemTap has another header
> +# with the same name, so we install these out of the way and provide a
> +# pkg-config file to pull them in.
> +
> +PROBE_HEADERS_INSTALL := common/sys/sdt.h \
> +                         common/sys/sdt_internal.h
> +
> +# The pkg-config files undergo a few translations with sed before installation.
> +
> +SHARE_PKG_CONFIG_INSTALL := dtrace_sdt.pc
>  
>  install::
>  	mkdir -p $(INSTINCLUDEDIR)/sys
>  	$(call describe-install-target,$(INSTINCLUDEDIR)/sys,$(notdir $(SYS_HEADERS_INSTALL)))
>  	cd $(uts_DIR) && install -m 644 $(SYS_HEADERS_INSTALL) $(INSTINCLUDEDIR)/sys
> +	mkdir -p $(INSTSDTINCLUDEDIR)/sys
> +	$(call describe-install-target,$(INSTSDTINCLUDEDIR)/sys,$(notdir $(PROBE_HEADERS_INSTALL)))
> +	cd $(uts_DIR) && install -m 644 $(PROBE_HEADERS_INSTALL) $(INSTSDTINCLUDEDIR)/sys
> +	mkdir -p $(INSTPKGCONFIGDIR)
> +	$(call describe-install-target,$(INSTPKGCONFIGDIR),$(SHARE_PKG_CONFIG_INSTALL))
> +	for name in $(SHARE_PKG_CONFIG_INSTALL); do \
> +		cd $(uts_DIR) && \
> +		    sed 's,@SDTINCLUDEDIR@,$(SDTINCLUDEDIR),g; s,@VERSION@,$(VERSION),g' < \
> +			$${name}.in > $(INSTPKGCONFIGDIR)/$$name; \
> +	done
> diff --git a/uts/dtrace_sdt.pc.in b/uts/dtrace_sdt.pc.in
> new file mode 100644
> index 0000000000000..c5506f9355d92
> --- /dev/null
> +++ b/uts/dtrace_sdt.pc.in
> @@ -0,0 +1,6 @@
> +sdtincludedir = @SDTINCLUDEDIR@
> +
> +Name: DTrace SDT
> +Description: DTrace raw SDT headers
> +Version: @VERSION@
> +Cflags: -I${sdtincludedir}
> 
> base-commit: 0f499300c150a3a8878361e31f3ba167f7d5b851
> -- 
> 2.45.1.275.g567cb0950c
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] build: install sdt*.h in /usr/lib64/dtrace/include/sys
  2024-05-23 18:41 ` Kris Van Hees
@ 2024-05-23 18:49   ` Nick Alcock
  2024-05-23 18:56     ` Kris Van Hees
  0 siblings, 1 reply; 5+ messages in thread
From: Nick Alcock @ 2024-05-23 18:49 UTC (permalink / raw)
  To: Kris Van Hees; +Cc: dtrace, dtrace-devel

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).

(Also, splitting them up runs the risk of your integrating one without
the other, and they really do go together.)

> 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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] build: install sdt*.h in /usr/lib64/dtrace/include/sys
  2024-05-23 18:49   ` Nick Alcock
@ 2024-05-23 18:56     ` Kris Van Hees
  2024-05-23 20:02       ` Nick Alcock
  0 siblings, 1 reply; 5+ messages in thread
From: Kris Van Hees @ 2024-05-23 18:56 UTC (permalink / raw)
  To: Nick Alcock; +Cc: Kris Van Hees, dtrace, dtrace-devel

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.

> (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.

> > 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.  If we are not caring about that right now, then
there is no need to introduce pkg-config just yet either, and definitely
not as part of the patch of moving the sdt header files.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] build: install sdt*.h in /usr/lib64/dtrace/include/sys
  2024-05-23 18:56     ` Kris Van Hees
@ 2024-05-23 20:02       ` Nick Alcock
  0 siblings, 0 replies; 5+ messages in thread
From: Nick Alcock @ 2024-05-23 20:02 UTC (permalink / raw)
  To: Kris Van Hees; +Cc: Nick Alcock, dtrace, dtrace-devel

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)

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-05-23 20:02 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-23 17:56 [PATCH] build: install sdt*.h in /usr/lib64/dtrace/include/sys Nick Alcock
2024-05-23 18:41 ` Kris Van Hees
2024-05-23 18:49   ` Nick Alcock
2024-05-23 18:56     ` Kris Van Hees
2024-05-23 20:02       ` Nick Alcock

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox