[PATCH BlueZ 0/4] Handful of build fixes and cleanups

[PATCH BlueZ 0/4] Handful of build fixes and cleanups

[Emil Velikov via B4 Relay]

Hello list,

The very first patch in this series fixes a regression I've introduced
recently - sorry about that.

Namely: two separate patch series introduced an `install-data-hook` and
I didn't properly test with both combined. The former is unconditionally
set while the latter is not.

So if obex is enabled, we fail to create the config files :facepalm:

The second patch is a log pre-existing issue - we install dbus/systemd
service files, for obexd even when building with --disable-obex

The last two are trivial cleanups, so feel free to drop them if they
don't strike your interest.

---
Emil Velikov (4):
      build: fold separate install-data-hooks
      build: install obexd (dbus,systemd} services as needed
      build: drop %.rules make rule
      build: simplify coverage handling

 Makefile.am                               | 27 ++++++++++++---------------
 Makefile.obexd                            |  9 ++-------
 tools/{hid2hci.rules => 97-hid2hci.rules} |  0
 3 files changed, 14 insertions(+), 22 deletions(-)
---
base-commit: b55d98e5cc97e4ff8c3980b84f46c84f3b1c1ee3
change-id: 20240214-hook-fixup-f26304b71366

Best regards,
-- 
Emil Velikov <emil.l.velikov@gmail.com>

[PATCH BlueZ 1/4] build: fold separate install-data-hooks

[Emil Velikov via B4 Relay]

From: Emil Velikov <emil.l.velikov@gmail.com>

Autoconfigure throws a useful error, that I've missed previously. As-is
with two identical hooks `make install` will fail.

Just fold them into single place - not particularly pretty but works.

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
---
 Makefile.am    | 9 +++++++++
 Makefile.obexd | 6 ------
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 5207c172b..e67551761 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -34,6 +34,15 @@ statedir = $(localstatedir)/lib/bluetooth
 install-data-hook:
 	install -dm555 $(DESTDIR)$(confdir)
 	install -dm700 $(DESTDIR)$(statedir)
+if OBEX
+if SYSTEMD
+	$(LN_S) -f obex.service $(DESTDIR)$(SYSTEMD_USERUNITDIR)/dbus-org.bluez.obex.service
+
+uninstall-hook:
+	rm -f $(DESTDIR)$(SYSTEMD_USERUNITDIR)/dbus-org.bluez.obex.service
+endif
+endif
+
 
 if DATAFILES
 dbusdir = $(DBUS_CONFDIR)/dbus-1/system.d
diff --git a/Makefile.obexd b/Makefile.obexd
index 81456544d..b91ca6644 100644
--- a/Makefile.obexd
+++ b/Makefile.obexd
@@ -2,12 +2,6 @@
 if SYSTEMD
 systemduserunitdir = $(SYSTEMD_USERUNITDIR)
 systemduserunit_DATA = obexd/src/obex.service
-
-install-data-hook:
-	$(LN_S) -f obex.service $(DESTDIR)$(SYSTEMD_USERUNITDIR)/dbus-org.bluez.obex.service
-
-uninstall-hook:
-	rm -f $(DESTDIR)$(SYSTEMD_USERUNITDIR)/dbus-org.bluez.obex.service
 endif
 
 dbussessionbusdir = $(DBUS_SESSIONBUSDIR)

-- 
2.43.1

Re: [PATCH BlueZ 1/4] build: fold separate install-data-hooks

[Luiz Augusto von Dentz]

Hi Emil,

On Wed, Feb 14, 2024 at 5:02 PM Emil Velikov via B4 Relay
<devnull+emil.l.velikov.gmail.com@kernel.org> wrote:
>
> From: Emil Velikov <emil.l.velikov@gmail.com>
>
> Autoconfigure throws a useful error, that I've missed previously. As-is
> with two identical hooks `make install` will fail.
>
> Just fold them into single place - not particularly pretty but works.
>
> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
> ---
>  Makefile.am    | 9 +++++++++
>  Makefile.obexd | 6 ------
>  2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/Makefile.am b/Makefile.am
> index 5207c172b..e67551761 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -34,6 +34,15 @@ statedir = $(localstatedir)/lib/bluetooth
>  install-data-hook:
>         install -dm555 $(DESTDIR)$(confdir)
>         install -dm700 $(DESTDIR)$(statedir)
> +if OBEX
> +if SYSTEMD
> +       $(LN_S) -f obex.service $(DESTDIR)$(SYSTEMD_USERUNITDIR)/dbus-org.bluez.obex.service
> +
> +uninstall-hook:
> +       rm -f $(DESTDIR)$(SYSTEMD_USERUNITDIR)/dbus-org.bluez.obex.service
> +endif
> +endif
> +
>  if DATAFILES
>  dbusdir = $(DBUS_CONFDIR)/dbus-1/system.d
> diff --git a/Makefile.obexd b/Makefile.obexd
> index 81456544d..b91ca6644 100644
> --- a/Makefile.obexd
> +++ b/Makefile.obexd
> @@ -2,12 +2,6 @@
>  if SYSTEMD
>  systemduserunitdir = $(SYSTEMD_USERUNITDIR)
>  systemduserunit_DATA = obexd/src/obex.service
> -
> -install-data-hook:
> -       $(LN_S) -f obex.service $(DESTDIR)$(SYSTEMD_USERUNITDIR)/dbus-org.bluez.obex.service
> -
> -uninstall-hook:
> -       rm -f $(DESTDIR)$(SYSTEMD_USERUNITDIR)/dbus-org.bluez.obex.service
>  endif

Hmm, isn't it possible to do target specific install hooks?

>  dbussessionbusdir = $(DBUS_SESSIONBUSDIR)
>
> --
> 2.43.1
>
>


-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ 1/4] build: fold separate install-data-hooks

[Emil Velikov]

Hi Luiz,

On Wed, 14 Feb 2024 at 22:23, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Emil,
>
> On Wed, Feb 14, 2024 at 5:02 PM Emil Velikov via B4 Relay
> <devnull+emil.l.velikov.gmail.com@kernel.org> wrote:
> >
> > From: Emil Velikov <emil.l.velikov@gmail.com>
> >
> > Autoconfigure throws a useful error, that I've missed previously. As-is
> > with two identical hooks `make install` will fail.
> >
> > Just fold them into single place - not particularly pretty but works.
> >
> > Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
> > ---
> >  Makefile.am    | 9 +++++++++
> >  Makefile.obexd | 6 ------
> >  2 files changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/Makefile.am b/Makefile.am
> > index 5207c172b..e67551761 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -34,6 +34,15 @@ statedir = $(localstatedir)/lib/bluetooth
> >  install-data-hook:
> >         install -dm555 $(DESTDIR)$(confdir)
> >         install -dm700 $(DESTDIR)$(statedir)
> > +if OBEX
> > +if SYSTEMD
> > +       $(LN_S) -f obex.service $(DESTDIR)$(SYSTEMD_USERUNITDIR)/dbus-org.bluez.obex.service
> > +
> > +uninstall-hook:
> > +       rm -f $(DESTDIR)$(SYSTEMD_USERUNITDIR)/dbus-org.bluez.obex.service
> > +endif
> > +endif
> > +
> >  if DATAFILES
> >  dbusdir = $(DBUS_CONFDIR)/dbus-1/system.d
> > diff --git a/Makefile.obexd b/Makefile.obexd
> > index 81456544d..b91ca6644 100644
> > --- a/Makefile.obexd
> > +++ b/Makefile.obexd
> > @@ -2,12 +2,6 @@
> >  if SYSTEMD
> >  systemduserunitdir = $(SYSTEMD_USERUNITDIR)
> >  systemduserunit_DATA = obexd/src/obex.service
> > -
> > -install-data-hook:
> > -       $(LN_S) -f obex.service $(DESTDIR)$(SYSTEMD_USERUNITDIR)/dbus-org.bluez.obex.service
> > -
> > -uninstall-hook:
> > -       rm -f $(DESTDIR)$(SYSTEMD_USERUNITDIR)/dbus-org.bluez.obex.service
> >  endif
>
> Hmm, isn't it possible to do target specific install hooks?
>

Good point - will have a try tomorrow/day after.
Hopefully we will be able to keep the obex specifics in Makefile.obex
without adding too many new if blocks.

Thanks
Emil

RE: Handful of build fixes and cleanups

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1211 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=826159

---Test result---

Test Summary:
CheckPatch                    PASS      1.12 seconds
GitLint                       PASS      0.89 seconds
BuildEll                      PASS      23.94 seconds
BluezMake                     PASS      724.63 seconds
MakeCheck                     PASS      11.86 seconds
MakeDistcheck                 PASS      164.98 seconds
CheckValgrind                 PASS      228.99 seconds
CheckSmatch                   PASS      331.74 seconds
bluezmakeextell               PASS      108.25 seconds
IncrementalBuild              FAIL      1354.33 seconds
ScanBuild                     PASS      971.65 seconds

Details
##############################
Test: IncrementalBuild - FAIL
Desc: Incremental build with the patches in the series
Output:

error: tools/97-hid2hci.rules: already exists in working directory
hint: Use 'git am --show-current-patch' to see the failed patch


---
Regards,
Linux Bluetooth

[PATCH BlueZ 2/4] build: install obexd (dbus,systemd} services as needed

[Emil Velikov via B4 Relay]

From: Emil Velikov <emil.l.velikov@gmail.com>

Currently we ship the service files, even when built with
--disable-obex.

Move the if OBEX guard few lines further up.

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
---
 Makefile.obexd | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile.obexd b/Makefile.obexd
index b91ca6644..4a89e0335 100644
--- a/Makefile.obexd
+++ b/Makefile.obexd
@@ -1,4 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
+if OBEX
+
 if SYSTEMD
 systemduserunitdir = $(SYSTEMD_USERUNITDIR)
 systemduserunit_DATA = obexd/src/obex.service
@@ -7,7 +9,6 @@ endif
 dbussessionbusdir = $(DBUS_SESSIONBUSDIR)
 dbussessionbus_DATA = obexd/src/org.bluez.obex.service
 
-if OBEX
 
 obex_plugindir = $(libdir)/obex/plugins
 

-- 
2.43.1

[PATCH BlueZ 3/4] build: drop %.rules make rule

[Emil Velikov via B4 Relay]

From: Emil Velikov <emil.l.velikov@gmail.com>

There's a single file, so we can drop the makefile rule and rename it
accordingly.

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
---
 Makefile.am                               | 8 +-------
 tools/{hid2hci.rules => 97-hid2hci.rules} | 0
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index e67551761..b0845e707 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -435,11 +435,9 @@ if HID2HCI
 rulesdir = $(UDEV_DIR)/rules.d
 
 rules_DATA = tools/97-hid2hci.rules
-
-CLEANFILES += $(rules_DATA)
 endif
 
-EXTRA_DIST += tools/hid2hci.rules
+EXTRA_DIST += tools/97-hid2hci.rules
 
 if TEST
 testdir = $(pkglibdir)/test
@@ -764,10 +762,6 @@ endif
 src/builtin.h: src/genbuiltin $(builtin_sources)
 	$(AM_V_GEN)$(srcdir)/src/genbuiltin $(builtin_modules) > $@
 
-tools/%.rules:
-	$(AM_V_at)$(MKDIR_P) tools
-	$(AM_V_GEN)cp $(srcdir)/$(subst 97-,,$@) $@
-
 $(lib_libbluetooth_la_OBJECTS): $(local_headers)
 
 lib/bluetooth/%.h: lib/%.h
diff --git a/tools/hid2hci.rules b/tools/97-hid2hci.rules
similarity index 100%
rename from tools/hid2hci.rules
rename to tools/97-hid2hci.rules

-- 
2.43.1

[PATCH BlueZ 4/4] build: simplify coverage handling

[Emil Velikov via B4 Relay]

