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

add doc_auto_cfg to relevant sdk crates #3121

Merged
merged 4 commits into from
Oct 12, 2024

Conversation

kevinheavey
Copy link

@kevinheavey kevinheavey commented Oct 9, 2024

Problem

The new SDK crates put a bunch of code behind feature gates, which means those items won't be visible on docs.rs. This wasn't a big issue with solana_sdk and solana_program because those crates enable almost all their features by default.

Summary of Changes

For each crate in sdk/ where this is an issue, add

all-features = true
rustdoc-args = ["--cfg=docsrs"]

to [package.metadata.docs.rs]
and add #![cfg_attr(docsrs, feature(doc_auto_cfg))] to each module that contains feature-gated public items.

This makes docs.rs show these items with a note that they require certain features (or targets etc). This solution is fairly common: tokio is a prominent crate that does this.

To emulate what docs.rs generates, you can cd to the individual crate directory and run RUSTDOCFLAGS="--cfg docsrs" cargo +nightly doc --all-features --no-deps

I published a toy example crate to demonstrate that this works on docs.rs: https://docs.rs/doc-feature-examples/latest/doc_feature_examples/

yihau
yihau previously approved these changes Oct 10, 2024
Copy link
Member

@yihau yihau left a comment

Choose a reason for hiding this comment

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

looks quite good to me! thank you!

@yihau
Copy link
Member

yihau commented Oct 10, 2024

if don't mind, we can wait @joncinque or @febo to have a chance to take a look

Copy link

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Thanks for noticing and fixing this! Just a few little things to clean up

@@ -1,4 +1,5 @@
#![no_std]
#![cfg_attr(docsrs, feature(doc_auto_cfg))]

Choose a reason for hiding this comment

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

Can you also add the change to sdk/hash/Cargo.toml for all-features and rustdoc-args?

@@ -1,3 +1,4 @@
#![cfg_attr(docsrs, feature(doc_auto_cfg))]

Choose a reason for hiding this comment

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

Is this required here since it's already enabled in lib.rs?

@@ -1,5 +1,6 @@
//! 64-byte signature type.
#![no_std]
#![cfg_attr(docsrs, feature(doc_auto_cfg))]

Choose a reason for hiding this comment

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

Can you add the appropriate flags to sdk/signature/Cargo.toml?

Copy link

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@joncinque joncinque added the automerge automerge Merge this Pull Request automatically once CI passes label Oct 11, 2024
Copy link

mergify bot commented Oct 11, 2024

automerge label removed due to a CI failure

@mergify mergify bot removed the automerge automerge Merge this Pull Request automatically once CI passes label Oct 11, 2024
@kevinheavey kevinheavey force-pushed the docs-features-cleanup branch from a06f515 to 8ac2ec3 Compare October 11, 2024 18:02
@joncinque joncinque added the automerge automerge Merge this Pull Request automatically once CI passes label Oct 11, 2024
Copy link

mergify bot commented Oct 11, 2024

automerge label removed due to a CI failure

@mergify mergify bot removed the automerge automerge Merge this Pull Request automatically once CI passes label Oct 11, 2024
@kevinheavey kevinheavey force-pushed the docs-features-cleanup branch from 8ac2ec3 to e2a47f5 Compare October 12, 2024 09:28
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 12, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 12, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 12, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 12, 2024
@kevinheavey kevinheavey added the automerge automerge Merge this Pull Request automatically once CI passes label Oct 12, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 12, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 12, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 12, 2024
@kevinheavey
Copy link
Author

@joncinque could you merge please? Looks like automerge is still disabled

kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 22, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 22, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 22, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 22, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 24, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 26, 2024
mergify bot pushed a commit that referenced this pull request Oct 28, 2024
* extract packet crate

* typo

* sort deps

* update digest

* add doc_auto_cfg like in #3121

* fix frozen-abi
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 28, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 28, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 28, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 29, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 29, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 29, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 29, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 29, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 29, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 29, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 30, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Nov 1, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Nov 1, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Nov 2, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Nov 5, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Nov 5, 2024
mergify bot pushed a commit that referenced this pull request Nov 6, 2024
* extract signer crate

* fmt

* fix bs58 std usage

* extract seed-derivable crate

* extract seed-phrase crate

* fix copy-pasted doc

* extract presigner crate

* make keypair module optional in solana-signer

* remove serde_json dep and fix test features

* remove thiserror from solana-signer

* lint

* more lint

* fix dev dep feature

* sort deps

* remove itertools from solana-signer

* move wasm impl to signer crate

* add doc_auto_cfg like in #3121

* post-rebase fix

* another post-rebase fix

* fmt

* make keypair functionality of solana-seed-phrase optional

* fix feature-gated imports

* Extract solana-keypair, move other things around

* Cleanup things that I missed

* Remove space

* Add deprecated notice

* Add deprecation notices

* Refactor SeedDerivable trait implementation

---------

Co-authored-by: Jon C <[email protected]>
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Nov 6, 2024
joncinque pushed a commit that referenced this pull request Nov 7, 2024
* minimize solana_sdk use in reserved_account_keys

* extract reserved-account-keys crate

* fix frozen-abi support in reserved-account-keys

* move id definitions to sdk-ids

* also move the IDs of zk_token_proof_program and zk_elgamal_proof_program

* fix some paths

* missing dev dep

* add doc_auto_cfg like in #3121

* fix copied sysvar id

* fix missing dep after rebase

* move stake::program::declare_id to stake::declare_id

* post-rebase fix
ray-kast pushed a commit to abklabs/agave that referenced this pull request Nov 27, 2024
ray-kast pushed a commit to abklabs/agave that referenced this pull request Nov 27, 2024
* extract account-decoder-client-types and convert UiAccount::encode to a free func encode_ui_account

* rip out more types and replace some usage in rpc-client and rpc-client-api

* rip out more types

* remove remaining account-decoder usage from rpc-client

* remove account-decoder from rpc-client-api and transaction-status-client-types

* make zstd optional

* fix import

* ignore some lint

* update some conversions

* missing import

* missing import

* unused imports

* fmt

* don't feature-gate any variants of UiAccountEncoding

* fmt

* update newer code that used UiAccount::encode

* remove sdk dep from account-decoder-client-types

* add doc_auto_cfg like in anza-xyz#3121
ray-kast pushed a commit to abklabs/agave that referenced this pull request Nov 27, 2024
* extract transaction-error crate

* update TransactionError usage in sdk

* fix frozen-abi support

* make serde optional in solana-transaction-error

* update lock file

* fmt

* add #[cfg(not(target_os = "solana"))] where applicable

* fmt

* put solana-transaction-error behind feature "full"

* use workspace lints

* update digest

* make rustc_version optional

* avoid frozen-abi build script

* add back workspace lints

* remove thiserror

* remove unused build deps

* use solana-instruction directly

* move AddressLoaderError and SanitizeMessageError to transaction-error, and fix its solana-instruction dep features

* lint

* move TransportError to transaction-error crate

* use doc_auto_cfg like in anza-xyz#3121

* missing comma

* add deprecations to re-exports
ray-kast pushed a commit to abklabs/agave that referenced this pull request Nov 27, 2024
* extract packet crate

* typo

* sort deps

* update digest

* add doc_auto_cfg like in anza-xyz#3121

* fix frozen-abi
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge automerge Merge this Pull Request automatically once CI passes need:merge-assist
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants