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
Add symbolic links for libssl and libcrypto to Debian 9 builds. #7609
Conversation
…both build and packaging.
…both build and packaging.
build.psm1
Outdated
# add two symbolic links to system shared libraries that libmi.so is dependent on to handle | ||
# platform specific changes. This appears to be a change in Debian 9; Debian 8 did not need these | ||
# symlinks. | ||
if (!(Test-Path -Path "$publishPath/libssl.so.1.0.0")) |
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.
The code for this is very similar to the code for RedHatFamily. Can we combine logic like this:
if($Environment.IsRedHatFamily -or $Environment.IsDebian9){
if($Environment.IsRedHatFamily) {
$sslTarget = "/lib64/libssl.so.10"
$cryptoTarget = "/lib64/libcrypto.so.10"
} elseif ($Enviroment.IsDebian9) {
…..
}
if(-not (Test-Path …) {
}
if(-not (Test-Path …) {
}
}
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.
fixed
build.psm1
Outdated
@@ -610,6 +610,22 @@ Fix steps: | |||
} | |||
} | |||
|
|||
if ($Environment.IsDebian9) | |||
{ |
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.
Please follow the pattern of {
being on the same line as if
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.
fixed
tools/packaging/packaging.psm1
Outdated
@@ -1102,6 +1102,15 @@ function New-AfterScripts | |||
$AfterRemoveScript = [io.path]::GetTempFileName() | |||
$packagingStrings.UbuntuAfterInstallScript -f "$Link" | Out-File -FilePath $AfterInstallScript -Encoding ascii | |||
$packagingStrings.UbuntuAfterRemoveScript -f "$Link" | Out-File -FilePath $AfterRemoveScript -Encoding ascii | |||
|
|||
if ($Environment.IsDebian9) |
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.
Please follow pattern for having {
to be on the same line as if
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.
fixed
if ($Environment.IsDebian9){ | ||
# NOTE: Debian 8 doesn't need these symlinks | ||
$sslTarget = "/usr/lib/x86_64-linux-gnu/libssl.so.1.0.2" | ||
$cryptoTarget = "/usr/lib/x86_64-linux-gnu/libcrypto.so.1.0.2" |
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.
I wonder that it doesn't work in CoreFX https://github.com/janvorli/corefx/blob/13e2cc977713f0a5edc7a702583eeabb5643006d/src/Native/Unix/System.Security.Cryptography.Native/opensslshim.cpp#L43 (from dotnet/corefx#27208)
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.
I suspect it's because they are loading explicitly by version; libmi doesn't have version detection logic.
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.
@adityapatwardhan: I've addressed your feedback.
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.
Do you mean that a root problem in libmi? if so it seems better to make a correction there than to receive surprises with each new linux distributive.
@adityapatwardhan can you update your review? |
PR Summary
On Debian 9, libmi cannot resolve libssl and libcrypto. This change adds symbolic links to packaging and build to the $PSHOME directory to resolve the issue.
The fix was verified interactively connecting to office 365, importing the session, and ensuring Get-MailBox and Get-User succeed.
Fix #7598
PR Checklist
.h
,.cpp
,.cs
,.ps1
and.psm1
files have the correct copyright headerWIP:
to the beginning of the title and remove the prefix when the PR is ready.[feature]
if the change is significant or affects feature tests