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.

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

Make Linux package universal. #15109

Merged
merged 20 commits into from
Apr 10, 2021
Merged

Conversation

TravisEz13
Copy link
Member

@TravisEz13 TravisEz13 commented Mar 26, 2021

PR Summary

Make Linux package universal.

  • Add packaging build for Linux
  • Consolidate to two universal packages one for RedHat and one for deb
  • Use post install and removal scripts to deal with symbolic links

NON-GOAL: Improve libmi support.

PR Context

PR Checklist

@ghost ghost assigned iSazonov Mar 26, 2021
.vsts-ci/linux/templates/packaging.yml Outdated Show resolved Hide resolved
.vsts-ci/linux.yml Outdated Show resolved Hide resolved
tools/packaging/packaging.psm1 Outdated Show resolved Hide resolved
tools/packaging/packaging.strings.psd1 Outdated Show resolved Hide resolved
@ghost ghost added Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept and removed Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept labels Mar 26, 2021
@TravisEz13 TravisEz13 added CL-BreakingChange Indicates that a PR should be marked as a breaking change in the Change Log CL-BuildPackaging Indicates that a PR should be marked as a build or packaging change in the Change Log labels Mar 27, 2021
@TravisEz13 TravisEz13 marked this pull request as ready for review March 30, 2021 22:14
@TravisEz13 TravisEz13 marked this pull request as draft March 30, 2021 22:14
@ghost ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Apr 3, 2021
@TravisEz13 TravisEz13 removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Apr 5, 2021
@TravisEz13 TravisEz13 marked this pull request as ready for review April 7, 2021 16:33
@TravisEz13 TravisEz13 merged commit 49288e4 into PowerShell:master Apr 10, 2021
@TravisEz13 TravisEz13 deleted the unified-deb-pkg branch April 10, 2021 16:53
@ghost
Copy link

ghost commented Apr 14, 2021

🎉v7.2.0-preview.5 has been released which incorporates this pull request.:tada:

Handy links:

@iSazonov iSazonov added this to the 7.2.0-preview.5 milestone Apr 15, 2021
TravisEz13 added a commit that referenced this pull request Apr 16, 2021
[7.2.0-preview.5] - 2021-04-14

* Breaking Changes