From: Emil Velikov <emil.l.velikov@gmail.com>

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
---
 Makefile.am | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index b0845e707..08eb32852 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -793,8 +793,8 @@ ell/ell.h: Makefile
 maintainer-clean-local:
 	-rm -rf ell
 
-if COVERAGE
 clean-coverage:
+if COVERAGE
 	@lcov --directory $(top_builddir) --zerocounters
 	$(RM) -r coverage $(top_builddir)/coverage.info
 
@@ -803,15 +803,9 @@ coverage: check
 				--output-file $(top_builddir)/coverage.info
 	$(AM_V_at)$(MKDIR_P) coverage
 	@genhtml -o coverage/ $(top_builddir)/coverage.info
+endif
 
 clean-local: clean-coverage
 	-find $(top_builddir) -name "*.gcno" -delete
 	-find $(top_builddir) -name "*.gcda" -delete
 	$(RM) -r lib/bluetooth
-
-else
-clean-local:
-	-find $(top_builddir) -name "*.gcno" -delete
-	-find $(top_builddir) -name "*.gcda" -delete
-	$(RM) -r lib/bluetooth
-endif

-- 
2.43.1

[PATCH BlueZ v2 01/10] build: rework {install-data,uninstall}-hook

[Emil Velikov via B4 Relay]

From: Emil Velikov <emil.l.velikov@gmail.com>

Currently autoconfigure throws a useful error, about a duplicate
install-data-hook.

Address that by making the two target specific and pulling them as
dependencies.

We have to create empty ones otherwise dependency tracking and
resolution will fail.
---
 Makefile.am    |  5 ++++-
 Makefile.obexd | 10 ++++++++--
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 5207c172b..450e931c6 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -31,7 +31,7 @@ AM_LDFLAGS = $(MISC_LDFLAGS)
 confdir = $(sysconfdir)/bluetooth
 statedir = $(localstatedir)/lib/bluetooth
 
-install-data-hook:
+bluetoothd-fix-permissions:
 	install -dm555 $(DESTDIR)$(confdir)
 	install -dm700 $(DESTDIR)$(statedir)
 
@@ -422,6 +422,9 @@ include Makefile.obexd
 include android/Makefile.am
 include Makefile.mesh
 
+install-data-hook: bluetoothd-fix-permissions obexd-add-service-symlink
+uninstall-hook: obexd-remove-service-symlink
+
 if HID2HCI
 rulesdir = $(UDEV_DIR)/rules.d
 
diff --git a/Makefile.obexd b/Makefile.obexd
index 81456544d..0e8bc6de7 100644
--- a/Makefile.obexd
+++ b/Makefile.obexd
@@ -3,11 +3,14 @@ if SYSTEMD
 systemduserunitdir = $(SYSTEMD_USERUNITDIR)
 systemduserunit_DATA = obexd/src/obex.service
 
-install-data-hook:
+obexd-add-service-symlink:
 	$(LN_S) -f obex.service $(DESTDIR)$(SYSTEMD_USERUNITDIR)/dbus-org.bluez.obex.service
 
-uninstall-hook:
+obexd-remove-service-symlink:
 	rm -f $(DESTDIR)$(SYSTEMD_USERUNITDIR)/dbus-org.bluez.obex.service
+else
+obexd-add-service-symlink:
+obexd-remove-service-symlink:
 endif
 
 dbussessionbusdir = $(DBUS_SESSIONBUSDIR)
@@ -102,6 +105,9 @@ obexd_src_obexd_CPPFLAGS = $(AM_CPPFLAGS) $(GLIB_CFLAGS) $(DBUS_CFLAGS) \
 
 obexd_src_obexd_CFLAGS = $(AM_CFLAGS) -fPIC
 
+else
+obexd-add-service-symlink:
+obexd-remove-service-symlink:
 endif
 
 obexd_src_obexd_SHORTNAME = obexd

-- 
2.43.1

RE: Handful of build fixes and cleanups

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 4741 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=826966

---Test result---

