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

(#3181) Let NuGet.Client get the system proxy if none is explicitly set. #3197

Merged
merged 5 commits into from
Jun 14, 2023

Conversation

corbob
Copy link
Member

@corbob corbob commented Jun 5, 2023

Description Of Changes

Remove the registry checking that we were doing for proxy settings. Override the NuGet.Client proxy with a null proxy so that it can gather the system proxy information instead.

Motivation and Context

Due to a commit where we were overriding the NuGet.Client proxy to prevent it loading the NuGet configuration file's proxy settings, we were inadvertently ignoring system proxy settings.

Testing

  1. Followed the testing steps listed in the NuGet.Client PR.
  2. Built locally and ran a local test kitchen against the system proxy settings test.

Operating Systems Testing

  • Windows 10
  • Windows Server 2019

Change Types Made

  • Bug fix (non-breaking change).
  • Feature / Enhancement (non-breaking change).
  • Breaking change (fix or feature that could cause existing functionality to change).
  • Documentation changes.
  • PowerShell code changes.

Change Checklist

  • Requires a change to the documentation.
  • Documentation has been updated.
  • Tests to cover my changes, have been added.
  • All new and existing tests passed?
  • PowerShell code changes: PowerShell v2 compatibility checked?

Related Issue

@corbob corbob changed the base branch from develop to master June 7, 2023 16:59
@corbob corbob changed the base branch from master to develop June 7, 2023 16:59
@corbob corbob force-pushed the proxies branch 4 times, most recently from ea043da to 987f3f0 Compare June 9, 2023 21:04
corbob added 3 commits June 13, 2023 08:39
We were checking the registry for proxy settings. Instead of checking
here and ignoring other settings, we can (and should) let NuGet.Client
poll the System proxy configuration. This commit removes the checking in
the registry for these settings as they're no longer necessary.
Previously we passed a new WebProxy to override the NuGet.Client proxy.
This commit changes that to pass in a null so that NuGet.Client can know
to check the system proxy settings instead.
Mark these tests as FossOnly for now as there is some differences with
licensed extension that we need to resolve.
@corbob corbob marked this pull request as ready for review June 13, 2023 16:01
@corbob corbob requested review from AdmiringWorm and gep13 June 13, 2023 16:01
These tests now fail because we're not reading the configuration to get
the information, and so it's not in the config object that these tests
can see. The system proxy is still tested successfully through it's own
Test Kitchen runs that configure the environment for having a proxy.
Copy link
Member

@gep13 gep13 left a comment

Choose a reason for hiding this comment

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

LGTM!

@gep13 gep13 merged commit bdba4f7 into chocolatey:develop Jun 14, 2023
@gep13
Copy link
Member

gep13 commented Jun 14, 2023

@corbob thank you very much for getting this fixed up!

@corbob corbob deleted the proxies branch June 14, 2023 11:26
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

Successfully merging this pull request may close these issues.

System proxy settings (automatically detect settings) are no longer used in v2.0.0
2 participants