Add --enable-documentation option#2273
Conversation
Also, simplify *.8 generation using .pl.in sources instead of the built scripts.
|
I'm not entirely sure it's beneficial to not automatically build man pages. What is the problem you'd like to solve and for whom? |
| SQUID_EMBED_BUILD_INFO | ||
|
|
||
| AC_ARG_ENABLE(documentation, | ||
| AS_HELP_STRING([--enable-documentation],[ |
There was a problem hiding this comment.
It feels wrong for the default Squid installation to have no manual pages (in environments where we can build those manual pages).
Please either keep the old "automated" default approach (i.e. enable the feature if we found the right tools) OR enable by default (breaking build in environments that do not supply those tools).
|
|
||
| AC_ARG_ENABLE(documentation, | ||
| AS_HELP_STRING([--enable-documentation],[ | ||
| Build optional documentation. |
There was a problem hiding this comment.
Most readers will not know what documentation we consider optional. This should be spelled out.
| @@ -5,11 +5,10 @@ | |||
| ## Please see the COPYING and CONTRIBUTORS files for details. | |||
| ## | |||
|
|
|||
There was a problem hiding this comment.
PR description: This option now required to generate documentation.
The above assertion is false because we still generate some documentation (e.g., squid.conf.documented) even when building with --disable-documentation. Please be more precise when describing the scope of this PR and the effect of the new configure parameter.
Once sufficient precision is achieved, we can discuss the best name for the new configure parameter(s).
Make documentation-specific tools optional depending on whether documentation is being generated.
Those tools were optional before this PR, right? Please rephrase to clarify what has changed in that regard.
Small refactoring to ensure that generated documentation (including release notes) absence does not break anything.
Changes to release notes generation should wait for the conclusion of the discussion at #2261 (comment)
| AC_MSG_WARN([pkg-config not found. Many optional features with external dependencies will not be enabled by default, usually without further warnings.]) | ||
| ]) | ||
|
|
||
|
|
There was a problem hiding this comment.
Please avoid out-of-scope formatting changes.
| AS_IF([test "x$enable_documentation" = "xyes"],[ | ||
| AC_CHECK_TOOL([LINUXDOC],[linuxdoc],[:]) | ||
| AS_IF([test "$LINUXDOC" = ":"],[ | ||
| AC_MSG_FAILURE([linuxdoc is required to compile Squid documentation. Please install linuxdoc tools and then re-run configure.]) |
There was a problem hiding this comment.
AFAICT, Autoconf documentation implies that we should use AC_MSG_ERROR() here because this is not a compilation error that is likely to be detailed in config.log. Please test and adjust if needed.
| SQUID_YESNO([$enableval],[--enable-documentation]) | ||
| ]) | ||
| AS_IF([test "x$enable_documentation" = "xyes"],[ | ||
| AC_CHECK_TOOL([LINUXDOC],[linuxdoc],[:]) |
There was a problem hiding this comment.
Unlike AC_CHECK_PROG() that Squid already uses, AC_CHECK_TOOL() implies that we are dealing with some cross-compilation concerns here. Do we have to use that function for linuxdoc?
There was a problem hiding this comment.
We should use this macro for these tools because they are only applicable for the build host. This avoids incorrectly finding target machine binaries in PATH and trying to use them.
Also, Squid is definitely cross-compiled on at least four OS. We should be using macros that sorts those problems out for all checks. But that is a wider problem out of scope here.
| linuxdoc -B html -T 2 --split=0 $< | ||
| perl -i -p -e "s%$@%%" $@ | ||
| cp -p $@ $(top_builddir)/RELEASENOTES.html | ||
| $(LINUXDOC) -B html -T 2 --split=0 $< && $(PERL) -i -p -e "s%$@%%" $@ |
There was a problem hiding this comment.
To keep this code more readable and simpler, please keep these two commands on their separate/dedicated lines.
| .sgml.html: | ||
| linuxdoc -B html -T 2 --split=0 $< | ||
| perl -i -p -e "s%$@%%" $@ | ||
| cp -p $@ $(top_builddir)/RELEASENOTES.html |
There was a problem hiding this comment.
With this RELEASENOTES.html action removed, do we no longer install that file (when the tools to generate it are available, etc.)?
There was a problem hiding this comment.
The install is done by make dist when packaging. It does not need to be done here as well.
yadij
added
the
M-ignored-by-merge-bots
3 participants
This option now required to generate documentation.
Make documentation-specific tools optional depending on whether
documentation is being generated.
Small refactoring to ensure that generated documentation
(including release notes) absence does not break anything.