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: enable socks proxy #8391

Open
wants to merge 2 commits into
base: 1.22-stable
Choose a base branch
from

Conversation

badeggg
Copy link

@badeggg badeggg commented Oct 14, 2020

Summary
This pull request makes yarn can be used behind a socks proxy. Hence fix #6525.

Notice

  • Since request library does not really handle socks proxy. I changed code to respect PROXY, HTTP_PROXY and HTTPS_PROXY environs in yarn. The reason for checking PROXY env is to keep consistent with npm
  • Logic to handle noproxy config between npm and yarn is different(not changing in this pull request)
  • I don't understand the text format of CHANGELOG.md, please help me to add a entry in the file

Test plan

# test file has following content

# step0 use changed yarn
yarn='/Users/zhaoxuxu/my_repos/yarn/bin/yarn'

# step1 make sure no proxy is configured
npm config delete proxy
npm config delete https-proxy
$yarn config delete proxy
$yarn config delete https-proxy
unset proxy PROXY  http_proxy HTTP_PROXY  https_proxy HTTPS_PROXY

# step2 set a env proxy
export proxy=socks5://127.0.01:1080 # or yarn config set proxy socks5://127.0.0.1:1080

# step3 make a request
mkdir testing
$yarn add lodash  # check your socks proxy server to validate that yarn is requesting via socks proxy

## repeat above steps except step2 should set proxy use different method
# output

zhaoxuxu:Desktop$  bash test 
yarn config v1.22.10
warning package.json: No license field
success Deleted "proxy".
✨  Done in 0.05s.
yarn config v1.22.10
warning package.json: No license field
success Deleted "https-proxy".
✨  Done in 0.06s.
yarn add v1.22.10
warning package.json: No license field
warning No license field
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...
[3/4] 🔗  Linking dependencies...
[4/4] 🔨  Building fresh packages...
warning No license field
success Saved 1 new dependency.
info Direct dependencies
└─ [email protected]
info All dependencies
└─ [email protected]
✨  Done in 8.18s.

@badeggg
Copy link
Author

badeggg commented Oct 14, 2020

And please help me to figure out the auto test failing if you think this pull request is rational.

@CaledoniaProject
Copy link

Why is it still not merged?

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.

2 participants