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

cargo update --config net.git-fetch-with-cli=true fetches using git2 #13991

Closed
epage opened this issue May 31, 2024 · 1 comment · Fixed by #13992
Closed

cargo update --config net.git-fetch-with-cli=true fetches using git2 #13991

epage opened this issue May 31, 2024 · 1 comment · Fixed by #13992
Labels
A-config-cli Area: --config CLI option C-bug Category: bug regression-from-stable-to-stable Regression in stable that worked in a previous stable release. S-triage Status: This issue is waiting on initial triage.

Comments

@epage
Copy link
Contributor

epage commented May 31, 2024

Problem

As reported in renovatebot/renovate#29277

  • Private git repo dependency fetching worked on 1.77 but not 1.78
  • They used net.git-fetch-with-cli=true on the command-line

I git bisected it to #13479, specifically f525e1f

Steps

$ git clone https://github.com/crate-ci/cargo-release.git
$ cd cargo-release
$ rm -rf ~/.cargo/git  # avoiding the github fast path
$ cargo update --config net.git-fetch-with-cli=true

Notice that the fetch is done using libgit2 instead of git. To make this more obvious, I ran from master with a dbg! around the git_fetch_with_cli check at

cargo/src/cargo/sources/git/utils.rs

Line 1008 in d04355a

if let Some(true) = gctx.net_config()?.git_fetch_with_cli {

Possible Solution(s)

No response

Notes

No response

Version

No response
@epage epage added C-bug Category: bug A-config-cli Area: --config CLI option regression-from-stable-to-stable Regression in stable that worked in a previous stable release. S-triage Status: This issue is waiting on initial triage. labels May 31, 2024
epage added a commit to epage/cargo that referenced this issue May 31, 2024
@epage
Revert "fix(context): Configure Shell before emitting messages"
72c6004 
This reverts commit f525e1f.

This removes color support from `cargo -Zhelp`

Fixes rust-lang#13991
epage added a commit to epage/cargo that referenced this issue May 31, 2024
@epage
Revert "fix(context): Configure Shell before emitting messages"
a9aff29 
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.

I had looked into finding the right balance to not have any downsides
but ran out my time box and having a working config is a higher
priority.

Fixes rust-lang#13991
@epage epage mentioned this issue May 31, 2024
Merged
bors added a commit that referenced this issue May 31, 2024
@bors
Auto merge of #13992 - epage:config, r=weihanglo
7a6fad0 
fix(config): Ensure `--config net.git-fetch-with-cli=true` is respected

### 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.
@weihanglo
Copy link
Member

I wonder why nobody has reported this. Maybe --config is not a thing people know its existence 😅

@bors bors closed this as completed in 4ce2b61 May 31, 2024
epage added a commit to epage/cargo that referenced this issue Jun 1, 2024
@epage
Revert "fix(context): Configure Shell before emitting messages"
d9388d5 
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
epage added a commit to epage/cargo that referenced this issue Jun 3, 2024
@epage
Revert "fix(context): Configure Shell before emitting messages"
3764730 
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-config-cli Area: --config CLI option C-bug Category: bug regression-from-stable-to-stable Regression in stable that worked in a previous stable release. S-triage Status: This issue is waiting on initial triage.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(config): Ensure --config net.git-fetch-with-cli=true is respected epage/cargo
2 participants
@epage @weihanglo