On 11/16/05, Raphael fenix@club-internet.fr wrote:
Changelog:
- semi implementation of advpack APIs LaunchINFSection/LaunchINFSectionEx
- semi implementation of advpack API ExtractFiles
- setupapi: correct and check pExtractFiles return code
I appreciate your work on advpack, but some comments that might help get it accepted more easily:
* separate the patch into as many independent chunks as you can. It seems like each function you implement can be sent in separate and linearly. * it would help to add test cases wherever you can even if it's just to check how the function handles parameters.
- if (!bRet) {
ERR("Failed to CopyFileW(%s, %s)\n", debugstr_w(src), debugstr_w(dst));
- }
The rest of the file uses standard C indentation, so it's best to stick with that:
if (x) {
}
but seeing as how it's just a one liner,
if (!bRet) ERR("...");
-- James Hawkins
Hi,
James Hawkins wrote:
- it would help to add test cases wherever you can even if it's just
to check how the function handles parameters.
I'm also working on advpack inf install functions. Perhaps we should coordinate our work: Here my current Roadmap: 1. get tests into CVS (see below) 2. fix doinfinstall 3. implement RunSetupCommand, Rewrite DoInfInstall, to call RunSetupCommand. 4. Implement LaunchInfSection, LaunchInfSectionEx on top of RunSetupCommand. 5. test installers, improve the tests (eg. test for Run{Post,Pre}SetupCommand, registry entries, copied files, ..) and fix the functions.
I'm currently on point 1: I have written test cases for DoInfInstall, RunSetupCommand, LaunchInfSection, LaunchInfSectionEx, and documented my findings (see attachement). They pass on my w2k. Can some one run this tests on win98? Any comments on the patch are highly welcome (my C skills are a little rosty).
Markus
Index: dlls/advpack/advpack.c =================================================================== RCS file: /home/wine/wine/dlls/advpack/advpack.c,v retrieving revision 1.14 diff -u -r1.14 advpack.c --- dlls/advpack/advpack.c 8 Nov 2005 12:43:35 -0000 1.14 +++ dlls/advpack/advpack.c 17 Nov 2005 14:12:57 -0000 @@ -56,20 +56,20 @@ * Executes an install section in an INF file or a program. * * PARAMS - * hWnd [I] Handle to parent window, NULL for quiet mode + * hWnd [I] Handle to parent window, NULL for desktop * szCmdName [I] Inf or EXE filename to execute * szInfSection [I] Inf section to install, NULL for DefaultInstall * szDir [I] Path to extracted files * szTitle [I] Title of all dialogs * phEXE [O] Handle of EXE to wait for - * dwFlags [I] Flags; see include/advpub.h + * dwFlags [I] Flags; see RSC_* in include/advpub.h * pvReserved [I] Reserved * * RETURNS * S_OK Everything OK * S_ASYNCHRONOUS OK, required to wait on phEXE * ERROR_SUCCESS_REBOOT_REQUIRED Reboot required - * E_INVALIDARG Invalid argument given + * E_INVALIDARG Invalid argument, szCmdName or szDir NULL * HRESULT_FROM_WIN32(ERROR_OLD_WIN_VERSION) * Not supported on this Windows version * E_UNEXPECTED Unexpected error @@ -92,34 +92,84 @@
/*********************************************************************** * LaunchINFSection (ADVPACK.@) + * + * Install an INF section without BACKUP/ROLLBACK capabilities. + * + * PARAMS + * hWnd [I] Handle to parent window, NULL for desktop. + * hInst [I] Optional program instance. + * cmdline [I] Coma seperated string with inf file name, section name, flags. + * show [I] Reboot behaviour, see LaunchINFSectionEx(). + * + * RETURNS + * Success: S_OK + * Failure: S_FALSE + * + * NOTES + * For possible cmdline flags see LIS_* in include/advpub.h. They differ from + * flags in LaunchINFSectionEx(). + * + * BUGS + * Unimplemented */ -void WINAPI LaunchINFSection( HWND hWnd, HINSTANCE hInst, LPCSTR cmdline, INT show ) +HRESULT WINAPI LaunchINFSection( HWND hWnd, HINSTANCE hInst, LPCSTR cmdline, INT show ) { FIXME("(%p %p %s %d): stub\n", hWnd, hInst, debugstr_a(cmdline), show ); + return S_FALSE; }
/*********************************************************************** * LaunchINFSectionEx (ADVPACK.@) + * + * Install an INF section with BACKUP/ROLLBACK capabilities. + * + * PARAMS + * hWnd [I] Handle to parent window, NULL for desktop. + * hInst [I] Optional program instance. + * cmdline [I] Coma seperated string with + * inf file name, section name, cabinet path, flags. + * show [I] Reboot behaviour: + * 'A' reboot always + * 'I' default, reboot if needed + * 'N' no reboot + * + * RETURNS + * S_OK Everything OK + * E_INVALIDARG Invalid argument, cmdline NULL + * HRESULT_FROM_WIN32(GetLastError()) Some other error + * + * NOTES + * For possible cmdline flags see ALINF_* in include/advpub.h. + * + * BUGS + * Unimplemented */ -void WINAPI LaunchINFSectionEx( HWND hWnd, HINSTANCE hInst, LPCSTR cmdline, INT show ) +HRESULT WINAPI LaunchINFSectionEx( HWND hWnd, HINSTANCE hInst, LPCSTR cmdline, INT show ) { FIXME("(%p %p %s %d): stub\n", hWnd, hInst, debugstr_a(cmdline), show ); + return E_NOTIMPL; }
-/* this structure very closely resembles parameters of RunSetupCommand() */ -typedef struct -{ - HWND hwnd; - LPCSTR title; - LPCSTR inf_name; - LPCSTR dir; - LPCSTR section_name; -} SETUPCOMMAND_PARAMS; - /*********************************************************************** * DoInfInstall (ADVPACK.@) + * + * Install an INF section. + * + * PARAMS + * setup [I] Structure containing install information. + * See INFINSTALLPARAMS in include/advpub.h. + * + * RETURNS + * S_OK Everything OK + * E_INVALIDARG Invalid argument, cmdline NULL + * HRESULT_FROM_WIN32(GetLastError()) Some other error + * + * NOTES + * The INFINSTALLPARAMS structure is undocumented by Microsoft. + * It looks like the arguments from RunSetupCommand() were packed + * into that structure. But the flags (DII_*) are different. */ -BOOL WINAPI DoInfInstall(const SETUPCOMMAND_PARAMS *setup) +HRESULT WINAPI DoInfInstall(LPCINFINSTALLPARAMS setup) { BOOL ret; HINF hinf; Index: dlls/advpack/tests/advpack.c =================================================================== RCS file: /home/wine/wine/dlls/advpack/tests/advpack.c,v retrieving revision 1.8 diff -u -r1.8 advpack.c --- dlls/advpack/tests/advpack.c 14 Nov 2005 12:27:39 -0000 1.8 +++ dlls/advpack/tests/advpack.c 17 Nov 2005 14:12:58 -0000 @@ -31,6 +31,10 @@ static HRESULT (WINAPI *pGetVersionFromFile)(LPSTR,LPDWORD,LPDWORD,BOOL); static HRESULT (WINAPI *pDelNode)(LPCSTR,DWORD); static HRESULT (WINAPI *pTranslateInfString)(LPSTR,LPSTR,LPSTR,LPSTR,LPSTR,DWORD,LPDWORD,LPVOID); +static HRESULT (WINAPI *pDoInfInstall)(LPCINFINSTALLPARAMS); +static HRESULT (WINAPI *pRunSetupCommand) (HWND,LPCSTR,LPCSTR,LPCSTR,LPCSTR,HANDLE,DWORD,LPVOID); +static HRESULT (WINAPI *pLaunchINFSection) (HWND,HINSTANCE,LPCSTR,INT); +static HRESULT (WINAPI *pLaunchINFSectionEx) (HWND,HINSTANCE,LPCSTR,INT);
static void version_test(void) { @@ -163,6 +167,13 @@ append_str(&ptr, "[Strings]\n"); append_str(&ptr, "DefaultAppPath="Application Name"\n"); append_str(&ptr, "LProgramF="Program Files"\n"); + append_str(&ptr, "[InfInstall]\n"); + append_str(&ptr, "AddReg=TestReg\n"); + append_str(&ptr, "[InfInstallFails]\n"); + append_str(&ptr, "AddReg=TestReg2\n"); + append_str(&ptr, "[TestReg]\n"); + append_str(&ptr, "[TestReg2]\n"); + append_str(&ptr, "foobar\n");
WriteFile(hf, data, ptr - data, &dwNumberOfBytesWritten, NULL); CloseHandle(hf); @@ -234,6 +245,118 @@ DeleteFile("c:\test.inf"); }
+/* TODO: check for the actual install results. */ +struct _infInstall { + UINT func; /* 0=DoInfInstall, 1=RunSetupCommand, + 2=LaunchInfSection, 3=LaunchInfSectionEx */ + BOOL todo; + LPCSTR file; + LPCSTR dir; + LPCSTR section; + DWORD flags; + HRESULT result; +}; + +static void infinstall_test() +{ + HRESULT hr = ERROR_SUCCESS; + char buffer[MAX_PATH]; + + INFINSTALLPARAMS infParams = { + NULL, + "WindowTitle", + NULL, + NULL, + NULL, + 0, + 0 + }; + + struct _infInstall infInstall[]={ + /* Tests for DoInfInstall */ + { 0, TRUE, NULL , NULL, NULL , 0, E_INVALIDARG }, + { 0, TRUE, "C:\a.inf" , NULL, NULL , DII_FLAG_QUIET, HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND) }, + { 0, TRUE, "C:\test.inf", NULL, NULL , 0, S_OK }, + { 0, TRUE, "C:\test.inf", NULL, "foobar" , 0, S_OK }, + { 0, TRUE, "C:\test.inf", NULL, "InfInstall" , 0, S_OK }, + { 0, TRUE, "C:\test.inf", NULL, "InfInstallFails", DII_FLAG_QUIET, HRESULT_FROM_WIN32(ERROR_BADKEY) }, + + /* Tests for RunSetupCommand */ + { 1, TRUE, NULL , NULL, NULL , 0, E_INVALIDARG }, + { 1, TRUE, "C:\a.inf" , NULL, NULL , 0, E_INVALIDARG }, + { 1, TRUE, "C:\a.inf" , "" , NULL , 0, HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND) }, + { 1, TRUE, "C:\test.inf", "" , NULL , 0, HRESULT_FROM_WIN32(ERROR_BAD_EXE_FORMAT) }, + { 1, TRUE, "C:\test.inf", "" , NULL , RSC_FLAG_INF, S_OK }, + { 1, TRUE, "C:\test.inf", "" , "foobar" , RSC_FLAG_INF, S_OK }, + { 1, TRUE, "C:\test.inf", "" , "InfInstall" , RSC_FLAG_INF, S_OK }, + { 1, TRUE, "C:\test.inf", "" , "InfInstallFails", RSC_FLAG_QUIET | RSC_FLAG_INF, HRESULT_FROM_WIN32(ERROR_BADKEY) }, + + /* Tests for LaunchINFSection */ + { 2, FALSE, NULL , NULL, NULL , LIS_QUIET, S_FALSE }, + { 2, FALSE, "C:\a.inf" , NULL, NULL , LIS_QUIET, S_FALSE }, + { 2, TRUE, "C:\test.inf", NULL, NULL , 0, S_OK }, + { 2, TRUE, "C:\test.inf", NULL, "foobar" , 0, S_OK }, + { 2, TRUE, "C:\test.inf", NULL, "InfInstall" , 0, S_OK }, + { 2, FALSE, "C:\test.inf", NULL, "InfInstallFails", LIS_QUIET, S_FALSE }, + + /* Tests for LaunchINFSectionEx */ + { 3, TRUE, NULL , NULL, NULL , ALINF_QUIET, E_INVALIDARG }, + /* + No way to supress this file not found message box on w2k. + Returns E_INVALIDARG, not file not found like RunSetupCommand! + { 3, TRUE, "C:\a.inf" , NULL, NULL , ALINF_QUIET, E_INVALIDARG }, + */ + { 3, TRUE, "C:\test.inf", NULL, NULL , 0, S_OK }, + { 3, TRUE, "C:\test.inf", NULL, "foobar" , 0, S_OK }, + { 3, TRUE, "C:\test.inf", NULL, "InfInstall" , 0, S_OK }, + { 3, TRUE, "C:\test.inf", NULL, "InfInstallFails", ALINF_QUIET, HRESULT_FROM_WIN32(ERROR_BADKEY) } + }; + int i, num_test = (sizeof(infInstall)/sizeof(struct _infInstall)); + + create_inf_file(); + + for (i=0; i < num_test; i++) + { + /* Execute given function */ + switch (infInstall[i].func) + { + /* DoInfInstall */ + case 0: + infParams.inf_name = infInstall[i].file; + infParams.dir = infInstall[i].dir; + infParams.section_name = infInstall[i].section; + infParams.flags = infInstall[i].flags; + hr = pDoInfInstall(&infParams); + break; + + /* RunSetupCommand */ + case 1: + hr = pRunSetupCommand(NULL, infInstall[i].file, infInstall[i].section, + infInstall[i].dir, "Window Title", NULL, infInstall[i].flags, NULL); + break; + + /* LaunchInfSection */ + case 2: + wsprintf (buffer, "%s,%s,%d", infInstall[i].file, infInstall[i].section, infInstall[i].flags); + hr = pLaunchINFSection (NULL, NULL, buffer, 'N'); + break; + + /* LaunchInfSectionEx */ + case 3: + wsprintf (buffer, "%s,%s,,%d", infInstall[i].file, infInstall[i].section, infInstall[i].flags); + hr = pLaunchINFSectionEx (NULL, NULL, buffer, 'N'); + break; + } + + if (infInstall[i].todo) + todo_wine ok (hr==infInstall[i].result, "i=%d Expected 0x%08x, got 0x%08x\n", i, (UINT)infInstall[i].result, (UINT)hr); + else + ok (hr==infInstall[i].result, "i=%d Expected 0x%08x, got 0x%08x\n", i, (UINT)infInstall[i].result, (UINT)hr); + } + DeleteFile("c:\a.inf"); + DeleteFile("c:\test.inf"); +} + START_TEST(advpack) { HMODULE hdll; @@ -245,12 +368,18 @@ pGetVersionFromFile = (void*)GetProcAddress(hdll, "GetVersionFromFile"); pDelNode = (void*)GetProcAddress(hdll, "DelNode"); pTranslateInfString = (void*)GetProcAddress(hdll, "TranslateInfString"); - if (!pGetVersionFromFile || !pDelNode || !pTranslateInfString) + pDoInfInstall = (void*)GetProcAddress(hdll, "DoInfInstall"); + pRunSetupCommand = (void*)GetProcAddress(hdll, "RunSetupCommand"); + pLaunchINFSection = (void*)GetProcAddress(hdll, "LaunchINFSection"); + pLaunchINFSectionEx = (void*)GetProcAddress(hdll, "LaunchINFSectionEx"); + if (!pGetVersionFromFile || !pDelNode || !pTranslateInfString || + !pDoInfInstall || !pRunSetupCommand || !pLaunchINFSection || !pLaunchINFSectionEx) return;
version_test(); delnode_test(); translateinfstring_test(); + infinstall_test();
FreeLibrary(hdll); } Index: include/advpub.h =================================================================== RCS file: /home/wine/wine/include/advpub.h,v retrieving revision 1.5 diff -u -r1.5 advpub.h --- include/advpub.h 27 Oct 2005 10:24:26 -0000 1.5 +++ include/advpub.h 17 Nov 2005 14:13:00 -0000 @@ -46,6 +46,20 @@ typedef const STRTABLE CSTRTABLE; typedef CSTRTABLE *LPCSTRTABLE;
+/* Structure for DoInfInstall */ +typedef struct _InfInstallParams{ + HWND hwnd; + LPCSTR title; + LPCSTR inf_name; + LPCSTR dir; + LPCSTR section_name; + DWORD unknown1; + DWORD flags; +} INFINSTALLPARAMS, *LPINFINSTALLPARAMS; + +typedef const INFINSTALLPARAMS CINFINSTALLPARAMS; +typedef CINFINSTALLPARAMS *LPCINFINSTALLPARAMS; + /* Flags for RunSetupCommand */ #define RSC_FLAG_INF 0x00000001 #define RSC_FLAG_SKIPDISKSPACECHECK 0x00000002 @@ -55,6 +69,23 @@ #define RSC_FLAG_DELAYREGISTEROCX 0x00000200 #define RSC_FLAG_SETUPAPI 0x00000400
+/* Flags for LaunchINFSectionEx */ +#define ALINF_QUIET RSC_FLAG_QUIET +#define ALINF_NGCONV RSC_FLAG_NGCONV +#define ALINF_UPDHLPDLLS RSC_FLAG_UPDHLPDLLS +#define ALINF_BKINSTALL 0x00000020 +#define ALINF_ROLLBACK 0x00000040 +#define ALINF_CHECKBKDATA 0x00000080 +#define ALINF_ROLLBKDOALL 0x00000100 +#define ALINF_DELAYREGISTEROCX RSC_FLAG_DELAYREGISTEROCX + +/* Flags for LaunchINFSection */ +#define LIS_QUIET 0x00000001 +#define LIS_NOGRPCONV 0x00000002 + +/* Flags for DoInfInstall */ +#define DII_FLAG_QUIET 0x00000001 + /* Flags for DelNode */ #define ADN_DEL_IF_EMPTY 0x00000001 #define ADN_DONT_DEL_SUBDIRS 0x00000002
On Thursday 17 November 2005 15:33, Markus Amsler wrote:
Hi,
Hi,
James Hawkins wrote:
- it would help to add test cases wherever you can even if it's just
to check how the function handles parameters.
I'm also working on advpack inf install functions. Perhaps we should coordinate our work: Here my current Roadmap:
- get tests into CVS (see below)
- fix doinfinstall
- implement RunSetupCommand, Rewrite DoInfInstall, to call
RunSetupCommand. 4. Implement LaunchInfSection, LaunchInfSectionEx on top of RunSetupCommand. 5. test installers, improve the tests (eg. test for Run{Post,Pre}SetupCommand, registry entries, copied files, ..) and fix the functions.
:) For me it is difficult to find a valid test as i had only one application who need this function (and no windows to valid behavior).
you seems to have planned to implement more functionnalities than me. So i think you can integrate my patch (or not) and continue :)
I'm currently on point 1: I have written test cases for DoInfInstall, RunSetupCommand, LaunchInfSection, LaunchInfSectionEx, and documented my findings (see attachement). They pass on my w2k. Can some one run this tests on win98? Any comments on the patch are highly welcome (my C skills are a little rosty).
Your patch seems good for me.
Only one comment, i started to think how to test LaunchINFSection (and before others complain about missing tests:) ) but complete tests need to have: - 3 differents .inf (to test uninstall, rollbacks, partial installs, ...) - 1 cabinet file (to test install from .inf extracted from a cabinet)
And currrently i don't know how to test cabinet part (only way i have found now is to deliver it on test directory)
Markus
Regards, Raphael
Hi,
I appreciate your work on advpack, but some comments that might help get it accepted more easily:
- separate the patch into as many independent chunks as you can. It
seems like each function you implement can be sent in separate and linearly.
Yes, but as they are really small changes i keep on patch for that
- it would help to add test cases wherever you can even if it's just
to check how the function handles parameters.
I have started to think about that but need a complex test to test all capabilities (ie other than parse of command line) (see answer to Markus)
if (!bRet) {
ERR("Failed to CopyFileW(%s, %s)\n", debugstr_w(src),debugstr_w(dst));
- }
The rest of the file uses standard C indentation, so it's best to stick with that:
forget to change that, good catch :)
-- James Hawkins
Regards, Raphael
-
James Hawkins -
Markus Amsler -
Raphael