diff mbox series

docs: Integrate rustdoc-generated output to Rust docs

Message ID 20221128201932.168313-1-carlos.bilbao@amd.com
State New
Headers
Series docs: Integrate rustdoc-generated output to Rust docs |

Commit Message

Carlos Bilbao Nov. 28, 2022, 8:19 p.m. UTC 
  Include HTML output generated from rustdoc into the Linux kernel
documentation on Rust. Add Makefile target `make htmlrust` to combine
make htmldocs and the generation of Rust documentation.

Signed-off-by: Carlos Bilbao <carlos.bilbao@amd.com>
---
 Documentation/Makefile         | 11 +++++++++++
 Documentation/rust/index.rst   |  1 +
 Documentation/rust/rustdoc.rst | 10 ++++++++++
 Makefile                       |  2 +-
 4 files changed, 23 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/rust/rustdoc.rst

Comments

Miguel Ojeda Nov. 28, 2022, 9:06 p.m. UTC | #1 
On Mon, Nov 28, 2022 at 9:19 PM Carlos Bilbao <carlos.bilbao@amd.com> wrote:
>
> Include HTML output generated from rustdoc into the Linux kernel
> documentation on Rust. Add Makefile target `make htmlrust` to combine
> make htmldocs and the generation of Rust documentation.

I think Konstantin (Cc'ing him) wanted us to integrate this into
`htmldocs`, rather than having a new target. That is, if `CONFIG_RUST`
is enabled, call the `rustdoc` target there.

In any case, I will also comment on this approach below. Thanks for this!

> +ifdef CONFIG_RUST
> +htmlrust:
> +       @make rustavailable

Why was this needed?

> +       @make LLVM=1 rustdoc
> +       @cp -r rust/doc/* Documentation/output/

We should change `rustdoc` to generate the docs directly in the right
place now that we have one.

Cheers,
Miguel
Jonathan Corbet Nov. 28, 2022, 10:23 p.m. UTC | #2 
Carlos Bilbao <carlos.bilbao@amd.com> writes:

> Include HTML output generated from rustdoc into the Linux kernel
> documentation on Rust. Add Makefile target `make htmlrust` to combine
> make htmldocs and the generation of Rust documentation.
>
> Signed-off-by: Carlos Bilbao <carlos.bilbao@amd.com>
> ---
>  Documentation/Makefile         | 11 +++++++++++
>  Documentation/rust/index.rst   |  1 +
>  Documentation/rust/rustdoc.rst | 10 ++++++++++
>  Makefile                       |  2 +-
>  4 files changed, 23 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/rust/rustdoc.rst

Thanks for doing this.  I do have a number of comments; please let me
know if you think I'm missing something somewhere.

> diff --git a/Documentation/Makefile b/Documentation/Makefile
> index 64d44c1ecad3..02ed01fa3499 100644
> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -95,6 +95,17 @@ htmldocs:
>  	@$(srctree)/scripts/sphinx-pre-install --version-check
>  	@+$(foreach var,$(SPHINXDIRS),$(call loop_cmd,sphinx,html,$(var),,$(var)))
>  
> +ifdef CONFIG_RUST
> +htmlrust:
> +	@make rustavailable
> +	@make LLVM=1 rustdoc
> +	@cp -r rust/doc/* Documentation/output/
> +	@make htmldocs
> +else
> +htmlrust:
> +	@echo "Error: CONFIG_RUST must be defined (see .config)"
> +endif

First, if at all possible, the Rust documentation should just be built
along with the rest; no need for a separate make command.  We don't have
separate build commands for any other subsystem's docs, and Rust should
be a first-class citizen here too.

Second, I'm not a big fan of that "cp" command, for a couple of reasons:

- It dumps a bunch of stuff into the main output directory, which risks
  overwriting something someday.  It seems like
  Documentation/output/html/rust might be a better place.

- Rather than copying, I'd suggest changing the rustdoc command that
  generates that output to just put it in the place where it should be.
  Preferably it should work properly when people use separate build
  trees as well.

It would also be nice to set up proper dependencies so that the Rust
docs are only regenerated if something has changed.

Does this all make sense?  Sorry to come back with all this stuff...I
really do want to see this happen.

Thanks,

jon
kernel test robot Nov. 30, 2022, 3:57 a.m. UTC | #3 
Hi Carlos,

I love your patch! Perhaps something to improve:

[auto build test WARNING on rust/rust-next]
[also build test WARNING on masahiroy-kbuild/for-next masahiroy-kbuild/fixes linus/master v6.1-rc7]
[cannot apply to lwn/docs-next next-20221129]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Carlos-Bilbao/docs-Integrate-rustdoc-generated-output-to-Rust-docs/20221129-042023
base:   https://github.com/Rust-for-Linux/linux rust-next
patch link:    https://lore.kernel.org/r/20221128201932.168313-1-carlos.bilbao%40amd.com
patch subject: [PATCH] docs: Integrate rustdoc-generated output to Rust docs
reproduce:
        # https://github.com/intel-lab-lkp/linux/commit/7a7a2a60d9cf38764f8668cee0ae67ec3880b66c
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Carlos-Bilbao/docs-Integrate-rustdoc-generated-output-to-Rust-docs/20221129-042023
        git checkout 7a7a2a60d9cf38764f8668cee0ae67ec3880b66c
        make menuconfig
        # enable CONFIG_COMPILE_TEST, CONFIG_WARN_MISSING_DOCUMENTS, CONFIG_WARN_ABI_ERRORS
        make htmldocs

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> Documentation/rust/rustdoc.rst:9: WARNING: Problems with "raw" directive path:

vim +/raw +9 Documentation/rust/rustdoc.rst

     8	
   > 9	.. raw:: html
Carlos Bilbao Nov. 30, 2022, 2:18 p.m. UTC | #4 
On 11/28/22 16:23, Jonathan Corbet wrote:

> Carlos Bilbao <carlos.bilbao@amd.com> writes:
>
>> Include HTML output generated from rustdoc into the Linux kernel
>> documentation on Rust. Add Makefile target `make htmlrust` to combine
>> make htmldocs and the generation of Rust documentation.
>>
>> Signed-off-by: Carlos Bilbao <carlos.bilbao@amd.com>
>> ---
>>   Documentation/Makefile         | 11 +++++++++++
>>   Documentation/rust/index.rst   |  1 +
>>   Documentation/rust/rustdoc.rst | 10 ++++++++++
>>   Makefile                       |  2 +-
>>   4 files changed, 23 insertions(+), 1 deletion(-)
>>   create mode 100644 Documentation/rust/rustdoc.rst
> Thanks for doing this.  I do have a number of comments; please let me
> know if you think I'm missing something somewhere.
>
>> diff --git a/Documentation/Makefile b/Documentation/Makefile
>> index 64d44c1ecad3..02ed01fa3499 100644
>> --- a/Documentation/Makefile
>> +++ b/Documentation/Makefile
>> @@ -95,6 +95,17 @@ htmldocs:
>>   	@$(srctree)/scripts/sphinx-pre-install --version-check
>>   	@+$(foreach var,$(SPHINXDIRS),$(call loop_cmd,sphinx,html,$(var),,$(var)))
>>   
>> +ifdef CONFIG_RUST
>> +htmlrust:
>> +	@make rustavailable
>> +	@make LLVM=1 rustdoc
>> +	@cp -r rust/doc/* Documentation/output/
>> +	@make htmldocs
>> +else
>> +htmlrust:
>> +	@echo "Error: CONFIG_RUST must be defined (see .config)"
>> +endif
> First, if at all possible, the Rust documentation should just be built
> along with the rest; no need for a separate make command.  We don't have
> separate build commands for any other subsystem's docs, and Rust should
> be a first-class citizen here too.
>
> Second, I'm not a big fan of that "cp" command, for a couple of reasons:
>
> - It dumps a bunch of stuff into the main output directory, which risks
>    overwriting something someday.  It seems like
>    Documentation/output/html/rust might be a better place.
>
> - Rather than copying, I'd suggest changing the rustdoc command that
>    generates that output to just put it in the place where it should be.
>    Preferably it should work properly when people use separate build
>    trees as well.
>
> It would also be nice to set up proper dependencies so that the Rust
> docs are only regenerated if something has changed.
>
> Does this all make sense?  Sorry to come back with all this stuff...I
> really do want to see this happen.


Thanks for your feedback, Jon and Miguel. Looks like you both agree on two
things: htmldocs should be the place to generate the extra Rust
documentation, and this should be done by making rustdoc use the proper
path directly (rather than by moving those contents). I will send a new
patch instead of v2, since it will be a whole other thing.


>
> Thanks,
>
> jon


Thanks,

Carlos
diff mbox series

Patch

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 64d44c1ecad3..02ed01fa3499 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -95,6 +95,17 @@  htmldocs:
 	@$(srctree)/scripts/sphinx-pre-install --version-check
 	@+$(foreach var,$(SPHINXDIRS),$(call loop_cmd,sphinx,html,$(var),,$(var)))
 
+ifdef CONFIG_RUST
+htmlrust:
+	@make rustavailable
+	@make LLVM=1 rustdoc
+	@cp -r rust/doc/* Documentation/output/
+	@make htmldocs
+else
+htmlrust:
+	@echo "Error: CONFIG_RUST must be defined (see .config)"
+endif
+
 linkcheckdocs:
 	@$(foreach var,$(SPHINXDIRS),$(call loop_cmd,sphinx,linkcheck,$(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..74581367417b
--- /dev/null
+++ b/Documentation/rust/rustdoc.rst
@@ -0,0 +1,10 @@ 
+.. SPDX-License-Identifier: GPL-2.0
+
+Rustdoc output
+==============
+
+If this documentation includes rustdoc-generated HTML, this is the
+entry point.
+
+.. raw:: html
+    :file: doc/kernel/index.html
diff --git a/Makefile b/Makefile
index f41ec8c8426b..c7e00bf928ed 100644
--- a/Makefile
+++ b/Makefile
@@ -1785,7 +1785,7 @@  $(help-board-dirs): help-%:
 # Documentation targets
 # ---------------------------------------------------------------------------
 DOC_TARGETS := xmldocs latexdocs pdfdocs htmldocs epubdocs cleandocs \
-	       linkcheckdocs dochelp refcheckdocs
+	       linkcheckdocs dochelp refcheckdocs htmlrust
 PHONY += $(DOC_TARGETS)
 $(DOC_TARGETS):
 	$(Q)$(MAKE) $(build)=Documentation $@