docs: Integrate rustdoc into Rust documentation

Message ID 20221130220825.1545758-1-carlos.bilbao@amd.com
State New
Headers
Series docs: Integrate rustdoc into Rust documentation |

Commit Message

Carlos Bilbao Nov. 30, 2022, 10:08 p.m. UTC
  Include HTML output generated with rustdoc into the Linux kernel
documentation on Rust. Change target `make htmldocs` to combine RST Sphinx
and the generation of Rust documentation, when support is available.

Signed-off-by: Carlos Bilbao <carlos.bilbao@amd.com>
---
 Documentation/Makefile         |  4 ++++
 Documentation/rust/index.rst   |  1 +
 Documentation/rust/rustdoc.rst |  7 +++++++
 rust/Makefile                  | 15 +++++++++------
 4 files changed, 21 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/rust/rustdoc.rst
  

Comments

Miguel Ojeda Dec. 1, 2022, 12:42 p.m. UTC | #1
On Wed, Nov 30, 2022 at 11:08 PM Carlos Bilbao <carlos.bilbao@amd.com> wrote:
>
> Include HTML output generated with rustdoc into the Linux kernel
> documentation on Rust. Change target `make htmldocs` to combine RST Sphinx
> and the generation of Rust documentation, when support is available.

Looks better, thanks for this v2! A few comments below...

> +ifdef CONFIG_RUST
> +       @make LLVM=1 rustdoc
> +endif

The Rust docs should probably be built with the build
system/config/... as given, whether it is GCC, LLVM, etc. This should
probably use `$(MAKE)` too; and if you intended to remove the command
line definitions, `MAKEOVERRIDES` too.

Ideally `htmldocs` would depend on `rustdoc`, though that would
require shuffling quite a few things since to build the Rust docs we
need a subset of the Rust dependencies built. Given we may be changing
things soon anyway on the Rust `Makefile`, we can leave that for the
future.

By the way, while checking this, I noticed we use some `CONFIG_`s in
this `Makefile`, but we do not perform a config sync for the `*docs`
targets, so one needs to do so manually, i.e. it can be a pitfall for
e.g. `CONFIG_WARN_MISSING_DOCUMENTS` and ` as well as a potential
`CONFIG_RUST`. Should this be fixed orthogonally, or is it intended?
(some targets do not need the sync, and the ones that need are
probably less used, so I guess that could be the reason?).

> +Rustdoc output
> +==============
> +
> +If this documentation includes rustdoc-generated HTML, the entry point
> +can be found `here. <rustdoc/kernel/index.html>`_

Perhaps this sentence could be moved to the top of the index file, so
that users do not need two clicks when visiting "Rust"? That way we
avoid one more file too.

> +RUSTDOC_OUTPUT=$(objtree)/Documentation/output/rust/rustdoc

Please add a space around the equal sign to be consistent with (most)
of the rest of the file.

Cheers,
Miguel
  
Carlos Bilbao Dec. 1, 2022, 8:40 p.m. UTC | #2
On 12/1/22 06:42, Miguel Ojeda wrote:

> On Wed, Nov 30, 2022 at 11:08 PM Carlos Bilbao <carlos.bilbao@amd.com> wrote:
>
>> +ifdef CONFIG_RUST
>> +       @make LLVM=1 rustdoc
>> +endif
> The Rust docs should probably be built with the build
> system/config/... as given, whether it is GCC, LLVM, etc. This should
> probably use `$(MAKE)` too; and if you intended to remove the command
> line definitions, `MAKEOVERRIDES` too.


Sounds good.


> By the way, while checking this, I noticed we use some `CONFIG_`s in
> this `Makefile`, but we do not perform a config sync for the `*docs`
> targets, so one needs to do so manually, i.e. it can be a pitfall for
> e.g. `CONFIG_WARN_MISSING_DOCUMENTS` and ` as well as a potential
> `CONFIG_RUST`. Should this be fixed orthogonally, or is it intended?
> (some targets do not need the sync, and the ones that need are
> probably less used, so I guess that could be the reason?).


I don't understand config sync. Perhaps that, e.g. Documentation/Makefile
checks for broken docs, for CONFIG_WARN_MISSING_DOCUMENTS, but we don't
do that for rust/Makefile? I'm not sure, but it does sound orthogonal, yes.


>
>> +Rustdoc output
>> +==============
>> +
>> +If this documentation includes rustdoc-generated HTML, the entry point
>> +can be found `here. <rustdoc/kernel/index.html>`_
> Perhaps this sentence could be moved to the top of the index file, so
> that users do not need two clicks when visiting "Rust"? That way we
> avoid one more file too.


Yes, that will be better :)


>
>> +RUSTDOC_OUTPUT=$(objtree)/Documentation/output/rust/rustdoc
> Please add a space around the equal sign to be consistent with (most)
> of the rest of the file.


Ack


>
> Cheers,
> Miguel


Thanks,
Carlos
  
Miguel Ojeda Dec. 2, 2022, 4:27 p.m. UTC | #3
On Thu, Dec 1, 2022 at 9:40 PM Carlos Bilbao <carlos.bilbao@amd.com> wrote:
>
> I don't understand config sync. Perhaps that, e.g. Documentation/Makefile
> checks for broken docs, for CONFIG_WARN_MISSING_DOCUMENTS, but we don't
> do that for rust/Makefile? I'm not sure, but it does sound orthogonal, yes.

Config sync is what needs to happen to make a bunch of files in
`include/` up to date with respect to the `.config`. It runs
automatically for some targets, but not always. For instance, the
`*docs` targets do not trigger it. So if you enable e.g.
`CONFIG_WARN_MISSING_DOCUMENTS`, and immediately afterwards run
`htmldocs`, it will not take it into account.

But don't worry about it: that part of my comment was directed at
others (e.g. Jon, Akira...) that may know the historical context or
the reason behind it -- no need to fix it here. I mentioned it here
since it affects `CONFIG_RUST` if we use it there (in the same way as
the other `CONFIG_*` used there).

Cheers,
Miguel
  
Carlos Bilbao Dec. 2, 2022, 4:30 p.m. UTC | #4
On 12/2/22 10:27, Miguel Ojeda wrote:

> On Thu, Dec 1, 2022 at 9:40 PM Carlos Bilbao <carlos.bilbao@amd.com> wrote:
>> I don't understand config sync. Perhaps that, e.g. Documentation/Makefile
>> checks for broken docs, for CONFIG_WARN_MISSING_DOCUMENTS, but we don't
>> do that for rust/Makefile? I'm not sure, but it does sound orthogonal, yes.
> Config sync is what needs to happen to make a bunch of files in
> `include/` up to date with respect to the `.config`. It runs
> automatically for some targets, but not always. For instance, the
> `*docs` targets do not trigger it. So if you enable e.g.
> `CONFIG_WARN_MISSING_DOCUMENTS`, and immediately afterwards run
> `htmldocs`, it will not take it into account.
>
> But don't worry about it: that part of my comment was directed at
> others (e.g. Jon, Akira...) that may know the historical context or
> the reason behind it -- no need to fix it here. I mentioned it here
> since it affects `CONFIG_RUST` if we use it there (in the same way as
> the other `CONFIG_*` used there).


Ah, that makes sense. I appreciate the clarification!


>
> Cheers,
> Miguel


Thanks,
Carlos
  

Patch

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 64d44c1ecad3..5e9435198cfb 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -92,6 +92,10 @@  quiet_cmd_sphinx = SPHINX  $@ --> file://$(abspath $(BUILDDIR)/$3/$4)
 	fi
 
 htmldocs:
+# If Rust support is available, add rustdoc generated contents
+ifdef CONFIG_RUST
+	@make LLVM=1 rustdoc
+endif
 	@$(srctree)/scripts/sphinx-pre-install --version-check
 	@+$(foreach var,$(SPHINXDIRS),$(call loop_cmd,sphinx,html,$(var),,$(var)))
 
diff --git a/Documentation/rust/index.rst b/Documentation/rust/index.rst
index 4ae8c66b94fa..5e7a9d39af90 100644
--- a/Documentation/rust/index.rst
+++ b/Documentation/rust/index.rst
@@ -13,6 +13,7 @@  in the kernel, please read the quick-start.rst guide.
     general-information
     coding-guidelines
     arch-support
