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: clarify config-relative paths for --config <path> #11079

Merged
merged 3 commits into from
Sep 13, 2022

Conversation

weihanglo
Copy link
Member

@weihanglo weihanglo commented Sep 13, 2022

What does this PR try to resolve?

Resolves #10991.

Instead of changing the behaviour (it might break things), we document the current one as @sunshowers suggesting.

How should we test and review this PR?

Check changes in

  • src/doc/src/reference/config.md and
  • src/doc/man/includes/section-options-common.md.
cargo help build
# Proofread the manpage change.
mdbook serve
# open http://[::1]:3000/reference/config.html#config-relative-paths
# and proofread the config-relative paths section

@rust-highfive
Copy link

r? @epage

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 13, 2022
@weihanglo weihanglo added the A-documenting-cargo-itself Area: Cargo's documentation label Sep 13, 2022
src/doc/src/reference/config.md Outdated Show resolved Hide resolved
Comment on lines 246 to 247
* For environment variables it is relative to the current working directory.
* For config files, that is relative to the parent directory of the directory where the value was defined.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also explicitly call out --config key=value?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not seeing the change for this

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like I'm missing something.

I see config.md covering

  • Environment variables
  • Implicit config files
  • Explicit config files via --config <path>

I'm not seeing --config key=value being discussed for where paths would be relative to.

Copy link
Member Author

Choose a reason for hiding this comment

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

Got it. Thank you for catching this! Fixed with 758ee5d.

Copy link
Member Author

Choose a reason for hiding this comment

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

Current implementation as a reference.

/// Root directory where this is defined.
///
/// If from a file, it is the directory above `.cargo/config`.
/// CLI and env are the current working directory.
pub fn root<'a>(&'a self, config: &'a Config) -> &'a Path {
match self {
Definition::Path(p) => p.parent().unwrap().parent().unwrap(),
Definition::Environment(_) | Definition::Cli => config.cwd(),
}
}

src/doc/src/reference/config.md Outdated Show resolved Hide resolved
@weihanglo
Copy link
Member Author

All issues are resolved. Does it look better?

@epage
Copy link
Contributor

epage commented Sep 13, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Sep 13, 2022

📌 Commit 758ee5d has been approved by epage

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 13, 2022
@bors
Copy link
Contributor

bors commented Sep 13, 2022

⌛ Testing commit 758ee5d with merge fedd172...

@bors
Copy link
Contributor

bors commented Sep 13, 2022

☀️ Test successful - checks-actions
Approved by: epage
Pushing fedd172 to master...

@bors bors merged commit fedd172 into rust-lang:master Sep 13, 2022
@weihanglo weihanglo deleted the issue-10991 branch September 13, 2022 17:09
weihanglo added a commit to weihanglo/rust that referenced this pull request Sep 13, 2022
10 commits in 646e9a0b9ea8354cc409d05f10e8dc752c5de78e..082503982ea0fb7a8fd72210427d43a2e2128a63
2022-09-02 14:29:28 +0000 to 2022-09-13 17:49:38 +0000
- Take priority into account within the pending queue (rust-lang/cargo#11032)
- fix(add): Clarify which version the features are added for (rust-lang/cargo#11075)
- doc: clarify config-relative paths for `--config &lt;path&gt;` (rust-lang/cargo#11079)
- Do not add home bin path to PATH if it's already there (rust-lang/cargo#11023)
- Don't use `for` loop on an `Option` (rust-lang/cargo#11081)
- Remove dead code (rust-lang/cargo#11080)
- Change progress indicator for sparse registries (rust-lang/cargo#11068)
- chore(ci): Ensure intradoc links are valid (rust-lang/cargo#11055)
- Cache index files based on contents hash (rust-lang/cargo#11044)
- fix: specifies the max length for crate name (rust-lang/cargo#11051)
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 13, 2022
Update cargo

10 commits in 646e9a0b9ea8354cc409d05f10e8dc752c5de78e..082503982ea0fb7a8fd72210427d43a2e2128a63 2022-09-02 14:29:28 +0000 to 2022-09-13 17:49:38 +0000
- Take priority into account within the pending queue (rust-lang/cargo#11032)
- fix(add): Clarify which version the features are added for (rust-lang/cargo#11075)
- doc: clarify config-relative paths for `--config <path>` (rust-lang/cargo#11079)
- Do not add home bin path to PATH if it's already there (rust-lang/cargo#11023)
- Don't use `for` loop on an `Option` (rust-lang/cargo#11081)
- Remove dead code (rust-lang/cargo#11080)
- Change progress indicator for sparse registries (rust-lang/cargo#11068)
- chore(ci): Ensure intradoc links are valid (rust-lang/cargo#11055)
- Cache index files based on contents hash (rust-lang/cargo#11044)
- fix: specifies the max length for crate name (rust-lang/cargo#11051)
@ehuss ehuss added this to the 1.65.0 milestone Sep 21, 2022
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 20, 2024
Update cargo

10 commits in 646e9a0b9ea8354cc409d05f10e8dc752c5de78e..082503982ea0fb7a8fd72210427d43a2e2128a63 2022-09-02 14:29:28 +0000 to 2022-09-13 17:49:38 +0000
- Take priority into account within the pending queue (rust-lang/cargo#11032)
- fix(add): Clarify which version the features are added for (rust-lang/cargo#11075)
- doc: clarify config-relative paths for `--config <path>` (rust-lang/cargo#11079)
- Do not add home bin path to PATH if it's already there (rust-lang/cargo#11023)
- Don't use `for` loop on an `Option` (rust-lang/cargo#11081)
- Remove dead code (rust-lang/cargo#11080)
- Change progress indicator for sparse registries (rust-lang/cargo#11068)
- chore(ci): Ensure intradoc links are valid (rust-lang/cargo#11055)
- Cache index files based on contents hash (rust-lang/cargo#11044)
- fix: specifies the max length for crate name (rust-lang/cargo#11051)
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 27, 2024
Update cargo

10 commits in 646e9a0b9ea8354cc409d05f10e8dc752c5de78e..082503982ea0fb7a8fd72210427d43a2e2128a63 2022-09-02 14:29:28 +0000 to 2022-09-13 17:49:38 +0000
- Take priority into account within the pending queue (rust-lang/cargo#11032)
- fix(add): Clarify which version the features are added for (rust-lang/cargo#11075)
- doc: clarify config-relative paths for `--config <path>` (rust-lang/cargo#11079)
- Do not add home bin path to PATH if it's already there (rust-lang/cargo#11023)
- Don't use `for` loop on an `Option` (rust-lang/cargo#11081)
- Remove dead code (rust-lang/cargo#11080)
- Change progress indicator for sparse registries (rust-lang/cargo#11068)
- chore(ci): Ensure intradoc links are valid (rust-lang/cargo#11055)
- Cache index files based on contents hash (rust-lang/cargo#11044)
- fix: specifies the max length for crate name (rust-lang/cargo#11051)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-documenting-cargo-itself Area: Cargo's documentation S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document that cargo --config <filename> has unintuitive behavior for config-relative paths
5 participants