Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build(cmake): set download_extract_timestamp to true #936

Conversation

jhlegarreta
Copy link
Member

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

Fixes:

CMake Warning (dev) at D:/a/_temp/-2103337693/cmake-3.24.2-windows-x86_64/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.

raised for exmaple in:
https://open.cdash.org/build/8983547/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 jhlegarreta force-pushed the FixExtractTimestampCMakePolicyWarnings branch from 32cf239 to 7b83ec5 Compare September 16, 2023 23:18
@jhlegarreta jhlegarreta changed the title build(CMake): set download_extract_timestamp to true build(cmake): set download_extract_timestamp to true Sep 16, 2023
@jhlegarreta
Copy link
Member Author

A number of other CMake warnings are present in the GHA workflow log:
https://github.com/InsightSoftwareConsortium/itk-wasm/actions/runs/5921543455/job/16054231396#step:7:132

But are not reported to CDash. Not sure why.

They may well deserve some attention.

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

Fixes:
```
CMake Warning (dev) at D:/a/_temp/-2103337693/cmake-3.24.2-windows-x86_64/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.
```

raised for exmaple in:
https://open.cdash.org/build/8983547/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 jhlegarreta force-pushed the FixExtractTimestampCMakePolicyWarnings branch from 7b83ec5 to 5ca66d6 Compare September 16, 2023 23:23
set(download_extract_timestamp_flag)
else()
set(download_extract_timestamp_flag DOWNLOAD_EXTRACT_TIMESTAMP TRUE)
endif()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is the appropriate place within the file.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

works!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolving then.

Copy link
Member Author

@jhlegarreta jhlegarreta Sep 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

@thewtex thewtex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jhlegarreta thank you so much! 💯

set(download_extract_timestamp_flag)
else()
set(download_extract_timestamp_flag DOWNLOAD_EXTRACT_TIMESTAMP TRUE)
endif()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

works!

@thewtex
Copy link
Member

thewtex commented Sep 18, 2023

Failing CI check addressed in #940.

@thewtex thewtex merged commit 343da2c into InsightSoftwareConsortium:main Sep 18, 2023
@jhlegarreta jhlegarreta deleted the FixExtractTimestampCMakePolicyWarnings branch September 18, 2023 22:19
@github-actions
Copy link

🎉 This PR is included in version 1.0.0-b.139 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants