View Issue Details

IDProjectCategoryView StatusLast Update
0004595The Dark ModTDM Updaterpublic16.11.2019 05:22
Reporterstgatilov Assigned Tostgatilov  
PrioritynormalSeveritynormalReproducibilityN/A
Status resolvedResolutionfixed 
Product VersionSVN 
Target VersionTDM 2.06Fixed in VersionTDM 2.06 
Summary0004595: Install VC++ redistributable in tdm_update
DescriptionSince now runtime (and MFC) are linked to TDM dynamically, it is necessary to deploy VC++ redistributable pack to players. Otherwise, TDM may crash on start on a virgin Windows machine.
Discussion here:
  http://forums.thedarkmod.com/topic/19019-mfc-crashes-and-dynamic-linking/
TagsNo tags attached.

Relationships

related to 0005062 resolvedstgatilov Disable VCRedist installation step in updater 

Activities

grayman

grayman

14.12.2017 15:09

administrator   ~0009786

Who's doing what to include the key files?
stgatilov

stgatilov

14.12.2017 17:01

developer   ~0009791

There is a hypothesis that we can simply add CRT dll-s to the package binaries. Given that TDM is not a security-critical application, this way may be easier to implement.
Otherwise someone has to patch tdm_update, so that it runs the vc_redist.exe which we have to somehow add to binaries anyway.
grayman

grayman

14.12.2017 20:03

administrator   ~0009798

Is it just these four, which NB mentioned in the thread?

msvcp140.dll
vcruntime140.dll
concrt140.dll
vccorlib140.dll

If so, where do I find them, and is it as simple as copying them into the darkmod folder and committing and merging to 2.06?
stgatilov

stgatilov

15.12.2017 03:30

developer   ~0009799

Last edited: 15.12.2017 03:33

View 3 revisions

I have checked our SVN version with procmon.

1. These are the DLLs that we surely need, according to both procmon and documentation:
32-bit:
  C:\Windows\SysWOW64\mfc120.dll
  C:\Windows\SysWOW64\msvcr120.dll
  C:\Windows\SysWOW64\msvcp120.dll
64-bit:
  C:\Windows\System32\mfc120.dll
  C:\Windows\System32\msvcr120.dll
  C:\Windows\System32\msvcp120.dll

2. This is the debug DLL that is needed by 64-bit build due to ill configuration (which must be corrected):
  C:\Windows\System32\msvcr120d.dll

3. These are the CRT-related DLL-s that are also loaded according to procmon, but which are not to be distributed according to MSDN:
32-bit:
  C:\Windows\SysWOW64\msvcrt.dll
  C:\Windows\SysWOW64\ucrtbase.dll
  C:\Windows\SysWOW64\msvcp_win.dll
64-bit:
  C:\Windows\System32\msvcrt.dll
  C:\Windows\System32\ucrtbase.dll
  C:\Windows\System32\msvcp_win.dll
Among them, my Win7 virtual machine (without MSVC) has only msvcrt.dll, which is different anyway. Probably they are loaded by other CRT dll-s depending on Windows version, who knows. We can try to omit them.

Note that we have to provide two sets of same-named DLLs: one for 32-bit version, one for 64-bit version.

P.S. By the way, I assume that user is smart enough to install OpenGL (graphics card drivers) and DirectX (this one is maybe even bundled in modern Windows) himself.

grayman

grayman

15.12.2017 15:25

administrator   ~0009801

"Note that we have to provide two sets of same-named DLLs: one for 32-bit version, one for 64-bit version."

So how would a player use them? Deliver them like this:

