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

fix(config): Ensure --config net.git-fetch-with-cli=true is respected #13992

Merged
merged 2 commits into from
May 31, 2024

Conversation

epage
Copy link
Contributor

@epage epage commented May 31, 2024

What does this PR try to resolve?

#13479 changed the global context initialization order so that command line stuff is processed after we read some config.
This had a side effect of breaking --config net.git-fetch-with-cli=true.
I reverted the change to restore support for --config.

Fixes #13991

How should we test and review this PR?

Additional information

This reverts commit f525e1f.

This removes color control from warnings for unstable features.
For some reason this removed color support from cargo -Zhelp in the
tests but I can't reproduce it locally.

The most important thing was getting the config fix in.
There are two follow ups

  • Can we have the config working and color?
  • Why did this fail for this field and not the others we already had
    tests for?

I ran out my immediate time box for looking into these.

epage added 2 commits May 31, 2024 14:56
This reverts commit f525e1f.

This removes color control from warnings for unstable features.
For some reason this removed color support from `cargo -Zhelp` in the
tests but I can't reproduce it locally.

The most important thing was getting the config fix in.
There are two follow ups
- Can we have the config working *and* color?
- Why did this fail for this field and not the others we already had
  tests for?

I ran out my immediate time box for looking into these.

Fixes rust-lang#13991
@rustbot
Copy link
Collaborator

rustbot commented May 31, 2024

r? @weihanglo

rustbot has assigned @weihanglo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-configuration Area: cargo config files and env vars S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 31, 2024
@epage
Copy link
Contributor Author

epage commented May 31, 2024

I suspect we should do a beta-backport for this (another reason to keep this PR simple).

Should we also do a 1.78.1? I suspect that 1.79 is close enough that it won't be worth it.

Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

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

Yes we should do a beta backport

@weihanglo
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented May 31, 2024

📌 Commit 4ce2b61 has been approved by weihanglo

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 May 31, 2024
@bors
Copy link
Contributor

bors commented May 31, 2024

⌛ Testing commit 4ce2b61 with merge 7a6fad0...

@bors
Copy link
Contributor

bors commented May 31, 2024

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 7a6fad0 to master...

@bors bors merged commit 7a6fad0 into rust-lang:master May 31, 2024
21 checks passed
@epage epage deleted the config branch June 1, 2024 01:32
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 1, 2024
Update cargo

9 commits in 431db31d0dbeda320caf8ef8535ea48eb3093407..7a6fad0984d28c8330974636972aa296b67c4513
2024-05-28 18:17:31 +0000 to 2024-05-31 22:26:03 +0000
- fix(config): Ensure `--config net.git-fetch-with-cli=true` is respected (rust-lang/cargo#13992)
- Fix libcurl proxy documentation link (rust-lang/cargo#13990)
- fix(new): Dont say were adding to a workspace when a regular package is in root (rust-lang/cargo#13987)
- fix: adjust custom err from cert-check due to libgit2 1.8 change (rust-lang/cargo#13970)
- fix(toml): Ensure targets are in a deterministic order (rust-lang/cargo#13989)
- doc(cargo-package): explain no guarantee of vcs provenance (rust-lang/cargo#13984)
- chore: fix some comments (rust-lang/cargo#13982)
- feat: stabilize `cargo update --precise <yanked>` (rust-lang/cargo#13974)
- Update openssl-src to 111.28.2+1.1.1w (rust-lang/cargo#13976)

r? ghost
@rustbot rustbot added this to the 1.80.0 milestone Jun 1, 2024
bors added a commit that referenced this pull request Jun 3, 2024
[beta-1.79] fix(config): Ensure `--config net.git-fetch-with-cli=true` is respected

Beta backports
- #13992

In order to make CI pass, the following PRs are also cherry-picked:
- #13964
bors added a commit that referenced this pull request Jun 3, 2024
refactor(source): Split `RecursivePathSource` out of `PathSource`

### What does this PR try to resolve?

`PathSource` serves a couple of roles
- When a dependency / patch uses `path` (non-recursive)
- As the implementation details of a `git` source (recursive)
- Dependency overrides (recursive)

Instead of using a `PathSource::new` vs `PathSouce::new_recursive`, this does `RecursivePathSource::new`.  This makes the intent a lot clearer and makes it easier to customize the behavior to each role that is played.

Specifically, there are two ways I expect to leverage this refactor
- Improve the interplay between `RecursivePathSource` and `read_packages` to reduce the duplicate package warnings for git sources (#13992)
- cargo script will change the semantics of path sources slightly and I'm assuming having a distinct source will make this easier

### How should we test and review this PR?

### Additional information
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-configuration Area: cargo config files and env vars 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.

cargo update --config net.git-fetch-with-cli=true fetches using git2
4 participants