Symlinking issues on Windows - Compile fails if build and src folder is on different drives.

[ESP-Marius]

Summary

After Generated files cmake was merged it seems like it is no longer possible to build the project on Windows if the source tree and the build tree are on different drives.

error.c and version_features.c are now always linked with link_to_source, but this function fails to create symlinks if the build conditions are as described earlier.

Error log:

D:\mbedtls>cmake C:\Users\Marius\Downloads\mbedtls-3.1.0\mbedtls-3.1.0
-- Building for: Visual Studio 15 2017
-- Selecting Windows SDK version 10.0.17763.0 to target Windows 10.0.19043.
-- The C compiler identification is MSVC 19.16.27045.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: C:/Program Files (x86)/Microsoft Visual Studio/2017/BuildTools/VC/Tools/MSVC/14.16.27023/bin/Hostx86/x86/cl.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Found Python3: C:/Python39/python.exe (found version "3.9.5") found components: Interpreter
CMake Error at CMakeLists.txt:138 (message):
  Could not create symbolic link for:
  C:\Users\Marius\Downloads\mbedtls-3.1.0\mbedtls-3.1.0\library\error.c -->
  The system cannot move the file to a different disk drive.

Call Stack (most recent call first):
  library/CMakeLists.txt:162 (link_to_source)

System information

Mbed TLS version (number or commit id): Release "Mbed TLS 3.1.0"
Operating system and version: Windows 10

Expected behavior

Fails during compilation due to unable to create symlinks.

Actual behavior

Successfully compiles.

Steps to reproduce

Run cmake /path/to/mbedtls_source from a folder located on a different drive from the source tree.

[tom-cosgrove-arm]

The problem here is that on Windows we are actually creating hard links to these files, rather than the symbolic links claimed by the comment in front of the link_to_source() definition.

This was changed by commit

commit 2a1edacb1b49fc17729527d2bc8f829d5f03bb84
Author: Darryl Green <darryl.green@arm.com>
Date:   Fri Jun 8 10:07:32 2018 +0100

    Change symlink to hardlink to avoid permission issues

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 764149182..b9a0ce02d 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -70,7 +70,7 @@ function(link_to_source base_name)
             if (IS_DIRECTORY ${target})
                 set(command cmd.exe /c mklink /j ${link} ${target})
             else()
-                set(command cmd.exe /c mklink ${link} ${target})
+                set(command cmd.exe /c mklink /h ${link} ${target})
             endif()
         endif()

merged as part of #1699 in 2018.

This suggests that we either use symbolic links and require elevated permissions to build on Windows, or keep hard links but then can't build on a different volume.

[ESP-Marius]

This came up as an issue in esp-idf, as some users are no longer able to build their project after the mbedtls version used in SDK was upgraded.

Maybe someone could help me understand why we need to link_to_source for e.g. error.c? As far as I could see this linked file is never used anywhere?

[tom-cosgrove-arm]

@mpg or @gilles-peskine-arm can you answer the question ^ above? This relates to the comment https://github.com/Mbed-TLS/mbedtls/pull/4514/files#r707221450 in #4514

[gilles-peskine-arm]

Some files, like tests/ssl-opt.sh and tests/data_files/*, need to be available in the build tree because the build scripts and test scripts don't support accessing some files from the source tree and some from the build tree. There needs to be a directory from which both tests/data_files/server2.key and programs/ssl/ssl_server2 are available. Since those files are source files, not generated, we avoid the hassle of copying them and instead link them (linking the whole directory where applicable), so that the build tree always has an up-to-date (and complete, for directories) version.

Regarding error.c and version_features.c specifically, I'm not sure they need to be present in the build tree. This was done in #4514, and the ostensible reason is to avoid modifying the source tree per general hygiene of out-of-tree builds, not for a technical reason. But for all I know there may be a technical reason as well.

[ESP-Marius]

Thanks for the quick feedback from both of you!

Linking it to the build-tree makes total sense, it was just that I couldn't actually see this linked/generated file being used anywhere in the build process that makes me a bit confused.

If being to able to compile on a separate volume is not something you will prioritize supporting (which is understandable!) we might just patch out this link_to_source in our fork of mbedtls. As long as I can't identify a good reason why we need to keep at least😄

[mpg]

Regarding error.c and version_features.c specifically, I'm not sure they need to be present in the build tree. This was done in #4514, and the ostensible reason is to avoid modifying the source tree per general hygiene of out-of-tree builds, not for a technical reason. But for all I know there may be a technical reason as well.

I don't think there's another reason. IIRC the first version of this PR was putting (some of) the generated files in the source tree and it was working fine, so I think the reason we're generating them in the build tree just trying keep the source tree clean as matter of principle. (Like, in principle your source tree could read-only and that shouldn't prevent you from building out-of-tree.)

Now, for uniformity between builds where error.c (etc) are generated during build, and those where they are present in the source tree, we link them to the build tree (instead of generating them) if they're present. That's really just for convenience, so it could probably be removed.

[gilles-peskine-arm]

Mbed TLS maintenance tends to attract people who are familiar with C and cryptography but often not with Windows and CMake. So it's quite possible that we're doing the wrong thing and not realizing it.

In terms of priorities, Windows is not our most important build platforms. But if you have a patch that works for you, and it doesn't seem to break other uses, we would welcome it. Since this is a somewhat unusual case on a non-first-tier platform, we wouldn't insist on a non-regression test.

[gilles-peskine-arm]

This was already an issue in 2.2x, and still is in 2.28.

Windows 10, cmake 3.25.1, Mbed TLS at 3917028 (shortly before mbedtls-2.28.2), c: is NTFS, z: is a VirtualBox drive:

C:\Temp\2.28>cmake z:\z\dev\mbedtls\2.2x -G "MinGW Makefiles"
-- The C compiler identification is GNU 5.3.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: C:/opt/mingw/bin/gcc.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Found Python3: C:/Program Files/python/python.exe (found version "3.7.2") found components: Interpreter
-- Performing Test C_COMPILER_SUPPORTS_WFORMAT_SIGNEDNESS
-- Performing Test C_COMPILER_SUPPORTS_WFORMAT_SIGNEDNESS - Success
CMake Error at CMakeLists.txt:167 (message):
  Could not create symbolic link for: Z:\z\dev\mbedtls\2.2x\include\mbedtls
  --> Local volumes are required to complete the operation.

Call Stack (most recent call first):
  include/CMakeLists.txt:20 (link_to_source)


-- Configuring incomplete, errors occurred!
See also "C:/Temp/2.28/CMakeFiles/CMakeOutput.log".

In contrast the same command works if I point it at 80f8c67 (shortly before mbedtls-3.3, with #6188), or at 2.28 plus a cherry-pick of the one commit of #6188.