COMP: Set `DOWNLOAD_EXTRACT_TIMESTAMP` to `TRUE` in wrapping modules #3709

jhlegarreta · 2022-10-29T20:32:39Z

Set DOWNLOAD_EXTRACT_TIMESTAMP to TRUE: the timestamps of the
extracted files will reflect the timestamps in the archive.

Configure/autoconf based projects like SWIG and PCRE have timestamped
files for their auto configuration dependencies. Thus, they depend on
the timestamps of the extracted files, and the external projects require
setting DOWNLOAD_EXTRACT_TIMESTAMP to TRUE to build robustly.

Set the CMP0135 policy to NEW for the CastXML CMake-based project.

Suppresses CMake warnings in wrapping modules.

Fixes:

CMake Warning (dev) at /usr/local/share/cmake-3.24/Modules/ExternalProject.cmake:3074 (message):
  The DOWNLOAD_EXTRACT_TIMESTAMP option was not given and policy CMP0135 is
  not set.  The policy's OLD behavior will be used.  When using a URL
  download, the timestamps of extracted files should preferably be that of
  the time of extraction, otherwise code that depends on the extracted
  contents might not be rebuilt if the URL changes.  The OLD behavior
  preserves the timestamps from the archive instead, but this is usually not
  what you want.  Update your project to the NEW behavior or specify the
  DOWNLOAD_EXTRACT_TIMESTAMP option with a value of true to avoid this
  robustness issue.
Call Stack (most recent call first):
  /usr/local/share/cmake-3.24/Modules/ExternalProject.cmake:4170 (_ep_add_download_command)
  Wrapping/Generators/SwigInterface/CMakeLists.txt:142 (ExternalProject_Add)
This warning is for project developers.  Use -Wno-dev to suppress it.

-- Found BISON: /usr/bin/bison (found version "3.5.1")
CMake Warning (dev) at /usr/local/share/cmake-3.24/Modules/ExternalProject.cmake:3074 (message):
  The DOWNLOAD_EXTRACT_TIMESTAMP option was not given and policy CMP0135 is
  not set.  The policy's OLD behavior will be used.  When using a URL
  download, the timestamps of extracted files should preferably be that of
  the time of extraction, otherwise code that depends on the extracted
  contents might not be rebuilt if the URL changes.  The OLD behavior
  preserves the timestamps from the archive instead, but this is usually not
  what you want.  Update your project to the NEW behavior or specify the
  DOWNLOAD_EXTRACT_TIMESTAMP option with a value of true to avoid this
  robustness issue.
Call Stack (most recent call first):
  /usr/local/share/cmake-3.24/Modules/ExternalProject.cmake:4170 (_ep_add_download_command)
  Wrapping/Generators/SwigInterface/CMakeLists.txt:243 (ExternalProject_Add)
This warning is for project developers.  Use -Wno-dev to suppress it.

CMake Warning (dev) at /usr/local/share/cmake-3.24/Modules/ExternalProject.cmake:3074 (message):
  The DOWNLOAD_EXTRACT_TIMESTAMP option was not given and policy CMP0135 is
  not set.  The policy's OLD behavior will be used.  When using a URL
  download, the timestamps of extracted files should preferably be that of
  the time of extraction, otherwise code that depends on the extracted
  contents might not be rebuilt if the URL changes.  The OLD behavior
  preserves the timestamps from the archive instead, but this is usually not
  what you want.  Update your project to the NEW behavior or specify the
  DOWNLOAD_EXTRACT_TIMESTAMP option with a value of true to avoid this
  robustness issue.
Call Stack (most recent call first):
  /usr/local/share/cmake-3.24/Modules/ExternalProject.cmake:4170 (_ep_add_download_command)
  Wrapping/Generators/CastXML/CMakeLists.txt:63 (ExternalProject_Add)
This warning is for project developers.  Use -Wno-dev to suppress it.