- Make PowerShell Linux deb and RPM packages universal (#15109)
- Enforce AppLocker Deny configuration before Execution Policy Bypass configuration (#15035)
- Disallow mixed dash and slash in command line parameter prefix (#15142) (Thanks @davidBar-On!)

* Experimental Features

- `PSNativeCommandArgumentPassing`: Use `ArgumentList` for native executable invocation (breaking change) (#14692)

* Engine Updates and Fixes

- Add `IArgumentCompleterFactory` for parameterized `ArgumentCompleters` (#12605) (Thanks @powercode!)

* General Cmdlet Updates and Fixes

- Fix SSH remoting connection never finishing with misconfigured endpoint (#15175)
- Respect `TERM` and `NO_COLOR` environment variables for `$PSStyle` rendering (#14969)
- Use `ProgressView.Classic` when Virtual Terminal is not supported (#15048)
- Fix `Get-Counter` issue with `-Computer` parameter (#15166) (Thanks @krishnayalavarthi!)
- Fix redundant iteration while splitting lines (#14851) (Thanks @hez2010!)
- Enhance `Remove-Item -Recurse` to work with OneDrive (#14902) (Thanks @iSazonov!)
- Change minimum depth to 0 for `ConvertTo-Json` (#14830) (Thanks @kvprasoon!)
- Allow `Set-Clipboard` to accept empty string (#14579)
- Turn on and off `DECCKM` to modify keyboard mode for Unix native commands to work correctly (#14943)
- Fall back to `CopyAndDelete()` when `MoveTo()` fails due to an `IOException` (#15077)

* Code Cleanup

<details>

<summary>

<p>We thank the following contributors!</p>
<p>@xtqqczze, @iSazonov, @ZhiZe-ZG</p>

</summary>

<ul>
<li>Update .NET to <code>6.0.0-preview.3</code> (#15221)</li>
<li>Add space before comma to hosting test to fix error reported by <code>SA1001</code> (#15224)</li>
<li>Add <code>SecureStringHelper.FromPlainTextString</code> helper method for efficient secure string creation (#14124) (Thanks @xtqqczze!)</li>
<li>Use static lambda keyword (#15154) (Thanks @iSazonov!)</li>
<li>Remove unnecessary <code>Array</code> -&gt; <code>List</code> -&gt; <code>Array</code> conversion in <code>ProcessBaseCommand.AllProcesses</code> (#15052) (Thanks @xtqqczze!)</li>
<li>Standardize grammar comments in Parser.cs (#15114) (Thanks @ZhiZe-ZG!)</li>
<li>Enable <code>SA1001</code>: Commas should be spaced correctly (#14171) (Thanks @xtqqczze!)</li>
<li>Refactor <code>MultipleServiceCommandBase.AllServices</code> (#15053) (Thanks @xtqqczze!)</li>
</ul>

</details>

* Tools

- Use Unix line endings for shell scripts (#15180) (Thanks @xtqqczze!)

* Tests

- Add the missing tag in Host Utilities tests (#14983)
- Update `copy-props` version in `package.json` (#15124)

* Build and Packaging Improvements

<details>

<summary>

<p>We thank the following contributors!</p>
<p>@JustinGrote</p>

</summary>

<ul>
<li>Fix <code>yarn-lock</code> for <code>copy-props</code> (#15225)</li>
<li>Make package validation regex accept universal Linux packages (#15226)</li>
<li>Bump NJsonSchema from 10.4.0 to 10.4.1 (#15190)</li>
<li>Make MSI and EXE signing always copy to fix daily build (#15191)</li>
<li>Sign internals of EXE package so that it works correctly when signed (#15132)</li>
<li>Bump Microsoft.NET.Test.Sdk from 16.9.1 to 16.9.4 (#15141)</li>
<li>Update daily release tag format to  work with new Microsoft Update work (#15164)</li>
<li>Feature: Add Ubuntu 20.04 Support to install-powershell.sh (#15095) (Thanks @JustinGrote!)</li>
<li>Treat rebuild branches like release branches (#15099)</li>
<li>Update WiX to 3.11.2 (#15097)</li>
<li>Bump NJsonSchema from 10.3.11 to 10.4.0 (#15092)</li>
<li>Allow patching of preview releases (#15074)</li>
<li>Bump Newtonsoft.Json from 12.0.3 to 13.0.1 (#15084, #15085)</li>
<li>Update the <code>minSize</code> build package filter to be explicit (#15055)</li>
<li>Bump NJsonSchema from 10.3.10 to 10.3.11 (#14965)</li>
</ul>

</details>

* Documentation and Help Content

- Merge `7.2.0-preview.4` changes to master (#15056)
- Update `README` and `metadata.json` (#15046)
- Fix broken links for `dotnet` CLI (#14937)

[7.2.0-preview.5]: v7.2.0-preview.4...v7.2.0-preview.5
@DHowett
Copy link

DHowett commented Apr 18, 2021

As a package maintainer, I'd like to lodge my dissent with making files (or symlinks, or permission changes) in postinst scripts. They can't be properly removed, the package manager doesn't know about them, and generally they circumvent the OS's mechanism for handling packaged data. 😄

@iSazonov
Copy link
Collaborator

Perhaps @heaths has thoughts how improve this.

@TravisEz13
Copy link
Member Author

@DHowett Please open an issue. This objection needs to be reviewed. As these symlinks are for a feature that eventually will no longer work, my proposal would be to just remove the links, but that breaks that feature for everyone.

Another alternative, to maintain compatibility, is to make a second package which this package has as an optional dependency on.

@heaths
Copy link
Contributor

heaths commented Apr 19, 2021

Is this question in response to tools/packaging/packaging.strings.psd1? As for build scripts, I see no problems with that and changes should be isolated (i.e. don't mess with the host outside of the build directory), but if in relation to the aforementioned file I really don't know. All I can say is that I don't see symlinking other packages' files by scripts that don't own them. Seems wrong to mess with another system like that - like in the old days before SFC on Windows when third-parties would replace Windows' bits.

@TravisEz13
Copy link
Member Author

@heaths libmi, provided by another team, has a hard coded path to load the libraries from. That team has decided not to support PowerShell anymore. To keep things functioning (until it completely brakes), this is our work around. symlinks would be our solution, if through the package manager or the script. I'm honestly happy to pull the symlinks from the package all together.

I don't think this is the same as replacing windows bits. We are symlinking other packages INTO our folder, not the other way around.

@jborean93
Copy link
Collaborator

jborean93 commented Apr 20, 2021

provided by another team, has a hard coded path to load the libraries from.

That's not true, the "universal" Linux build they have provided you are linked to the following OpenSSL libs

  • libssl.so.1.0.0
  • libcrypto.so.1.0.0

These are not absolute paths but library names that the loader checks in the various library paths on the host. You can see that in the following snippet, note I don't have OpenSSL 1.0.x installed and deleted the symlinks to show you what actual linked name so not found is expected.

[root@713b4cbcd41d /]# ldd /opt/microsoft/powershell/7/libmi.so
ldd: warning: you do not have execution permission for `/opt/microsoft/powershell/7/libmi.so'
        linux-vdso.so.1 (0x00007ffeb393a000)
        libpthread.so.0 => /lib64/libpthread.so.0 (0x00007fb157362000)
        libdl.so.2 => /lib64/libdl.so.2 (0x00007fb15715e000)
        libpam.so.0 => /lib64/libpam.so.0 (0x00007fb156f4e000)
        libssl.so.1.0.0 => not found
        libcrypto.so.1.0.0 => not found
        libc.so.6 => /lib64/libc.so.6 (0x00007fb156b8b000)
        /lib64/ld-linux-x86-64.so.2 (0x00007fb157582000)
        libaudit.so.1 => /lib64/libaudit.so.1 (0x00007fb156961000)
        libcap-ng.so.0 => /lib64/libcap-ng.so.0 (0x00007fb15675b000)

Typically these libraries are just picked up in the normal load path like /usr/lib, or the one specified by LD_LIBRARY_PATH. The libmi.so binary also has an RPATH set to $ORIGIN which allows it to also check the origin path (same dir it is placed in) which is what allows you to use the symlinks

[root@2cd102a3a066 /]# objdump -x /opt/microsoft/powershell/7/libmi.so | grep RPATH
  RPATH                $ORIGIN

But with the exception of EL7, which I go into below, you don't need the symlinks at all as lib[crypto|ssl].so.1.0.0 is in the normal system paths for distributions that ship with OpenSSL 1.0.x. On Ubuntu 16.04 I deleted the symlinks and you can see that it picked up the system locations just fine.

root@9d42597878f5:/# ldd /opt/microsoft/powershell/7/libmi.so
        linux-vdso.so.1 =>  (0x00007ffc80db9000)
        libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007f0741cf0000)
        libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007f0741aec000)
        libpam.so.0 => /lib/x86_64-linux-gnu/libpam.so.0 (0x00007f07418de000)
        libssl.so.1.0.0 => /lib/x86_64-linux-gnu/libssl.so.1.0.0 (0x00007f0741676000)
        libcrypto.so.1.0.0 => /lib/x86_64-linux-gnu/libcrypto.so.1.0.0 (0x00007f0741231000)
        libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f0740e67000)
        /lib64/ld-linux-x86-64.so.2 (0x00007f0741f0d000)
        libaudit.so.1 => /lib/x86_64-linux-gnu/libaudit.so.1 (0x00007f0740c40000)

In a normal situation you should just be picking up these libraries as they are installed, this actually works just fine if you have OpenSSL 1.0.x installed with only 2 caveats

  • Enterprise Linux 7 (CentOS 7, RHEL 7, etc) uses .10 as the suffix for the OpenSSL libs in /usr/lib64 so they won't be picked up
    • This is one case where you probably need the symlinks or to patch the ELF headers to point to the distro specific path for that distribution only
    • Other distributions that still package OpenSSL 1.0.x use the lib[ssl|crypto].so.1.0.0 file so they just work
  • macOS does have a hardcoded path to /usr/local/opt/openssl/lib/lib[ssl|crypto].1.0.0.dylib
    • This is a different distribution package and IIRC it doesn't contain symlinks so I consider it a separate problem

The other issue is that the PowerShell package for newer distributions have a symlink to point lib[ssl|crypto].1.0.0 to the OpenSSL 1.1.x paths which is completely wrong. This causes a seg fault in the process when libmi tries to use any OpenSSL APIs.

So ultimately what I think should happen is

  • Patch the libmi.so library to change the suffix of libssl and libcrypto to .10 for the RPM/Enterprise Linux package
  • Do nothing, no symlinks on the other Linux distributions

I also think you should be consuming the newer releases from the omi repo where that team has actually provided an official "universal" OpenSSL 1.1.x build which you can use on newer distributions that don't ship OpenSSL 1.0.x (read mostly all of them). It also means that patched libmi.so library is only shipped on EL7, everything else shouldn't require symlinks.

macOS is another problem and there's not much you can do there without recompling your own version of omi on OpenSSL 1.1.x which is understandable why you don't want to do.

@TravisEz13
Copy link
Member Author

@jborean93 This issue is not to improve libmi behavior. Please open a new issue for making libmi behavior better.

As for resolving this, I'll open a new issue for resolving creating the symlinks in the postscript. Thinking about it, I think the best approach is separate packages with the symlink.

@jborean93
Copy link
Collaborator

jborean93 commented Apr 20, 2021

I would love to resolve the libmi but I know the team doesn’t want to look into it. My posts are talking about the issues with how the current libmi is packaged. The package has symlinks which is not needed all the time (only EL7). The package symlinks point to OpenSSL 1.1.x which causes seg faults in PowerShell whenever you try to use any of the TLS components in libmi. How is that not related to the packaging components and thus this PR.

there are comments asking why things are done the way they are, I’m explaining the problem and why the existing packaging setup is wrong and giving possible fixes for it. I can’t really raise the PR myself because there are a lot of internal moving parts in the packaging process so my comments are a way of trying to stress what is wrong and hopefully illustrate what could be done to solve it.

@daxian-dbw
Copy link
Member

@jborean93 How about making the PSWSMan module the only source of libmi and libpsrpclient binaries for PowerShell remoting over WSMan on Linux and macOS? Namely, powershell will not ship those libs, but instead, tell users to leverage the PSWSMan module to enable WSMan remoting on Linux and macOS.

Please see the discussion topic #15310, and let's discuss the feasibility there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CL-BreakingChange Indicates that a PR should be marked as a breaking change in the Change Log CL-BuildPackaging Indicates that a PR should be marked as a build or packaging change in the Change Log
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants