Backport NMakeDeps quoting bugfix (conan-io#15288)

Try to address NMakeDeps quoting issues (conan-io#15140) * Better spacing and quotes for defines in NMakeDeps * No need to escape parenthesis in defines * Fix tests * Follow proper syntax definition * Add numeric test * Ensure functional tests cover the quoting * Add test for space in values * fix values with whitespace in nmaketoolchain defines * fix nmakedeps and test * simplify * review --------- Co-authored-by: danimtb <[email protected]>
jasal82 · Dec 18, 2023 · 18e1f84 · 18e1f84
1 parent 22ca754
commit 18e1f84
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 7 deletions.
diff --git a/conan/tools/microsoft/nmakedeps.py b/conan/tools/microsoft/nmakedeps.py
@@ -50,9 +50,9 @@ def format_define(define):
                     # https://learn.microsoft.com/en-us/cpp/build/reference/cl-environment-variables
                     macro, value = define.split("=", 1)
                     if value and not value.isnumeric():
-                        value = f'\\"{value}\\"'
+                        value = f'\"{value}\"'
                     define = f"{macro}#{value}"
-                return f"/D{define}"
+                return f"/D\"{define}\""
 
             cl_flags = [f'-I"{p}"' for p in cpp_info.includedirs or []]
             cl_flags.extend(cpp_info.cflags or [])

diff --git a/conan/tools/microsoft/nmaketoolchain.py b/conan/tools/microsoft/nmaketoolchain.py
@@ -36,7 +36,7 @@ def _format_defines(self, defines):
                 if value and not value.isnumeric():
                     value = f'\\"{value}\\"'
                 define = f"{macro}#{value}"
-            formated_defines.append(f"/D{define}")
+            formated_defines.append(f"/D\"{define}\"")
         return formated_defines
 
     @property

diff --git a/conans/test/functional/toolchains/test_nmake_toolchain.py b/conans/test/functional/toolchains/test_nmake_toolchain.py
@@ -13,7 +13,8 @@
     [
         ("msvc", "190", "dynamic", "14", "Release", [], [], [], [], []),
         ("msvc", "190", "dynamic", "14", "Release",
-         ["TEST_DEFINITION1", "TEST_DEFINITION2=0", "TEST_DEFINITION3=", "TEST_DEFINITION4=TestPpdValue4"],
+         ["TEST_DEFINITION1", "TEST_DEFINITION2=0", "TEST_DEFINITION3=", "TEST_DEFINITION4=TestPpdValue4",
+          "TEST_DEFINITION5=__declspec(dllexport)", "TEST_DEFINITION6=foo bar"],
          ["/GL"], ["/GL"], ["/LTCG"], ["/LTCG"]),
         ("msvc", "191", "static", "17", "Debug", [], [], [], [], []),
     ],

diff --git a/conans/test/integration/toolchains/microsoft/test_nmakedeps.py b/conans/test/integration/toolchains/microsoft/test_nmakedeps.py
@@ -28,7 +28,10 @@ def package_info(self):
                 self.cpp_info.components["pkg-3"].defines = ["TEST_DEFINITION3="]
                 self.cpp_info.components["pkg-3"].requires = ["pkg-1", "pkg-2"]
                 self.cpp_info.components["pkg-4"].libs = ["pkg-4"]
-                self.cpp_info.components["pkg-4"].defines = ["TEST_DEFINITION4=foo"]
+                self.cpp_info.components["pkg-4"].defines = ["TEST_DEFINITION4=foo",
+                                                            "TEST_DEFINITION5=__declspec(dllexport)",
+                                                            "TEST_DEFINITION6=foo bar",
+                                                            "TEST_DEFINITION7=7"]
     """)
     client.save({"conanfile.py": conanfile})
     client.run("create . -s arch=x86_64")
@@ -37,8 +40,11 @@ def package_info(self):
     bat_file = client.load("conannmakedeps.bat")
     # Checking that defines are added to CL
     for flag in (
-        r"/DTEST_DEFINITION1", r"/DTEST_DEFINITION2#0",
-        r"/DTEST_DEFINITION3#", r'/DTEST_DEFINITION4#\\"foo\\"',
+        '/D"TEST_DEFINITION1"', '/D"TEST_DEFINITION2#0"',
+        '/D"TEST_DEFINITION3#"', '/D"TEST_DEFINITION4#"foo""',
+        '/D"TEST_DEFINITION5#"__declspec\(dllexport\)""',
+        '/D"TEST_DEFINITION6#"foo bar""',
+        '/D"TEST_DEFINITION7#7"'
     ):
         assert re.search(fr'set "CL=%CL%.*\s{flag}(?:\s|")', bat_file)
     # Checking that libs and system libs are added to _LINK_