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

documentation of environment variables is lacking #920

Open
5 tasks
Be-ing opened this issue Jan 26, 2024 · 13 comments
Open
5 tasks

documentation of environment variables is lacking #920

Be-ing opened this issue Jan 26, 2024 · 13 comments

Comments

@Be-ing
Copy link
Contributor

Be-ing commented Jan 26, 2024

Currently, the documentation on what environment variables this crate reads and what effects they have is lacking. For example, the try_flags_from_environment documentation merely says "Normally the cc crate will consult with the standard set of environment variables (such as CFLAGS and CXXFLAGS) to construct the compiler invocation." cc-rs uses more than CFLAGS and CXXFLAGS. Here are a few topics lacking documentation:

  • How to use CC/CXX to specify the compiler
  • How CC/CXX can specify compiler caches and compiler flags
  • How to use target-specific environment variables
  • How RUSTC_WRAPPER can be used for C & C++ compiler caching with sccache
  • VCINSTALLDIR

Are there other uses of environment variables that should be documented?

@Be-ing Be-ing changed the title documentation on environment variables is lacking documentation of environment variables is lacking Jan 26, 2024
@NobodyXu
Copy link
Collaborator

NobodyXu commented Jan 26, 2024

There are a few more from

cc-rs/src/lib.rs

Line 3238 in 8cf5455

fn get_target(&self) -> Result<Arc<str>, Error> {

  • TARGET set the default target if user does not specify one
  • HOST
  • OPT_LEVEL
  • DEBUG

@NobodyXu
Copy link
Collaborator

Also

@NobodyXu
Copy link
Collaborator

NobodyXu commented Jan 26, 2024

There are also target prefixed version of CFLAGS/CXXFLAGS

cc-rs/src/lib.rs

Line 2180 in 8cf5455

let flags_env_var_name = if self.cpp { "CXXFLAGS" } else { "CFLAGS" };

Prefixed NVCC

cc-rs/src/lib.rs

Line 2657 in 8cf5455

let nvcc = match self.getenv_with_target_prefixes("NVCC") {

CXXSTDLIB

cc-rs/src/lib.rs

Line 2826 in 8cf5455

if let Ok(stdlib) = self.getenv_with_target_prefixes("CXXSTDLIB") {

@NobodyXu
Copy link
Collaborator

Also prefixed in CC and CXX

cc-rs/src/lib.rs

Line 2558 in 8cf5455

.env_tool(env)

cc-rs/src/lib.rs

Line 2755 in 8cf5455

let tool = match self.getenv_with_target_prefixes(name) {

@NobodyXu
Copy link
Collaborator

NobodyXu commented Jan 26, 2024

Prefixed AR and RANLIB

cc-rs/src/lib.rs

Line 2949 in 8cf5455

fn get_base_archiver_variant(&self, env: &str, tool: &str) -> Result<(Command, String), Error> {

And ARFLAGS, RANLIBFLAGS

cc-rs/src/lib.rs

Line 3364 in 8cf5455

fn envflags(&self, name: &str) -> Result<Vec<String>, Error> {

@Be-ing
Copy link
Contributor Author

Be-ing commented Jan 27, 2024

Oh, there is some documentation in README.md, but not on https://docs.rs. I think that should be moved to the crate documentation in lib.rs so it shows up on https://docs.rs. I expect to find crate usage documentation on https://docs.rs, not a README.md in the source repository.

@NobodyXu
Copy link
Collaborator

Maybe we could #[doc = include_str!("../../README.md")] to include them in docs.rs/cc?

@Be-ing
Copy link
Contributor Author

Be-ing commented Jan 27, 2024

I think it would be simpler to just link to https://docs.rs from README.md and keep the documentation in one place (lib.rs)

@NobodyXu
Copy link
Collaborator

Well, if we add new env to main, then anybody who checks the doc in README would use it while it doesn't work in practice.

@Be-ing
Copy link
Contributor Author

Be-ing commented Jan 27, 2024

Yes, I've run into that problem with another Rust library. I got confused because the README.md documented some new features that were not available in the latest release at the time.

@NobodyXu
Copy link
Collaborator

Yeah, that's why I think #[doc = include_str!("../../README.md")] makes more sense, since it would guarantee the consistency

@Be-ing
Copy link
Contributor Author

Be-ing commented Jan 28, 2024

#[doc = include_str!("../../README.md")] would keep README.md and docs.rs consistent. However, that leaves the possibility of someone going to https://github.com/rust-lang/cc-rs and getting confused by new information in README.md that hasn't been released yet.

@NobodyXu
Copy link
Collaborator

Yeah that's true, my other project simply leaves a link to latest doc on crates.io/crates/cargo-binstall to maintain consistency

Be-ing added a commit to Be-ing/cc-rs that referenced this issue Jan 28, 2024
so it appears on https://docs.rs and inconsistencies aren't
created between README.md and lib.rs

per discussion in rust-lang#920

The old introductory text in lib.rs has been replaced with
the text from README.md and the introductory paragraph revised.
Otherwise no major changes to the text have been made.
NobodyXu pushed a commit that referenced this issue Feb 12, 2024
so it appears on https://docs.rs and inconsistencies aren't
created between README.md and lib.rs

per discussion in #920

The old introductory text in lib.rs has been replaced with
the text from README.md and the introductory paragraph revised.
Otherwise no major changes to the text have been made.
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

No branches or pull requests

2 participants