Skip to content
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

#[doc(html_root_url)] #517

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

#[doc(html_root_url)] #517

wants to merge 1 commit into from

Conversation

arcnmx
Copy link
Contributor

@arcnmx arcnmx commented Jan 31, 2022

This can fix up cargo doc --no-deps URLs, which is nice for anyone hosting their own crate's docs online in the same way glib does - particularly if it targets the development git branch.

There are a few things to address here:

  • If this is merged, there should be an associated/backported PR targeting stable 0.15 branch with a different URL root (https://gtk-rs.org/gtk-rs-core/stable/0.xx/docs/)
  • Whenever a release is made, these URLs would need to be updated after branching off but before tagging a release.
    • I believe it's common practice to add a reminder comment to Cargo.toml as a prompt for anyone manually updating the version number when a major release occurs
    • Another approach might be to encode the requirement directly into CI using something like version-sync
    • I'm not sure if this is the current deployment practice or if master is tagged directly before branching off instead?
  • I'd like to also include the same change to the sys crates, but since they're auto-generated by gir I'm not entirely sure how to approach that? I've had this problem with gir before, and am not sure if it provides any ability to customize the sys/lib.rs generation?

@bilelmoussaoui
Copy link
Member

We already handle that automatically as part of the CI using the gir-rustdoc script, are there any external dependencies that are not properly linked?

@arcnmx
Copy link
Contributor Author

arcnmx commented Jan 31, 2022

We already handle that automatically as part of the CI using the gir-rustdoc script, are there any external dependencies that are not properly linked?

Sorry it's not clear to me what you mean by this. Do you mean that the CI deployment steps modify the source code before uploading it to crates.io? It's not that glib's external dependencies aren't properly linked, it's that a project where glib is an external dependency isn't properly linked!

As far as I can tell, gir-rustdoc is used for generating your online docs, and it uses --extern-html-root-url in some fashion to override the URLs. This only applies to its own docs (and any downstream crates that might happen to also use the same script) though. That seems like a bit of a hack when rustdoc already has built-in support for conveying the documentation URL to dependents - #![doc(html_root_url)]! Requiring the use of specific scripts and unstable flags for generating the docs of any crate that depends on glib doesn't seem ideal, and is particularly unhelpful when trying to use cargo doc --open locally.

(I'll note that I am currently using that flag to generate docs, which is what prompted me to open this PR. I'm not using that script, though at a glance the sys crates do indeed appear to be missing?)

@bilelmoussaoui
Copy link
Member

As far as I can tell, gir-rustdoc is used for generating your online docs, and it uses --extern-html-root-url in some fashion to override the URLs. This only applies to its own docs (and any downstream crates that might happen to also use the same script) though. That seems like a bit of a hack when rustdoc already has built-in support for conveying the documentation URL to dependents - #![doc(html_root_url)]! Requiring the use of specific scripts and unstable flags for generating the docs of any crate that depends on glib doesn't seem ideal, and is particularly unhelpful when trying to use cargo doc --open local

The docs already depends on rust nightly for other reasons (the cfg doc feature) so why not use other nightly features?

(I'll note that I am currently using that flag to generate docs, which is what prompted me to open this PR. I'm not using that script, though at a glance the sys crates do indeed appear to be missing?)

You should probably run python3 generate.py --embed-docs first then

Tbh, this solution would require more work in general as you will have to always update all the files to point to the right docs link. I don't think that is ideal neither :)

Anyway, I will summon our docs expert @GuillaumeGomez for his opinion

@arcnmx
Copy link
Contributor Author

arcnmx commented Jan 31, 2022

The docs already depends on rust nightly for other reasons (the cfg doc feature) so why not use other nightly features?

This is actually partially my argument for why this should be done. Anyone stuck on a stable rustc would benefit from being able to use cargo doc locally (without --features glib/dox) and have it automagically pull in / link to the docs generated properly with all the bells and whistles!

You should probably run python3 generate.py --embed-docs first then

Hm, how does that affect whether --extern-html-root-url glib-sys=X is passed or not?

Tbh, this solution would require more work in general as you will have to always update all the files to point to the right docs link. I don't think that is ideal neither :)

Yes, the maintenance burden does suck - I feel like it provides material value to downstream users though. I'd probably push for this to be integrated into gir or other tools in order to (at least mostly) automate it. I've resorted to some odd hacks myself after failing to wrangle gir to generate sys crates in particular ways .-.

@bilelmoussaoui
Copy link
Member

So what should we do here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants