Repository: conan-io/conan
Pull request: Test for universal binaries macOS using Ninja
Changelog: Fix: Replace `|` character in generated CMake and Environment files.
Docs: Omit

Closes: https://github.com/conan-io/conan/issues/17009


Problem statement:
[bug] Generated CMake files for macOS universal binaries not compatible with Ninja
### Describe the bug

Using generated CMake files for macOS universal binaries aren't compatible with Ninja. For example:
```
# conanfile.txt
[requires]
zlib/1.3.1

[generators]
CMakeToolchain
CMakeDeps
```

```
# CMakeLists.txt
cmake_minimum_required(VERSION 3.18)
project(ninjatest CXX)

find_package(ZLIB CONFIG REQUIRED)
```

Please run the following commands:
```
$ conan install . -s arch="armv8|x86_64" -b missing -of build
$ cd build
$ cmake .. -GNinja \
    -DCMAKE_TOOLCHAIN_FILE=conan_toolchain.cmake \
    -DCMAKE_POLICY_DEFAULT_CMP0091=NEW \
    -DCMAKE_BUILD_TYPE=Release
```

Then you'll get something like
```
CMake Error:
  Running

   'ninja' '-C' '.../build' '-t' 'cleandead'

  failed with:

   ninja: error: build.ninja:81: expected newline, got '|'

```

The reason for this error is the phony target which refreshes the CMake cache
```
build ../CMakeLists.txt ... ZLIB-release-armv8|x86_64-data.cmake ...
```
which uses a filename with the non-escaped reserved symbol `|` in Ninja syntax.

Of course, one could argue that CMake should escape the referenced files in the generated `build.ninja` file correctly. On the other hand, it seems that Ninja doesn't support any escape sequence for the pipe character. So CMake got its hands tied in this matter. What is your take on this issue?

### How to reproduce it

_No response_

Hints from issue discussion:
Hi @phlipsi 

Thanks for the report.

Just to make sure, is this happening if using the ``Unix Makefiles`` CMake generator instead of the Ninja one?

> ZLIB-release-armv8|x86_64-data.cmake

This seems the cause, I am now a bit surprised that this doesn't generate issues with other generators. I guess that we could do a replace of the ``|`` character too for the file names? cc @czoido 
> Just to make sure, is this happening if using the `Unix Makefiles` CMake generator instead of the Ninja one?

I think the reason why it only fails in Ninja is because `|` is a reserved character in Ninja files and when making reference to files that have that character in the filename Ninja interprets them incorrectly, that's why it does not fail with `Unix Makefiles`.

> This seems the cause, I am now a bit surprised that this doesn't generate issues with other generators. I guess that we could do a replace of the `|` character too for the file names? cc @czoido

 Yes, I think the only solution we can bring would be to do a replace of the character `|` in the filenames. Let's check that solution.

Commit message: expand test

Patch to review:
diff --git a/conan/tools/cmake/cmakedeps/templates/target_data.py b/conan/tools/cmake/cmakedeps/templates/target_data.py
index 35330ee091d..1e5a6e2862a 100644
--- a/conan/tools/cmake/cmakedeps/templates/target_data.py
+++ b/conan/tools/cmake/cmakedeps/templates/target_data.py
@@ -23,7 +23,8 @@ def filename(self):
         if self.arch:
             data_fname += "-{}".format(self.arch)
         data_fname += "-data.cmake"
-        return data_fname
+        # https://github.com/conan-io/conan/issues/17009
+        return data_fname.replace("|", "_")
 
     @property
     def _build_modules_activated(self):
diff --git a/conan/tools/env/virtualbuildenv.py b/conan/tools/env/virtualbuildenv.py
index 0da4b925066..49e9d484745 100644
--- a/conan/tools/env/virtualbuildenv.py
+++ b/conan/tools/env/virtualbuildenv.py
@@ -34,7 +34,7 @@ def _filename(self):
         if configuration:
             f += "-" + configuration.replace(".", "_")
         if arch:
-            f += "-" + arch.replace(".", "_")
+            f += "-" + arch.replace(".", "_").replace("|", "_")
         return f
 
     def environment(self):
diff --git a/conan/tools/env/virtualrunenv.py b/conan/tools/env/virtualrunenv.py
index c7308418b82..890e8cd1930 100644
--- a/conan/tools/env/virtualrunenv.py
+++ b/conan/tools/env/virtualrunenv.py
@@ -52,7 +52,7 @@ def _filename(self):
         if self.configuration:
             f += "-" + self.configuration.replace(".", "_")
         if self.arch:
