[PATCH] Automatic man page update from rhd_id.c strings
All the required building blocks are contained in this patch:
* A program to be linked against rhd_id.c which prints the chipset
strings by calling RHDIdentify().
* sed(1) expressions and shell code to actually update the man page
using those strings.
* Example "make update-man" target which interactively updates the
man page in $(srcdir).
Whether/How exactly to integrate this will require some discussion.
The author can imagine multiple options to make use of this code:
* "make check" testcase making sure man/radeon.man and a newly
updated man page match
* "make dist" hook doing the same
* Non-interactive update of man/radeonhd.man, possibly to be run every
time rhd_id.c has changed, so that man/radeonhd.man will never be out
of sync with rhd_id.c. Requires compatibility work on sed expression
(does not work with SED="/sbin/busybox sed")
* Interactive update of man page in source (this code).
Each option has its own number of (dis)advantages.
---
Makefile.am | 6 ++++
man/Makefile.am | 34 ++++++++++++++++++++++-
man/print-device-list.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++
man/radeonhd.man | 2 +
4 files changed, 107 insertions(+), 2 deletions(-)
create mode 100644 man/print-device-list.c
diff --git a/Makefile.am b/Makefile.am
index 36f5bf2..b723d71 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1,2 +1,8 @@
AUTOMAKE_OPTIONS = foreign
SUBDIRS = man src
+
+if MAINTAINER_MODE
+# Interactively update man page from rhd_id.c RHDIdentify() output.
+update-man:
+ cd man && $(MAKE) update-man
+endif
diff --git a/man/Makefile.am b/man/Makefile.am
index bf7ec17..07350aa 100644
--- a/man/Makefile.am
+++ b/man/Makefile.am
@@ -35,8 +35,6 @@ EXTRA_DIST = @DRIVER_NAME@.man
CLEANFILES = $(driverman_DATA)
-SED = sed
-
# Strings to replace in man pages
XORGRELSTRING = @PACKAGE_STRING@
XORGMANNAME = X Version 11
@@ -57,3 +55,35 @@ SUFFIXES = .$(DRIVER_MAN_SUFFIX) .man
.man.$(DRIVER_MAN_SUFFIX):
sed $(MAN_SUBSTS) < $< > $@
+
+EXTRA_PROGRAMS = print-device-list
+
+print_device_list_SOURCES = print-device-list.c $(top_srcdir)/src/rhd_id.c
+print_device_list_CFLAGS = $(AM_CFLAGS) -I$(top_srcdir)/src $(XORG_CFLAGS) $(WARN_CFLAGS) $(PEDANTIC_CFLAGS)
+
+if MAINTAINER_MODE
+CLEANFILES += radeonhd.man.new
+radeonhd.man.new: print-device-list $(srcdir)/radeonhd.man
+ (\
+ $(SED) -n '1,/^\.\\" START_DEVICE_LIST marker - do not delete/p' "$(srcdir)/radeonhd.man"; \
+ ./print-device-list \
+ | $(SED) \
+ -e '/^$$/d' \
+ -e '/^RADEONHD:/d' \
+ -e 's/\.$$//' \
+ -e 's/^ \([A-Z0-9]\{1,\}\) \{1,\}: \(.*\)/.TP 8\n.B \1\n\2/'; \
+ $(SED) -n '/^\.\\" END_DEVICE_LIST marker - do not delete/,$$p' "$(srcdir)/radeonhd.man" \
+ ) < "$(srcdir)/radeonhd.man" > radeonhd.man.new
+
+# Interactively update man page from rhd_id.c RHDIdentify() output.
+update-man: radeonhd.man.new
+ if diff -u "$(srcdir)/radeonhd.man" radeonhd.man.new; then :; else \
+ echo "New man pages differs from old one."; \
+ echo "Overwrite old with new? Press ENTER to continue, Ctrl-C to abort."; \
+ if read < /dev/tty; then \
+ mv -f radeonhd.man.new "$(srcdir)/radeonhd.man"; \
+ else \
+ exit 1; \
+ fi; \
+ fi
+endif
diff --git a/man/print-device-list.c b/man/print-device-list.c
new file mode 100644
index 0000000..059aff6
--- /dev/null
+++ b/man/print-device-list.c
@@ -0,0 +1,67 @@
+/* print-device-list.c - print radeonhd device list
+ * Copyright 2007 Hans Ulrich Niedermann
On Oct 29, 07 13:21:21 +0100, Hans Ulrich Niedermann wrote:
All the required building blocks are contained in this patch:
* A program to be linked against rhd_id.c which prints the chipset strings by calling RHDIdentify(). * sed(1) expressions and shell code to actually update the man page using those strings. * Example "make update-man" target which interactively updates the man page in $(srcdir).
Question: why isn't rhd_id.c parsed instead of compiling a build tool for extracting that information? Having rhd_id.c parsed would e.g. be more cross-compiling friendly (not that I claim that cross compiling works with this driver - I guess this is untested).
* Interactive update of man page in source (this code).
I would rather do this as an intermediate step, performing this on each
and every man page compilation. A string like %CARD_IDS% could be
replaced by the card names.
Matthias
--
Matthias Hopf
Matthias Hopf wrote:
On Oct 29, 07 13:21:21 +0100, Hans Ulrich Niedermann wrote:
All the required building blocks are contained in this patch:
* A program to be linked against rhd_id.c which prints the chipset strings by calling RHDIdentify(). * sed(1) expressions and shell code to actually update the man page using those strings. * Example "make update-man" target which interactively updates the man page in $(srcdir).
Question: why isn't rhd_id.c parsed instead of compiling a build tool for extracting that information? Having rhd_id.c parsed would e.g. be more cross-compiling friendly.
Two reasons: a) I guessed that the output strings could be more easily made to be something reasonable, even if someone decided to rewrite RHDIdentify() to use the device list structs in rhd_id.c to produce its output. That guess might be wrong - I'm not part of your group of core developers, obviously :) b) Parsing a C program is definitely non-trivial compared to parsing some simple text. Executing the code instead of parsing all the #ifdefs and comments with sed seemed to be easier. Well... it might be possible to use the proper C preprocessor in a first step and only then parse the result with sed, but that seems a lot less straightforward. It would remain to be seen how portable that can be made. We currently do not distinguish between CFLAGS and CPPFLAGS e.g., which requires more changes all over the place. I try not to propose too invasive changes to avoid the natural "All those gazillion changes for just that little effect?" reactions, so I'd quickly rule out such a CPPFLAGS/CFLAGS separation as a precursor to the man page check.
* Interactive update of man page in source (this code).
I would rather do this as an intermediate step, performing this on each and every man page compilation. A string like %CARD_IDS% could be replaced by the card names.
This would boldly break cross-compilation on purpose, which I would not dare suggest as a non-core developer :-) Besides, I don't see a need to run all that complicated stuff on every build - when it only needs to be run after a subset of rhd_id.c changes. A good way IMHO would probably be to run the updater (whichever we end up choosing) everytime "make all" is run in src/, if MAINTAINER_MODE. That would remind the devopler who has modified the device list in rhd_id.c but forgotten to update the man page - and that is the point of the exercise, right? -- Hans Ulrich Niedermann
On Oct 29, 07 15:53:13 +0100, Hans Ulrich Niedermann wrote:
On Oct 29, 07 13:21:21 +0100, Hans Ulrich Niedermann wrote: Question: why isn't rhd_id.c parsed instead of compiling a build tool for extracting that information? Having rhd_id.c parsed would e.g. be more cross-compiling friendly. Two reasons: a) I guessed that the output strings could be more easily made to be something reasonable, even if someone decided to rewrite RHDIdentify() to use the device list structs in rhd_id.c to
Matthias Hopf wrote: produce its output. b) Parsing a C program is definitely non-trivial compared to parsing some simple text. Executing the code instead of parsing all the #ifdefs and comments with sed seemed to be easier.
Both very reasonable. Still, as both (man page generation and source code) are in the same tree, I think it is reasonable to assume that the layout won't change so much that the man page generation would break (read: if it changes, the man page generation can be updated as well). I don't think any #ifdefs are involved here, ever. But I think both Luc and Stefan should comment on that as well.
that seems a lot less straightforward. It would remain to be seen how portable that can be made. We currently do not distinguish between CFLAGS and CPPFLAGS e.g., which requires more changes all over the place.
Not too much, I think. Also I don't think we need a praeprocessor in this case. I could come up with a short perl, awk, or sed script for that.
* Interactive update of man page in source (this code). I would rather do this as an intermediate step, performing this on each and every man page compilation. A string like %CARD_IDS% could be replaced by the card names. This would boldly break cross-compilation on purpose, which I would not dare suggest as a non-core developer :-)
Hm. Why? Ok, in your original setting I understand that :-P
Besides, I don't see a need to run all that complicated stuff on every build - when it only needs to be run after a subset of rhd_id.c changes.
Right. Ok, if it is a separate build rule, fine. But that you won't notice changes automatically. I like automatic updates :)
That would remind the devopler who has modified the device list in rhd_id.c but forgotten to update the man page - and that is the point of the exercise, right?
Actually, the point is to automate this :P
CU
Matthias
--
Matthias Hopf
Matthias Hopf wrote:
On Oct 29, 07 15:53:13 +0100, Hans Ulrich Niedermann wrote:
On Oct 29, 07 13:21:21 +0100, Hans Ulrich Niedermann wrote: Question: why isn't rhd_id.c parsed instead of compiling a build tool for extracting that information? Having rhd_id.c parsed would e.g. be more cross-compiling friendly. Two reasons: a) I guessed that the output strings could be more easily made to be something reasonable, even if someone decided to rewrite RHDIdentify() to use the device list structs in rhd_id.c to
Matthias Hopf wrote: produce its output. b) Parsing a C program is definitely non-trivial compared to parsing some simple text. Executing the code instead of parsing all the #ifdefs and comments with sed seemed to be easier.
Both very reasonable. Still, as both (man page generation and source code) are in the same tree, I think it is reasonable to assume that the layout won't change so much that the man page generation would break (read: if it changes, the man page generation can be updated as well). I don't think any #ifdefs are involved here, ever.
But I think both Luc and Stefan should comment on that as well.
Well... I couldn't stop myself from implementing a simple sed script parsing rhd_id.c - that may be useful at least for illustration purposes. See my reply to this mail.
Besides, I don't see a need to run all that complicated stuff on every build - when it only needs to be run after a subset of rhd_id.c changes.
Right. Ok, if it is a separate build rule, fine. But that you won't notice changes automatically. I like automatic updates :)
Generally, I concur about automatic updates being very nice. In this case, however, I prefer having the developer who is about to commit a rhd_id.c with a modified RHDIdentify() function to give the man page change going with that at least a cursory look.
That would remind the devopler who has modified the device list in rhd_id.c but forgotten to update the man page - and that is the point of the exercise, right?
Actually, the point is to automate this :P
Let's comment the others on how far this automation should go. To illustrate my point, I'll reply to this mail with a patch that a) uses sed to parse rhd_id.c directly b) updates the man page if rhd_id.c RHDIdentify() has changed c) alerts the developer on every first "make" run after such a change -- Hans Ulrich Niedermann -- To unsubscribe, e-mail: radeonhd+unsubscribe@opensuse.org For additional commands, e-mail: radeonhd+help@opensuse.org
From 19576806ddda04d08bbb2db022b8134fb0abc5fd Mon Sep 17 00:00:00 2001 From: Hans Ulrich Niedermann
Date: Tue, 30 Oct 2007 01:55:04 +0100 Subject: [PATCH] Automatically update man page from rhd_id.c X-See-URL: http://radeonhd.lauft.net/patches/
In a normal build in src/ ("make", "make all"), ensure that the man page (man/radeonhd.man) is consistent with rhd_id.c. If that requires man/radeonhd.man to be changed, display a warning message and abort build. --- Makefile.am | 3 ++- man/radeonhd.man | 6 +++--- src/Makefile.am | 31 +++++++++++++++++++++++++++++++ src/rhd_id.c | 2 ++ 4 files changed, 38 insertions(+), 4 deletions(-) diff --git a/Makefile.am b/Makefile.am index 36f5bf2..4b99d66 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1,2 +1,3 @@ AUTOMAKE_OPTIONS = foreign -SUBDIRS = man src +# src before man: src/ may update sources in man/ +SUBDIRS = src man diff --git a/man/radeonhd.man b/man/radeonhd.man index 7acfe6f..98ae47b 100644 --- a/man/radeonhd.man +++ b/man/radeonhd.man @@ -36,9 +36,8 @@ features. The .B radeonhd driver supports video cards based on the following ATI chips: -.\" The following list was generated from "X -logverbose 7" by the following command: -.\" sed -n '/^\t[RM][A-Z0-9]\+ * : /{ s/\.$//; s/^\t\([A-Z0-9]\+\) \+: \(.*\)/.TP 8\n.B \1\n\2/; p};' /var/log/Xorg.0.log -.\" The list replicates the output of RHDIdentify() in src/rhd_id.c. +.\" This list is generated from the RHDIdentify() function in src/rhd_id.c. +.\" START_DEVICE_LIST marker - do not delete .TP 8 .B RV505 Radeon X1550, X1550 64bit @@ -120,6 +119,7 @@ Radeon X1200 .TP 8 .B RS740 RS740, RS740M +.\" END_DEVICE_LIST marker - do not delete .\" .\" .SH CONFIGURATION DETAILS diff --git a/src/Makefile.am b/src/Makefile.am index 225a2f6..8d365fc 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -79,3 +79,34 @@ libatom_la_SOURCES = \ radeonhd_drv_la_LIBADD = libatom.la endif + +if MAINTAINER_MODE +SRCMAN = $(top_srcdir)/man/radeonhd.man + +CLEANFILES = radeonhd.man.new +radeonhd.man.new: rhd_id.c $(SRCMAN) + echo "Generating radeon.man.new..."; \ + ( $(SED) -n '1,/^\.\\" START_DEVICE_LIST marker - do not delete/p' "$(SRCMAN)"; \ + $(SED) -e '1,/^[ ]*\/\* START_DEVICE_LIST marker - do not delete \*\/ *$$/d' \ + -e '/^[ ]*\/\* END_DEVICE_LIST marker - do not delete \*\/ *$$/,$$d' \ + -e '/^[ ]\{1,\}xf86Msg/d' \ + -e 's/^[ ]\{1,\}"//' \ + -e 's/\.\?\\n\(\\t"\|");\)$$//' \ + -e 's/^\([A-Z0-9]\{1,\}\) \{1,\}: \(.*\)/.TP 8\n.B \1\n\2/' \ + "$(srcdir)/rhd_id.c"; \ + $(SED) -n '/^\.\\" END_DEVICE_LIST marker - do not delete/,$$p' "$(SRCMAN)" \ + ) > radeonhd.man.new + +# Update man page from rhd_id.c RHDIdentify() output. +# The exit(1) makes sure the update does not go unnoticed. +all-local: radeonhd.man.new + @if diff -u "$(SRCMAN)" radeonhd.man.new; then \ + echo "radeon(4) man page is current."; \ + else \ + echo "WARNING: New man pages differs from old one."; \ + echo " Please check the changes to $(SRCMAN) and commit them together with your"; \ + echo " changes to $(srcdir)/rhd_id.c."; \ + mv -f radeonhd.man.new "$(SRCMAN)"; \ + exit 1; \ + fi +endif diff --git a/src/rhd_id.c b/src/rhd_id.c index 79f95e8..d0fac6b 100644 --- a/src/rhd_id.c +++ b/src/rhd_id.c @@ -235,6 +235,7 @@ void RHDIdentify(int flags) { xf86Msg(X_INFO, "%s: X driver for the following AMD GPG (ATI) graphics devices:\n", RHD_NAME); + /* START_DEVICE_LIST marker - do not delete */ xf86Msg(X_NONE, "\t" "RV505 : Radeon X1550, X1550 64bit.\n\t" "RV515 : Radeon X1300, X1550, X1600; FireGL V3300, V3350.\n\t" @@ -265,6 +266,7 @@ RHDIdentify(int flags) "M76 : Mobility Radeon HD 2600; (Gemini ATI) Mobility Radeon HD 2600 XT.\n\t" "RS690 : Radeon X1200.\n\t" "RS740 : RS740, RS740M\n"); + /* END_DEVICE_LIST marker - do not delete */ xf86Msg(X_NONE, "\n"); } -- 1.5.3.4 -- To unsubscribe, e-mail: radeonhd+unsubscribe@opensuse.org For additional commands, e-mail: radeonhd+help@opensuse.org
participants (2)
-
Hans Ulrich Niedermann
-
Matthias Hopf