=== Consolidate GetMsiProperty() === '''Background:''' GetMsiProperty() is being copy'n'pasted from file to file, see [http://opengrok.libreoffice.org/search?q=GetMsiProperty&project=components&defs=&refs=&path=&hist= this link]. Instead of the copy'n'paste, we should define it as an inline in a header, and use it where appropriate. '''Skills:''' Building on Windows, simple C++
Deteted "Easyhack" from summary
Same should be done for GetMsiProp(), in fact GetMsiProperty() calls could be converted to GetMsiProp() calls, too.
I think we can do the same for SetMsiProperty Since all those functions will require #include <msiquery.h>, should we replace all these with a #include "msiprop.hxx" or just the ones which are used by the inlined functions ? And I plan to put msipro.hxx in setup_native/source/win32customactions/inc (new folder). Is it ok ?
Another advice is needed: In the code, I found TCHAR *, LPTSTR and wchar_t *, beside string & TCHAR in code targeted y the issue. I started to use LPTSTR for pointers, as it is standard in Windows, but am not sure about readability. I am a cpp fresher, so any advice welcome.
Mathias Michel committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=95ee7d9cd3a0b0f397def8e607759c81feb8c592 fix for fdo#39632 : Consolidate GetMsiProperty() The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Good job, thanks.
Andras Timar committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=55fc2b96bb808381fff62b3d34f0e25c13de84a4 Revert "fix for fdo#39632 : Consolidate GetMsiProperty()" The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
I'm sorry, but I had to revert your patch. It looked OK at the first glance, but after some testing it showed problems. I tested two CustonActions, DotNetCHeck and SelectLanguage, neither of them worked. Even when I uncommented the commented MessageBox calls in DotNetCheck, I got no MessageBoxes at all. Can you please have a second look at the patch? Thanks.
I tried activating Message boxes in setup_native/source/win32/customactions/shellextensions/dotnetcheck.cxx, because I think this action is always called. I did so by uncommenting lines in the extern "C" UINT __stdcall DotNetCheck(MSIHANDLE handle) function I did it on a standard commit : commit 484bb96aa975d834e326d927d36ee17808b8b6b5 Author: Tor Lillqvist <tml@iki.fi> Date: Sat Jan 12 15:34:28 2013 +0200 Boxes did not appear and installer complained it was interrupt before the end (although all is working fine and desktop shortcut is created. My leads are: 1) I need to do more than just uncommenting code to activate message boxes 2) dotnetcheck custom action is not called unconditionally If 1), could you give me some pointers, or do you have any idea on how to go further ? If 2), As I checked in the idt files it seems to always trigger. Some pointers on how to understand idt files needed, then.
adding LibreOffice developer list as CC to unresolved EasyHacks for better visibility. see e.g. http://nabble.documentfoundation.org/minutes-of-ESC-call-td4076214.html for details
Migrating Whiteboard tags to Keywords: (EasyHack DifficultyBeginner SkillWindows SkillCpp ) [NinjaEdit]
removing skillWindows keyword. There is no such thing at https://wiki.documentfoundation.org/Development/Easy_Hacks/lists/by_Required_Skill and having a "skill*" keyword makes the easyHack not even appear on that page even in "uncategorized".
JanI is default CC for Easy Hacks (Add Jan; remove LibreOffice Dev List from CC) [NinjaEdit]
A polite ping, still working on this issue?
This should be largely addressed by these set of fixes in the Windows installer module submitted against tdf#72606 'EasyHack _tstring/TCHAR elimination' http://cgit.freedesktop.org/libreoffice/core/commit/?id=8a4dd6f45b12e7d44ad595bc0fadc37075061119 http://cgit.freedesktop.org/libreoffice/core/commit/?id=163dcad72e03e214d842e74d1f71ed025cbdd870 http://cgit.freedesktop.org/libreoffice/core/commit/?id=074bd09ee6f3113792b60ee721aabb731c5d7ace http://cgit.freedesktop.org/libreoffice/core/commit/?id=68502698d29e577a7a451f1a796677128901cfe3 http://cgit.freedesktop.org/libreoffice/core/commit/?id=7fe92c766adf97bdeb4d844ffe6d0650a964572e http://cgit.freedesktop.org/libreoffice/core/commit/?id=29e0b587df4e509558c22fa478992b07486828d1 Note that there are a mix of ANSI/Wide calls used in the Windows installer so there isn't one single #include file defining GetMsiProperty() across the whole installer but this at least minimises the former copy'n'paste mess.
I want to work on this bug.
Have a look at this step-by-step guide: https://wiki.documentfoundation.org/Development/GetInvolved
Actually I would recommend that we consider this issue closed. The patches I submitted (ref 2016-06-17) go 90% towards Björn's original concerns about copy-pasta; because of the remaining ANSI/Wide issues (that will be fixed as part of tdf#72606) I wouldn't try to progress this specific issue further.
A polite ping, still working on this issue ?
yes
A polite ping, still working on this bug ? I cannot see any patches so far ?
This looks like something I can do -- but again, not clear what the current status is from this log last updated months ago. I will assign to myself and give it a go.
(In reply to Guy Marcus from comment #22) > This looks like something I can do -- but again, not clear what the current > status is from this log last updated months ago. I will assign to myself and > give it a go. Closing this as a bug per suggestion of Stewart above and the fact that GetMsiProp() and the like do not even appear to be present in the source any more. Following the link provided to opengrok in the initial post yields zero results.