[v2] docs: Integrate rustdoc into Rust documentation

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

Commit Message

Carlos Bilbao Dec. 1, 2022, 8:48 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>
---

Changes since V1:
 - Work on top of v6.1-rc1.
 - Don't use rustdoc.rst, instead add link to Documentation/rust/index.rst.
 - In Documentation/Makefile, replace @make rustdoc for $(Q)$(MAKE) rustdoc.
 - Don't do LLVM=1 for all rustdoc generation within `make htmldocs`.
 - Add spaces on definition of RUSTDOC_OUTPUT, for consistency.

---
 Documentation/Makefile       |  4 ++++
 Documentation/rust/index.rst |  3 +++
 rust/Makefile                | 15 +++++++++------
 3 files changed, 16 insertions(+), 6 deletions(-)


base-commit: 9abf2313adc1ca1b6180c508c25f22f9395cc780
  

Comments

Akira Yokosawa Dec. 5, 2022, 1:06 a.m. UTC | #1
Hi,

On Thu, 1 Dec 2022 14:48:14 -0600, Carlos Bilbao 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.
> 
> Signed-off-by: Carlos Bilbao <carlos.bilbao@amd.com>
> ---
> 
> Changes since V1:
>  - Work on top of v6.1-rc1.

Thank you for the rebase.

>  - Don't use rustdoc.rst, instead add link to Documentation/rust/index.rst.
>  - In Documentation/Makefile, replace @make rustdoc for $(Q)$(MAKE) rustdoc.
>  - Don't do LLVM=1 for all rustdoc generation within `make htmldocs`.
>  - Add spaces on definition of RUSTDOC_OUTPUT, for consistency.
> 
> ---
>  Documentation/Makefile       |  4 ++++
>  Documentation/rust/index.rst |  3 +++
>  rust/Makefile                | 15 +++++++++------
>  3 files changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/Makefile b/Documentation/Makefile
> index 64d44c1ecad3..f537cf558af6 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
> +	$(Q)$(MAKE) rustdoc
> +endif
>  	@$(srctree)/scripts/sphinx-pre-install --version-check
>  	@+$(foreach var,$(SPHINXDIRS),$(call loop_cmd,sphinx,html,$(var),,$(var)))

So, this means "make htmldocs" will require kernel .config if CONFIG_RUST=y.
I'm not sure this new requirement is acceptable for kernel documentation
testers who just want to build kernel documentation.

You are doing three things in this patch.

 1) Change the destination of rustdoc to under Documentation/output/
 2) Add a cross reference to the generated rustdoc in
    Documentation/rust/index.rst.
 3) Integrate rustdoc generation into htmldocs.

I'm OK with 1) and 2).
Can you separate 3) into another patch and respin?

By the way, is rustdoc's requirement of .config only for CONFIG_RUST?
In other words, are contents of rustdoc affected by other config settings?

If not, I think rustdoc can be generated regardless of config settings as
far as necessary tools (rustc, bindgen, etc.) are available.

>  
> diff --git a/Documentation/rust/index.rst b/Documentation/rust/index.rst
> index 4ae8c66b94fa..4005326c3ba9 100644
> --- a/Documentation/rust/index.rst
> +++ b/Documentation/rust/index.rst
> @@ -6,6 +6,9 @@ Rust
>  Documentation related to Rust within the kernel. To start using Rust
>  in the kernel, please read the quick-start.rst guide.
>  
> +If this documentation includes rustdoc-generated HTML, the entry point can
> +be found `here. <rustdoc/kernel/index.html>`_

This cross reference will only make sense in htmldocs build.
Perhaps, you can use the "only::" directive [1] as follows:

.. only:: html

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

[1]: https://www.sphinx-doc.org/en/master/usage/restructuredtext/directives.html#directive-only

        Thanks, Akira

> +
>  .. toctree::
>      :maxdepth: 1
>  
> diff --git a/rust/Makefile b/rust/Makefile
> index 7700d3853404..080c07048065 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
>  
> @@ -58,7 +61,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 $<
>  
> @@ -75,15 +78,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 \
> @@ -141,7 +144,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-,,$@) $<
>  
>  # We cannot use `-Zpanic-abort-tests` because some tests are dynamic,
> 
> base-commit: 9abf2313adc1ca1b6180c508c25f22f9395cc780
  
