[v5,0/2] docs: Integrate rustdoc into Rust documentation

  Include HTML output generated with rustdoc into the Linux kernel
documentation on Rust.

Carlos Bilbao:
 docs: Move rustdoc output, cross-reference it
 docs: Integrate rustdoc generation into htmldocs

---
Changes since V4:
 - Limit rustdoc note only to html outputs.

Changes since V3:
 - Added Reviewed-Bys from Akira Yokosawa.
 - PATCH 1/2: Avoid error 404 adding tag `rustdoc` for Sphinx.
 - PATCH 1/2: Don't use "here" as link text, describe destination instead.
 - PATCH 2/2: Check CONFIG_RUST in a way that allows us to skip generation.
 - PATCH 2/2: Reoder Sphinx runs so they complete even if rustdoc fails.

Changes since V2:
 - Split v2 into two-patch series.
 - Add "only:: html" directive in Documentation/rust/index.rst reference

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       |  8 ++++++++
 Documentation/rust/index.rst |  8 ++++++++
 rust/Makefile                | 15 +++++++++------
 3 files changed, 25 insertions(+), 6 deletions(-)
  

Carlos Bilbao <carlos.bilbao@amd.com> writes:

> Include HTML output generated with rustdoc into the Linux kernel
> documentation on Rust.
>
> Carlos Bilbao:
>  docs: Move rustdoc output, cross-reference it
>  docs: Integrate rustdoc generation into htmldocs

OK, so I just gave this a try...

- It forces the generation of a kernel configuration, something that the
  docs build has never done until now.  What are our changes of
  eliminating that?

- It did a bunch of other building, starting with objtool - again, never
  needed for the docs build before.

In the end, it died with:

> BINDGEN rust/bindings/bindings_generated.rs
> Failed to run rustfmt: No such file or directory (os error 2) (non-fatal, continuing)
>   BINDGEN rust/bindings/bindings_helpers_generated.rs
> error: Found argument '--blacklist-type' which wasn't expected, or isn't valid in this context
> 
> 	Did you mean '--blocklist-type'?

Perhaps this is because I ignored the warnings about my Rust toolchain
being too new? (Rust 1.65.0, bindgen 0.63.0).  I get that only one
version is really supported, but it would be nice to fail a bit more
gracefully if at all possible.

Anyway, I've unapplied these for now; thoughts on all this?

Thanks,

jon
  

On 1/2/23 17:53, Jonathan Corbet wrote:

> Carlos Bilbao <carlos.bilbao@amd.com> writes:
>
>> Include HTML output generated with rustdoc into the Linux kernel
>> documentation on Rust.
>>
>> Carlos Bilbao:
>>   docs: Move rustdoc output, cross-reference it
>>   docs: Integrate rustdoc generation into htmldocs
> OK, so I just gave this a try...
>
> - It forces the generation of a kernel configuration, something that the
>    docs build has never done until now.  What are our changes of
>    eliminating that?


Yes, this means "make htmldocs" will require kernel .config, but only if we
want CONFIG_RUST=y. AFAIK this is a limitation of Rust in the kernel at the
moment, not something particular to this patch.


>
> - It did a bunch of other building, starting with objtool - again, never
>    needed for the docs build before.


Yes, building rustdoc requires building new things, no way around that
either, IMHO.


>
> In the end, it died with:
>
>> BINDGEN rust/bindings/bindings_generated.rs
>> Failed to run rustfmt: No such file or directory (os error 2) (non-fatal, continuing)
>>    BINDGEN rust/bindings/bindings_helpers_generated.rs
>> error: Found argument '--blacklist-type' which wasn't expected, or isn't valid in this context
>>
>> 	Did you mean '--blocklist-type'?
> Perhaps this is because I ignored the warnings about my Rust toolchain
> being too new? (Rust 1.65.0, bindgen 0.63.0).  I get that only one


Yes, it is important to have the expected Rust toolchain. You can try
running:

rustup override set $(scripts/min-tool-version.sh rustc)

there's more information about this on the Rust Quick Start [1]. It may be
annoying but you will need this for any future Rust-kernel work too.


> version is really supported, but it would be nice to fail a bit more
> gracefully if at all possible.
>
> Anyway, I've unapplied these for now; thoughts on all this?