raised for example in:
https://open.cdash.org/build/8245525/configure

Related documentation:
https://cmake.org/cmake/help/latest/module/ExternalProject.html?highlight=download_extract_timestamp
https://cmake.org/cmake/help/latest/policy/CMP0135.html

PR Checklist

No API changes were made (or the changes have been approved)
No major design changes were made (or the changes have been approved)

jhlegarreta · 2022-10-29T20:34:08Z

Not sure if there is a way to inherit them from
e3d6c94#diff-1e7de1ae2d059d21e1dd75d5812d5a34b0222cef273b7c3a2af62eb747f9d20aR23
I put the statements as close as possible to the lines triggering the warnings. Let me know if there is a better place if the above cannot be done.

jhlegarreta · 2022-10-30T01:04:26Z

The warning is no longer present:

Previously:
ITK.Linux.Python: https://dev.azure.com/itkrobotlinuxpython/ITK.Linux.Python/_build/results?buildId=7740&view=logs&j=0da5d1d9-276d-5173-c4c4-9d4d4ed14fdb&t=f58be928-45a1-58e7-c1ad-04869565b3f4&l=1145
ITK.macOS.Python: https://dev.azure.com/itkrobotmacospython/ITK.macOS.Python/_build/results?buildId=7812&view=logs&j=a5e52b91-c83f-5429-4a68-c246fc63a4f7&t=5add3200-7efa-59c0-0a45-43bce0c77c69&l=1124

Now:
ITK.Linux.Python: https://dev.azure.com/itkrobotlinuxpython/ITK.Linux.Python/_build/results?buildId=7741&view=logs&j=0da5d1d9-276d-5173-c4c4-9d4d4ed14fdb&t=f58be928-45a1-58e7-c1ad-04869565b3f4&l=1144
ITK.macOS.Python: https://dev.azure.com/itkrobotmacospython/ITK.macOS.Python/_build/results?buildId=7813&view=logs&j=a5e52b91-c83f-5429-4a68-c246fc63a4f7&t=5add3200-7efa-59c0-0a45-43bce0c77c69&l=1123

N-Dekker · 2022-10-31T12:10:51Z

Thanks for addressing this warning Jon. Just looking at https://cmake.org/cmake/help/latest/policy/CMP0135.html Is the new CMake default behavior indeed preferable, for external projects CastXML and PCRE?

dzenanz · 2022-10-31T12:15:28Z

As those are usually extracted only once, it should probably not matter?

jhlegarreta · 2022-10-31T12:22:37Z

Thanks for addressing this warning Jon. Just looking at https://cmake.org/cmake/help/latest/policy/CMP0135.html Is the new CMake default behavior indeed preferable, for external projects CastXML and PCRE?

TBH, I do not know what the impact of the OLD vs. NEW policy would be: i.e. if timestamps are checked somehow. Maybe to download again the contents depending on its value? Not sure. In any case, I believe we should have no warnings. If setting the DOWNLOAD_EXTRACT_TIMESTAMP variable is preferred over this, I'd happy to hear the rationale/suggestions. Also, then the change in PR #3552 should be maybe commented as to why it should apply only to specific parts.

blowekamp · 2022-10-31T12:23:32Z

I have been encountering a new problem in SimpleITK related to automate needing to be rerun in PCRE and SWIG. It appears related to timestamps of file triggering the need to re-run. It may be related to this CMP.

So setting this policy to new may not be with out side effects. I'm currently investigating....

blowekamp · 2022-10-31T12:58:17Z

My tests are confirming that configure/autoconf based projects like SWIG and PCRE[2] depend on the time stamps of the extracted files and the external project require setting DOWNLOAD_EXTRACT_TIMESTAMP to 0 to build robustly. This should be changed in this PR.

The reported build error is related to the aclocal (or specific version of it) executable missing. Which is caused but autoconf being triggered due to timestamps. For this error to occur the system must have CMake >2.24, missing the required aclocal. Additionally the "extract time" for the extract files may vary based on the time it takes to extract the files from the archive, and the order of the files in the archive.