+    rustdoc
 
 .. only::  subproject and html
 
diff --git a/Documentation/rust/rustdoc.rst b/Documentation/rust/rustdoc.rst
new file mode 100644
index 000000000000..aa63a550b34c
--- /dev/null
+++ b/Documentation/rust/rustdoc.rst
@@ -0,0 +1,7 @@ 
+.. SPDX-License-Identifier: GPL-2.0
+
+Rustdoc output
+==============
+
+If this documentation includes rustdoc-generated HTML, the entry point
+can be found `here. <rustdoc/kernel/index.html>`_
diff --git a/rust/Makefile b/rust/Makefile
index f32df0e49815..eb2ff9260a6c 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -1,5 +1,8 @@ 
 # SPDX-License-Identifier: GPL-2.0
 
+# Where to place rustdoc generated documentation
+RUSTDOC_OUTPUT=$(objtree)/Documentation/output/rust/rustdoc
+
 always-$(CONFIG_RUST) += target.json
 no-clean-files += target.json
 
@@ -67,7 +70,7 @@  quiet_cmd_rustdoc = RUSTDOC $(if $(rustdoc_host),H, ) $<
 	OBJTREE=$(abspath $(objtree)) \
 	$(RUSTDOC) $(if $(rustdoc_host),$(rust_common_flags),$(rust_flags)) \
 		$(rustc_target_flags) -L$(objtree)/$(obj) \
-		--output $(objtree)/$(obj)/doc \
+		--output $(RUSTDOC_OUTPUT) \
 		--crate-name $(subst rustdoc-,,$@) \
 		@$(objtree)/include/generated/rustc_cfg $<
 
@@ -84,15 +87,15 @@  quiet_cmd_rustdoc = RUSTDOC $(if $(rustdoc_host),H, ) $<
 # and then retouch the generated files.
 rustdoc: rustdoc-core rustdoc-macros rustdoc-compiler_builtins \
     rustdoc-alloc rustdoc-kernel
-	$(Q)cp $(srctree)/Documentation/images/logo.svg $(objtree)/$(obj)/doc
-	$(Q)cp $(srctree)/Documentation/images/COPYING-logo $(objtree)/$(obj)/doc
-	$(Q)find $(objtree)/$(obj)/doc -name '*.html' -type f -print0 | xargs -0 sed -Ei \
+	$(Q)cp $(srctree)/Documentation/images/logo.svg $(RUSTDOC_OUTPUT)
+	$(Q)cp $(srctree)/Documentation/images/COPYING-logo $(RUSTDOC_OUTPUT)
+	$(Q)find $(RUSTDOC_OUTPUT) -name '*.html' -type f -print0 | xargs -0 sed -Ei \
 		-e 's:rust-logo\.svg:logo.svg:g' \
 		-e 's:rust-logo\.png:logo.svg:g' \
 		-e 's:favicon\.svg:logo.svg:g' \
 		-e 's:<link rel="alternate icon" type="image/png" href="[./]*favicon-(16x16|32x32)\.png">::g'
 	$(Q)echo '.logo-container > img { object-fit: contain; }' \
-		>> $(objtree)/$(obj)/doc/rustdoc.css
+		>> $(RUSTDOC_OUTPUT)/rustdoc.css
 
 rustdoc-macros: private rustdoc_host = yes
 rustdoc-macros: private rustc_target_flags = --crate-type proc-macro \
@@ -153,7 +156,7 @@  quiet_cmd_rustdoc_test = RUSTDOC T $<
 		@$(objtree)/include/generated/rustc_cfg \
 		$(rustc_target_flags) $(rustdoc_test_target_flags) \
 		--sysroot $(objtree)/$(obj)/test/sysroot $(rustdoc_test_quiet) \
-		-L$(objtree)/$(obj)/test --output $(objtree)/$(obj)/doc \
+		-L$(objtree)/$(obj)/test --output $(RUSTDOC_OUTPUT) \
 		--crate-name $(subst rusttest-,,$@) $<
 
 quiet_cmd_rustdoc_test_kernel = RUSTDOC TK $<