My two cents is that these are limitations of Rust in the kernel, at least
on its current state, and so adding rustdoc to the Documentation was
going to come with them. But if someone has any ideas to make it less
painful, I'm all ears too :)


>
> Thanks,
>
> jon


Thanks,

Carlos


[1] 
https://github.com/Rust-for-Linux/linux/blob/rust/Documentation/rust/quick-start.rst
  

On Tue, Jan 3, 2023 at 12:54 AM Jonathan Corbet <corbet@lwn.net> wrote:
>
> - It forces the generation of a kernel configuration, something that the
>   docs build has never done until now.  What are our changes of
>   eliminating that?

We could workaround it by providing a fixed, preset config that does
not interfere with the usual `.config`.

We would still need to compile some things like we normally would do,
though (see next point).

> - It did a bunch of other building, starting with objtool - again, never
>   needed for the docs build before.

Yeah, rustdoc, like the compiler, requires dependencies to be
available to understand the code. Thus some things need to be
compiled, like for the normal build.

This is definitely different than the current docs, of course, which
is why I raised these questions back then.

> In the end, it died with:
>
> > BINDGEN rust/bindings/bindings_generated.rs
> > Failed to run rustfmt: No such file or directory (os error 2) (non-fatal, continuing)

This one is unrelated -- it happens when rustfmt is not installed, so
that those interested in only building (but not developing) the kernel
can avoid it. We could hide the message, though for developers it is
useful to know.

This is one instance where knowing in the build system whether the
user intends to developer the kernel or not could be useful (e.g. we
could hide it for some of the distribution/packaging targets); but I
would prefer to simply make rustfmt mandatory, since in principle
there could be a rustfmt bug that makes a behavioral change, and it
would simplify things (it comes with the compiler anyway).

> >   BINDGEN rust/bindings/bindings_helpers_generated.rs
> > error: Found argument '--blacklist-type' which wasn't expected, or isn't valid in this context
> >
> >       Did you mean '--blocklist-type'?
>
> Perhaps this is because I ignored the warnings about my Rust toolchain
> being too new? (Rust 1.65.0, bindgen 0.63.0).  I get that only one

Yeah, that is due to bindgen 0.63.0 removing [1] some flags deprecated
[2] in 0.58.0.

[1] https://github.com/rust-lang/rust-bindgen/blob/v0.63.0/CHANGELOG.md#removed-1
[2] https://github.com/rust-lang/rust-bindgen/blob/v0.58.0/CHANGELOG.md#deprecated-1

> version is really supported, but it would be nice to fail a bit more
> gracefully if at all possible.

Do you mean failing in the `scripts/rust_is_available.sh` step instead
of warning? We could also add versioning information to that script,
so that it knows more about which versions work etc., but I guess at
that point it would be best to simply start supporting several
versions, which may be a bit too early to split CI runs on that since
it would require some degree of testing.

Cheers,
Miguel
  

Carlos Bilbao <carlos.bilbao@amd.com> writes:

> On 1/2/23 17:53, Jonathan Corbet wrote:
>> Perhaps this is because I ignored the warnings about my Rust toolchain
>> being too new? (Rust 1.65.0, bindgen 0.63.0).  I get that only one
>
> Yes, it is important to have the expected Rust toolchain. You can try
> running:
>
> rustup override set $(scripts/min-tool-version.sh rustc)
>
> there's more information about this on the Rust Quick Start [1]. It may be
> annoying but you will need this for any future Rust-kernel work too.

I get this part.  I do wish it would fail a bit more gracefully, but I
*was* warned.

(I got away with building the 6.1 stuff with my out-of-spec toolchain,
but luck always runs out at some point :)

>> version is really supported, but it would be nice to fail a bit more
>> gracefully if at all possible.
>>
>> Anyway, I've unapplied these for now; thoughts on all this?
>
> My two cents is that these are limitations of Rust in the kernel, at least
> on its current state, and so adding rustdoc to the Documentation was
> going to come with them. But if someone has any ideas to make it less
> painful, I'm all ears too :)

