mbedtls
mbedtls copied to clipboard
Mbed-TLS
Copy files instead of hard-linking on Windows
Description
Fixes an issue on Windows where when the source and build directory are on different drives hard-linking to files or directory fails as it doesn't work across filesystem boundaries. Note also that symlinking is also not possible because it requires administrator privileges on Windows.
~~The solution copies the files using the built-in cmake file(COPY) command.~~
The solution copies the files using the built-in cmake configure_file(src dest COPYONLY) command.
As this command only operates on files, if a directory is specified the files will be globbed recursively
and through symlinks.
Status
READY
Additional comments
Fixes #5751
Todos
- [x] Changelog updated
@N3xed Would you consider using configure_file(src dest COPY_ONLY) instead of file(COPY)?
I think file(COPY) has two downsides:
- It will update the destination file modification time if CMake configure stage runs again, which will result in the source file being rebuilt, even if the content is the same.
- It will not detect changes to the original file and won't copy it again (unless CMake configure stage runs).
configure_file(src dest COPY_ONLY) would address both issues.
@N3xed Would you consider using
configure_file(src dest COPY_ONLY)instead offile(COPY)?
I think this would be better, indeed. But as a result, when a directory is specified, the files are now globbed because configure_file doesn't work with directories.
Additionally, instead of copying every time on Windows, we could check if the ${target} and ${link} are on the same drive and then hard-link instead. I've opted not to do this since this would complicate it even more and the current implementation already works. I could implement that as well, though, if requested.
I've opted not to do this since this would complicate it even more and the current implementation already works. I could implement that as well, though, if requested.
Thant makes sense to me, I think copying is simpler and more reliable than detecting if files are on the same drive.
Looks like this version passed CI, so at least it doesn't break anything it seems.
From the perspective of the esp-idf project where the issue was originally reported, this change looks good to me. Hope it can be merged upstream.