Miguel Ojeda Dec. 5, 2022, 4:08 p.m. UTC | #2
On Mon, Dec 5, 2022 at 2:06 AM Akira Yokosawa <akiyks@gmail.com> wrote:
>
> So, this means "make htmldocs" will require kernel .config if CONFIG_RUST=y.
> I'm not sure this new requirement is acceptable for kernel documentation
> testers who just want to build kernel documentation.

If the worry is that "full tree testers" (or CIs in general) cannot
fully test the docs anymore without a config, that is definitely a
change, and one of the reasons why I initially didn't add it to
`htmldocs`.

However, full tree testers will need other changes anyway (at least
installing the Rust toolchain), even if there was no need for a
config. So it may be not too bad, and putting it in `htmldocs` means
not having to call another target in the CIs; and for humans, less
chance of forgetting etc.

(It is also why I wondered above about
`CONFIG_WARN_MISSING_DOCUMENTS`: if `Documentation/` intended to
require a config as a whole, then it would be fine. I assume that is
not the case, though, but not doing the sync is nevertheless a bit
confusing)

> By the way, is rustdoc's requirement of .config only for CONFIG_RUST?
> In other words, are contents of rustdoc affected by other config settings?
>
> If not, I think rustdoc can be generated regardless of config settings as
> far as necessary tools (rustc, bindgen, etc.) are available.

Yeah, at the moment the config affects what gets generated. However,
that may change in the future: there has been some movement around the
Rust features needed for this recently, so I want to try that approach
again (it would require some other changes, though).

Cheers,
Miguel
  
Carlos Bilbao Dec. 5, 2022, 4:36 p.m. UTC | #3
On 12/4/22 19:06, Akira Yokosawa wrote:

> Hi,
>
> On Thu, 1 Dec 2022 14:48:14 -0600, Carlos Bilbao 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.
>>
>> Signed-off-by: Carlos Bilbao <carlos.bilbao@amd.com>
>> ---
>>
>> Changes since V1:
>>   - Work on top of v6.1-rc1.
> Thank you for the rebase.
>
>>   - Don't use rustdoc.rst, instead add link to Documentation/rust/index.rst.
>>   - In Documentation/Makefile, replace @make rustdoc for $(Q)$(MAKE) rustdoc.
>>   - Don't do LLVM=1 for all rustdoc generation within `make htmldocs`.
>>   - Add spaces on definition of RUSTDOC_OUTPUT, for consistency.
>>
>> ---
>>   Documentation/Makefile       |  4 ++++
>>   Documentation/rust/index.rst |  3 +++
>>   rust/Makefile                | 15 +++++++++------
>>   3 files changed, 16 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/Makefile b/Documentation/Makefile
>> index 64d44c1ecad3..f537cf558af6 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
>> +	$(Q)$(MAKE) rustdoc
>> +endif
>>   	@$(srctree)/scripts/sphinx-pre-install --version-check
>>   	@+$(foreach var,$(SPHINXDIRS),$(call loop_cmd,sphinx,html,$(var),,$(var)))
> So, this means "make htmldocs" will require kernel .config if CONFIG_RUST=y.
> I'm not sure this new requirement is acceptable for kernel documentation
> testers who just want to build kernel documentation.


This is already kind of the case for Rust-related business.


>
> You are doing three things in this patch.
>
>   1) Change the destination of rustdoc to under Documentation/output/
>   2) Add a cross reference to the generated rustdoc in
>      Documentation/rust/index.rst.
>   3) Integrate rustdoc generation into htmldocs.
>
> I'm OK with 1) and 2).
> Can you separate 3) into another patch and respin?


Glad we can agree on 1) and 2). Why moving 3)? This is a small patch with
one overall purpose (Integrate rustdoc into website).


>
> By the way, is rustdoc's requirement of .config only for CONFIG_RUST?
> In other words, are contents of rustdoc affected by other config settings?
>
> If not, I think rustdoc can be generated regardless of config settings as
> far as necessary tools (rustc, bindgen, etc.) are available.


Yes, but someone with the tools may not want to use them. Keep in mind that
generating rustdoc takes a few extra seconds.


>
>>   
>> diff --git a/Documentation/rust/index.rst b/Documentation/rust/index.rst
>> index 4ae8c66b94fa..4005326c3ba9 100644
>> --- a/Documentation/rust/index.rst
>> +++ b/Documentation/rust/index.rst
>> @@ -6,6 +6,9 @@ Rust
>>   Documentation related to Rust within the kernel. To start using Rust
>>   in the kernel, please read the quick-start.rst guide.
>>   
>> +If this documentation includes rustdoc-generated HTML, the entry point can
>> +be found `here. <rustdoc/kernel/index.html>`_
> This cross reference will only make sense in htmldocs build.
> Perhaps, you can use the "only::" directive [1] as follows:
>
> .. only:: html


This I can gladly do on a V3. I will wait for an answer on issues above.


>
>      If this documentation includes rustdoc-generated HTML, the entry point can
>      be found `here. <rustdoc/kernel/index.html>`_
>
> [1]: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.sphinx-doc.org%2Fen%2Fmaster%2Fusage%2Frestructuredtext%2Fdirectives.html%23directive-only&amp;data=05%7C01%7Ccarlos.bilbao%40amd.com%7C163763a795284a542e4f08dad65cf4c6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638057991984040258%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=DU8nmQp7gCCGNMUR6urtHHCz5nXAtomeV17%2BzB%2F4L38%3D&amp;reserved=0
>
>          Thanks, Akira
>
>> +
>>   .. toctree::
>>       :maxdepth: 1
>>   
>> diff --git a/rust/Makefile b/rust/Makefile
>> index 7700d3853404..080c07048065 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
>>   
>> @@ -58,7 +61,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 $<
>>   
>> @@ -75,15 +78,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 \
>> @@ -141,7 +144,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-,,$@) $<
>>   
>>   # We cannot use `-Zpanic-abort-tests` because some tests are dynamic,
>>
>> base-commit: 9abf2313adc1ca1b6180c508c25f22f9395cc780


Thanks,
Carlos
  
Akira Yokosawa Dec. 6, 2022, 1:32 p.m. UTC | #4
On Mon, 5 Dec 2022 17:08:53 +0100, Miguel Ojeda wrote:
> On Mon, Dec 5, 2022 at 2:06 AM Akira Yokosawa <akiyks@gmail.com> wrote:
>>
>> So, this means "make htmldocs" will require kernel .config if CONFIG_RUST=y.
>> I'm not sure this new requirement is acceptable for kernel documentation
>> testers who just want to build kernel documentation.
> 
> If the worry is that "full tree testers" (or CIs in general) cannot
> fully test the docs anymore without a config, that is definitely a
> change, and one of the reasons why I initially didn't add it to
> `htmldocs`.

Well, I'm actually worrying about additional TAT when I want to test
a particular change in a .rst file and test-build under the relevant
subdirectory using a command, e.g., "make SPHINXDIRS=doc-guide htmldocs".

This completes almost instantly when CONFIG_RUST is not set.

With CONFIG_RUST=y, in my test, it runs RUSTDOC even when rustdoc is
already generated once, as shown below:

------
  CALL    scripts/checksyscalls.sh
  DESCEND objtool
  RUSTDOC   /home/foo/.rustup/toolchains/1.62.0-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/lib.rs
  RUSTDOC H rust/macros/lib.rs
  RUSTDOC   rust/compiler_builtins.rs
  RUSTDOC   rust/alloc/lib.rs
  RUSTDOC   rust/kernel/lib.rs
  SPHINX  htmldocs --> file:///linux/Documentation/output/doc-guide
make[2]: Nothing to be done for 'html'.
Using classic theme
source directory: doc-guide
------

It takes 10 seconds or so (under a not-fast PC), 97% or more of which
is wasted for redundant RUSTDOC runs.

Why is RUSTDOC invoked when there is no change in rustdoc?

If those redundant runs can be resolved, I might change my mind and ack
the integration of generating rustdoc and htmldocs.

> 
> However, full tree testers will need other changes anyway (at least
> installing the Rust toolchain), even if there was no need for a
> config. So it may be not too bad, and putting it in `htmldocs` means
> not having to call another target in the CIs; and for humans, less
> chance of forgetting etc.

Full tree testers wouldn't mind the redundant runs of RUSTDOC.

I think you can add a new target in the top-devel Makefile which
runs both rustdoc and htmldocs for CIs. Something like 'htmldocsboth'
or 'htmldocsall'???

htmldocs and other *docs targets are the most primitive ones for
running Sphinx, so my gut feeling tells me _not_ to contaminate
htmldocs with rustdoc or vice versa.

> 
> (It is also why I wondered above about
> `CONFIG_WARN_MISSING_DOCUMENTS`: if `Documentation/` intended to
> require a config as a whole, then it would be fine. I assume that is
> not the case, though, but not doing the sync is nevertheless a bit
> confusing)