I'm worrying now that I asked you to do the wrong thing, sorry.  If
building the Rust docs by default is going to make building the docs in
general harder (and break it for some people), then we need to not do
that.  Unless this can be made to work without forcing users to create a
kernel configuration or breaking the build if the right toolchain isn't
present, then we need to go back to having a separate make subcommand to
build the Rust docs.

My apologies, it wasn't my purpose to make extra useless work for you,
honest...

Thanks again for working on this,

jon
  

Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> writes:

>> - It did a bunch of other building, starting with objtool - again, never
>>   needed for the docs build before.
>
> Yeah, rustdoc, like the compiler, requires dependencies to be
> available to understand the code. Thus some things need to be
> compiled, like for the normal build.

Does it really need objtool?

A certain amount of extra building is OK as long as it doesn't radically
slow down the (already glacial) docs build.  I'd like it to not *break*
the docs build if the right dependencies aren't there, though.

>> version is really supported, but it would be nice to fail a bit more
>> gracefully if at all possible.
>
> Do you mean failing in the `scripts/rust_is_available.sh` step instead
> of warning? We could also add versioning information to that script,
> so that it knows more about which versions work etc., but I guess at
> that point it would be best to simply start supporting several
> versions, which may be a bit too early to split CI runs on that since
> it would require some degree of testing.

It seems like that step should fail regardless, not just for the docs
build, no?

Otherwise, though, it would suffice to turn a failure to build the Rust
docs into a warning-level event for the docs build; I'm mostly concerned
about it breaking the build as a whole.  Supporting multiple Rust
versions would be nice, but it's up to you to decide when you think you
can do that; I don't think the docs build should drive it.

Thanks,

jon
  

On Wed, Jan 4, 2023 at 1:25 AM Jonathan Corbet <corbet@lwn.net> wrote:
>
> Does it really need objtool?

No, it does not. That is a byproduct of using the `prepare` target to
setup Rust for the descend, but we could rearrange some things for
`rustdoc`.

> A certain amount of extra building is OK as long as it doesn't radically
> slow down the (already glacial) docs build.  I'd like it to not *break*
> the docs build if the right dependencies aren't there, though.

I agree if we go with a fixed/preset/configless approach, because in
that case we will always have `CONFIG_RUST=y` and therefore the
generation of Rust docs is really just an attempt that may or may not
fail (or we could only attempt to do so if the dependencies are met
exactly as expected).

On the other hand, if we went with the current setup, where a config
is used, then if the user has specified `CONFIG_RUST=y`, I think it is
fair to fail, since the operation cannot be completed, just like the
normal build. Of course, we could also do the "just attempt it"
approach and print a loud message if it failed, but I think, as a
user, would still prefer as a user if it just failed.

> It seems like that step should fail regardless, not just for the docs
> build, no?

The bindgen step should fail the same way for both normal builds and
docs, indeed.

I think I understand now what you meant by "fail more gracefully". I
thought you meant fail with a better/proper message given versioning
information or similar, but you primarily meant avoid breaking the
entire docs build if the Rust part fails, right?

Cheers,
Miguel
  

Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> writes:

> I think I understand now what you meant by "fail more gracefully". I
> thought you meant fail with a better/proper message given versioning
> information or similar, but you primarily meant avoid breaking the
> entire docs build if the Rust part fails, right?

Both would be nice, but not breaking the docs build would be at the top
of my list.

Thanks,

jon
  

On 1/4/23 08:53, Jonathan Corbet wrote:

> Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> writes:
>
>> I think I understand now what you meant by "fail more gracefully". I
>> thought you meant fail with a better/proper message given versioning
>> information or similar, but you primarily meant avoid breaking the
>> entire docs build if the Rust part fails, right?
> Both would be nice, but not breaking the docs build would be at the top
> of my list.


There's a bunch of simple workarounds we can use to keep going even if
rustdoc fails, if that is agreeable. I'd test but maybe something like:

ifeq ($(CONFIG_RUST),y)
     $(Q)$(MAKE) rustdoc || true
endif

or:

ifeq ($(CONFIG_RUST),y)
     $(Q)$(MAKE) -k rustdoc
endif

or:

ifeq ($(CONFIG_RUST),y)
     -$(Q)$(MAKE) -k rustdoc
endif

> Thanks,
>
> jon


Thanks,

Carlos