Test Summary:
CheckPatch                    PASS      4.37 seconds
GitLint                       PASS      4.58 seconds
BuildEll                      PASS      24.34 seconds
BluezMake                     FAIL      12.09 seconds
MakeCheck                     FAIL      6.42 seconds
MakeDistcheck                 FAIL      34.23 seconds
CheckValgrind                 FAIL      11.26 seconds
CheckSmatch                   FAIL      12.22 seconds
bluezmakeextell               FAIL      11.08 seconds
IncrementalBuild              FAIL      2081.36 seconds
ScanBuild                     PASS      999.98 seconds

Details
##############################
Test: BluezMake - FAIL
Desc: Build BlueZ
Output:

In file included from ./src/eir.h:14,
                 from src/shared/ad.c:24:
./lib/bluetooth/sdp.h:22:10: fatal error: bluetooth/bluetooth.h: No such file or directory
   22 | #include <bluetooth/bluetooth.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [Makefile:8511: src/shared/libshared_mainloop_la-ad.lo] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:4647: all] Error 2
##############################
Test: MakeCheck - FAIL
Desc: Run Bluez Make Check
Output:

In file included from client/mgmt.c:34:
./lib/bluetooth/sdp.h:22:10: fatal error: bluetooth/bluetooth.h: No such file or directory
   22 | #include <bluetooth/bluetooth.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [Makefile:7801: client/mgmt.o] Error 1
make: *** [Makefile:12164: check] Error 2
##############################
Test: MakeDistcheck - FAIL
Desc: Run Bluez Make Distcheck
Output:

In file included from ../../src/eir.h:14,
                 from ../../src/shared/ad.c:24:
../../lib/bluetooth/sdp.h:22:10: fatal error: bluetooth/bluetooth.h: No such file or directory
   22 | #include <bluetooth/bluetooth.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[2]: *** [Makefile:8511: src/shared/libshared_mainloop_la-ad.lo] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [Makefile:4647: all] Error 2
make: *** [Makefile:12085: distcheck] Error 1
##############################
Test: CheckValgrind - FAIL
Desc: Run Bluez Make Check with Valgrind
Output:

In file included from ./src/eir.h:14,
                 from src/shared/ad.c:24:
./lib/bluetooth/sdp.h:22:10: fatal error: bluetooth/bluetooth.h: No such file or directory
   22 | #include <bluetooth/bluetooth.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [Makefile:8511: src/shared/libshared_mainloop_la-ad.lo] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:12164: check] Error 2
##############################
Test: CheckSmatch - FAIL
Desc: Run smatch tool with source
Output:

src/shared/crypto.c:271:21: warning: Variable length array is used.
src/shared/crypto.c:272:23: warning: Variable length array is used.
src/shared/ad.c: note: in included file (through src/eir.h):
./lib/bluetooth/sdp.h:22:11: error: unable to open 'bluetooth/bluetooth.h'
./lib/bluetooth/sdp.h:22:11: error: using '/github/workspace/src/src/lib/bluetooth/bluetooth.h'
In file included from ./src/eir.h:14,
                 from src/shared/ad.c:24:
./lib/bluetooth/sdp.h:22:10: fatal error: bluetooth/bluetooth.h: No such file or directory
   22 | #include <bluetooth/bluetooth.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [Makefile:8511: src/shared/libshared_mainloop_la-ad.lo] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:4647: all] Error 2
##############################
Test: bluezmakeextell - FAIL
Desc: Build Bluez with External ELL
Output:

In file included from ./src/eir.h:14,
                 from src/shared/ad.c:24:
./lib/bluetooth/sdp.h:22:10: fatal error: bluetooth/bluetooth.h: No such file or directory
   22 | #include <bluetooth/bluetooth.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [Makefile:8511: src/shared/libshared_mainloop_la-ad.lo] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:4647: all] Error 2
##############################
Test: IncrementalBuild - FAIL
Desc: Incremental build with the patches in the series
Output:

error: tools/97-hid2hci.rules: already exists in working directory
hint: Use 'git am --show-current-patch' to see the failed patch


---
Regards,
Linux Bluetooth