I have no idea. (Note: I was not around when the kernel documentation
transitioned to Sphinx.)

> 
>> By the way, is rustdoc's requirement of .config only for CONFIG_RUST?
>> In other words, are contents of rustdoc affected by other config settings?
>>
>> If not, I think rustdoc can be generated regardless of config settings as
>> far as necessary tools (rustc, bindgen, etc.) are available.
> 
> Yeah, at the moment the config affects what gets generated. However,
> that may change in the future: there has been some movement around the
> Rust features needed for this recently, so I want to try that approach
> again (it would require some other changes, though).

Interesting.
So rustdoc needs .config. I'm convinced.

        Thanks, Akira

> 
> Cheers,
> Miguel
  
Akira Yokosawa Dec. 6, 2022, 2:22 p.m. UTC | #5
On Mon, 5 Dec 2022 10:36:11 -0600, Carlos Bilbao wrote:
> On 12/4/22 19:06, Akira Yokosawa wrote:
> 
>> Hi,
>>
>> On Thu, 1 Dec 2022 14:48:14 -0600, Carlos Bilbao 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.
>>>
>>> Signed-off-by: Carlos Bilbao <carlos.bilbao@amd.com>
>>> ---
>>>
>>> Changes since V1:
>>>   - Work on top of v6.1-rc1.
>> Thank you for the rebase.
>>
>>>   - Don't use rustdoc.rst, instead add link to Documentation/rust/index.rst.
>>>   - In Documentation/Makefile, replace @make rustdoc for $(Q)$(MAKE) rustdoc.
>>>   - Don't do LLVM=1 for all rustdoc generation within `make htmldocs`.
>>>   - Add spaces on definition of RUSTDOC_OUTPUT, for consistency.
>>>
>>> ---
>>>   Documentation/Makefile       |  4 ++++
>>>   Documentation/rust/index.rst |  3 +++
>>>   rust/Makefile                | 15 +++++++++------
>>>   3 files changed, 16 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/Documentation/Makefile b/Documentation/Makefile
>>> index 64d44c1ecad3..f537cf558af6 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
>>> +    $(Q)$(MAKE) rustdoc
>>> +endif
>>>       @$(srctree)/scripts/sphinx-pre-install --version-check
>>>       @+$(foreach var,$(SPHINXDIRS),$(call loop_cmd,sphinx,html,$(var),,$(var)))
>> So, this means "make htmldocs" will require kernel .config if CONFIG_RUST=y.
>> I'm not sure this new requirement is acceptable for kernel documentation
>> testers who just want to build kernel documentation.
> 
> 
> This is already kind of the case for Rust-related business.
> 
> 
>>
>> You are doing three things in this patch.
>>
>>   1) Change the destination of rustdoc to under Documentation/output/
>>   2) Add a cross reference to the generated rustdoc in
>>      Documentation/rust/index.rst.
>>   3) Integrate rustdoc generation into htmldocs.
>>
>> I'm OK with 1) and 2).
>> Can you separate 3) into another patch and respin?
> 
> 
> Glad we can agree on 1) and 2). Why moving 3)? This is a small patch with
> one overall purpose (Integrate rustdoc into website).

Yes, I agree that 3) is a small change. I understand what you want to
do. But there are a couple of options for _how_ to do it.
My current position is that Documentation/Makefile is _not_ the right
place for the change, as mentioned in my reply to Miguel.

> 
> 
>>
>> By the way, is rustdoc's requirement of .config only for CONFIG_RUST?
>> In other words, are contents of rustdoc affected by other config settings?
>>
>> If not, I think rustdoc can be generated regardless of config settings as
>> far as necessary tools (rustc, bindgen, etc.) are available.
> 
> 
> Yes, but someone with the tools may not want to use them. Keep in mind that
> generating rustdoc takes a few extra seconds.

As mentioned in another reply, I'm convinced of the dependency on .config.

> 
> 
>>
>>>   diff --git a/Documentation/rust/index.rst b/Documentation/rust/index.rst
>>> index 4ae8c66b94fa..4005326c3ba9 100644
>>> --- a/Documentation/rust/index.rst
>>> +++ b/Documentation/rust/index.rst
>>> @@ -6,6 +6,9 @@ Rust
>>>   Documentation related to Rust within the kernel. To start using Rust
>>>   in the kernel, please read the quick-start.rst guide.
>>>   +If this documentation includes rustdoc-generated HTML, the entry point can
>>> +be found `here. <rustdoc/kernel/index.html>`_
>> This cross reference will only make sense in htmldocs build.
>> Perhaps, you can use the "only::" directive [1] as follows:
>>
>> .. only:: html
> 
> 
> This I can gladly do on a V3. I will wait for an answer on issues above.

