Fix translation map issues

mentioned in merge request !540 (merged)

mentioned in merge request utils/kwsys!14 (closed)

In this context CMake also seems to be influenced by the PWD environment variable. In particular, if getcwd() does not change, but PWD does, CMake forces a rebuild. This can happen easily if cmake is called from anything other than a shell script. For example, python -c "import subprocess; subprocess.check_call(['cmake', '--build', '.'], cwd='/path/to/project')" will execute CMake in /path/to/project, while PWD still contains the working directory of the caller. As a workaround, I am now setting PWD in addition to cwd, but I don't see why this should be necessary. CMake should use getcwd() to determine the current working directory and it should ignore the PWD environment variable. It is only meaningful in the context of a shell.

My branch checks whether PWD is valid or not by comparing real paths with getcwd(). PWD is ignored if they don't agree. The reason that PWD is cared about at all is that there are machines where the home directory is actually a symlink to an unstable directory path:

/home/user -> /mnt/net/asdfasdf

and the asdfasdf part is not the same across all boots and/or logins to the machine. Yes, setups such as that are annoying, but they exist and with CMake's behavior of refusing to run if the path to the source or build tree is changed, something with PWD needs to be done. When this support was initially implemented, it turns out that it has way too much influence across the behavior of CMake and this issue is to make that behavior have as narrow a scope as possible.

We've sketched out a solution that needs to be implemented yet (it's a bit tedious to type out here right now; it's on a whiteboard) that basically tries to salvage whatever it can from PWD rather than just using it blindly.

Transferring this from my whiteboard:

def compute_logical_path(given_path):
    path = os.path.join(os.environ.get('PWD', os.getcwd()), given_path)
    logical_path = '/'
    for component in path.split('/'):
        if component == '..':
            working_path = real_path(os.path.join(logical_path, component))

            # I'm not 100% sure about the logic that follows here (the whiteboard is unclear); tests will need to be performed.
            #     if rp(lp') is prefix of wp, done <---\
            #     else strip 1 component of lp'    ----/
            real_logical_path = real_path(logical_path)
            if working_path.has_path_prefix(real_logical_path):
                return real_path(path)
            else:
                logical_path = os.path.dirname(logical_path)
        elif components == '.':
            pass
        else:
            logical_path = os.path.join(logical_path, component)
    return logical_path

There may also be an additional option:

if (!cmsys::SystemTools::GetEnv("CMAKE_FORCE_REAL_PATH")) {
    builddir = compute_logical_path(input_builddir);
    srcdir = compute_logical_path(input_srcdir);
} else {
    builddir = cmsys::SystemTools::CollapseFullPath(input_builddir);
    srcdir = cmsys::SystemTools::CollapseFullPath(input_srcdir);
}

IIRC our conversation that resulted in that whiteboard algorithm identified that the goal is to produce a logical path that contains no .. components (or .) and preserves as much of the original path as possible, including symlinks. That way a path to the source tree constructed via ../src => $PWD/../src => ... will preserve the symlinks it can while removing .. components. This is sufficient for the homedir-symlink use case.

Yes, I just need to figure out what the code version of that block is. The whiteboard has some context-dependent shorthand I need to translate :) .

mentioned in issue #16736 (closed)

The current logic breaks in SystemTools::ClassInitialize() breaks building when the build directory path is contains a symlink that resolves to a subpath of that path: /home/x/prj/pkg/src/prj/part/build where /home/x/prj/pkg/src/prj is a symlink to /home/x/prj.

The eventual error is "add_subdirectory not given a binary directory but the given source diretory." The root cause is because this nasty translation map has replaced /home/x/prj/ with /home/x/proj/pkg/src/prj in paths that already contain /home/x/prj/pkg/src/prj, leading to paths /home/x/prj/pkg/src/prj/pkg/src/prj, and similar ones up containing up to three of these spurious replacements.

