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

Add powershell app based on PowerShell Core 6.1, #1440

Closed
wants to merge 4 commits into from

Conversation

theli-ua
Copy link

functionality enough to satisfy installers requiring powershell

For example this allows Magic Arena ( https://magic.wizards.com/en/mtgarena ) installer work in wine

Note: https://docs.microsoft.com/en-us/powershell/scripting/install/installing-powershell-core-on-windows?view=powershell-6 says WMF is a prerequisite but I have not found a way to install it. And installing it this way is at least enough for installer to work (thus warns).
It is also not clear what dotnet version it requires ... it works with 462 but probably any 45+ will work (not sure how to reflect such dependency)

@theli-ua theli-ua mentioned this pull request Nov 27, 2019
src/winetricks Outdated
Comment on lines 14989 to 14990
for file in *\\*; do target="${file//\\//}"; mkdir -p "${target%/*}"; mv -v "$file" "$target"; done
w_append_path "$W_SYSTEM32_DLLS"/WindowsPowerShell/v1.0
Copy link
Author

Choose a reason for hiding this comment

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

Reason for this is this is a non-compliant zip file that has back slashes. While unzip detects that and extracts archive reversing slashes it fails to successfully extract archive due to a different reasn (for w/e reason it creates one of directories without executable bit and then fails to extract files into it with permission denied)

@Kreyren
Copy link
Contributor

Kreyren commented Nov 27, 2019

So you don't need WMF to satisfy MTGA? o.O

@theli-ua
Copy link
Author

theli-ua commented Nov 29, 2019 via email

@theli-ua
Copy link
Author

theli-ua commented Dec 1, 2019

So you don't need WMF to satisfy MTGA? o.O

no, you don't

src/winetricks Outdated
{
if test "$W_ARCH" = "win64"; then
w_die "Only implemented for 32bit arch at the moment"
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use w_package_unsupported_win64 for that.

Copy link
Author

Choose a reason for hiding this comment

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

I can technically add win64 support if you would prefer that. I don't really see an issue adding it.
However if you would like that indeed - what would be the best example of handling separate x86/64 archives?

Copy link
Contributor

Choose a reason for hiding this comment

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

That would be preferable, but not required.

There are several examples, e.g., amstream:
https://github.com/Winetricks/winetricks/blob/master/src/winetricks#L5586

you'll want to unconditionally install win32 (as win64 always supports win32), then selectively install win64 if $W_ARCH = win64.

Copy link
Author

Choose a reason for hiding this comment

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

I am honestly not sure what is the right way to install this properly and integrate into system and how PATH is supposed to be set up in the right way if both 32 and 64 bit things are installed so I guess I'll just leave it as is here

src/winetricks Outdated

w_warn "Installing Powershell into $W_SYSTEM32_DLLS_WIN/WindowsPowerShell/v1.0/"
w_warn "So that apps that require psh scripts to be executed would find it"
w_warn "Note that some functionality may not work without Windows Management Framework"
Copy link
Contributor

Choose a reason for hiding this comment

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

This will give 3 popups in GUI mode, please combine it into one.

src/winetricks Outdated

w_call dotnet462

w_download https://github.com/PowerShell/PowerShell/releases/download/v6.1.6/PowerShell-6.1.6-win-x86.zip 1dc690c9a1091d0ed5b71de903ab3cd7b58ba9638acdfeb9f9fe7031abaf47e9 PowerShell-6.1.6-win-x86.zip
Copy link
Contributor

Choose a reason for hiding this comment

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

Since you're not changing the filename, there's no need to specify it as the last argument.

src/winetricks Outdated
w_try_7z "$W_SYSTEM32_DLLS"/WindowsPowerShell/v1.0/ "${W_CACHE}/${W_PACKAGE}/${file1}"
w_try_cd "$W_SYSTEM32_DLLS"/WindowsPowerShell/v1.0
w_try cp -f pwsh.exe powershell.exe
for file in *\\*; do target="${file//\\//}"; mkdir -p "${target%/*}"; mv -v "$file" "$target"; done
Copy link
Contributor

Choose a reason for hiding this comment

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

mkdir/mv should be behind w_try().

This also introduces a bashism, causing travis to fail.

@tannisroot
Copy link
Contributor

64-bit prefix support would be really nice! There's an app (MTGA) that needs powershell for the installation process but it seems to crash in 32-bit prefix for some reason.
And it's also very likely that other potentials apps that will depend on powershell are going to be 64-bit.
Also, perhaps powershell could be categorized not as an app but as a Windows component, since it is one?

@theli-ua
Copy link
Author

theli-ua commented Dec 6, 2019

64-bit prefix support would be really nice! There's an app (MTGA) that needs powershell for the installation process but it seems to crash in 32-bit prefix for some reason.

well, MTGA installer is actually what this PR has been tested with. There is no reason it should require 64bit prefix

@tannisroot
Copy link
Contributor

tannisroot commented Dec 7, 2019

There is no reason it should require 64bit prefix

oh, weird, you're right. My prefix must have gotten borked during testing.
In that case, I guess 64-bit support can be added whenever a new 64-bit app will need powershell.

@PietJankbal
Copy link
Contributor

Hi, i attached script that also installs 64 bit powershell here:
https://bugs.winehq.org/attachment.cgi?id=65974
Again , script has hard coded "Program Files" (not localized) because i dunno how to get around that issue. After installation powershell.exe can be run to start it

@tannisroot
Copy link
Contributor

@PietJankbal does this actually work? because it specified w_package_unsupported_win64, and I am pretty sure that won't allow you to install the verb into 64-bit prefix.

@tannisroot
Copy link
Contributor

I think I might have found a valid usecase for a 64-bit powershell verb.
I've discovered that MTGA doesn't seem to render properly with WineD3D on Ultra settings, but it works just fine with DXVK. However, to work correctly, DXVK requires you to launch 32-bit apps in forced Large Address Aware mode so that it doesn't run out of memory. However, that option doesn't work in 32-bit prefixes, only in 64-bit ones.
So it would be really lovely to add 64-bit support to avoid these issues that might occur with 32-bit prefixes.

@PietJankbal
Copy link
Contributor

@PietJankbal does this actually work? because it specified w_package_unsupported_win64, and I am pretty sure that won't allow you to install the verb into 64-bit prefix.

That is commented out, it should be reoved really

@tannisroot
Copy link
Contributor

tannisroot commented Dec 14, 2019

Oh, didn't notice it's commented out :D

Copy link
Contributor

@austin987 austin987 left a comment

Choose a reason for hiding this comment

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

A couple minor things, then this can go in as far as I'm concerned (unless you want to get 64-bit working before merging).


load_powershell()
{
w_package_unsupported_win64
Copy link
Contributor

Choose a reason for hiding this comment

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

Can be removed, as you said (proper 64-bit support can be a followup PR).

Copy link
Author

Choose a reason for hiding this comment

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

I wasn't planning on adding win64 support

w_override_dlls native powershell.exe

# remove builtin placeholders
rm -f "$W_SYSTEM32_DLLS"/powershell.exe
Copy link
Contributor

Choose a reason for hiding this comment

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

Prepend with w_try

# remove builtin placeholders
rm -f "$W_SYSTEM32_DLLS"/powershell.exe


Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove the extra newline.

@theli-ua
Copy link
Author

theli-ua commented Jan 7, 2020

A couple minor things, then this can go in as far as I'm concerned (unless you want to get 64-bit working before merging).

frankly, I'm not sure anymore this has much value. It does work after being installed but I'm not sure why anyone would use it. Original intent was to make Magic Arena installer work but as it happens just moving wine's stub to a proper location fixes that (in 5.0-rc4).

@PietJankbal
Copy link
Contributor

Hi,

A few remarks. As theli-ua already pointed out wine`s stub has moved so this line is not up to date any more:
rm -f "$W_SYSTEM32_DLLS"/powershell.exe

Anyway, i still think its useful to have powershell installed and (a bit of)-functional so heres my five cents (did also a bit of googling on this):

Powershell 6 and up are really kind of "add-ons" to Windows so those versions of powershell should be called by "wine pwsh.exe". The installers for powershell ver. 6 and up work fine if you put a fake pwrshlplugin.dll (with high enough version resource) in system32/wow64.
But the pwsh.exe`s should really remain in Program Files/Powershell and not be copied (or linked) to system32/WindowsPowerShell/v1.0/powershell.exe

The right way to go is i think install powershell2.0 (i have a script for this and it installs and with a bit of aftermath repair work because of bug 25740 it has at least minimal functionality like reporting version correctly) Version 2.0 is btw also the version that is reported on my win7 partition when you start powershell (see e.g. https://appuals.com/how-to-check-powershell-version-on-windows-10-8-and-7/)

Id first like to finish my internet explorer server_2003 script but after that id like to pick this up , ok?

Regards

@theli-ua
Copy link
Author

theli-ua commented Jan 7, 2020

@PietJankbal sure, feel free to pick this up

@PietJankbal
Copy link
Contributor

See https://raw.githubusercontent.com/PietJankbal/temporary_stuff/master/pwsh20.diff

I`ll do PR if my other Pull request are settled down

@tannisroot
Copy link
Contributor

@PietJankbal the last link is now broken as you removed your repository

@PietJankbal
Copy link
Contributor

@PietJankbal the last link is now broken as you removed your repository

Hi, i rearranged some files, it`s here: https://github.com/PietJankbal/storage/blob/master/pwsh20.diff
I stopped trying to get things into winetricks (too frustating...) if anyone would like to pick this up please go ahead

@allentiak
Copy link

Hi, @austin987, @theli-ua

I could take over here. Should I copy this branch into a new PR? Or I can someone assign this one to me?

@austin987
Copy link
Contributor

@allentiak I'm not sure if GH will let you push over another user's branch (it tells me I can, but I'm the maintainer).

I'd suggest making your own PR.

@allentiak
Copy link

Then, I'll make a new PR out of this one. Give me a few days.

@austin987 austin987 closed this May 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants