Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Try to address NMakeDeps quoting issues #15140

Merged
merged 11 commits into from
Dec 17, 2023
Merged

Try to address NMakeDeps quoting issues #15140

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
94ca2f5
Better spacing and quotes for defines in NMakeDeps
AbrilRBS Nov 20, 2023
789c8ab
No need to escape parenthesis in defines
AbrilRBS Nov 20, 2023
b23a56c
Fix tests
AbrilRBS Nov 20, 2023
559b9aa
Follow proper syntax definition
AbrilRBS Nov 24, 2023
ae0321e
Add numeric test
AbrilRBS Dec 12, 2023
9b2502d
Ensure functional tests cover the quoting
AbrilRBS Dec 12, 2023
36e182c
Add test for space in values
AbrilRBS Dec 12, 2023
74cb858
fix values with whitespace in nmaketoolchain defines
danimtb Dec 13, 2023
2fe9776
fix nmakedeps and test
danimtb Dec 13, 2023
db5ab43
simplify
danimtb Dec 14, 2023
ad47ba0
review
danimtb Dec 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions conan/tools/microsoft/nmakedeps.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,8 @@ def format_define(define):
# CL env-var can't accept '=' sign in /D option, it can be replaced by '#' sign:
# https://learn.microsoft.com/en-us/cpp/build/reference/cl-environment-variables
macro, value = define.split("=", 1)
if value and not value.isnumeric():
memsharded marked this conversation as resolved.
Show resolved Hide resolved
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 [])
Expand Down
3 changes: 2 additions & 1 deletion conans/test/functional/toolchains/test_nmake_toolchain.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
AbrilRBS marked this conversation as resolved.
Show resolved Hide resolved
"TEST_DEFINITION5=__declspec(dllexport)"],
["/GL"], ["/GL"], ["/LTCG"], ["/LTCG"]),
("msvc", "191", "static", "17", "Debug", [], [], [], [], []),
],
Expand Down
12 changes: 9 additions & 3 deletions conans/test/integration/toolchains/microsoft/test_nmakedeps.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -38,8 +41,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\\"',
r'/DTEST_DEFINITION1', r'/DTEST_DEFINITION2#0',
r'/DTEST_DEFINITION3#', r'/DTEST_DEFINITION4#foo',
r'/DTEST_DEFINITION5#__declspec(dllexport)',
r'"/DTEST_DEFINITION6#foo bar"',
r'/DTEST_DEFINITION7#7'
):
assert re.search(fr'set "CL=%CL%.*\s{flag}(?:\s|")', bat_file)
# Checking that libs and system libs are added to _LINK_
Expand Down