OK.

So if you split this into a two-patch series, 1/2 for 1) and 2), and 2/2
for 3) (or an updated one), I'm glad to give my RB tag to 1/2 for Miguel
to be able take it for v6.2 (timing is tight!). 2/2 will need at least
a couple of respins, I guess.

        Thanks, Akira
> 
> 
>>
>>      If this documentation includes rustdoc-generated HTML, the entry point can
>>      be found `here. <rustdoc/kernel/index.html>`_
>>
>> [1]: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.sphinx-doc.org%2Fen%2Fmaster%2Fusage%2Frestructuredtext%2Fdirectives.html%23directive-only&amp;data=05%7C01%7Ccarlos.bilbao%40amd.com%7C163763a795284a542e4f08dad65cf4c6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638057991984040258%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=DU8nmQp7gCCGNMUR6urtHHCz5nXAtomeV17%2BzB%2F4L38%3D&amp;reserved=0
>>
>>          Thanks, Akira
>>
>>> +
>>>   .. toctree::
>>>       :maxdepth: 1
>>>   diff --git a/rust/Makefile b/rust/Makefile
>>> index 7700d3853404..080c07048065 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
>>>   @@ -58,7 +61,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 $<
>>>   @@ -75,15 +78,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 \
>>> @@ -141,7 +144,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-,,$@) $<
>>>     # We cannot use `-Zpanic-abort-tests` because some tests are dynamic,
>>>
>>> base-commit: 9abf2313adc1ca1b6180c508c25f22f9395cc780
> 
> 
> Thanks,
> Carlos
  
Carlos Bilbao Dec. 6, 2022, 2:55 p.m. UTC | #6
On 12/6/22 08:22, Akira Yokosawa wrote:

> On Mon, 5 Dec 2022 10:36:11 -0600, Carlos Bilbao wrote:
>> On 12/4/22 19:06, Akira Yokosawa wrote:
>>
>>> Hi,
>>>
>>> On Thu, 1 Dec 2022 14:48:14 -0600, Carlos Bilbao 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.
>>>>
>>>> Signed-off-by: Carlos Bilbao <carlos.bilbao@amd.com>
>>>> ---
>>>>
>>>> Changes since V1:
>>>>    - Work on top of v6.1-rc1.
>>> Thank you for the rebase.
>>>
>>>>    - Don't use rustdoc.rst, instead add link to Documentation/rust/index.rst.
>>>>    - In Documentation/Makefile, replace @make rustdoc for $(Q)$(MAKE) rustdoc.
>>>>    - Don't do LLVM=1 for all rustdoc generation within `make htmldocs`.
>>>>    - Add spaces on definition of RUSTDOC_OUTPUT, for consistency.
>>>>
>>>> ---
>>>>    Documentation/Makefile       |  4 ++++
>>>>    Documentation/rust/index.rst |  3 +++
>>>>    rust/Makefile                | 15 +++++++++------
>>>>    3 files changed, 16 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/Documentation/Makefile b/Documentation/Makefile
>>>> index 64d44c1ecad3..f537cf558af6 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
>>>> +    $(Q)$(MAKE) rustdoc
>>>> +endif
>>>>        @$(srctree)/scripts/sphinx-pre-install --version-check
>>>>        @+$(foreach var,$(SPHINXDIRS),$(call loop_cmd,sphinx,html,$(var),,$(var)))
>>> So, this means "make htmldocs" will require kernel .config if CONFIG_RUST=y.
>>> I'm not sure this new requirement is acceptable for kernel documentation
>>> testers who just want to build kernel documentation.
>>
>> This is already kind of the case for Rust-related business.
>>
>>
>>> You are doing three things in this patch.
>>>
>>>    1) Change the destination of rustdoc to under Documentation/output/
>>>    2) Add a cross reference to the generated rustdoc in
>>>       Documentation/rust/index.rst.
>>>    3) Integrate rustdoc generation into htmldocs.
>>>
>>> I'm OK with 1) and 2).
>>> Can you separate 3) into another patch and respin?
>>
>> Glad we can agree on 1) and 2). Why moving 3)? This is a small patch with
>> one overall purpose (Integrate rustdoc into website).
> Yes, I agree that 3) is a small change. I understand what you want to
> do. But there are a couple of options for _how_ to do it.
> My current position is that Documentation/Makefile is _not_ the right
> place for the change, as mentioned in my reply to Miguel.
>
>>
>>> By the way, is rustdoc's requirement of .config only for CONFIG_RUST?
>>> In other words, are contents of rustdoc affected by other config settings?
>>>
>>> If not, I think rustdoc can be generated regardless of config settings as
>>> far as necessary tools (rustc, bindgen, etc.) are available.
>>
>> Yes, but someone with the tools may not want to use them. Keep in mind that
>> generating rustdoc takes a few extra seconds.
> As mentioned in another reply, I'm convinced of the dependency on .config.
>
>>
>>>>    diff --git a/Documentation/rust/index.rst b/Documentation/rust/index.rst
>>>> index 4ae8c66b94fa..4005326c3ba9 100644
>>>> --- a/Documentation/rust/index.rst
>>>> +++ b/Documentation/rust/index.rst
>>>> @@ -6,6 +6,9 @@ Rust
>>>>    Documentation related to Rust within the kernel. To start using Rust
>>>>    in the kernel, please read the quick-start.rst guide.
>>>>    +If this documentation includes rustdoc-generated HTML, the entry point can
>>>> +be found `here. <rustdoc/kernel/index.html>`_
>>> This cross reference will only make sense in htmldocs build.
>>> Perhaps, you can use the "only::" directive [1] as follows:
>>>
>>> .. only:: html
>>
>> This I can gladly do on a V3. I will wait for an answer on issues above.
> OK.
>
> So if you split this into a two-patch series, 1/2 for 1) and 2), and 2/2
> for 3) (or an updated one), I'm glad to give my RB tag to 1/2 for Miguel
> to be able take it for v6.2 (timing is tight!). 2/2 will need at least
> a couple of respins, I guess.


