Bug 96518 - MSI Installer: Test for existing non-empty directory (CheckInstallDirectory) will always return OK
Summary: MSI Installer: Test for existing non-empty directory (CheckInstallDirectory) ...
Status: RESOLVED INSUFFICIENTDATA
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Installation (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: All Windows (All)
: medium normal
Assignee: Not Assigned
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-12-15 18:10 UTC by Stuart Swales
Modified: 2017-12-04 12:39 UTC (History)
1 user (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Stuart Swales 2015-12-15 18:10:57 UTC
One of our custom install functions CheckInstallDirectory (in Library_shlxtmsi, compiled from setup_native/source/win32/customactions/shellextensions/checkdirectory.cxx) will always claim that the target installation directory is empty, and therefore OK to install into, as it is checking for the wrong file.

It does this as it inserts the string "LibreOffice 5" between the actual install location and the file it's trying to test for.

Spotted this whilst looking to eliminate _tstring/TCHAR and Win32 ANSI API calls.

Verified by adding debug output to this custom installer DLL in a build of 5.1 beta2 and forcing LO install into a differently named directory containing a program\setup.ini file.

git diff \
 -r 4dd1fa636a7408ba04777435911f8b1cb5da7587 \
 -r d5c1f2e15d2b80346f02612919dc5de014153bb8 \
 setup_native/source/win32/customactions/shellextensions/checkdirectory.cxx

-    std::_tstring sSetupIniPath = sInstallPath + TEXT("program\\setup.ini");
+    std::_tstring sSetupIniPath = sInstallPath + sOfficeHostnamePath + TEXT("\\program\\setup.ini");

*** BUT the worst thing is that this function is called too late in the MSI InstallExecuteSequence AFTER all the files have been copied anyway, so it would end up testing for a file which always DID exist...

I think that the best thing to do is to actually eliminate this function from the installer.
Comment 1 Xisco Faulí 2017-04-13 09:43:44 UTC Comment hidden (obsolete)
Comment 2 QA Administrators 2017-10-30 10:54:06 UTC Comment hidden (obsolete)
Comment 3 QA Administrators 2017-12-04 12:39:14 UTC
Dear Bug Submitter,

Please read this message in its entirety before proceeding.

Your bug report is being closed as INSUFFICIENTDATA due to inactivity and
a lack of information which is needed in order to accurately
reproduce and confirm the problem. We encourage you to retest
your bug against the latest release. If the issue is still
present in the latest stable release, we need the following
information (please ignore any that you've already provided):

a) Provide details of your system including your operating
   system and the latest version of LibreOffice that you have
   confirmed the bug to be present

b) Provide easy to reproduce steps – the simpler the better

c) Provide any test case(s) which will help us confirm the problem

d) Provide screenshots of the problem if you think it might help

e) Read all comments and provide any requested information

Once all of this is done, please set the bug back to UNCONFIRMED
and we will attempt to reproduce the issue. Please do not:

a) respond via email 

b) update the version field in the bug or any of the other details
   on the top section of our bug tracker

Warm Regards,
QA Team

MassPing-NeedInfo-20171204