[cmake-developers] [Patch 1/5] Improved WIX support

[Stuermer, Michael SP/HZA-ZSEP]

> -----Original Message-----
> From: Nils Gladitz [mailto:nilsgladitz at gmail.com]
> Sent: Thursday, July 21, 2016 8:56 AM
> To: Stuermer, Michael SP/HZA-ZSEP; CMake Developers
> Subject: Re: [cmake-developers] [Patch 1/5] Improved WIX support
> 
> On 07/20/2016 03:58 PM, Stuermer, Michael SP/HZA-ZSEP wrote:
> 
> > Using the patchfile support I managed to implement the service installation
> issue I had, so the unnecessary features from the last patch are removed
> now.
> >
> > I tested all patches separately and hope they work now.
> >
> > looking forward for feedback,
> >
> 
> To start with I don't think I really like the first patch as it is.
> 
> We should either require that install file locations are canonical (which is what
> I went for when I initially implemented them; I think I would still prefer it that
> way), or perform more complete and consistent canonicalization. e.g.
> 
> - The implementation as-is only works with cmake's internal path separation
> (forward slash).
>      Given the canonical path "foo/bar" the path "foo\bar" does not currently
> work. So neither should a backslash work in a prefix e.g.
> ".\foo/bar".
>      I'd also like to think of these paths as portable (should any other CPack
> generator choose to implement install properties as well) which is why I think
> we should not support (canonicalize) windows path separators anywhere in
> the path.
> 
> - Handling "." only as a singular prefix is inconsistent.
>      If we do implement this then ".." should also be supported and
> canonicalization should work anywhere in the path.
>      e.g. given the canonical path "foo/bar/baz" these should refer to the same
> path:
>          - "./foo/bar/baz"
>          - "././foo/bar/baz"
>          - "foo/./bar/baz"
>          - "foo/../foo/bar/baz"
>          etc.
> 
> Nils

Thanks for the detailed feedback! Proper canonicalization when setting up the map of installed files is a better way to go. I just had the impression the key of the file in the map ("./foo", "foo", ...) should not be changed. If this is not the case and we can change/improve the canonicalization where the installed files are set up we can discard this patch and I'll have a look at it again in the future.

For my current task I do not need the install properties anymore, so there is no dependency to this patch from the others.

Michael