-
Notifications
You must be signed in to change notification settings - Fork 478
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
Use install-dotnet scripts for installation #71
Conversation
Tests should not concern internal workings of install-dotnet as those are covered by the appropriate repo and team.
// Fallback to \n if initial split doesn't work (not consistent across versions) | ||
if (lines.length === 1) { | ||
lines = output.split('\n'); | ||
let scriptArguments: string[] = []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to pass proxy settings into the script when being called
src/installer.ts
Outdated
.replace(/'/g, "''"); | ||
let command = `& '${escapedScript}'`; | ||
if (this.version) { | ||
command += ` -Version ${this.version}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to pass in proxy settings
Any updates on this? |
@RehanSaeed I’ve been using for a while now. I know it’s not the best solution but you’re able to use this by referencing the branch name whole this gets merged.
|
The install scripts are incompatible with proxy settings added recently. The product team has also pushed back on autoupgrade scenarios being added. Addressing #25 will need to be done separately. |
Required for actions/setup-dotnet#71. Agents can set proxy settings for their runs which include addresses which should not go through the proxy. This script doesn't allow that value to be specified for windows. In the shell script this is handled by wget and curl both honoring the environment variables https_proxy and no_proxy.
6190d3b
to
b7f3cf0
Compare
Is this now released? |
@RehanSaeed Looks like a new PR was created for side by side installs #124 |
There were a lot of changes since this PR, so I created a clean PR |
Fixes sxs issues (#25), caching, allows using global.json (#14), better error messages (#54)
Default install location is used rather than the toolkit's cache, this enables side by side installs to work properly.