Thanks Guillaume, Miguel.
This is looking good to me.
On Thu, 9 Oct 2025 at 17:14, Miguel Ojeda ojeda@kernel.org wrote:
On Tue, 17 Jun 2025 16:00:33 +0200 Guillaume Gomez guillaume1.gomez@gmail.com wrote:
The goal of this patch is to remove the use of 2 unstable rustdoc features (`--no-run` and `--test-builder`) and replace it with a stable feature: `--output-format=doctest`, which was added in https://github.com/rust-lang/rust/pull/134531.
Before this patch, the code was using very hacky methods in order to retrieve doctests, modify them as needed and then concatenate all of them in one file.
Now, with this new flag, it instead asks rustdoc to provide the doctests code with their associated information such as file path and line number.
Signed-off-by: Guillaume Gomez guillaume1.gomez@gmail.com
(Procedural bit: normally we provide a changelog between versions after this `---` line so that reviewers now what changed so far.)
I finally took a look at this again, so I rebased it and got:
thread 'main' panicked at scripts/rustdoc_test_gen.rs:92:15: No path candidates found for `rust_kernel_alloc_allocator.rs`.This is likely a bug in the build system, or some files went away while compiling.which brings me to the bigger point: the main reason to have the new output format is to avoid all these hacks, including the "find the real path back to the original file" hack here. More generally, to avoid the 2 scripts approach.
So now we can finally get rid of all that and simplify. That is, we can just merge it all in a single script that reads the JSON and builds the result directly, since now we have everything we need (originally I needed the 2 scripts approach since `rustdoc` executed the test builder once per test so I had to somehow collect the results).
i.e. no more hundreds of generated files/processes, just a simple pipe.
Yeah, this definitely seems the right way -- even though outputting the generated rustdoc.json to a file is sometimes useful for debugging it. But unless this actually breaks often (which hopefully it will less frequently with this setup), just passing things around directly in a pipe is neater.
Anyway, just to check we had everything we needed, I did a quick try -- please see the draft patch below.
I gave it a go -- please see the draft patch below. The diff w.r.t. your patch would be something like +217 -341, i.e. we get rid of quite a lot of lines. I added as well some more context in the commit message, and put the right docs in the unified script. This also improves the sorting of the tests (it now follows the line number better).
We still have to preserve the support for the old compilers, so what I think I will do is just have the new script separately, keeping the old ones as-is until we can remove them when we upgrade the minimum for e.g. the next Debian Stable.
Agreed: we definitely need to keep the old way running for a little bit longer. Whether or not it makes sense to have both versions running in parallel, or just delay this until we no-longer need the old implementation is a matter of taste, I suspect. Having just one "active" implementation at a time has a bunch of advantages (particularly in the case there's some inconsistency in, e.g., test names between compiler versions) -- it's probably what I'd go for (mostly because it's easier), but I've no objection to having both running in parallel, assuming it doesn't cause any more issues than the code getting more complicated. And I personally don't feel that having both is likely to be a big enough burden to be worth bumping the compiler version alone.
Cc'ing David and KUnit, since this is closer to getting ready -- please let me know if this raises alarms for anyone.
Apart from the fact that it breaks the older compilers, this looks like a definite improvement. (I was a little worried about adding a whole json parser in, but it's nicely small and self-contained, so I actually quite like it.)
So I'm happy with this from the KUnit side, so long as we either don't remove support for the old system/old compilers, or delay merging this until we can drop support for those altogether.
Cheers, -- David