If I remove Realpath() call from SystemTools::ClassInitialize(), all works fine: https://gitlab.kitware.com/cmake/cmake/blob/master/Source/kwsys/SystemTools.cxx#L4672

Good enough fix?

PS. This translation map "feature" is trying to support some exotic use case at expense of breaking the trivial case. Why is cmake concerned about the mounts on a user's system? It should be up to the user to make sure their paths match across their builds if they don't want rebuilding.

@alexei The old behavior has been there for years now and I'd rather it be fixed rather than trying to implement a workaround that may break the existing support (nasty as it may be). I agree 100% with the exotic/trivial use case assessment, but it was implemented upon the request of users and wasn't fully designed before implemented and CMake workflow policy is to not break backwards compatibility whenever possible. AFAIK, this is also too low-level to be implemented behind a new policy change (all of this logic happens before any CMake code is read to know whether to do it or not).

Thanks for the reply. If removing Realpath is a breaking change, and given that this behavior happens before the code is read, then prospect for a fix seems bleak; unless the mapping behavior can be changed to not add that particular entry into the map, if this can be done in a backcompatible way (seems dubious). In my case, I relocated my build directory in the tree; luckily I could move it up, before the symlink.

The fix is to remove the translation map completely and to detect the previous use case specifically and handle it by just setting CMAKE_SOURCE_DIR and CMAKE_BINARY_DIR to the symlink-using names. All other cases would get real paths.

mentioned in issue #18001

mentioned in merge request !3307 (closed)

mentioned in issue #20515

mentioned in issue #20107

From the various comments, it is not clear what are the cases, involving symbolic links, which must be supported if translation map is removed.

Anyway, I think the current approach is, by design, erroneous because to get a logical path, the only way is to pick-up $PWD shell variable content which is not at all always defined.

IMHO, a clean approach can be:

Never use $PWD shell variable to ensure the same behavior regardless the environment
Always translate paths given to CMake to real paths except specific case (item 3)
When options -S and -B are given, specifying absolute paths, and a specific option is given (like --keep-logical-paths), use them without translation to real paths.

The option 3 enable the user to control efficiently what are the root paths used by CMake.

This approach breaks the compatibility, but again, I think it is not reasonable to try to support an erroneous design...

From an implementation point of view, to ensure uniform behavior, it is required to be able to manipulate paths from a logical point of view (not evolving file system) to ensure logical paths will not be translated to real paths unintentionally (std::filesystem::path answers to these constraints).

mentioned in merge request !4869 (closed)

@marc.chevrier the primary use case is for some environments in which one's home directory only appears stable over time due to it being a symbolic link to a machine-specific possibly-changing-on-reboot mount point, e.g. /home/user -> /mnt/xyz/usr which can become /home/user -> /mnt/abc/usr later. In this use case, cmake ../src needs to use $PWD rather than getcwd() in order to get a logical working directory that preserves the symlink. The use case came up pretty early in CMake's development and the path translation map was added to address it. However, the path translation map handles the problem at a level that is too low and causes other problems documented here.

Logical paths should be preserved whenever possible, particularly when given to -S and -B. If the caller didn't want to use the logical path they could have called realpath() themselves. This gives them maximum control. This is the same as a compile that if given -g -c /path/with/symlink/to/src.c will put that path in the debug symbols and not call realpath(). The main difference if for /path/with/symlink/to/../src where the compiler can preserve that path as given but CMake needs to get rid of the ../.

A few years ago Ben and I came up with a solution that involves preserving logical paths by only doing special processing when a path is first encountered (e.g. on the cmake command line). The key operation is a CollapseLogicalPath function that takes an absolute path that may contain symlinks and ../ sequences and collapses it to a logical path that contains no ../ but represents the same physical path while preserving as many of the leading symlinks as possible. This operation can be applied to input paths up front, and after that we need no further special handling unless new ../ sequences are encountered.