Sounds good, I will divide for v3.


>
>          Thanks, Akira
>>
>>>       If this documentation includes rustdoc-generated HTML, the entry point can
>>>       be found `here. <rustdoc/kernel/index.html>`_
>>>
>>> [1]: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.sphinx-doc.org%2Fen%2Fmaster%2Fusage%2Frestructuredtext%2Fdirectives.html%23directive-only&amp;data=05%7C01%7Ccarlos.bilbao%40amd.com%7Cbed091b8fae74b3d314408dad7954fc2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638059333550278394%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=xGbmhYsge%2BoPo%2BRrsc6fCW8htY3yh0dHk4E8gQBQV9o%3D&amp;reserved=0
>>>
>>>           Thanks, Akira
>>>
>>>> +
>>>>    .. toctree::
>>>>        :maxdepth: 1
>>>>    diff --git a/rust/Makefile b/rust/Makefile
>>>> index 7700d3853404..080c07048065 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
>>>>    @@ -58,7 +61,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 $<
>>>>    @@ -75,15 +78,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 \
>>>> @@ -141,7 +144,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-,,$@) $<
>>>>      # We cannot use `-Zpanic-abort-tests` because some tests are dynamic,
>>>>
>>>> base-commit: 9abf2313adc1ca1b6180c508c25f22f9395cc780
>>
>> Thanks,
>> Carlos
  
Carlos Bilbao Dec. 6, 2022, 3:01 p.m. UTC | #7
On 12/6/22 08:22, Akira Yokosawa wrote:

> On Mon, 5 Dec 2022 10:36:11 -0600, Carlos Bilbao wrote:
>> On 12/4/22 19:06, Akira Yokosawa wrote:
>>
>>> Hi,
>>>
>>> On Thu, 1 Dec 2022 14:48:14 -0600, Carlos Bilbao 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.
>>>>
>>>> Signed-off-by: Carlos Bilbao <carlos.bilbao@amd.com>
>>>> ---
>>>>
>>>> Changes since V1:
>>>>    - Work on top of v6.1-rc1.
>>> Thank you for the rebase.
>>>
>>>>    - Don't use rustdoc.rst, instead add link to Documentation/rust/index.rst.
>>>>    - In Documentation/Makefile, replace @make rustdoc for $(Q)$(MAKE) rustdoc.
>>>>    - Don't do LLVM=1 for all rustdoc generation within `make htmldocs`.
>>>>    - Add spaces on definition of RUSTDOC_OUTPUT, for consistency.
>>>>
>>>> ---
>>>>    Documentation/Makefile       |  4 ++++
>>>>    Documentation/rust/index.rst |  3 +++
>>>>    rust/Makefile                | 15 +++++++++------
>>>>    3 files changed, 16 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/Documentation/Makefile b/Documentation/Makefile
>>>> index 64d44c1ecad3..f537cf558af6 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
>>>> +    $(Q)$(MAKE) rustdoc
>>>> +endif
>>>>        @$(srctree)/scripts/sphinx-pre-install --version-check
>>>>        @+$(foreach var,$(SPHINXDIRS),$(call loop_cmd,sphinx,html,$(var),,$(var)))
>>> So, this means "make htmldocs" will require kernel .config if CONFIG_RUST=y.
>>> I'm not sure this new requirement is acceptable for kernel documentation
>>> testers who just want to build kernel documentation.
>>
>> This is already kind of the case for Rust-related business.
>>
>>
>>> You are doing three things in this patch.
>>>
>>>    1) Change the destination of rustdoc to under Documentation/output/
>>>    2) Add a cross reference to the generated rustdoc in
>>>       Documentation/rust/index.rst.
>>>    3) Integrate rustdoc generation into htmldocs.
>>>
>>> I'm OK with 1) and 2).
>>> Can you separate 3) into another patch and respin?
>>
>> Glad we can agree on 1) and 2). Why moving 3)? This is a small patch with
>> one overall purpose (Integrate rustdoc into website).
> Yes, I agree that 3) is a small change. I understand what you want to
> do. But there are a couple of options for _how_ to do it.
> My current position is that Documentation/Makefile is _not_ the right
> place for the change, as mentioned in my reply to Miguel.


Actually, I originally used `make rustdocs` [1] but good arguments were
given to use `make htmldocs` instead, and I was convinced too.


>
>>
>>> By the way, is rustdoc's requirement of .config only for CONFIG_RUST?
>>> In other words, are contents of rustdoc affected by other config settings?
>>>
>>> If not, I think rustdoc can be generated regardless of config settings as
>>> far as necessary tools (rustc, bindgen, etc.) are available.
>>
>> Yes, but someone with the tools may not want to use them. Keep in mind that
>> generating rustdoc takes a few extra seconds.
> As mentioned in another reply, I'm convinced of the dependency on .config.
>
>>
>>>>    diff --git a/Documentation/rust/index.rst b/Documentation/rust/index.rst
>>>> index 4ae8c66b94fa..4005326c3ba9 100644
>>>> --- a/Documentation/rust/index.rst
>>>> +++ b/Documentation/rust/index.rst
>>>> @@ -6,6 +6,9 @@ Rust
>>>>    Documentation related to Rust within the kernel. To start using Rust
>>>>    in the kernel, please read the quick-start.rst guide.
>>>>    +If this documentation includes rustdoc-generated HTML, the entry point can
>>>> +be found `here. <rustdoc/kernel/index.html>`_
>>> This cross reference will only make sense in htmldocs build.
>>> Perhaps, you can use the "only::" directive [1] as follows:
>>>
>>> .. only:: html
>>
>> This I can gladly do on a V3. I will wait for an answer on issues above.
> OK.
>
> So if you split this into a two-patch series, 1/2 for 1) and 2), and 2/2
> for 3) (or an updated one), I'm glad to give my RB tag to 1/2 for Miguel
> to be able take it for v6.2 (timing is tight!). 2/2 will need at least
> a couple of respins, I guess.


Sounds good, I will divide for v3.


>
>          Thanks, Akira
>>
>>>       If this documentation includes rustdoc-generated HTML, the entry point can
>>>       be found `here. <rustdoc/kernel/index.html>`_
>>>
>>> [1]: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.sphinx-doc.org%2Fen%2Fmaster%2Fusage%2Frestructuredtext%2Fdirectives.html%23directive-only&amp;data=05%7C01%7Ccarlos.bilbao%40amd.com%7Cbed091b8fae74b3d314408dad7954fc2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638059333550278394%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=xGbmhYsge%2BoPo%2BRrsc6fCW8htY3yh0dHk4E8gQBQV9o%3D&amp;reserved=0
>>>
>>>           Thanks, Akira
>>>
>>>> +
>>>>    .. toctree::
>>>>        :maxdepth: 1
>>>>    diff --git a/rust/Makefile b/rust/Makefile
>>>> index 7700d3853404..080c07048065 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
>>>>    @@ -58,7 +61,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 $<
>>>>    @@ -75,15 +78,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 \
>>>> @@ -141,7 +144,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-,,$@) $<
>>>>      # We cannot use `-Zpanic-abort-tests` because some tests are dynamic,
>>>>
>>>> base-commit: 9abf2313adc1ca1b6180c508c25f22f9395cc780
>>
>> Thanks,
>> Carlos