jhlegarreta · 2022-10-31T13:45:33Z

💯 Thanks Brad. I've amended the commit.

N-Dekker · 2022-10-31T14:05:09Z

DOWNLOAD_EXTRACT_TIMESTAMP is introduced with CMake 3.24: https://cmake.org/cmake/help/latest/release/3.24.html?highlight=download_extract_timestamp#modules Does that mean that cmake_minimum_required should then be adjusted as well?

ITK/CMakeLists.txt

Lines 13 to 15 in d81a02c

    
           set(ITK_OLDEST_VALIDATED_POLICIES_VERSION "3.16.3") 
        
           set(ITK_NEWEST_VALIDATED_POLICIES_VERSION "3.19.7") 
        
           cmake_minimum_required(VERSION ${ITK_OLDEST_VALIDATED_POLICIES_VERSION}...${ITK_NEWEST_VALIDATED_POLICIES_VERSION} FATAL_ERROR)

Wrapping/Generators/CastXML/CMakeLists.txt

blowekamp · 2022-10-31T14:13:47Z

DOWNLOAD_EXTRACT_TIMESTAMP is introduced with CMake 3.24

The minimum require version certainly should not be updated to 3.24. ExternalProject_Add appears to ignore unknown names arguments, however I am not sure this is behavior that should be relied upon?

The alternate would be conditionally assign DOWNLOAD_EXTRACT_TIMESTAMP FALSE to a variable and use that.

N-Dekker · 2022-10-31T14:22:40Z

The minimum require version certainly should not be updated to 3.24.

OK, but we now have set(ITK_NEWEST_VALIDATED_POLICIES_VERSION "3.19.7"), in the root CMakeLists.

ITK/CMakeLists.txt

Line 14 in d81a02c

set(ITK_NEWEST_VALIDATED_POLICIES_VERSION "3.19.7")

Shouldn't that already have preserved the old behavior, even when using CMake 3.24?

jhlegarreta · 2022-10-31T23:52:33Z

Shouldn't that already have preserved the old behavior, even when using CMake 3.24?

@N-Dekker According to

ITK/CMakeLists.txt

Line 10 in 49150c8

    
           # CMake versions greater than the ITK_NEWEST_VALIDATED_POLICIES_VERSION policies will

# CMake versions greater than the ITK_NEWEST_VALIDATED_POLICIES_VERSION policies will
# continue to generate policy warnings "CMake Warning (dev)...Policy CMP0XXX is not set:"

So I guess they will appear for 3.24.

Yes, for this project likely either behavior of the timestamp should be OK. However, I thought there was motivation to use the NEW CMP135 behavior where it could be used, like the castXML project. To do that the CMP135 could be set to new, and this line removed, or the value could be set to TRUE.

The minimum require version certainly should not be updated to 3.24. ExternalProject_Add appears to ignore unknown names arguments, however I am not sure this is behavior that should be relied upon?
The alternate would be conditionally assign DOWNLOAD_EXTRACT_TIMESTAMP FALSE to a variable and use that.

@blowekamp Thanks for the suggestions and directions forward. Tried to do them in 6e20e06.

The warning in the CastXML has been silenced.
They are back for SWIG though, and the variable contents are not properly parsed by ExternalProject_Add:
https://open.cdash.org/build/8248990/configure

CMake Error at /usr/local/share/cmake-3.24/Modules/ExternalProject.cmake:3479 (get_property):
  get_property could not find TARGET DOWNLOAD_EXTRACT_TIMESTAMP FALSE.
  Perhaps it has not yet been created.
Call Stack (most recent call first):
  /usr/local/share/cmake-3.24/Modules/ExternalProject.cmake:3661 (_ep_get_file_deps)
  /usr/local/share/cmake-3.24/Modules/ExternalProject.cmake:4173 (_ep_add_configure_command)
  Wrapping/Generators/SwigInterface/CMakeLists.txt:254 (ExternalProject_Add)

