> -----Original Message----- > From: Nils Gladitz [mailto:nilsglad...@gmail.com] > Sent: Friday, August 12, 2016 1:50 PM > To: Stuermer, Michael SP/HZA-ZSEP; CMake Developers > Subject: Re: [cmake-developers] [Patch 5/5] Improved WIX support > > On 08/12/2016 11:50 AM, Stuermer, Michael SP/HZA-ZSEP wrote: > > > > >> Patch 5 seems to implement patching of FeatureRef rather than the > >> original Feature elements. > >> I am not sure if this is what you intended but this could be error > >> prone given that there could in theory be any number (0-n) FeatureRef > >> elements for any corresponding Feature element. > >> > >> Nils > > The intention was to be able to add custom components that are added as > extra .wxs source files to certain features. If there are more convenient ways > to do this I will be happy to change the implementation or adapt my WIX > project. But so far this seemed to be a very easy and intuitive solution: the > additional component references are added in the same place where all > other component references are added as well. > > I understand the general intention but not why you elected to patch > FeatureRef elements instead of the Feature elements themselves. > After having look at the code for some minutes I remember why patching the ref instead oft he feature was my way to go:
The feature is written to file in cmWIXFeaturesSourceWriter::EmitFeatureForComponent(), where I do not have any patch information available. This means I'd have to change the signature of both EmitFeatureForComponent and EmitFeatureForComponentGroup and pass a reference to the patch instance along. Multiple occurrence of IDs can happen, but the patch will only be applied once because applied fragments are erased immediately after writing them to the stream. So after all for me this was a consideration of a 1-line change vs. changing class interfaces an passing object instances to where it might not be desirable. I agree the commit message of the patch is not accurate enough and if there is way to add custom WIX-components to features without changing the cpack source I'd be happy to do so. But so far I tried several approaches and neither worked (see below). > This would not be any more convenient but would certainly match > expectations and be less ill defined. > e.g. when I specify a patch for a Feature I expect that the Feature with the > given ID gets patched. > > Arguments against patching a FeatureRef instead are: > - There can be n FeatureRef elements for any Feature element in which case > it would not be obvious if the patch should be applied to one > (which?) or all of these The way the patch was implemented only the featurerefs in the generated features.wxs file would be patched and there should not be any double occurences of a feature ref. > - While similar FeatureRef and Feature don't take the same Child elements Right, and if both Feature and FeatureRef would be patchable we would be in trouble. For the lazy one: this is not the case at the moment so we would not need to worry about it (but it's very nice). For the correct one: We could introduce another attribute to CPackWixFragment called "Type" where type of the XML node to be patched could be stored. But this would introduce additional complexity to the cmWIXPatch class... > - You can already define your own FeatureRef elements (referencing any of > the pre-existing Feature elements if wanted) without having to use the > patch mechanism > I tried this like this (in a separate additional .wxs source file added with CPACK_WIX_EXTRA_SOURCES): <Wix xmlns="http://schemas.microsoft.com/wix/2006/wi";> <Fragment> <Component Id="SetRegistryValues" Guid="@random_guid@" Directory="INSTALL_ROOT"> <RegistryKey Root="HKLM" Key="@regkey@"> <RegistryValue Type="integer" Name="@name@" Value="@val@" /> </RegistryKey> </Component> <FeatureRef Id="@feature_id_from_features.wxs@" IgnoreParent="yes"> <ComponentRef Id="SetRegistryValues" /> </FeatureRef> </Fragment> </Wix> Did not work, the registry value was not set. Using the proposed approach it worked. Do I have to reference it somehow different? > Nils > . Michael -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