[1] 
https://lore.kernel.org/lkml/a019a3f1-7ff1-15b2-d930-e1d722847e0c@gmail.com/T/


Thanks,
Carlos
  
Jonathan Corbet Dec. 6, 2022, 3:02 p.m. UTC | #8
Akira Yokosawa <akiyks@gmail.com> writes:

> Well, I'm actually worrying about additional TAT when I want to test
> a particular change in a .rst file and test-build under the relevant
> subdirectory using a command, e.g., "make SPHINXDIRS=doc-guide htmldocs".
>
> This completes almost instantly when CONFIG_RUST is not set.
>
> With CONFIG_RUST=y, in my test, it runs RUSTDOC even when rustdoc is
> already generated once, as shown below:

This is something that would be nice to avoid if we can; narrowing
things with SPHINXDIRS should avoid building anything that the user
isn't asking for.  I'm not sure how much makefile pain would be required
to make that happen...Documentation/Makefile is not the easiest place to
make changes, alas.

> I think you can add a new target in the top-devel Makefile which
> runs both rustdoc and htmldocs for CIs. Something like 'htmldocsboth'
> or 'htmldocsall'???
>
> htmldocs and other *docs targets are the most primitive ones for
> running Sphinx, so my gut feeling tells me _not_ to contaminate
> htmldocs with rustdoc or vice versa.

Well, I *would* like for a bare "make htmldocs" to make *all* of the
docs; I don't think Rust should be special in that regard.

>> (It is also why I wondered above about
>> `CONFIG_WARN_MISSING_DOCUMENTS`: if `Documentation/` intended to
>> require a config as a whole, then it would be fine. I assume that is
>> not the case, though, but not doing the sync is nevertheless a bit
>> confusing)
>
> I have no idea. (Note: I was not around when the kernel documentation
> transitioned to Sphinx.)

I think we're just seeing the implementation as was rammed in by
somebody in a hurry; I don't doubt it could be improved.

Thanks,

jon  (currently traveling and scrambling to get ready for the merge window)
  
Miguel Ojeda Dec. 6, 2022, 3:39 p.m. UTC | #9
On Tue, Dec 6, 2022 at 2:32 PM Akira Yokosawa <akiyks@gmail.com> wrote:
>
> Why is RUSTDOC invoked when there is no change in rustdoc?

I think we can give proper dependencies to `rustdoc`, so if that is
the main concern, then it should be OK.

But note that `htmldocs` also calls Sphinx every run. It does a subset
of the work, but perhaps we can save some time there if we could
detect whether anything changed.

> I think you can add a new target in the top-devel Makefile which
> runs both rustdoc and htmldocs for CIs. Something like 'htmldocsboth'
> or 'htmldocsall'???
>
> htmldocs and other *docs targets are the most primitive ones for
> running Sphinx, so my gut feeling tells me _not_ to contaminate
> htmldocs with rustdoc or vice versa.

That is reasonable, but others wanted it in `htmldocs` (and the goal
is to eventually do so, so if we can do it already, even better).

Cheers,
Miguel
  

Patch

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 64d44c1ecad3..f537cf558af6 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
+	$(Q)$(MAKE) 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..4005326c3ba9 100644
--- a/Documentation/rust/index.rst
+++ b/Documentation/rust/index.rst
@@ -6,6 +6,9 @@  Rust
 Documentation related to Rust within the kernel. To start using Rust
 in the kernel, please read the quick-start.rst guide.
 
+If this documentation includes rustdoc-generated HTML, the entry point can
+be found `here. <rustdoc/kernel/index.html>`_
+
 .. toctree::
     :maxdepth: 1
 
diff --git a/rust/Makefile b/rust/Makefile
index 7700d3853404..080c07048065 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
 
@@ -58,7 +61,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 $<
 
@@ -75,15 +78,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 \
@@ -141,7 +144,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-,,$@) $<
 
 # We cannot use `-Zpanic-abort-tests` because some tests are dynamic,