The variable value seems to be OK, though. Not sure what's going on here; I run out of ideas.

Wrapping/Generators/CastXML/CMakeLists.txt

thewtex

Outstanding, thank you so much for addressing this, @jhlegarreta !

blowekamp · 2022-11-23T12:53:39Z

@jhlegarreta I apologize. It appears I had the logic of DOWNLOAD_EXTRACT_TIMESTAMP backwards. I corrected it in SimpleITK, but not here.

DOWNLOAD_EXTRACT_TIMESTAMP should be set to TRUE for SWIG and PCRE.

I have reread the documentation for the CMP0135 and it's clarity should be improved.. Setting DOWNLOAD_EXTRACT_TIMESTAMP to TRUE is the OLD behavior. The OLD behaviors preserved the timestamp in the archive. This is required for SWIG and PCRE which have timed stamped files for autoconf dependencies.

@bradking FYI for CMake maintainer.

Set `DOWNLOAD_EXTRACT_TIMESTAMP` to `TRUE`: the timestamps of the extracted files will reflect the timestamps in the archive. Configure/autoconf based projects like SWIG and PCRE have timestamped files for their auto configuration dependencies. Thus, they depend on the timestamps of the extracted files, and the external projects require setting `DOWNLOAD_EXTRACT_TIMESTAMP` to `TRUE` to build robustly. Set the `CMP0135` policy to `NEW` for the CastXML CMake-based project. Suppresses CMake warnings in wrapping modules. Fixes: ``` CMake Warning (dev) at /usr/local/share/cmake-3.24/Modules/ExternalProject.cmake:3074 (message): The DOWNLOAD_EXTRACT_TIMESTAMP option was not given and policy CMP0135 is not set. The policy's OLD behavior will be used. When using a URL download, the timestamps of extracted files should preferably be that of the time of extraction, otherwise code that depends on the extracted contents might not be rebuilt if the URL changes. The OLD behavior preserves the timestamps from the archive instead, but this is usually not what you want. Update your project to the NEW behavior or specify the DOWNLOAD_EXTRACT_TIMESTAMP option with a value of true to avoid this robustness issue. Call Stack (most recent call first): /usr/local/share/cmake-3.24/Modules/ExternalProject.cmake:4170 (_ep_add_download_command) Wrapping/Generators/SwigInterface/CMakeLists.txt:142 (ExternalProject_Add) This warning is for project developers. Use -Wno-dev to suppress it. -- Found BISON: /usr/bin/bison (found version "3.5.1") CMake Warning (dev) at /usr/local/share/cmake-3.24/Modules/ExternalProject.cmake:3074 (message): The DOWNLOAD_EXTRACT_TIMESTAMP option was not given and policy CMP0135 is not set. The policy's OLD behavior will be used. When using a URL download, the timestamps of extracted files should preferably be that of the time of extraction, otherwise code that depends on the extracted contents might not be rebuilt if the URL changes. The OLD behavior preserves the timestamps from the archive instead, but this is usually not what you want. Update your project to the NEW behavior or specify the DOWNLOAD_EXTRACT_TIMESTAMP option with a value of true to avoid this robustness issue. Call Stack (most recent call first): /usr/local/share/cmake-3.24/Modules/ExternalProject.cmake:4170 (_ep_add_download_command) Wrapping/Generators/SwigInterface/CMakeLists.txt:243 (ExternalProject_Add) This warning is for project developers. Use -Wno-dev to suppress it. CMake Warning (dev) at /usr/local/share/cmake-3.24/Modules/ExternalProject.cmake:3074 (message): The DOWNLOAD_EXTRACT_TIMESTAMP option was not given and policy CMP0135 is not set. The policy's OLD behavior will be used. When using a URL download, the timestamps of extracted files should preferably be that of the time of extraction, otherwise code that depends on the extracted contents might not be rebuilt if the URL changes. The OLD behavior preserves the timestamps from the archive instead, but this is usually not what you want. Update your project to the NEW behavior or specify the DOWNLOAD_EXTRACT_TIMESTAMP option with a value of true to avoid this robustness issue. Call Stack (most recent call first): /usr/local/share/cmake-3.24/Modules/ExternalProject.cmake:4170 (_ep_add_download_command) Wrapping/Generators/CastXML/CMakeLists.txt:63 (ExternalProject_Add) This warning is for project developers. Use -Wno-dev to suppress it. ``` raised for example in: https://open.cdash.org/build/8245525/configure Related documentation: https://cmake.org/cmake/help/latest/module/ExternalProject.html?highlight=download_extract_timestamp https://cmake.org/cmake/help/latest/policy/CMP0135.html

