Re: msi: ACTION_RegisterProduct store all InstallProperties

[Adam Strzelecki]

Hello,
> I think you need to read through the implementation of
> MSI_GetProductInfo more closely and also read the MsiGetProductInfo
> tests in tests/msi.c and the test_publish:RegisterProduct tests in
> tests/install.c.

I think I did, and looking at the tests I see they are testing  
properties only at Uninstall key, and I think this is wrong, or at  
least incomplete, as quoted below:
> res = RegOpenKeyA(uninstall, prodcode, &prodkey);
> ok(res == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", res);
>
> CHECK_REG_DWORD(prodkey, "Version", 0x1010001);
> CHECK_REG_DWORD(prodkey, "VersionMajor", 1);
> CHECK_REG_DWORD(prodkey, "VersionMinor", 1);
> CHECK_REG_DWORD(prodkey, "WindowsInstaller", 1);

While MSI_GetProductInfo is reading VersionMajor, VersionMinor, ...  
etc from InstallProperties key (from szInstallProperties_fmt or  
szInstaller_LocalSystemProductCodes_fmt), but NOT from Uninstall key  
as quotes below:
> if (classes)
>     MSIREG_OpenLocalSystemProductKey(szProduct, &userdata, FALSE);
> else
>     MSIREG_OpenInstallPropertiesKey(szProduct, &userdata, FALSE);
>
> if (!lstrcmpW(szAttribute, INSTALLPROPERTY_HELPLINKW) ||
>     !lstrcmpW(szAttribute, INSTALLPROPERTY_HELPTELEPHONEW) ||
>     !lstrcmpW(szAttribute, INSTALLPROPERTY_INSTALLDATEW) ||
>     !lstrcmpW(szAttribute,  
>
> INSTALLPROPERTY_INSTALLEDPRODUCTNAMEW) ||
>         !lstrcmpW(szAttribute, INSTALLPROPERTY_INSTALLLOCATIONW) ||
>         !lstrcmpW(szAttribute, INSTALLPROPERTY_INSTALLSOURCEW) ||
>         !lstrcmpW(szAttribute, INSTALLPROPERTY_LOCALPACKAGEW) ||
>         !lstrcmpW(szAttribute, INSTALLPROPERTY_PUBLISHERW) ||
>         !lstrcmpW(szAttribute, INSTALLPROPERTY_URLINFOABOUTW) ||
>         !lstrcmpW(szAttribute, INSTALLPROPERTY_URLUPDATEINFOW) ||
>         !lstrcmpW(szAttribute, INSTALLPROPERTY_VERSIONMINORW) ||
>         !lstrcmpW(szAttribute, INSTALLPROPERTY_VERSIONMAJORW) ||
>         !lstrcmpW(szAttribute, INSTALLPROPERTY_VERSIONSTRINGW) ||
>         !lstrcmpW(szAttribute, INSTALLPROPERTY_PRODUCTIDW) ||
>         !lstrcmpW(szAttribute, INSTALLPROPERTY_REGCOMPANYW) ||
>         !lstrcmpW(szAttribute, INSTALLPROPERTY_REGOWNERW))
>     {
>         if (!prodkey)
>         {
>             r = ERROR_UNKNOWN_PRODUCT;
>             goto done;
>         }
>     if (!userdata)
>         return ERROR_UNKNOWN_PROPERTY;
>
>     if (!lstrcmpW(szAttribute,  
>
> INSTALLPROPERTY_INSTALLEDPRODUCTNAMEW))
>             szAttribute = display_name;
>         else if (!lstrcmpW(szAttribute,  
> INSTALLPROPERTY_VERSIONSTRINGW))
>             szAttribute = display_version;
>     val = msi_reg_get_value(userdata, szAttribute, &type);
>     if (!val)
>         val = empty;
> }

So it does not find any of those properties in the registry, because  
they're not written there by ACTION_RegisterProduct.
And fixing this is the intention of my patch.

Regards,

msi: ACTION_RegisterProduct store all InstallProperties

[James Hawkins]

On Sat, Mar 1, 2008 at 4:02 PM, Adam Strzelecki  wrote:
> Hello,
> > I think you need to read through the implementation of
> > MSI_GetProductInfo more closely and also read the MsiGetProductInfo
> > tests in tests/msi.c and the test_publish:RegisterProduct tests in
> > tests/install.c.
>
> I think I did, and looking at the tests I see they are testing
>  properties only at Uninstall key, and I think this is wrong, or at
>  least incomplete, as quoted below:
> > res = RegOpenKeyA(uninstall, prodcode, &prodkey);
> > ok(res == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", res);
> >
> > CHECK_REG_DWORD(prodkey, "Version", 0x1010001);
> > CHECK_REG_DWORD(prodkey, "VersionMajor", 1);
> > CHECK_REG_DWORD(prodkey, "VersionMinor", 1);
> > CHECK_REG_DWORD(prodkey, "WindowsInstaller", 1);
>
> While MSI_GetProductInfo is reading VersionMajor, VersionMinor, ...
>  etc from InstallProperties key (from szInstallProperties_fmt or
>  szInstaller_LocalSystemProductCodes_fmt), but NOT from Uninstall key
>  as quotes below:
> > if (classes)
> >     MSIREG_OpenLocalSystemProductKey(szProduct, &userdata, FALSE);
> > else
> >     MSIREG_OpenInstallPropertiesKey(szProduct, &userdata, FALSE);
> >
> > if (!lstrcmpW(szAttribute, INSTALLPROPERTY_HELPLINKW) ||
> >     !lstrcmpW(szAttribute, INSTALLPROPERTY_HELPTELEPHONEW) ||
> >     !lstrcmpW(szAttribute, INSTALLPROPERTY_INSTALLDATEW) ||
> >     !lstrcmpW(szAttribute,
> >
> > INSTALLPROPERTY_INSTALLEDPRODUCTNAMEW) ||
> >         !lstrcmpW(szAttribute, INSTALLPROPERTY_INSTALLLOCATIONW) ||
> >         !lstrcmpW(szAttribute, INSTALLPROPERTY_INSTALLSOURCEW) ||
> >         !lstrcmpW(szAttribute, INSTALLPROPERTY_LOCALPACKAGEW) ||
> >         !lstrcmpW(szAttribute, INSTALLPROPERTY_PUBLISHERW) ||
> >         !lstrcmpW(szAttribute, INSTALLPROPERTY_URLINFOABOUTW) ||
> >         !lstrcmpW(szAttribute, INSTALLPROPERTY_URLUPDATEINFOW) ||
> >         !lstrcmpW(szAttribute, INSTALLPROPERTY_VERSIONMINORW) ||
> >         !lstrcmpW(szAttribute, INSTALLPROPERTY_VERSIONMAJORW) ||
> >         !lstrcmpW(szAttribute, INSTALLPROPERTY_VERSIONSTRINGW) ||
> >         !lstrcmpW(szAttribute, INSTALLPROPERTY_PRODUCTIDW) ||
> >         !lstrcmpW(szAttribute, INSTALLPROPERTY_REGCOMPANYW) ||
> >         !lstrcmpW(szAttribute, INSTALLPROPERTY_REGOWNERW))
> >     {
> >         if (!prodkey)
> >         {
> >             r = ERROR_UNKNOWN_PRODUCT;
> >             goto done;
> >         }
> >     if (!userdata)
> >         return ERROR_UNKNOWN_PROPERTY;
> >
> >     if (!lstrcmpW(szAttribute,
> >
> > INSTALLPROPERTY_INSTALLEDPRODUCTNAMEW))
> >             szAttribute = display_name;
> >         else if (!lstrcmpW(szAttribute,
> > INSTALLPROPERTY_VERSIONSTRINGW))
> >             szAttribute = display_version;
> >     val = msi_reg_get_value(userdata, szAttribute, &type);
> >     if (!val)
> >         val = empty;
> > }
>
> So it does not find any of those properties in the registry, because
>  they're not written there by ACTION_RegisterProduct.
>  And fixing this is the intention of my patch.

Which is wrong, which you'll see when I submit my next patch which
tests your theory.

msi: ACTION_RegisterProduct store all InstallProperties

[Adam Strzelecki]

> Which is wrong, which you'll see when I submit my next patch which
> tests your theory.

I don't mind you pointing me out my mistakes. So I kindly waiting for  
your patch. However right now I presented my arguments quoting Wine  
sources, but you just respond that I'm wrong without presenting yours.

All I can say, I was testing this is MSXML 6.0 + Visual Studio 2005  
installer, and current Wine trunk causes VS2005 installer think that  
MSXML 6.0 is regardless of anything, just because VerionMajor &  
VersionMinor fails on MsiGetProductInfo.

msi: ACTION_RegisterProduct store all InstallProperties

[James Hawkins]

On Sat, Mar 1, 2008 at 4:33 PM, Adam Strzelecki  wrote:
> > Which is wrong, which you'll see when I submit my next patch which
> > tests your theory.
>
> I don't mind you pointing me out my mistakes. So I kindly waiting for
>  your patch. However right now I presented my arguments quoting Wine
>  sources, but you just respond that I'm wrong without presenting yours.
>
> All I can say, I was testing this is MSXML 6.0 + Visual Studio 2005
>  installer, and current Wine trunk causes VS2005 installer think that
>  MSXML 6.0 is regardless of anything, just because VerionMajor &
>  VersionMinor fails on MsiGetProductInfo.

Have some patience...I *just* wrote the tests, and I'm doing this on a
Saturday.  You'll see them soon enough.

msi: ACTION_RegisterProduct store all InstallProperties

[Adam Strzelecki]

Thanks for your latest MSI patches fixing the issue.

Please mark my patch obsolete.

Regards,