Fix crate build on docs.rs #281
Conversation
Use dynamic code-gen to build-around non-availabiltiy of some packages.
|
The documenation assumed GDAL 3.2 (the sys versions do not matter as |
Use latest GDAL 3.5 pre-bindings for compiling for docs.
| fn main() { | ||
| let out_path = PathBuf::from(env::var("OUT_DIR").unwrap()).join("bindings.rs"); | ||
|
|
||
| // Hardcode a prebuilt binding version while generating docs. | ||
| // Otherwise docs.rs will explode due to not actually having libgdal installed. | ||
| if std::env::var("DOCS_RS").is_ok() { | ||
| let version = Version::parse("3.2.0").expect("invalid version for docs.rs"); | ||
| let version = Version::parse("3.5.0").expect("invalid version for docs.rs"); |
There was a problem hiding this comment.
Suggestion: this seems a bit buried here, what about pulling the version out as global variables at the top, to make them easier to maintain as new versions are released?
At top:
// Set the following to the latest released version of GDAL for use by docs.rs
const DOCS_RS_GDAL_VERSION: &str = "3.5.0";
const DOCS_RS_GDAL_VERSION_NUM: &str = "3050000";Here
let version = Version::parse(DOCS_RS_GDAL_VERSION).expect(...);
add_docs_rs_helper(Some(DOCS_RS_GDAL_VERSION_NUM));Also - is there a way to programmatically derive the "3050000" value from "3.5.0", or does it vary somewhat independently? (presumably the major / minor parts of it follow the semver components). If the latter, might be good to include a comment on where to find this value; I didn't find it in the prebuilt bindings for 3.5, so it doesn't seem easily obvious for future versions.
There was a problem hiding this comment.
The number is defined like so in gdal source:
/* GDAL_COMPUTE_VERSION macro introduced in GDAL 1.10 */
/* Must be used ONLY to compare with version numbers for GDAL >= 1.10 */
#ifndef GDAL_COMPUTE_VERSION
#define GDAL_COMPUTE_VERSION(maj,min,rev) ((maj)*1000000+(min)*10000+(rev)*100)
#endifWe'll probably refactor the version handling soon-ish as the recent gdal versions are not strictly semver (like 3.5.0.2). The semver crate seems to fail on these at present.
|
Do we still need in |
|
Closes #268 |
|
can we merge this? |
|
I'm a bit behind on this, but do we know why the current approach isn't working? Was it a docs.rs bug? |
|
@jdroenner Yes, we should merge this and closes PR #241 if it builds on docs.rs. I've checked this locally on the docs.rs docker images and it works. |
|
Did we try to ask someone working on docs.rs why the cfg is no longer set? I'm surprised there aren't more crates running into this, and the workaround is TBH quite complex. |
|
Well it's been open for a while (ref. #268) and affects all crates depending on gdal. We could always un-complicate it if docs.rs supports a cfg attribute. |
|
Yeah, so I just re-discovered rust-lang/docs.rs#1580. We might be able to use https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#host-config instead, but it's probably not worth the hassle of an unstable option. bors d+ |
|
✌️ rmanoka can now approve this pull request. To approve and merge a pull request, simply reply with |
|
bors r+ |
|
rust-lang/docs.rs#1580 just got fixed though. |
|
Build succeeded: |
|
I'm fine with whatever gets docs compiling on publish. |
296: Increased documentation content of README and root rustdoc page. r=lnicola a=metasim - [X] I agree to follow the project's [code of conduct](https://github.com/georust/gdal/blob/master/CODE_OF_CONDUCT.md). - [X] I added an entry to `CHANGES.md` if knowledge of this change could be valuable to users. --- I wanted to kickstart some work to level-up the documentation. Wanting to take an incremental approach, making it feel a bit less intimidating to make progress in the future. I don't see this as the only PR I do on this, just a start. All feedback welcome :). Related: * #268 * #281 Co-authored-by: Simeon H.K. Fitch <fitch@astraea.io>
Use dynamic code-gen to build-around non-availabiltiy of some packages.
CHANGES.mdif knowledge of this change could be valuable to users.