Skip to content

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

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
Merged
daxian-dbw merged 4 commits into from
Dec 6, 2017
Merged

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

daxian-dbw merged 4 commits into from
Dec 6, 2017

Conversation

@markekraus
Copy link
Contributor

@markekraus markekraus commented Dec 5, 2017 •
edited
Loading

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

markekraus commented Dec 5, 2017

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
Loading

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

# 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

@TravisEz13 TravisEz13 Dec 6, 2017

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

@daxian-dbw daxian-dbw Dec 6, 2017

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

@markekraus markekraus Dec 6, 2017

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

@markekraus markekraus Dec 6, 2017

Choose a reason for hiding this comment

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

#5635

CHANGELOG.md
- 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

@TravisEz13 TravisEz13 Dec 6, 2017

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

daxian-dbw commented Dec 6, 2017

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

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.

4 participants

@markekraus @daxian-dbw @TravisEz13 @joeyaiello