[PATCH libmnl 0/6] Doxygen Build Improvements

[PATCH libmnl 0/6] Doxygen Build Improvements

[Jeremy Sowden]

These changes were prompted by Mark Mentovai's request to remove a hard-coded
`/bin/bash` path from the rule that creates the man-page sym-links.  Hitherto,
the doxygen Makefile has jumped through a number of hoops to make sure
everything works with `make distcheck` and parallel builds.  This patch-set
makes some doxygen config changes that obviate the need for them, fixes a bug in
`make clean`, updates .gitignore and moves the shell-script out of the Makefile
into a separate file for ease of maintenance.  In the process, the hard-coded
`/bin/bash` is removed.

One thing I have left is the setting of `-p` when running the shell-script.  The
comment reads "`bash -p` prevents import of functions from the environment".
Why is this a problem?

Jeremy Sowden (6):
  build: add `make dist` tar-balls to .gitignore
  doc: add .gitignore for Doxygen artefacts
  doc: change `INPUT` doxygen setting to `@top_srcdir@`
  doc: move doxygen config file into doxygen directory
  doc: move man-page sym-link shell-script into a separate file
  doc: fix doxygen `clean-local` rule

 .gitignore                               |  3 +-
 configure.ac                             | 15 ++++++-
 doxygen/.gitignore                       |  4 ++
 doxygen/Makefile.am                      | 53 +++---------------------
 doxygen.cfg.in => doxygen/doxygen.cfg.in |  4 +-
 doxygen/finalize_manpages.sh             | 40 ++++++++++++++++++
 6 files changed, 67 insertions(+), 52 deletions(-)
 create mode 100644 doxygen/.gitignore
 rename doxygen.cfg.in => doxygen/doxygen.cfg.in (91%)
 create mode 100644 doxygen/finalize_manpages.sh

-- 
2.35.1

[PATCH libmnl 1/6] build: add `make dist` tar-balls to .gitignore

[Jeremy Sowden]

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 .gitignore | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/.gitignore b/.gitignore
index 9a7d86c78ae2..0276c98fb3a5 100644
--- a/.gitignore
+++ b/.gitignore
@@ -17,3 +17,5 @@ Makefile.in
 
 /doxygen.cfg
 /libmnl.pc
+
+/libmnl-*.tar.bz2
-- 
2.35.1

[PATCH libmnl 2/6] doc: add .gitignore for Doxygen artefacts

[Jeremy Sowden]

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 doxygen/.gitignore | 3 +++
 1 file changed, 3 insertions(+)
 create mode 100644 doxygen/.gitignore

diff --git a/doxygen/.gitignore b/doxygen/.gitignore
new file mode 100644
index 000000000000..a23345c2c599
--- /dev/null
+++ b/doxygen/.gitignore
@@ -0,0 +1,3 @@
+doxyfile.stamp
+html/
+man/
-- 
2.35.1

[PATCH libmnl 3/6] doc: change `INPUT` doxygen setting to `@top_srcdir@`

[Jeremy Sowden]

It avoids the need to move src directories in doxygen/Makefile.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 doxygen.cfg.in      |  2 +-
 doxygen/Makefile.am | 11 -----------
 2 files changed, 1 insertion(+), 12 deletions(-)

diff --git a/doxygen.cfg.in b/doxygen.cfg.in
index ae31dbe4c16c..d6db0048a1f7 100644
--- a/doxygen.cfg.in
+++ b/doxygen.cfg.in
@@ -6,7 +6,7 @@ ABBREVIATE_BRIEF       =
 FULL_PATH_NAMES        = NO
 TAB_SIZE               = 8
 OPTIMIZE_OUTPUT_FOR_C  = YES
-INPUT                  = .
+INPUT                  = @top_srcdir@
 FILE_PATTERNS          = */src/*.c
 RECURSIVE              = YES
 EXCLUDE_SYMBOLS        = EXPORT_SYMBOL mnl_nlmsg_batch mnl_socket
diff --git a/doxygen/Makefile.am b/doxygen/Makefile.am
index 29078dee122a..bca5092b4aec 100644
--- a/doxygen/Makefile.am
+++ b/doxygen/Makefile.am
@@ -4,18 +4,7 @@ doc_srcs = $(shell find $(top_srcdir)/src -name '*.c')
 
 doxyfile.stamp: $(doc_srcs) Makefile.am
 	rm -rf html man
-
-# Test for running under make distcheck.
-# If so, sibling src directory will be empty:
-# move it out of the way and symlink the real one while we run doxygen.
-	[ -f ../src/Makefile.in ] || \
-{ set -x; cd ..; mv src src.distcheck; ln -s $(top_srcdir)/src; }
-
 	cd ..; doxygen doxygen.cfg >/dev/null
-
-	[ ! -d ../src.distcheck ] || \
-{ set -x; cd ..; rm src; mv src.distcheck src; }
-
 # We need to use bash for its associative array facility
 # (`bash -p` prevents import of functions from the environment).
 # The command has to be a single line so the functions work
-- 
2.35.1

[PATCH libmnl 4/6] doc: move doxygen config file into doxygen directory

[Jeremy Sowden]

Now that the `INPUT` directory is correct, we can update `OUTPUT_DIRECTORY` to
`.` and we don't need to cd out of the doxygen directory to run doxygen.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 .gitignore                               |  1 -
 configure.ac                             | 15 ++++++++++++++-
 doxygen/.gitignore                       |  1 +
 doxygen/Makefile.am                      |  2 +-
 doxygen.cfg.in => doxygen/doxygen.cfg.in |  2 +-
 5 files changed, 17 insertions(+), 4 deletions(-)
 rename doxygen.cfg.in => doxygen/doxygen.cfg.in (95%)

diff --git a/.gitignore b/.gitignore
index 0276c98fb3a5..b6b8d60db5dc 100644
--- a/.gitignore
+++ b/.gitignore
@@ -15,7 +15,6 @@ Makefile.in
 /libtool
 /stamp-h1
 
-/doxygen.cfg
 /libmnl.pc
 
 /libmnl-*.tar.bz2
diff --git a/configure.ac b/configure.ac
index 314481dae87e..dcdd4245175e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -27,7 +27,20 @@ regular_CFLAGS="-Wall -Waggregate-return -Wmissing-declarations \
 	-Wformat=2 -pipe"
 AC_SUBST([regular_CPPFLAGS])
 AC_SUBST([regular_CFLAGS])
-AC_CONFIG_FILES([Makefile src/Makefile include/Makefile include/libmnl/Makefile include/linux/Makefile include/linux/netfilter/Makefile examples/Makefile examples/genl/Makefile examples/kobject/Makefile examples/netfilter/Makefile examples/rtnl/Makefile libmnl.pc doxygen.cfg doxygen/Makefile])
+AC_CONFIG_FILES([Makefile
+		 src/Makefile
+		 include/Makefile
+		 include/libmnl/Makefile
+		 include/linux/Makefile
+		 include/linux/netfilter/Makefile
+		 examples/Makefile
+		 examples/genl/Makefile
+		 examples/kobject/Makefile
+		 examples/netfilter/Makefile
+		 examples/rtnl/Makefile
+		 libmnl.pc
+		 doxygen/doxygen.cfg
+		 doxygen/Makefile])
 
 AC_ARG_WITH([doxygen], [AS_HELP_STRING([--with-doxygen],
 	    [create doxygen documentation])],
diff --git a/doxygen/.gitignore b/doxygen/.gitignore
index a23345c2c599..2196cf8c442b 100644
--- a/doxygen/.gitignore
+++ b/doxygen/.gitignore
@@ -1,3 +1,4 @@
 doxyfile.stamp
+doxygen.cfg
 html/
 man/
diff --git a/doxygen/Makefile.am b/doxygen/Makefile.am
index bca5092b4aec..3f0b1e9a8ab4 100644
--- a/doxygen/Makefile.am
+++ b/doxygen/Makefile.am
@@ -4,7 +4,7 @@ doc_srcs = $(shell find $(top_srcdir)/src -name '*.c')
 
 doxyfile.stamp: $(doc_srcs) Makefile.am
 	rm -rf html man
-	cd ..; doxygen doxygen.cfg >/dev/null
+	doxygen doxygen.cfg >/dev/null
 # We need to use bash for its associative array facility
 # (`bash -p` prevents import of functions from the environment).
 # The command has to be a single line so the functions work
diff --git a/doxygen.cfg.in b/doxygen/doxygen.cfg.in
similarity index 95%
rename from doxygen.cfg.in
rename to doxygen/doxygen.cfg.in
index d6db0048a1f7..24089ac0cb95 100644
--- a/doxygen.cfg.in
+++ b/doxygen/doxygen.cfg.in
@@ -1,7 +1,7 @@
 # Difference with default Doxyfile 1.8.20
 PROJECT_NAME           = @PACKAGE@
 PROJECT_NUMBER         = @VERSION@
-OUTPUT_DIRECTORY       = doxygen
+OUTPUT_DIRECTORY       = .
 ABBREVIATE_BRIEF       =
 FULL_PATH_NAMES        = NO
 TAB_SIZE               = 8
-- 
2.35.1

[PATCH libmnl 5/6] doc: move man-page sym-link shell-script into a separate file

[Jeremy Sowden]

We use `$(SHELL)` to run the script and exec bash if `$(SHELL)` is something
else.  We don't hard-code the path to bash.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 doxygen/Makefile.am          | 36 +++-----------------------------
 doxygen/finalize_manpages.sh | 40 ++++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 33 deletions(-)
 create mode 100644 doxygen/finalize_manpages.sh

diff --git a/doxygen/Makefile.am b/doxygen/Makefile.am
index 3f0b1e9a8ab4..4770fc788068 100644
--- a/doxygen/Makefile.am
+++ b/doxygen/Makefile.am
@@ -1,42 +1,10 @@
 if HAVE_DOXYGEN
-
 doc_srcs = $(shell find $(top_srcdir)/src -name '*.c')
 
 doxyfile.stamp: $(doc_srcs) Makefile.am
 	rm -rf html man
 	doxygen doxygen.cfg >/dev/null
-# We need to use bash for its associative array facility
-# (`bash -p` prevents import of functions from the environment).
-# The command has to be a single line so the functions work
-# and so `make` gives all lines to `bash -c`
-# (hence ";\" at the end of every line but the last).
-	/bin/bash -p -c 'declare -A renamed_page;\
-main(){ set -e; cd man/man3; rm -f _*;\
-  count_real_pages;\
-  rename_real_pages;\
-  make_symlinks;\
-};\
-count_real_pages(){ page_count=0;\
-  for i in $$(ls -S);\
-  do head -n1 $$i | grep -E -q '^\.so' && break;\
-    page_count=$$(($$page_count + 1));\
-  done;\
-  first_link=$$(($$page_count + 1));\
-};\
-rename_real_pages(){ for i in $$(ls -S | head -n$$page_count);\
-  do for j in $$(ls -S | tail -n+$$first_link);\
-    do grep -E -q $$i$$ $$j && break;\
-    done;\
-    mv -f $$i $$j;\
-    renamed_page[$$i]=$$j;\
-  done;\
-};\
-make_symlinks(){ for j in $$(ls -S | tail -n+$$first_link);\
-  do ln -sf $${renamed_page[$$(cat $$j | cut -f2 -d/)]} $$j;\
-  done;\
-};\
-main'
-
+	$(SHELL) $(top_srcdir)/doxygen/finalize_manpages.sh
 	touch doxyfile.stamp
 
 CLEANFILES = doxyfile.stamp
@@ -53,3 +21,5 @@ install-data-local:
 uninstall-local:
 	rm -r $(DESTDIR)$(mandir) man html doxyfile.stamp
 endif
+
+EXTRA_DIST = finalize_manpages.sh
diff --git a/doxygen/finalize_manpages.sh b/doxygen/finalize_manpages.sh
new file mode 100644
index 000000000000..6f230b12cbc0
--- /dev/null
+++ b/doxygen/finalize_manpages.sh
@@ -0,0 +1,40 @@
+#
+# We need to use bash for its associative array facility
+#
+[ "$BASH" ] || exec bash $0
+#
+# (`bash -p` prevents import of functions from the environment).
+#
+set -p
+
+declare -A renamed_page
+
+main(){ set -e; cd man/man3; rm -f _*
+  count_real_pages
+  rename_real_pages
+  make_symlinks
+}
+
+count_real_pages(){ page_count=0
+  for i in $(ls -S)
+  do head -n1 $i | grep -E -q '^\.so' && break
+    page_count=$(($page_count + 1))
+  done
+  first_link=$(($page_count + 1))
+}
+
+rename_real_pages(){ for i in $(ls -S | head -n$page_count)
+  do for j in $(ls -S | tail -n+$first_link)
+    do grep -E -q $i$ $j && break
+    done
+    mv -f $i $j
+    renamed_page[$i]=$j
+  done
+}
+
+make_symlinks(){ for j in $(ls -S | tail -n+$first_link)
+  do ln -sf ${renamed_page[$(cat $j | cut -f2 -d/)]} $j
+  done
+}
+
+main
-- 
2.35.1

[PATCH libmnl 6/6] doc: fix doxygen `clean-local` rule

[Jeremy Sowden]

The html and man directories are created in the current working directory,
$(top_builddir)/doxygen, not $(top_srcdir)/doxygen.

Remove doxyfile.stamp in `clean-local` and remove `CLEANFILES` in order to do
all cleaning in one place.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 doxygen/Makefile.am | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/doxygen/Makefile.am b/doxygen/Makefile.am
index 4770fc788068..90154c2eb869 100644
--- a/doxygen/Makefile.am
+++ b/doxygen/Makefile.am
@@ -7,11 +7,9 @@ doxyfile.stamp: $(doc_srcs) Makefile.am
 	$(SHELL) $(top_srcdir)/doxygen/finalize_manpages.sh
 	touch doxyfile.stamp
 
-CLEANFILES = doxyfile.stamp
-
 all-local: doxyfile.stamp
 clean-local:
-	rm -rf $(top_srcdir)/doxygen/man $(top_srcdir)/doxygen/html
+	rm -rf doxyfile.stamp html man
 install-data-local:
 	mkdir -p $(DESTDIR)$(mandir)/man3
 	cp --no-dereference --preserve=links,mode,timestamps man/man3/*.3\
-- 
2.35.1

Re: [PATCH libmnl 5/6] doc: move man-page sym-link shell-script into a separate file

[Jan Engelhardt]

On Wednesday 2022-08-03 22:12, Jeremy Sowden wrote:

>We use `$(SHELL)` to run the script and exec bash if `$(SHELL)` is something
>else.  We don't hard-code the path to bash.

Does it matter in practice? I don't recall seeing libmnl targeting
a BSD platform where bash prominently isn't /bin/bash.

Re: [PATCH libmnl 5/6] doc: move man-page sym-link shell-script into a separate file

[Jeremy Sowden]

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

On 2022-08-03, at 22:56:32 +0200, Jan Engelhardt wrote:
> On Wednesday 2022-08-03 22:12, Jeremy Sowden wrote:
> > We use `$(SHELL)` to run the script and exec bash if `$(SHELL)` is
> > something else.  We don't hard-code the path to bash.
> 
> Does it matter in practice? I don't recall seeing libmnl targeting
> a BSD platform where bash prominently isn't /bin/bash.

Apparently, there are cross-building use-cases.

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH libmnl 0/6] Doxygen Build Improvements

[Duncan Roe]

Hi Jeremy,

On Wed, Aug 03, 2022 at 09:12:41PM +0100, Jeremy Sowden wrote:
> These changes were prompted by Mark Mentovai's request to remove a hard-coded
> `/bin/bash` path from the rule that creates the man-page sym-links.  Hitherto,
> the doxygen Makefile has jumped through a number of hoops to make sure
> everything works with `make distcheck` and parallel builds.  This patch-set
> makes some doxygen config changes that obviate the need for them, fixes a bug in
> `make clean`, updates .gitignore and moves the shell-script out of the Makefile
> into a separate file for ease of maintenance.  In the process, the hard-coded
> `/bin/bash` is removed.
>
> One thing I have left is the setting of `-p` when running the shell-script.  The
> comment reads "`bash -p` prevents import of functions from the environment".
> Why is this a problem?
>
> Jeremy Sowden (6):
>   build: add `make dist` tar-balls to .gitignore
>   doc: add .gitignore for Doxygen artefacts
>   doc: change `INPUT` doxygen setting to `@top_srcdir@`
>   doc: move doxygen config file into doxygen directory
>   doc: move man-page sym-link shell-script into a separate file
>   doc: fix doxygen `clean-local` rule
>
>  .gitignore                               |  3 +-
>  configure.ac                             | 15 ++++++-
>  doxygen/.gitignore                       |  4 ++
>  doxygen/Makefile.am                      | 53 +++---------------------
>  doxygen.cfg.in => doxygen/doxygen.cfg.in |  4 +-
>  doxygen/finalize_manpages.sh             | 40 ++++++++++++++++++
>  6 files changed, 67 insertions(+), 52 deletions(-)
>  create mode 100644 doxygen/.gitignore
>  rename doxygen.cfg.in => doxygen/doxygen.cfg.in (91%)
>  create mode 100644 doxygen/finalize_manpages.sh
>
> --
> 2.35.1
Thanks for this but ... I think it would be better to use the netfilter-log
model.

That way there is no new shell script to maintain - libmnl would pick up
libnetfilter-queue's copy.

I found the time-consuming part was checking the new SYNOPSIS lines to be
sufficient. I have far less time nowadays (no Covid lockdowns, &c.) but I will
try to get it done soon-ish.

In the meantime I think Mark's 1-line patch to the existing mess is adequate.

The bin/bash thing is easily resolved: I've tested this in libnetfilter-queue:

> --- a/doxygen/build_man.sh
> +++ b/doxygen/build_man.sh
> @@ -1,4 +1,5 @@
> -#!/bin/bash -p
> +#!/bin/sh
> +[ -n "$BASH" ] || exec bash -p $0
>
>  # Script to process man pages output by doxygen.
>  # We need to use bash for its associative array facility.

Cheers ... Duncan.

Re: [PATCH libmnl 0/6] Doxygen Build Improvements

[Jeremy Sowden]

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

On 2022-08-04, at 13:45:04 +1000, Duncan Roe wrote:
> On Wed, Aug 03, 2022 at 09:12:41PM +0100, Jeremy Sowden wrote:
> > These changes were prompted by Mark Mentovai's request to remove a
> > hard-coded `/bin/bash` path from the rule that creates the man-page
> > sym-links.  Hitherto, the doxygen Makefile has jumped through a
> > number of hoops to make sure everything works with `make distcheck`
> > and parallel builds.  This patch-set makes some doxygen config
> > changes that obviate the need for them, fixes a bug in `make clean`,
> > updates .gitignore and moves the shell-script out of the Makefile
> > into a separate file for ease of maintenance.  In the process, the
> > hard-coded `/bin/bash` is removed.
> >
> > One thing I have left is the setting of `-p` when running the
> > shell-script.  The comment reads "`bash -p` prevents import of
> > functions from the environment".  Why is this a problem?
> >
> > Jeremy Sowden (6):
> >   build: add `make dist` tar-balls to .gitignore
> >   doc: add .gitignore for Doxygen artefacts
> >   doc: change `INPUT` doxygen setting to `@top_srcdir@`
> >   doc: move doxygen config file into doxygen directory
> >   doc: move man-page sym-link shell-script into a separate file
> >   doc: fix doxygen `clean-local` rule
> >
> >  .gitignore                               |  3 +-
> >  configure.ac                             | 15 ++++++-
> >  doxygen/.gitignore                       |  4 ++
> >  doxygen/Makefile.am                      | 53 +++---------------------
> >  doxygen.cfg.in => doxygen/doxygen.cfg.in |  4 +-
> >  doxygen/finalize_manpages.sh             | 40 ++++++++++++++++++
> >  6 files changed, 67 insertions(+), 52 deletions(-)
> >  create mode 100644 doxygen/.gitignore
> >  rename doxygen.cfg.in => doxygen/doxygen.cfg.in (91%)
> >  create mode 100644 doxygen/finalize_manpages.sh
> >
> > --
> > 2.35.1
>
> Thanks for this but ... I think it would be better to use the
> netfilter-log model.
>
> That way there is no new shell script to maintain - libmnl would pick
> up libnetfilter-queue's copy.
>
> I found the time-consuming part was checking the new SYNOPSIS lines to
> be sufficient. I have far less time nowadays (no Covid lockdowns, &c.)
> but I will try to get it done soon-ish.
>
> In the meantime I think Mark's 1-line patch to the existing mess is
> adequate.

Cool.

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]