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

Make Travis CI use libcurl+openssl+gssapi for macOS #5629

Merged
merged 4 commits into from Dec 6, 2017
Merged

Make Travis CI use libcurl+openssl+gssapi for macOS #5629

merged 4 commits into from Dec 6, 2017

Conversation

markekraus
Copy link
Contributor

@markekraus markekraus commented Dec 5, 2017 •
edited

Closes #5590

  • makes Travis CI use the brew installed libcurl that uses OpenSSL for the crypto provider and include the gssapi option. The native libcurl provides inconsistent feature support across OS versions.
  • re-enable tests marked pending in Make the '-SslProtocol' tests pending #5605
  • GSSAPI is required for PowerShellGet in order to support HttpClientHandler.UseDefaultCredentials which it sets as true (unless you supply credentials).

PR Checklist

Note: Please mark anything not applicable to this PR NA.

PR Summary

@markekraus markekraus added the Documentation Needed in this repo Documentation is needed in this repo label Dec 5, 2017
@markekraus markekraus changed the title Make Travis CI use libcurl+openssl Make Travis CI use libcurl+openssl+gssapi Dec 5, 2017
@markekraus
Copy link
Contributor Author

yay.. all passed this time.

So.. the question is where do I document this at? in the web cmdlets documentations? somewhere in this repo (install documentation)? both?

@daxian-dbw
Copy link
Member

daxian-dbw commented Dec 5, 2017 •
edited

We need to document it in the web cmdlets help content, and point out what parameters are affected by this. docs/FAQ.md might also be a good place to keep this information.

TravisEz13
TravisEz13 approved these changes Dec 6, 2017
View reviewed changes
Copy link
Member

@TravisEz13 TravisEz13 left a comment

Choose a reason for hiding this comment

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

One comment about another issue this PR brings up, Please file an issue or submit a PR for this and link in this PR.

build.psm1
@@ -1533,7 +1533,7 @@ function Start-PSBootstrap {
Start-NativeExecution { brew install $Deps } -IgnoreExitcode

# Install patched version of curl
Start-NativeExecution { brew install curl --with-openssl } -IgnoreExitcode
Start-NativeExecution { brew install curl --with-openssl --with-gssapi } -IgnoreExitcode
Copy link
Member

Choose a reason for hiding this comment

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

We should update the PowerShell brew recipe as well.
https://github.com/caskroom/homebrew-cask/blob/master/Casks/powershell.rb#L33

Copy link
Member

Choose a reason for hiding this comment

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

We need to figure out what to do with the powershell package about this limitation on macOS.
CoreCLR has explicitly moved away from OpenSSL on mac, see this PR: dotnet/corefx#17011.
Please open an issue to track the macOS packaging about this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

opened Homebrew/homebrew-cask#41673

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#5635

CHANGELOG.md Outdated
@@ -5,6 +5,8 @@
- Update the contribution guideline to note that updating the changelog is required. (#5586)
- Remove Pester as a module include with the PowerShell Packages.
In the future, you should be able to add it by running `Install-Module Pester`. (#5623, #5631)
- Make Travis CI use `libcurl+openssl` (#5629, @markekraus)
- Make Travis CI use `libcurl+openssl+gssapi` (#5629, @markekraus)
Copy link
Member

Choose a reason for hiding this comment

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

should only have one line for a single PR

@daxian-dbw daxian-dbw self-assigned this Dec 6, 2017
This was referenced Dec 6, 2017
Merged
Closed
@daxian-dbw
Copy link
Member

I didn't add the [feature] tag when resolving a conflict. Add the [feature] to trigger full test run.

@daxian-dbw daxian-dbw merged commit ee7fbed into PowerShell:master Dec 6, 2017
@daxian-dbw daxian-dbw changed the title Make Travis CI use libcurl+openssl+gssapi Make Travis CI use libcurl+openssl+gssapi for macOS Dec 6, 2017
TravisEz13 pushed a commit to TravisEz13/PowerShell that referenced this pull request Dec 7, 2017
@markekraus @TravisEz13
Make Travis CI use libcurl+openssl+gssapi for macOS (PowerShell#5629)
171ee52
@TravisEz13 TravisEz13 mentioned this pull request Dec 7, 2017
Merged
TravisEz13 pushed a commit that referenced this pull request Dec 7, 2017
@markekraus @TravisEz13
Make Travis CI use libcurl+openssl+gssapi for macOS (#5629)
721d3e9
@TravisEz13 TravisEz13 added this to the 6.0.0-RC.2 milestone Dec 8, 2017
@markekraus markekraus deleted the macOsLibcurlFix branch January 19, 2018 19:00
@joeyaiello joeyaiello mentioned this pull request Oct 15, 2018
Closed
@joeyaiello joeyaiello removed the Documentation Needed in this repo Documentation is needed in this repo label Oct 15, 2018
@joeyaiello joeyaiello removed the Documentation Needed in this repo Documentation is needed in this repo label Oct 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daxian-dbw daxian-dbw daxian-dbw approved these changes

@TravisEz13 TravisEz13 TravisEz13 approved these changes

@adityapatwardhan adityapatwardhan Awaiting requested review from adityapatwardhan adityapatwardhan is a code owner

Assignees

@daxian-dbw daxian-dbw

Labels
None yet
Projects
None yet
Milestone
6.0.0-RC.2
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@markekraus @daxian-dbw @TravisEz13 @joeyaiello