llvm dev - Sep 2016 - change in CMake variable names breaks existing uses and does not conform to CMake conventions

Hi Chris and everyone else,

I just noticed that some of my builds broke due to commit 280013, as
LLVM_INCLUDE_DIRS was renamed to LLVM_INCLUDE_DIR. In and of itself, not much of
an issue as the fix is just to remove one character (in a couple of places).
However, I would like to discuss if this rename is desirable at all. Sure,
in-tree LLVM_INCLUDE_DIR is used everywhere, however not providing an
${NAME}_INCLUDE_DIRS variable goes against CMake conventions. See for example
the find scripts included with CMake itself:

https://cmake.org/cmake/help/v3.6/module/FindZLIB.html -> provides
ZLIB_INCLUDE_DIRS (and not ZLIB_INCLUDE_DIR) even though it is typically only
one directory

And the same holds true for most  other Find${NAME}.cmake scripts I had a look
at (Boost, Bullet, CUDA, Freetype, XMLRPC, Zlib). I'll admit there are 1 or
2 exceptions (such as OpenGL) but in general I think it would be better to stick
to the more common/conventional case.

Thoughts?

Regards
Johannes


Fraunhofer-Institut für Graphische Datenverarbeitung IGD
Fraunhoferstr. 5  |  64283 Darmstadt  |  Germany
Tel +49 6151 155-606  |  Fax +49 6151 155-139
johannes.mueller-roemer at igd.fraunhofer.de<mailto:johannes.mueller-roemer
at igd.fraunhofer.de> |
www.igd.fraunhofer.de<http://www.igd.fraunhofer.de>

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160901/30b86c2c/attachment.html>

Hi Everyone, (resending because I can't get the hang of Reply All)

I second Johannes.

cmake's own Find* man page suggests using the _DIRS variant "to keep
things
consistent":
https://cmake.org/cmake/help/git-master/manual/cmake-developer.7.html#standard-variable-names

Granted, we're supporting Config-mode and not Module-mode, but i think the
variable name rules should still apply for consistency.

Best
Philip

2016-09-01 13:31 GMT+02:00 Mueller-Roemer, Johannes Sebastian via llvm-dev <
llvm-dev at lists.llvm.org>:
> Hi Chris and everyone else,
>
>
>
> I just noticed that some of my builds broke due to commit 280013, as
> LLVM_INCLUDE_DIRS was renamed to LLVM_INCLUDE_DIR. In and of itself, not
> much of an issue as the fix is just to remove one character (in a couple of
> places). However, I would like to discuss if this rename is desirable at
> all. Sure, in-tree LLVM_INCLUDE_DIR is used everywhere, however not
> providing an ${NAME}_INCLUDE_DIRS variable goes against CMake conventions.
> See for example the find scripts included with CMake itself:
>
>
>
> https://cmake.org/cmake/help/v3.6/module/FindZLIB.html -> provides
> ZLIB_INCLUDE_DIRS (and not ZLIB_INCLUDE_DIR) even though it is typically
> only one directory
>
>
>
> And the same holds true for most  other Find${NAME}.cmake scripts I had a
> look at (Boost, Bullet, CUDA, Freetype, XMLRPC, Zlib). I’ll admit there are
> 1 or 2 exceptions (such as OpenGL) but in general I think it would be
> better to stick to the more common/conventional case.
>
>
>
> Thoughts?
>
>
>
> Regards
>
> Johannes
>
>
>
>
>
> Fraunhofer-Institut für Graphische Datenverarbeitung IGD
>
> Fraunhoferstr. 5  |  64283 Darmstadt  |  Germany
>
> Tel +49 6151 155-606  |  Fax +49 6151 155-139
>
> johannes.mueller-roemer at igd.fraunhofer.de | www.igd.fraunhofer.de
>
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160901/498c6d2e/attachment.html>

Hi Philip,

thanks for the hint about the man page. Totally forgot about that.

Even though Config-mode generally has different rules, we don’t truly support
that anyways as none of the exported targets have the
INTERFACE_INCLUDE_DIRECTORIES (in which case the LLVM_INCLUDE_DIRS variable
would become unnecessary anyways).

Regards,
Johannes

From: Philip Pfaffe [mailto:philip.pfaffe at gmail.com]
Sent: Thursday, September 1, 2016 14:01
To: Mueller-Roemer, Johannes Sebastian <Johannes.Sebastian.Mueller-Roemer at
igd.fraunhofer.de>
Cc: Chris Bieneman (cbieneman at apple.com) <cbieneman at apple.com>;
llvm-dev at lists.llvm.org
Subject: Re: [llvm-dev] change in CMake variable names breaks existing uses and
does not conform to CMake conventions

Hi Everyone, (resending because I can't get the hang of Reply All)

I second Johannes.

cmake's own Find* man page suggests using the _DIRS variant "to keep
things consistent":
https://cmake.org/cmake/help/git-master/manual/cmake-developer.7.html#standard-variable-names

Granted, we're supporting Config-mode and not Module-mode, but i think the
variable name rules should still apply for consistency.

Best
Philip

2016-09-01 13:31 GMT+02:00 Mueller-Roemer, Johannes Sebastian via llvm-dev
<llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>>:
Hi Chris and everyone else,

I just noticed that some of my builds broke due to commit 280013, as
LLVM_INCLUDE_DIRS was renamed to LLVM_INCLUDE_DIR. In and of itself, not much of
an issue as the fix is just to remove one character (in a couple of places).
However, I would like to discuss if this rename is desirable at all. Sure,
in-tree LLVM_INCLUDE_DIR is used everywhere, however not providing an
${NAME}_INCLUDE_DIRS variable goes against CMake conventions. See for example
the find scripts included with CMake itself:

https://cmake.org/cmake/help/v3.6/module/FindZLIB.html -> provides
ZLIB_INCLUDE_DIRS (and not ZLIB_INCLUDE_DIR) even though it is typically only
one directory

And the same holds true for most  other Find${NAME}.cmake scripts I had a look
at (Boost, Bullet, CUDA, Freetype, XMLRPC, Zlib). I’ll admit there are 1 or 2
exceptions (such as OpenGL) but in general I think it would be better to stick
to the more common/conventional case.

Thoughts?

Regards
Johannes


Fraunhofer-Institut für Graphische Datenverarbeitung IGD
Fraunhoferstr. 5  |  64283 Darmstadt  |  Germany
Tel +49 6151 155-606<tel:%2B49%206151%20155-606>  |  Fax +49 6151
155-139<tel:%2B49%206151%20155-139>
johannes.mueller-roemer at igd.fraunhofer.de<mailto:johannes.mueller-roemer
at igd.fraunhofer.de> |
www.igd.fraunhofer.de<http://www.igd.fraunhofer.de>


_______________________________________________
LLVM Developers mailing list
llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160901/7502477a/attachment.html>