[PATCH v3 0/2] MR6827: msi: Fix MsiSummaryInfoPersist not saving data to disk
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55859 When a db is transacted, we need to call `Commit` to actually save the changes, but tests show this is not how msi works, so I changed `MsiGetSummaryInformationW` to open the db directly instead. -- v3: msi: Make MsiGetSummaryInformationW open database as direct instead of transacted msi: Add more tests for MsiSummaryInfoPersist https://gitlab.winehq.org/wine/wine/-/merge_requests/6827
From: Fabian Maurer <dark.shadow4(a)web.de> --- dlls/msi/tests/suminfo.c | 147 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 147 insertions(+) diff --git a/dlls/msi/tests/suminfo.c b/dlls/msi/tests/suminfo.c index 59b56a2cbba..753f8bb53e5 100644 --- a/dlls/msi/tests/suminfo.c +++ b/dlls/msi/tests/suminfo.c @@ -278,6 +278,153 @@ static void test_suminfo(void) r = MsiCloseHandle(hsuminfo); ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n"); + /* try persisting on transacted msi */ + r = MsiOpenDatabaseW(msifileW, MSIDBOPEN_TRANSACT, &hdb); + ok(r == ERROR_SUCCESS, "MsiOpenDatabase failed\n"); + + r = MsiGetSummaryInformationA(hdb, NULL, 1, &hsuminfo); + ok(r == ERROR_SUCCESS, "MsiGetSummaryInformation wrong error\n"); + + r = MsiSummaryInfoSetPropertyA(hsuminfo, PID_AUTHOR, VT_LPSTR, 1, &ft, "Fabian"); + ok(r == ERROR_SUCCESS, "MsiSummaryInfoSetProperty wrong error\n"); + + r = MsiSummaryInfoPersist(hsuminfo); + ok(r == ERROR_SUCCESS, "MsiSummaryInfoPersist wrong error %u\n", r); + + r = MsiCloseHandle(hsuminfo); + ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n"); + + r = MsiCloseHandle(hdb); + ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n"); + + r = MsiOpenDatabaseW(msifileW, MSIDBOPEN_READONLY, &hdb); + ok(r == ERROR_SUCCESS, "MsiOpenDatabase failed\n"); + + r = MsiGetSummaryInformationA(hdb, NULL, 1, &hsuminfo); + ok(r == ERROR_SUCCESS, "MsiGetSummaryInformation wrong error\n"); + + sz = 0x10; + strcpy(buf,"x"); + r = MsiSummaryInfoGetPropertyA(hsuminfo, PID_AUTHOR, &type, NULL, NULL, buf, &sz); + ok(r == ERROR_SUCCESS, "MsiSummaryInfoGetPropertyA wrong error\n"); + ok(!strcmp(buf,"Mike"), "buffer was wrong: %s\n", buf); + + r = MsiCloseHandle(hsuminfo); + ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n"); + + r = MsiCloseHandle(hdb); + ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n"); + + /* try persisting on transacted msi with commit */ + r = MsiOpenDatabaseW(msifileW, MSIDBOPEN_TRANSACT, &hdb); + ok(r == ERROR_SUCCESS, "MsiOpenDatabase failed\n"); + + r = MsiGetSummaryInformationA(hdb, NULL, 1, &hsuminfo); + ok(r == ERROR_SUCCESS, "MsiGetSummaryInformation wrong error\n"); + + r = MsiSummaryInfoSetPropertyA(hsuminfo, PID_AUTHOR, VT_LPSTR, 1, &ft, "Fabian"); + ok(r == ERROR_SUCCESS, "MsiSummaryInfoSetProperty wrong error\n"); + + r = MsiSummaryInfoPersist(hsuminfo); + ok(r == ERROR_SUCCESS, "MsiSummaryInfoPersist wrong error %u\n", r); + + r = MsiCloseHandle(hsuminfo); + ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n"); + + r = MsiDatabaseCommit(hdb); + ok(r == ERROR_SUCCESS, "MsiDatabaseCommit wrong error %u\n", r); + + r = MsiCloseHandle(hdb); + ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n"); + + r = MsiOpenDatabaseW(msifileW, MSIDBOPEN_READONLY, &hdb); + ok(r == ERROR_SUCCESS, "MsiOpenDatabase failed\n"); + + r = MsiGetSummaryInformationA(hdb, NULL, 1, &hsuminfo); + ok(r == ERROR_SUCCESS, "MsiGetSummaryInformation wrong error\n"); + + sz = 0x10; + strcpy(buf,"x"); + r = MsiSummaryInfoGetPropertyA(hsuminfo, PID_AUTHOR, &type, NULL, NULL, buf, &sz); + ok(r == ERROR_SUCCESS, "MsiSummaryInfoGetPropertyA wrong error\n"); + ok(!strcmp(buf,"Fabian"), "buffer was wrong: %s\n", buf); + + r = MsiCloseHandle(hsuminfo); + ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n"); + + r = MsiCloseHandle(hdb); + ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n"); + + /* try persisting on direct msi */ + r = MsiOpenDatabaseW(msifileW, MSIDBOPEN_DIRECT, &hdb); + ok(r == ERROR_SUCCESS, "MsiOpenDatabase failed\n"); + + r = MsiGetSummaryInformationA(hdb, NULL, 1, &hsuminfo); + ok(r == ERROR_SUCCESS, "MsiGetSummaryInformation wrong error\n"); + + r = MsiSummaryInfoSetPropertyA(hsuminfo, PID_AUTHOR, VT_LPSTR, 1, &ft, "Fabian2"); + ok(r == ERROR_SUCCESS, "MsiSummaryInfoSetProperty wrong error\n"); + + r = MsiSummaryInfoPersist(hsuminfo); + ok(r == ERROR_SUCCESS, "MsiSummaryInfoPersist wrong error %u\n", r); + + r = MsiCloseHandle(hsuminfo); + ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n"); + + r = MsiCloseHandle(hdb); + ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n"); + + r = MsiOpenDatabaseW(msifileW, MSIDBOPEN_READONLY, &hdb); + ok(r == ERROR_SUCCESS, "MsiOpenDatabase failed\n"); + + r = MsiGetSummaryInformationA(hdb, NULL, 1, &hsuminfo); + ok(r == ERROR_SUCCESS, "MsiGetSummaryInformation wrong error\n"); + + sz = 0x10; + strcpy(buf,"x"); + r = MsiSummaryInfoGetPropertyA(hsuminfo, PID_AUTHOR, &type, NULL, NULL, buf, &sz); + ok(r == ERROR_SUCCESS, "MsiSummaryInfoGetPropertyA wrong error\n"); + ok(!strcmp(buf,"Fabian2"), "buffer was wrong: %s\n", buf); + + r = MsiCloseHandle(hsuminfo); + ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n"); + + r = MsiCloseHandle(hdb); + ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n"); + + /* try persisting on indirectly opened msi */ + r = MsiGetSummaryInformationA(0, msifile, 2, &hsuminfo); + ok(r == ERROR_SUCCESS, "MsiGetSummaryInformation wrong error %u\n", r); + + r = MsiSummaryInfoSetPropertyA(hsuminfo, PID_AUTHOR, VT_LPSTR, 1, &ft, "Fabian3"); + ok(r == ERROR_SUCCESS, "MsiSummaryInfoSetProperty wrong error\n"); + + r = MsiSummaryInfoPersist(hsuminfo); + ok(r == ERROR_SUCCESS, "MsiSummaryInfoPersist wrong error %u\n", r); + + r = MsiCloseHandle(hsuminfo); + ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n"); + + r = MsiOpenDatabaseW(msifileW, MSIDBOPEN_READONLY, &hdb); + ok(r == ERROR_SUCCESS, "MsiOpenDatabase failed\n"); + + r = MsiGetSummaryInformationA(hdb, NULL, 1, &hsuminfo); + ok(r == ERROR_SUCCESS, "MsiGetSummaryInformation wrong error\n"); + + sz = 0x10; + strcpy(buf,"x"); + r = MsiSummaryInfoGetPropertyA(hsuminfo, PID_AUTHOR, &type, NULL, NULL, buf, &sz); + ok(r == ERROR_SUCCESS, "MsiSummaryInfoGetPropertyA wrong error\n"); + todo_wine + ok(!strcmp(buf,"Fabian3"), "buffer was wrong: %s\n", buf); + + r = MsiCloseHandle(hsuminfo); + ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n"); + + r = MsiCloseHandle(hdb); + ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n"); + + /* Cleanup */ r = DeleteFileA(msifile); ok(r, "DeleteFile failed\n"); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6827
From: Fabian Maurer <dark.shadow4(a)web.de> This fixes MsiSummaryInfoPersist not saving data to disk Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55859 --- dlls/msi/suminfo.c | 2 +- dlls/msi/tests/suminfo.c | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/dlls/msi/suminfo.c b/dlls/msi/suminfo.c index b1934cf41c7..8670904c59b 100644 --- a/dlls/msi/suminfo.c +++ b/dlls/msi/suminfo.c @@ -515,7 +515,7 @@ UINT WINAPI MsiGetSummaryInformationW( MSIHANDLE hDatabase, const WCHAR *szDatab if( szDatabase && szDatabase[0] ) { - LPCWSTR persist = uiUpdateCount ? MSIDBOPEN_TRANSACT : MSIDBOPEN_READONLY; + LPCWSTR persist = uiUpdateCount ? MSIDBOPEN_DIRECT : MSIDBOPEN_READONLY; ret = MSI_OpenDatabaseW( szDatabase, persist, &db ); if( ret != ERROR_SUCCESS ) diff --git a/dlls/msi/tests/suminfo.c b/dlls/msi/tests/suminfo.c index 753f8bb53e5..42e68c16d84 100644 --- a/dlls/msi/tests/suminfo.c +++ b/dlls/msi/tests/suminfo.c @@ -415,7 +415,6 @@ static void test_suminfo(void) strcpy(buf,"x"); r = MsiSummaryInfoGetPropertyA(hsuminfo, PID_AUTHOR, &type, NULL, NULL, buf, &sz); ok(r == ERROR_SUCCESS, "MsiSummaryInfoGetPropertyA wrong error\n"); - todo_wine ok(!strcmp(buf,"Fabian3"), "buffer was wrong: %s\n", buf); r = MsiCloseHandle(hsuminfo); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6827
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=149761 Your paranoid android. === debian11b (64 bit WoW report) === user32: input.c:4305: Test succeeded inside todo block: button_down_hwnd_todo 1: got MSG_TEST_WIN hwnd 00000000019D00DA, msg WM_LBUTTONDOWN, wparam 0x1, lparam 0x320032 win.c:4070: Test failed: Expected active window 0000000005710150, got 0000000000000000. win.c:4071: Test failed: Expected focus window 0000000005710150, got 0000000000000000.
On Tue Nov 19 08:23:09 2024 +0000, Hans Leidekker wrote:
Your test shows that calling MsiSummaryInfoPersist() has no effect if the database is transacted. That makes sense because changes should only be stored when MsiDatabaseCommit() is called. It would be nice if you could add a test that shows this. Then the commit message 'Fix MsiSummaryInfoPersist not saving data to disk' should be more specific. Your patch only handles the case where MsiGetSummaryInformationW() opens the database. Thanks, changed the commit message and added another test using MsiDatabaseCommit to persistent the changes using a transacted database.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6827#note_87975
This merge request was approved by Hans Leidekker. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6827
participants (4)
-
Fabian Maurer -
Fabian Maurer (＠DarkShadow44)
-
Hans Leidekker (＠hans)
-
Marvin