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

Rewrite new style wow64 support #2191

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

vitlav
Copy link
Contributor

@vitlav vitlav commented Feb 20, 2024

This rewrite done in continue to #2030 and #2143 and pretent to be better improvement than #2189.

The first commit removed wine binary arch detection (these binaries can be wrapper scripts) and add new style wow64 detection by missed wine64 command.
The second commit fix environment expand macro to get correct %ProgramFiles% path.
And w_try_regsvr is renamed to w_try_regsvr32 to be more consisency with other 32/64 functions. Now some code is already using w_try_regsvr32 and this code is broken without this commit.

src/winetricks Outdated
test -x "${WINE64}" || WINE64="${WINE}"
fi

# if we can't detect wine64 command, it is the new wow64 mode
Copy link
Contributor

Choose a reason for hiding this comment

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

The wine-wow64 package on Arch AUR creates a wine64 -> wine symlink, so this doesn't work in that situation:
https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=wine-wow64#n100

No idea if other platforms do this, though. I imagine they might do it for backwards compatibility.

Maybe we could check if WINE64 is a symlink to WINE?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The wine-wow64 package on Arch AUR creates a wine64 -> wine symlink, so this doesn't work in that situation: https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=wine-wow64#n100

No idea if other platforms do this, though. I imagine they might do it for backwards compatibility.

Maybe we could check if WINE64 is a symlink to WINE?

I would prefer they remove wine64 link, but we can add workaround too (check if WINE64 is a symlink to WINE).

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure how common this scenario is. Most distros don't seem to ship the new WoW64 yet (as in, the --enable-archs=x86_64,i386 is set in the build script), so I have no idea how common this symlink is. Fedora, Ubuntu and Arch don't seem to ship the new WoW 64 build.

Copy link
Collaborator

Choose a reason for hiding this comment

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

On Gentoo Linux, we allow for multiple Wine versions to be installed at the same time, including different flavours (Vanilla, Staging, and Proton for example), so in general 'wine' will be a symlink to '/etc/eselect/wine/bin/wine' which will be a link to the currently selected 'main' provider.

We also have a USE-flag to toggle the new style 'wow64' build, which is not handled differently with regards to the links, and as such, this change will not help Winetricks to detect either.

May be tricksy to catch them all. : ]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This check needed only for text message. I can just remove it. Our near future is new WoW64 in any case.

Choose a reason for hiding this comment

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

I agree with @Chiitoo. We should not remove the symlink, some applications in the past appeared to be broken without it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, I have added a patch to remove wine64 checking and a bit obsoleted warning message too.

SurenIV pushed a commit to SurenIV/winetricks that referenced this pull request Feb 27, 2024
8e00767fb441c90240129b40798aecb1941514d1 (in Wine) moves the output of
uninstaller from stdout to stderr.

See https://bugs.winehq.org/show_bug.cgi?id=56010 for more details.

Closes Winetricks#2191
@vitlav
Copy link
Contributor Author

vitlav commented Mar 22, 2024

I've been testing these changes for a month now, and everything is working. Can someone else report the results of their testing?

@jre-wine
Copy link
Contributor

jre-wine commented Mar 24, 2024

I tested it on Debian testing (wine 9.0~repack-4, old style WoW64 in /usr/lib/wine/, with /usr/bin/wine being a wrapper script and /usr/bin/wine64 not existing):

32-bit and 64-bit prefixes are detected correctly.

Your implementation removed the distinction between classic and new style WoW64. I don't know how important that is for debugging, but I prefer not knowing this over a non-working Winetricks.

Unfortunately the Debian-specific missing /usr/bin/wine64 still leads to
warning: wine cmd.exe /c echo '%AppData%' returned empty string, error message ""

So to get a functional Winetricks on current Debian I need either

@vitlav
Copy link
Contributor Author

vitlav commented Apr 6, 2024

Any suggestions?

@austin987
Copy link
Contributor

I don't think this is the right approach, while its simpler, it doesn't allow us to detect the different wine setups.

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.

6 participants