[cmake-developers] Questions on staged topic branches

[Eric Noulard]

2012/9/4 David Cole <david.cole at kitware.com>:
> If you are on the "To" line, please reply to this email, and answer
> the questions below:
>
> Brad and I have questions on these staged topic branches before we
> consider them for merging to 'master'.
>
> ##########
> # Peter Kuemmel and Eric Noulard:
> #                                    cpack-stripping | master=0 next=1
>
> This one should not be necessary. Can you tell us what problem it
> fixes? The only valid install path that should ever go under DESTDIR
> has to be an absolute path already anyways. So it will either start
> with a "/" already, or a "$" because it begins with
> "${CMAKE_INSTALL_PREFIX}"... so this change is unnecessary, and in
> fact harmful in the case where DESTDIR doesn't end with a "/" but the
> install rule does begin with ${CMAKE_INSTALL_PREFIX}. (Or are we
> misreading this somehow?)
>
>
> ##########
> # Eric Noulard:
> #               CPackRPM_handleAttrDirectiveProperly | master=0 next=1
>
> The character set used in the REGEX here has errors in it: [A-Za-z\(\)-\,]
>
> 1) The comma does not need to be escaped in this character set
> context, but "\\" does if you mean to include the backslash character.

Only comma and hypen should be matched. Not backslash.

> 2) The hyphen, if meant to be matched is not matched here, but used as
> a range separator from the \) to the \,

Right really sorry I overlooked that one.

> What's the intent of the change? To handle commas and hyphens, you
> would add ",-" at the end of the existing character set. If more
> characters need to be matched, then add them, but keep the hyphen last
> or make it first or escape it in order to match it here.

I'll do that, but may be only this Week-End.

This corresponds to this bug
http://public.kitware.com/Bug/view.php?id=0013468
I did reopen it.

-- 
Erk
Le gouvernement représentatif n'est pas la démocratie --
http://www.le-message.org