jhlegarreta · 2022-11-23T14:28:21Z

@blowekamp It's fine, thanks Brad. I should have done a better job understanding as well. I've amended the commit, changed the subject and message body according to what you described. See if it now matches the appropriate behavior.

dzenanz · 2022-11-23T14:40:04Z

@blowekamp For an open-source project like CMake, if you think that documentation should be improved, the usual response/expectation is to propose the change you think would improve it. I took a look at the doc you pointed out, and I didn't an obvious improvement opportunity.

jhlegarreta · 2022-11-23T18:36:56Z

Warnings are gone:
ITK.Linux.Python: https://dev.azure.com/itkrobotlinuxpython/ITK.Linux.Python/_build/results?buildId=7862&view=logs&j=0da5d1d9-276d-5173-c4c4-9d4d4ed14fdb&t=f58be928-45a1-58e7-c1ad-04869565b3f4&l=1150
ITK.macOS.Python: https://dev.azure.com/itkrobotmacospython/ITK.macOS.Python/_build/results?buildId=7932&view=logs&j=a5e52b91-c83f-5429-4a68-c246fc63a4f7&t=5add3200-7efa-59c0-0a45-43bce0c77c69&l=1129
(the warnings appeared around those lines)

https://open.cdash.org/index.php?project=Insight#!#Experimental

github-actions bot added type:Compiler Compiler support or related warnings type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots labels Oct 29, 2022

dzenanz approved these changes Oct 31, 2022

View reviewed changes

jhlegarreta force-pushed the FixWrappingCMakePolicyWarnings branch from e7072ad to 4622287 Compare October 31, 2022 13:37

jhlegarreta changed the title ~~COMP: Set CMake Policy 135 to NEW to suppress warnings in wrapping~~ COMP: Set DOWNLOAD_EXTRACT_TIMESTAMP to FALSE in wrapping modules Oct 31, 2022

jhlegarreta force-pushed the FixWrappingCMakePolicyWarnings branch from 4622287 to 0277e73 Compare October 31, 2022 13:40

dzenanz approved these changes Oct 31, 2022

View reviewed changes

blowekamp reviewed Oct 31, 2022

View reviewed changes

Wrapping/Generators/CastXML/CMakeLists.txt Outdated Show resolved Hide resolved

jhlegarreta force-pushed the FixWrappingCMakePolicyWarnings branch 4 times, most recently from ac40322 to 6e20e06 Compare October 31, 2022 23:38

blowekamp reviewed Nov 1, 2022

View reviewed changes

Wrapping/Generators/CastXML/CMakeLists.txt Show resolved Hide resolved

jhlegarreta force-pushed the FixWrappingCMakePolicyWarnings branch 3 times, most recently from 729dac8 to 8cd8afc Compare November 23, 2022 01:27

thewtex approved these changes Nov 23, 2022

View reviewed changes

jhlegarreta force-pushed the FixWrappingCMakePolicyWarnings branch from 8cd8afc to 331b474 Compare November 23, 2022 14:25