We've worked on this intermittently since then, and a few months ago I made some sweeping cleanups working toward it that are already merged. I've just posted !4869 (closed) to show the rest of the WIP I have so far. See its CollapseLogicalPath and GetLogicalWorkingDirectory functions in particular. Overall, the approach:

Preserves logical paths given by the user on the command line.
Removes ../ from logical paths safely using CollapseLogicalPath.
Never calls realpath() except as part of the CollapseLogicalPath implementation or other special circumstances.

Note that this is all conditional on realpath(getenv("PWD")) == getcwd(). If this is not the case, we're not in a trustworthy shell and we doing any logical path work and everything ends up with resolved paths.

So, CMake works, without user customization, only when launched from a shell supporting the definition of environment variable $PWD.

This is, by far, too restrictive to be reasonable. See, for example, launch from python which is not possible without special tweak.

It works without $PWD too, but you're restricted to only using realpaths. /path/to/symlink/.. is always /target/of/symlink/.. unless you do logical path transformations. Asking the kernel about the former will never give /path/to (except when the target is a sibling of the link itself). Only the shell really does these kinds of things.

@brad.king Globally, I disagree with your approach because the corner stone, i.e. $PWD, is a side effect of shell (not even sure this variable is available regardless the shell type) not a general behavior.

And asking the user to rely on realpath to avoid any problems with logical paths is not acceptable because:

realpath is counterintuitive
realpath is not always easily accessible depending on of the environment used to launch CMake
Requiring for the common use case to tweak the paths because of the support of some specific cases is not a rational approach: The common use case must be the easiest one. Requiring some additional configuration steps for specific cases seems reasonable.

This is why I suggest handling logical paths only when -S and -B options are specified: no need of $PWD!

An alternate approach should be to use translation map specified by the user. This map can be an ordered list of regular expressions applied to the path to get the final path. For example: ^/mnt/[a-z]+/usr/(.+)$ is translated to /home/usr/$1

So, again, no need of $PWD because only real paths are handled and, on user request (i.e. translation map), logical path can be produced from real path.

The cornerstone of our approach is the preservation of logical paths while removing ../.

The $PWD treatment is one example of how we get a logical path that contains symlinks. We can decide orthogonally whether to include it or not. Even if we don't include it, users could still run cmake $PWD/../src to give us a logical path that contains both symlinks and ../, and we should preserve as much of the logical path as possible.

I don't know any other way than $PWD to get logical path.

cmake /path/with/symlinks/to/src

This is not my point. How to you get a logical path for the current working directory without using $PWD? getcwd() returns always the real path.

getcwd() returns always the real path.

Yes, and that is what breaks the /home/user -> /mnt/xyz/user use case if we don't use $PWD and the user runs cmake ../src. The translation map is seeded with $PWD right now. We end up choosing /mnt/xyz/user/src and then the translation map converts it back to /home/user/src. The problem in this issue is that the translation map is applied deeply rather than only when first encountering paths. We need to get rid of the translation map while still providing a way to support the existing use case.

I'm proposing that we preserve the existing support for $PWD to allow the use case to work as it always has. IIUC you're proposing that we drop $PWD and instead require cmake -B "$PWD" -S "$PWD/../src" to support the use case.

Most of the work involved in fixing this is needed regardless of what we choose for that. Logical paths must be preserved without removing symlinks from them. ../ must be removed from such paths while preserving as many leading symlinks as possible.

Note that all this must also cover paths that come from find_package and other such calls. Those searches can us environment variables like SomePackage_ROOT, and the user may set those with logical paths.

So, if I understand correctly:

The behavior of CMake depends on the environment: it will be different if launched from a shell (bash for example) and from a non-shell environment (python for example).
The rule is to preserve logical paths over real paths.

Based on these assumptions, Let me know if the following behaviors are what you expect to provide:

First scenario: sources are under /root1/src (no symlinks), build is under /root1/src/build and build is a symlink to /root2/build

launch from a shell:

> cd /root1/src/build
> cmake ..

In this case, because $PWD is defined, logical paths are preserved (CMAKE_SOURCE_DIR=/root1/src and CMAKE_BINARY_DIR=/root1/src/build). This scenario generates, with the current implementation, at least for Unix Makefiles generator, a broken environment because the makefiles generated use relative paths which are not valid because make do not use logical paths, only real paths.

Second scenario, same file tree as first scenario but launch from python:

> cd /root1
> python -c "import subprocess; subprocess.check_call(['cmake', '..'], cwd='/root1/src/build')"

As is, it does not work because $PWD is not defined, so retry with a full pathname for sources:

> cd /root1
> python -c "import subprocess; subprocess.check_call(['cmake', '/root1/src'], cwd='/root1/src/build')"

In this case, because $PWD is not usable, logical paths are not used (CMAKE_SOURCE_DIR=/root1/src and CMAKE_BINARY_DIR=/root2/build). In this case, makefiles generated are OK because real paths are used. But, cmake cannot be launched in the way as scenario 1 and the result is different (logical versus real paths).

Third scenario, use the famous home path symlinked to some temporary path: /home/usr -> /mnt/xyz/usr. We have /home/usr/src for sources and /home/usr/src/build for build. Using shell (same as scenario 1), we get the expected result (CMAKE_SOURCE_DIR=/home/usr/src and CMAKE_BINARY_DIR=/home/usr/src/build). And the generated makefiles are OK because src and build are under the same real file tree so relative paths are OK.

Fourth scenario, same file tree as the third one but use python to launch cmake.

> cd /home/usr
> python -c "import subprocess; subprocess.check_call(['cmake', '..'], cwd='/home/usr/src/build')"

In this case, $PWD is not usable so real paths are used. You didn't get the needed behavior (CMAKE_SOURCE_DIR=/mnt/xyz/usr/src and CMAKE_BINARY_DIR=/mnt/xyz/usr/src/build)!! /home/usr paths are not preserved.

In summary, depending on how cmake is used, we get what is expected or a different result or, even worse, some cryptic failure at build execution. I am afraid nobody will never understand what is the logic of cmake...

In scenario 1: with CollapseLogicalPath, cmake .. will not blindly remove one component that may or may not be a symlink. Instead, CollapseLogicalPath would convert /root1/src/build/.. to /root2 and generation would fail because there is no CMakeLists.txt.

The $PWD thing is a heuristic and should not be the main focus of discussion here. Let's assume we'll drop it.

My main point is that cmake -S /root1/src -B /root1/src/build should honor the user's logical path as specified. Part of the fix will need to involve updating the rules about when relative paths are allowed to cross out of the build tree.

Scenario 1 is broken, yes. This is because of the translation map masking the real paths for relative path computations; it's just a case that wasn't considered when implemented. I actually use a setup like this all the time, but I end up doing cmake $OLDPWD instead of cmake ../src to avoid the translation map getting things wrong.

Scenario 4 makes sense because we can't go searching for every possible symlink on the machine. We don't get the cwd= path except through $PWD, so it can't be fixed without it.

I am afraid nobody will [ever] understand what is the logic of cmake...

I'm sure I could say the same for the current behavior too :) . But because CMake generates absolute paths in its build trees, we need to do something for the home directory symlink case otherwise such users cannot use CMake in their home directory (lest it be broken on every login to the machine).

@marc.chevrier as a side note here, std::filesystem is not likely something we can use inside CMake without a C++11 compatibility implementation like we have for cm::optional and cm::string_view.

Yes, I know. I am currently investigating the possibility to develop std::filesystem::path class for compilers not supporting c++17. It should quite easy because it is pure algorithmic (not interaction with OS API). For other stuff of std::filesystem, it is more complex and costly because deeply interaction with native filesystem API is required...

mentioned in issue #20900 (closed)

mentioned in merge request !4963 (merged)

mentioned in merge request !4968 (merged)

Fix translation map issues

Child items ...

Activity