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

@markekrausmarkekraus 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

@markekrausmarkekraus added the Documentation Needed in this repo Documentation is needed in this repo label Dec 5, 2017
@markekrausmarkekraus changed the title Make Travis CI use libcurl+opensslMake Travis CI use libcurl+openssl+gssapiDec 5, 2017
@markekraus
Copy link
ContributorAuthor

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

@TravisEz13TravisEz13 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

@TravisEz13TravisEz13Dec 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-dbwdaxian-dbwDec 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
ContributorAuthor

@markekrausmarkekrausDec 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
ContributorAuthor

@markekrausmarkekrausDec 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

@TravisEz13TravisEz13Dec 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-dbwdaxian-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-dbwdaxian-dbw merged commit ee7fbed into PowerShell:masterDec 6, 2017
@daxian-dbwdaxian-dbw changed the title Make Travis CI use libcurl+openssl+gssapiMake Travis CI use libcurl+openssl+gssapi for macOSDec 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
@TravisEz13TravisEz13 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
@TravisEz13TravisEz13 added this to the 6.0.0-RC.2 milestone Dec 8, 2017
@markekrausmarkekraus deleted the macOsLibcurlFix branch January 19, 2018 19:00
@joeyaiellojoeyaiello mentioned this pull request Oct 15, 2018
Closed
@joeyaiellojoeyaiello removed the Documentation Needed in this repo Documentation is needed in this repo label Oct 15, 2018
@joeyaiellojoeyaiello removed the Documentation Needed in this repo Documentation is needed in this repo label Oct 15, 2018
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@daxian-dbwdaxian-dbwdaxian-dbw approved these changes

@TravisEz13TravisEz13TravisEz13 approved these changes

@adityapatwardhanadityapatwardhanAwaiting requested review from adityapatwardhan

Assignees

@daxian-dbwdaxian-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