-            f += "-" + self.arch.replace(".", "_")
+            f += "-" + self.arch.replace(".", "_").replace("|", "_")
         return f
 
     def environment(self):
diff --git a/test/functional/toolchains/cmake/test_universal_binaries.py b/test/functional/toolchains/cmake/test_universal_binaries.py
index 45dcc2a5fe6..7e4b4582d34 100644
--- a/test/functional/toolchains/cmake/test_universal_binaries.py
+++ b/test/functional/toolchains/cmake/test_universal_binaries.py
@@ -99,3 +99,47 @@ def test(self):
     client.run_command("lipo -info './build/armv8|armv8.3|x86_64/Release/foo'")
 
     assert "foo are: x86_64 arm64 arm64e" in client.out
+
+
+@pytest.mark.skipif(platform.system() != "Darwin", reason="Only OSX")
+@pytest.mark.tool("cmake", "3.23")
+def test_create_universal_binary_ninja():
+    client = TestClient()
+
+    client.run("new cmake_lib -d name=mylibrary -d version=1.0")
+
+    client.run('export .')
+
+    conanfile = textwrap.dedent("""
+        [requires]
+        mylibrary/1.0
+        [generators]
+        CMakeToolchain
+        CMakeDeps
+        VirtualBuildEnv
+        VirtualRunEnv
+        """)
+
+    cmake = textwrap.dedent("""
+        cmake_minimum_required(VERSION 3.23)
+        project(ninjatest CXX)
+
+        find_package(mylibrary CONFIG REQUIRED)
+        """)
+
+    client.save({"conanfile.txt": conanfile,
+                 "CMakeLists.txt": cmake},
+                 clean_first=True)
+
+    client.run('install . -s=arch="armv8|x86_64" --build=missing -of=build')
+
+    with client.chdir("build"):
+        client.run_command("cmake .. -GNinja "
+                           "-DCMAKE_TOOLCHAIN_FILE=conan_toolchain.cmake "
+                           "-DCMAKE_POLICY_DEFAULT_CMP0091=NEW "
+                           "-DCMAKE_BUILD_TYPE=Release")
+
+    assert "ninja: error: build.ninja:87: expected newline, got '|'" not in client.out
+    assert "Build files have been written to:" in client.out
+    # test that there are no files with the "|" character in the build folder
+    assert not any("|" in f for f in os.listdir(os.path.join(client.current_folder, "build")))


Code review comments:
diff_hunk: @@ -99,3 +99,47 @@ def test(self):
     client.run_command("lipo -info './build/armv8|armv8.3|x86_64/Release/foo'")
 
     assert "foo are: x86_64 arm64 arm64e" in client.out
+
+
+@pytest.mark.skipif(platform.system() != "Darwin", reason="Only OSX")
+@pytest.mark.tool("cmake", "3.23")
+def test_create_universal_binary_ninja():
+    client = TestClient()
+
+    client.run("new cmake_lib -d name=mylibrary -d version=1.0")
+
+    client.run('export .')
+
+    conanfile = textwrap.dedent("""
+        [requires]
+        mylibrary/1.0
+        [generators]
+        CMakeToolchain
+        CMakeDeps
+        VirtualBuildEnv
+        VirtualRunEnv
+        """)
+
+    cmake = textwrap.dedent("""
+        cmake_minimum_required(VERSION 3.23)
+        project(ninjatest CXX)
+
+        find_package(mylibrary CONFIG REQUIRED)
+        """)
+
+    client.save({"conanfile.txt": conanfile,
+                 "CMakeLists.txt": cmake},
+                 clean_first=True)
+
+    client.run('install . -s=arch="armv8|x86_64" --build=missing -of=build')
+
+    with client.chdir("build"):
+        client.run_command("cmake .. -GNinja "
+                           "-DCMAKE_TOOLCHAIN_FILE=conan_toolchain.cmake "
+                           "-DCMAKE_POLICY_DEFAULT_CMP0091=NEW "
+                           "-DCMAKE_BUILD_TYPE=Release")
+
+    assert "ninja: error: build.ninja:87: expected newline, got '|'" not in client.out
path: test/functional/toolchains/cmake/test_universal_binaries.py
text: @author:
```suggestion
    assert "expected newline, got '|'" not in client.out
```