[llvm-commits] [PATCH][CMake] PR10050.

[arrowdodger]

On Thu, Jan 19, 2012 at 8:06 PM, arrowdodger <6yearold at gmail.com> wrote:

> On Thu, Jan 12, 2012 at 6:28 PM, arrowdodger <6yearold at gmail.com> wrote:
>
>> On Sun, Jan 8, 2012 at 3:36 PM, arrowdodger <6yearold at gmail.com> wrote:
>>
>>> On Mon, Jan 2, 2012 at 9:36 PM, Chandler Carruth <chandlerc at google.com>wrote:
>>>
>>>> Thanks, but there are some real problems with how this is structured.
>>>> The first two are easy to fix, but the last may not be.
>>>>
>>>> 1) The macros are strangely formed in a few ways. First, please avoid
>>>> magical arrays filled in with the results of your macro. They're very hard
>>>> to understand. I'd much rather that each macro produces a new target which
>>>> other things can then depend on, or populates an array the name of which is
>>>> passed into the macro. Also, why make the user write a foreach loop around
>>>> the files? It seems like you could have them provide a list of files rather
>>>> than a single file, and do the looping inside the macro. Finally, please
>>>> allow the user to specify the actual filename rather than the basename of
>>>> the podfile. Compute the basename yourself, as this will make both the use
>>>> of the macro easier to understand and it's implementation more clear.
>>>>
>>>
>>> Does it looks better now?
>>>
>>>
>>>> 2) Please use something more distinct such as 'llvm-docs-*' as the
>>>> custom target names; 'llvm-ps' looks too much like it could be one of the
>>>> actual LLVM tools.
>>>>
>>>
>>> Done.
>>>
>>>
>>>> 3) I don't understand why you can't use normal CMake dependencies to
>>>> connect the install rules to the actual build steps. It seems like you
>>>> could use install(FILES ${HTML_FILES} ...) or some equivalent construct.
>>>> CMake should be handling the dependency computations and ensuring that
>>>> those files are build for 'make install'.
>>>>
>>>
>>> Yeah, it should. But it does not. I've even asked on StackOverflow about
>>> this.
>>>
>>>
>>>> If CMake cannot do this naturally with its own dependency information,
>>>> then I fear it may not be worth adding this to the CMake builds, and they
>>>> will be less functional than the autotools builds. I think re-executing the
>>>> make tool is an unacceptable design wart.
>>>>
>>>
>>> Why do you think so? Isn't that hack is what CMake should actually do -
>>> just call `make <target>` during `make install`? If CMake fix it, this hack
>>> can be removed by altering only 3 lines in AddLLVM.cmake.
>>>
>>> I also have trouble believing CMake can't represent this pattern, but I
>>>> haven't tried my hand at it...
>>>>
>>>
>>> If you find cleaner way to solve this, i would be happy to redo the
>>> patch.
>>>
>>
>> Bump.
>>
>
> Another bump.
>

Chandler, you aren't interested in this? Should i stop spamming maillist?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120126/0142fb4a/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clang.cmake.docs.patch
Type: text/x-patch
Size: 4064 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120126/0142fb4a/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: llvm.cmake.docs.patch
Type: text/x-patch
Size: 8875 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120126/0142fb4a/attachment-0001.bin>