jhlegarreta changed the title ~~COMP: Set DOWNLOAD_EXTRACT_TIMESTAMP to FALSE in wrapping modules~~ COMP: Set DOWNLOAD_EXTRACT_TIMESTAMP to TRUE in wrapping modules Nov 23, 2022

dzenanz approved these changes Nov 23, 2022

View reviewed changes

thewtex added this to the ITK 5.3.0 milestone Nov 23, 2022

thewtex mentioned this pull request Nov 23, 2022

Update vxl 2022-11-15 #3740

Merged

2 tasks

dzenanz merged commit 0bfebf5 into InsightSoftwareConsortium:master Nov 23, 2022

jhlegarreta deleted the FixWrappingCMakePolicyWarnings branch November 23, 2022 18:43

jhlegarreta mentioned this pull request Nov 24, 2022

COMP: Set DOWNLOAD_EXTRACT_TIMESTAMP to TRUE for Windows wrapping #3763

Merged

2 tasks

jhlegarreta mentioned this pull request Jan 4, 2023

CMake reporting DOWNLOAD_EXTRACT_TIMESTAMP option and CMP0135 policy-related warning for remotes #3846

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

COMP: Set `DOWNLOAD_EXTRACT_TIMESTAMP` to `TRUE` in wrapping modules #3709

COMP: Set `DOWNLOAD_EXTRACT_TIMESTAMP` to `TRUE` in wrapping modules #3709

jhlegarreta commented Oct 29, 2022 •

edited

Loading

jhlegarreta commented Oct 29, 2022

jhlegarreta commented Oct 30, 2022 •

edited

Loading

N-Dekker commented Oct 31, 2022

dzenanz commented Oct 31, 2022

jhlegarreta commented Oct 31, 2022

blowekamp commented Oct 31, 2022

blowekamp commented Oct 31, 2022

jhlegarreta commented Oct 31, 2022

N-Dekker commented Oct 31, 2022

blowekamp commented Oct 31, 2022

N-Dekker commented Oct 31, 2022

jhlegarreta commented Oct 31, 2022 •

edited

Loading

thewtex left a comment

blowekamp commented Nov 23, 2022

jhlegarreta commented Nov 23, 2022

dzenanz commented Nov 23, 2022

jhlegarreta commented Nov 23, 2022

COMP: Set DOWNLOAD_EXTRACT_TIMESTAMP to TRUE in wrapping modules #3709

COMP: Set DOWNLOAD_EXTRACT_TIMESTAMP to TRUE in wrapping modules #3709

Conversation

jhlegarreta commented Oct 29, 2022 • edited Loading

PR Checklist

jhlegarreta commented Oct 29, 2022

jhlegarreta commented Oct 30, 2022 • edited Loading

N-Dekker commented Oct 31, 2022

dzenanz commented Oct 31, 2022

jhlegarreta commented Oct 31, 2022

blowekamp commented Oct 31, 2022

blowekamp commented Oct 31, 2022

jhlegarreta commented Oct 31, 2022

N-Dekker commented Oct 31, 2022

blowekamp commented Oct 31, 2022

N-Dekker commented Oct 31, 2022

jhlegarreta commented Oct 31, 2022 • edited Loading

thewtex left a comment

Choose a reason for hiding this comment

blowekamp commented Nov 23, 2022

jhlegarreta commented Nov 23, 2022

dzenanz commented Nov 23, 2022

jhlegarreta commented Nov 23, 2022

COMP: Set `DOWNLOAD_EXTRACT_TIMESTAMP` to `TRUE` in wrapping modules #3709

COMP: Set `DOWNLOAD_EXTRACT_TIMESTAMP` to `TRUE` in wrapping modules #3709

jhlegarreta commented Oct 29, 2022 •

edited

Loading

jhlegarreta commented Oct 30, 2022 •

edited

Loading

jhlegarreta commented Oct 31, 2022 •

edited

Loading