Fix extraction of msvcp140.dll in vcrun2022#2465
Fix extraction of msvcp140.dll in vcrun2022#2465gbdrbob wants to merge 2 commits intoWinetricks:masterfrom
Conversation
src/winetricks
Outdated
| w_try_cabextract --directory="${W_TMP}/win32" "${W_CACHE}"/"${W_PACKAGE}"/vc_redist.x86.exe -F 'a10' | ||
| w_try_cabextract --directory="${W_SYSTEM32_DLLS}" "${W_TMP}/win32/a10" -F 'msvcp140.dll' | ||
| w_try_cabextract --directory="${W_TMP}/win32" "${W_TMP}/win32/a10" -F 'msvcp140.dll_x86' | ||
| w_try mv "${W_TMP}/win32/msvcp140.dll_x86" "${W_SYSTEM32_DLLS}/msvcp140.dll" |
There was a problem hiding this comment.
Pretty sure this should be w_try mv -f since the dll would already exist.
There was a problem hiding this comment.
Not necessary? mv removes any exisiting destination file unless that file has a mode which restricts writing to it, and by default all the files in the wineprefix are user writable.
Setting -f would cause winetricks to silently overwrite a file that a user has deliberately made read-only. I'm fine with being overruled, but my instinct is not to do that.
See this stackexchange question for more details, or the POSIX specification for mv.
There was a problem hiding this comment.
I looked into this a little further:
The native vc_redist* installers from MS do silently overwrite read only files, so despite my reservations expressed above @Gcenx is correct, and I've updated this PR to use mv -f as winetricks should probably do the same as the native installers.
There was a problem hiding this comment.
Honestly this should be expanded as the dlls where marking as native,builtin won’t be installed now that wine-11.1 shows the same version.
currently only one dll is getting extracted but all of the dlls that now have version resources should be manually installed.
msvcp140.dll workaround to ensure vcrun2022 verb correctly installs native dlls even when the builtin version number is higher was broken due to multi-arch suffixes in the MS vc_redist cabinet archives. This commit is intended to fix that.
| w_try_cabextract --directory="${W_TMP}/win64" "${W_CACHE}"/"${W_PACKAGE}"/vc_redist.x64.exe -F 'a12' | ||
| w_try_cabextract --directory="${W_SYSTEM64_DLLS}" "${W_TMP}/win64/a12" -F 'msvcp140.dll' | ||
| w_try_cabextract --directory="${W_TMP}/win64" "${W_TMP}/win64/a12" -F 'msvcp140.dll_amd64' | ||
| w_try mv -f "${W_TMP}/win64/msvcp140.dll_amd64" "${W_SYSTEM64_DLLS}/msvcp140.dll" |
There was a problem hiding this comment.
Do you mind clarifying why you decided to only extract and move one dll from the cabinet file?
There was a problem hiding this comment.
When this was originally implemented only this dll needed to be extracted, now most wine dlls have version resources
As of wine-11.1 wines dlls will have the same version as the ones from vcrun
There was a problem hiding this comment.
Then wouldn't this change to the verb imply a dependency of wine 11.1 and that prefixes not created by wine 11.1 will be impacted if this were merged?
There was a problem hiding this comment.
There wasn’t a wine version required when this was originally added so no point adding one now.
It makes more sense to do this regardless of wine version.
This pull is intened to fix the bug described in issue #2464 - where msvcp140.dll is not extracted from vc_redist.x86.exe and vc_redist.x64.exe due to suffixes on the filenames in the archive.