> -----Original Message-----
> From: Nils Gladitz [mailto:nilsglad...@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
--
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