darkmod/CRT_DLL_32/*.dll
darkmod/CRT_DLL_64/*.dll

and the player needs to drop them where? Into the darkmod folder, or into Windows/SysWOW64 and Windows/System32?
grayman

grayman

15.12.2017 15:36

administrator   ~0009802

Tell you what ...

Instead of me asking all these questions, why don't you set up what we need in the SVN darkmod folder, commit the files, and I'll take it from there.
stgatilov

stgatilov

15.12.2017 16:01

developer   ~0009803

In the simplest case, the DLLs need to be in the directory of EXE file. So 32-bit DLLs must be near TheDarkMod.exe for it to work, and 64-bit DLLs must be near TheDarkModx64.exe for it to work. Notice the dilemma =(

Asking user to copy DLLs to system directory is not an option in my opinion.

Maybe patching tdm_update, so that it runs vc_redist.exe, is a better idea.
grayman

grayman

15.12.2017 16:14

administrator   ~0009804

I assume that the player will either run 32-bit or 64-bit, so he copies the correct set and he's done.

"Maybe patching tdm_update, so that it runs vc_redist.exe, is a better idea."

Can you handle that, or does taaaki need to do it?
stgatilov

stgatilov

15.12.2017 16:21

developer   ~0009805

I'll try to add redistributable install to tdm_update.
I think I'll run it after update each time if at least something was downloaded (or maybe if EXE file was downloaded).
stgatilov

stgatilov

16.12.2017 04:12

developer   ~0009807

Ok, the fact that msvcr120d.dll was used (p.2) was some random build issue. I have rebuilt everything from scratch, and now it is not loaded.
grayman

grayman

16.12.2017 13:38

administrator   ~0009808

Not loaded == not needed?
stgatilov

stgatilov

16.12.2017 14:31

developer   ~0009809

Last edited: 16.12.2017 14:33

View 2 revisions

Committed tdm_update changes in SVN rev 7345, and added packages in SVN rev 15119.

In the new version, tdm_update tries to run vcredist packs at the end of PostUpdateCleanup phase. Note that there are two packages: one is 32-bit, the other one is 64-bit. The new code is excluded from all non-Windows builds.
1) tdm_update determines the bitness of Windows OS using GetSystemWow64Directory call. If it succeeds, then OS is 64-bit and both 32-bit and 64-bit packs must be installed. If the call fails, then OS is 32-bit and only 32-bit package is installed.
2) tdm_update checks if the package is already installed. This is done by looking at registry value SOFTWARE\Classes\Installer\Dependencies\{some_guid}.Version, according to https://stackoverflow.com/a/34209692/556899. If this registry key exists and has proper value, then redistributable pack is already installed and nothing has to be done.
3) tdm_update runs vcredist_x86.exe or/and vcredist_x64.exe from target directory (using CreateProcess). The installer is run with "/install /passive /norestart", where "/passive" means no user interaction. The installer needs admin rights, so it shows the UAC dialog, where publisher is displayed as "Microsoft Corporation", which should assure the user that it is OK to install it =) tdm_update waits for vcredist installer to complete, because running both 32-bit and 64-bit installers in parallel can cause issues.

As for the packages themselves, I have added them to SVN and to the manifest in \devel\manifests\base.txt. So they must be already in the target directory of user's machine by the time PostUpdateCleanup starts (given that you generate new package).

Note that:
1. If redistributable is already installed on user's machine, then admin rights are NOT necessary. But if it is not installed, then admin rights are needed to install it. If user rejects the UAC dialog, then he'll have to install DLLs himself (by running vcredist installers manually, or downloading the DLLs and putting them nearby).
2. The packages (vcredist_x86.exe and vcredist_x64.exe) are not deleted. They take about 13 MB of space together, and I hope it is not important.

stgatilov

stgatilov

16.12.2017 14:36

developer   ~0009810

One problem I face now is that the code is hard to test.
It may work a bit differently depending on OS bitness, whether packages are already installed or not, working under user/admin. It is rather complicated to find a Windows machine without redist already installed...
I have tried to comment out the check for already installed redist, and verified that the installers are run properly then. I have a relatively virgin Win7 in a VM, I'll probably try to run tdm_update there.
stgatilov

stgatilov

16.12.2017 15:56

developer   ~0009811

Last edited: 16.12.2017 15:56

View 2 revisions

Tested on Win 7 32-bit: it installs redistributable pack properly.

stgatilov

stgatilov

31.12.2017 07:10

developer   ~0009945

Two updates without complaints... I hope it works =)
In case of trouble, reopen the issue.

Issue History

Date Modified Username Field Change
31.07.2017 03:48 stgatilov New Issue
21.09.2017 19:13 nbohr1more Status new => feedback
14.12.2017 15:09 grayman Note Added: 0009786
14.12.2017 17:01 stgatilov Note Added: 0009791
14.12.2017 17:01 stgatilov Status feedback => new
14.12.2017 20:03 grayman Note Added: 0009798
15.12.2017 03:30 stgatilov Note Added: 0009799
15.12.2017 03:31 stgatilov Note Edited: 0009799 View Revisions
15.12.2017 03:33 stgatilov Note Edited: 0009799 View Revisions
15.12.2017 15:25 grayman Note Added: 0009801
15.12.2017 15:36 grayman Note Added: 0009802
15.12.2017 16:01 stgatilov Note Added: 0009803
15.12.2017 16:14 grayman Note Added: 0009804
15.12.2017 16:19 stgatilov Assigned To => stgatilov
15.12.2017 16:19 stgatilov Status new => assigned
15.12.2017 16:21 stgatilov Note Added: 0009805
16.12.2017 04:12 stgatilov Note Added: 0009807
16.12.2017 13:38 grayman Note Added: 0009808
16.12.2017 14:31 stgatilov Note Added: 0009809
16.12.2017 14:33 stgatilov Note Edited: 0009809 View Revisions
16.12.2017 14:36 stgatilov Note Added: 0009810
16.12.2017 15:56 stgatilov Note Added: 0009811
16.12.2017 15:56 stgatilov Note Edited: 0009811 View Revisions
31.12.2017 07:10 stgatilov Note Added: 0009945
31.12.2017 07:10 stgatilov Status assigned => resolved
31.12.2017 07:10 stgatilov Fixed in Version => TDM 2.06
31.12.2017 07:10 stgatilov Resolution open => fixed
16.11.2019 05:22 stgatilov Relationship added related to 0005062