[PATCH] D35377: [CMAKE] PR33717 When LLVM_APPEND_VC_REV = OFF, only create VCSRevision.h if it doesn't already exist.

[David Blaikie via llvm-commits]

I'm OK if this already does the right thing - but would like to know that,
if you could check :) Otherwise, yeah, adding the empty-test would be good,
I would think.

On Thu, Jul 13, 2017 at 1:03 PM don hinton <hintonda at gmail.com> wrote:

> I think so, but we could also check to see if it's empty.  That way you'll
> always have an empty file if it's OFF.
>
> On Thu, Jul 13, 2017 at 12:51 PM, David Blaikie <dblaikie at gmail.com>
> wrote:
>
>> Does this do the correct/desired thing if LLVM_APPEND_VC_REV is changed
>> from ON to OFF - or does it leave the last rev in the file indefinitely?
>> That could eb confusing to developers.
>>
>>
>> On Thu, Jul 13, 2017 at 12:40 PM don hinton via Phabricator <
>> reviews at reviews.llvm.org> wrote:
>>
>>> hintonda created this revision.
>>> Herald added a subscriber: mgorny.
>>>
>>> If LLVM_APPEND_VC_REV = OFF, find_first_existing_vc_file will
>>> not set llvm_vc and VCSRevision.h will be rewritten every time cmake
>>> is run, which causes everything VCSRevision.h depends on to get
>>> rebuilt and relinked even if nothing else changed.
>>>
>>>
>>> https://reviews.llvm.org/D35377
>>>
>>> Files:
>>>   include/llvm/Support/CMakeLists.txt
>>>
>>>
>>> Index: include/llvm/Support/CMakeLists.txt
>>> ===================================================================
>>> --- include/llvm/Support/CMakeLists.txt
>>> +++ include/llvm/Support/CMakeLists.txt
>>> @@ -55,7 +55,9 @@
>>>      PROPERTIES GENERATED TRUE
>>>                 HEADER_FILE_ONLY TRUE)
>>>  else()
>>> -  file(WRITE "${version_inc}" "")
>>> +  if(NOT EXISTS "${version_inc}")
>>> +    file(WRITE "${version_inc}" "")
>>> +       endif()
>>>  endif()
>>>
>>>  add_custom_target(llvm_vcsrevision_h DEPENDS "${version_inc}")
>>>
>>>
>>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170713/6cad38e6/attachment.html>