Skip to content

Docs don't inform that given class is gated behind experimental-godot-api feature #1163

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Yarwin opened this issue May 13, 2025 · 11 comments · Fixed by godot-rust/godot-rust.github.io#12
Labels
documentation Improvements or additions to documentation quality-of-life No new functionality, but improves ergonomics/internals

Comments

@Yarwin
Copy link
Contributor

Yarwin commented May 13, 2025

Back in June of 2024 docs were informing if given class was gated behind feature or not:

image from 19.06.2024

Image

Nowadays they don't do so despite the fact that given classes are still gated behind said feature:

image from 13.05.2025

Image

https://godot-rust.github.io/docs/gdext/master/godot/classes/struct.NavigationAgent2D.html

list of experimental classes
"AudioSample",
"AudioSamplePlayback",
"Compositor",
"CompositorEffect",
"GraphEdit",
"GraphElement",
"GraphFrame",
"GraphNode",
"NavigationAgent2D",
"NavigationAgent3D",
"NavigationLink2D",
"NavigationLink3D",
"NavigationMesh",
"NavigationMeshSourceGeometryData2D",
"NavigationMeshSourceGeometryData3D",
"NavigationObstacle2D",
"NavigationObstacle3D",
"NavigationPathQueryParameters2D",
"NavigationPathQueryParameters3D",
"NavigationPathQueryResult2D",
"NavigationPathQueryResult3D",
"NavigationPolygon",
"NavigationRegion2D",
"NavigationRegion3D",
"NavigationServer2D",
"NavigationServer3D",
"Parallax2D",
"SkeletonModification2D",
"SkeletonModification2DCCDIK",
"SkeletonModification2DFABRIK",
"SkeletonModification2DJiggle",
"SkeletonModification2DLookAt",
"SkeletonModification2DPhysicalBones",
"SkeletonModification2DStackHolder",
"SkeletonModification2DTwoBoneIK",
"SkeletonModificationStack2D",
"StreamPeerGZIP",
"XRBodyModifier3D",
"XRBodyTracker",
"XRFaceModifier3D",
"XRFaceTracker",
@Yarwin Yarwin added documentation Improvements or additions to documentation quality-of-life No new functionality, but improves ergonomics/internals labels May 13, 2025
@Bromeon
Copy link
Member

Bromeon commented May 13, 2025

Also, experimental-threads and feature(since_api = "4.5") 👍

@Yarwin Yarwin moved this to Backlog in priorities May 13, 2025
@fpdotmonkey
Copy link
Contributor

This suggests that it's a nightly feature. Not sure how it could've worked before, though. https://users.rust-lang.org/t/how-to-document-optional-features-in-api-docs/64577/3

@Bromeon
Copy link
Member

Bromeon commented May 14, 2025

This suggests that it's a nightly feature.

That's not the problem though, our invocation has used cargo +nightly doc.

In essence, in a post-processing step we parse #[cfg(...)] attributes and insert additional #[doc(cfg(...))] ones. I assume this broke at some point, would be good to find out where and why the cargo doc command didn't cause an error.

Ideally it should also work for https://docs.rs/godot, which is handled in this CI workflow.

@Yarwin
Copy link
Contributor Author

Yarwin commented May 14, 2025

so silly – we just need to pass RUSTDOCFLAGS flag while building the docs. I'll push the fix later this day.

RUSTFLAGS="--cfg published_docs -A unused_imports -A dead_code -A unexpected_cfgs -A unused_doc_comments -A unused_attributes" RUSTDOCFLAGS="--cfg published_docs" cargo +nightly doc --lib -p godot --features experimental-godot-api,experimental-threads,serde --no-deps --open

Image

@Bromeon
Copy link
Member

Bromeon commented May 14, 2025

Nice, thanks a lot! Do you know if this changed recently, as it used to work? Or did we accidentally change it?

@Yarwin
Copy link
Contributor Author

Yarwin commented May 15, 2025

Nice, thanks a lot! Do you know if this changed recently, as it used to work? Or did we accidentally change it?

The issue was on our side – we started gating docs behind cfg attribute published_docs while never providing it in the first place (see target/doc/release/godot-…/lib-godot.json and rustflags key).

I also messed for a while with doc_auto_cfg (see RFC: https://github.com/rust-lang/rfcs/blob/master/text/3631-rustdoc-cfgs-handling.md) but I can't get it to work for some reason 🤔 (I tried various combinations of !#[feature(..)]). For now I'll leave everything as it is and add a proper rustdocflag.

@Bromeon
Copy link
Member

Bromeon commented May 15, 2025

The issue was on our side – we started gating docs behind cfg attribute published_docs while never providing it in the first place

Wait, but for docs.rs we do provide it (also in other crates):

rustdoc-args = ["--cfg", "published_docs"]

Is rustdoc-args not forwarded properly to the command line / env var?


And for the website workflow, we have the following. So the issue is that we use RUSTFLAGS instead of RUSTDOCFLAGS?

export RUSTFLAGS="--cfg published_docs -A unused_imports -A dead_code -A ..."

@Yarwin
Copy link
Contributor Author

Yarwin commented May 15, 2025

Is rustdoc-args not forwarded properly to the command line / env var?

Huh, it confused me too – as the https://docs.rs/about/metadata states: You can customize docs.rs builds by defining (…) it is for customizing docs.rs builds (the one for these docs: https://docs.rs/godot/latest/godot/index.html).

Good that you reminded me about it tho, we need to add rustc-args there too


(…) So the issue is that we use RUSTFLAGS instead of RUSTDOCFLAGS?

I haven't dig into rustdoc behavior (and I don't intent so) but from what I see it is still required. cargo doc (...) firstly builds the package, and then documents it. I've tried RUSTDOCFLAGS="--cfg published_docs (...)" cargo +nightly doc --lib -p godot (...) few times and from what I see, the cfg parameter is being forwarded only to godot

Image

while doubling it with Rustflags results in propagating this cfg value to all the built crates:

Image

(I could probably use find or something instead of print screen, but I'm too lazy).

@Yarwin Yarwin moved this from Backlog to in review in priorities May 15, 2025
@Bromeon
Copy link
Member

Bromeon commented May 15, 2025

All good, let's not spend too much time on this 🙂

@Bromeon
Copy link
Member

Bromeon commented May 15, 2025

Another random bit I just found:

Merged years ago, but in my testing last year I don't think this was the case? It's still nightly-only, but I wonder if we need something special to activate it?

@Yarwin
Copy link
Contributor Author

Yarwin commented May 15, 2025

but I wonder if we need something special to activate it?

Yeah, for some reason I couldn't get it to work (obviously I edited macros, codegen, lib.rs feature activation and whatnot). We can open issue for that later, for now I would just like to fix (not solve) the problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation quality-of-life No new functionality, but improves ergonomics/internals
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants