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.

Sign up for GitHub

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

Jump to bottom

Only enable vergen on docsrs #533

Merged
merged 1 commit into from
Oct 9, 2023
Merged

Only enable vergen on docsrs #533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ regex = ["dep:regex-automata"]
# Enable serde serialization support
serde = ["dep:serde"]

# Enable dependencies only needed for generation of documentation on docs.rs
docsrs = ["dep:vergen"]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you checked that this actually gets enabled with generating docsrs docs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't tested it with the live docsrs, since I'm not sure how one would go about doing so.

My understanding is that the docsrs feature gets enabled by

chumsky/Cargo.toml

Lines 67 to 69 in af4e2b1

[package.metadata.docs.rs]
all-features = true
rustdoc-args = ["--cfg", "docsrs"]

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this would enable such a feature. The cfg flag just allows you to do #[cfg(docsrs)] (which would probably be sufficient for your purposes, fwiw, instead of adding a cargo feature).

Copy link
Contributor Author

@stefnotch stefnotch Sep 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I tried a few things in that direction, but haven't quite managed to get it to work as expected.

So I first tested the code with RUSTFLAGS="--cfg docsrs" cargo doc --open. (Because someone said that it's a workaround of some sort.) That only seems to pass the info to the Cargo.toml part. It doesn't pass the info to the lib.rs file, since #[cfg(docsrs)] in lib.rs had no effect.

Then I tested it with cargo rustdoc --open -- --cfg docsrs. This lets me do #[cfg(docsrs)] in lib.rs, but does not pass it to the build.rs script. And importantly, it doesn't seem to let me make vergen an optional feature.

I could probably find a variation where I get it to work, but it might almost be easier to make the docsrs stuff a feature. all-features = true should turn on the docsrs feature. Unless of course, you've got a good idea. I'm definitely willing to try out more stuff.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tried the command at the top of guide.rs?

RUSTDOCFLAGS="--cfg docsrs" cargo +nightly doc --all-features

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I missed that command, thank you.

So I tested it just now:
Conditionally compiling vergen does not seem to be possible with

[target.'cfg(docsrs)'.build-dependencies]
vergen = { version = "=8.1.1", features = ["git", "gitoxide"] }

Conditionally compiling it with --all-features and then docsrs = ["dep:vergen"] does seem to work.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that's unfortunate :(

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, not sure I understood this message. So this does work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, this pull request should work.


# An alias of all features that work with the stable compiler.
# Do not use this feature, its removal is not considered a breaking change and its behaviour may change.
# If you're working on chumsky and you're adding a feature that does not require nightly support, please add it to this list.
Expand Down Expand Up @@ -94,7 +97,7 @@ lasso = "0.7"
slotmap = "1.0"

[build-dependencies]
vergen = { version = "=8.1.1", features = ["git", "gitoxide"] }
vergen = { version = "=8.1.1", optional = true, features = ["git", "gitoxide"] }

[target.'cfg(unix)'.dev-dependencies]
pprof = { version = "0.11", features = ["flamegraph", "criterion"] }
Expand Down
12 changes: 12 additions & 0 deletions build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,20 @@
use std::error::Error;
#[cfg(feature = "docsrs")]
use vergen::EmitBuilder;

fn main() -> Result<(), Box<dyn Error>> {
emit_git_metadata()?;
Ok(())
}

#[cfg(feature = "docsrs")]
fn emit_git_metadata() -> Result<(), Box<dyn Error>> {
// Emit the instructions
EmitBuilder::builder().all_git().emit()?;
Ok(())
}

#[cfg(not(feature = "docsrs"))]
fn emit_git_metadata() -> Result<(), Box<dyn Error>> {
Ok(())
}
11 changes: 11 additions & 0 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
extern crate alloc;
extern crate core;

#[cfg(feature = "docsrs")]
macro_rules! blob_url_prefix {
() => {
concat!(
Expand All @@ -34,6 +35,16 @@ macro_rules! blob_url_prefix {
};
}

#[cfg(not(feature = "docsrs"))]
macro_rules! blob_url_prefix {
() => {
concat!(
"https://github.com/zesterer/chumsky/blob/",
env!("CARGO_PKG_VERSION")
)
};
}

macro_rules! go_extra {
( $O :ty ) => {
#[inline(always)]
Expand Down