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.

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

[bug] NMakeDeps: cpp_info.system_libs and cpp_info.defines of dependencies are not injected #12943

Closed
SpaceIm opened this issue Jan 21, 2023 · 1 comment · Fixed by #12944
Closed
Milestone

Comments

@SpaceIm
Copy link
Contributor

SpaceIm commented Jan 21, 2023

Environment details

  • Operating System+version: Windows 11
  • Compiler+version: Visual Studio 2022
  • Conan version: 1.57.0
  • Python version: 3.11.1

Steps to reproduce

  1. checkout gdal < 3.5 : conan v2 support conan-center-index#15386, where recipe is migrated to conan v2 (from VisualStudioBuildEnvironment to NMakeToolchain + NMakeDeps for Visual Studio build)
  2. conan create . gdal/3.4.3@ -b missing
  3. see link errors in test package

Root cause is that cpp_info.defines of dependencies are not injected in CL, only includedirs. Since GDAL depends on libjpeg, and libjpeg static must come with LIBJPEG_STATIC definition, this missing definition leads to incorrect mangling, and afterwards lot of link warnings (of libjpeg symbols but also proj symbols) and few link errors when an executable is linked to gdal & libjpeg static libs.

Logs

https://c3i.jfrog.io/c3i/misc/logs/pr/15386/1-configs/windows-visual_studio/gdal/3.4.3//42b95a8783a99826180d6f5c9cae4fe6df641006-test.txt

@SpaceIm
Copy link
Contributor Author

SpaceIm commented Jan 21, 2023

After this fix in NMakeDeps, gdal is properly compiled and test package works fine:

--- a/conan/tools/microsoft/nmakedeps.py
+++ b/conan/tools/microsoft/nmakedeps.py
@@ -41,11 +41,22 @@ class NMakeDeps(object):
             ret.extend(cpp_info.exelinkflags or [])
             ret.extend(cpp_info.sharedlinkflags or [])
             ret.extend([format_lib(lib) for lib in cpp_info.libs or []])
+            ret.extend([format_lib(lib) for lib in cpp_info.system_libs or []])
             link_args = " ".join(ret)
 
+            def format_define(define):
+                if "=" in 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)
+                    value = fr'\"{value}\"' if value and not value.isnumeric() else value
+                    define = f"{macro}#{value}"
+                return f"/D{define}"
+
             cl_flags = [f'-I"{p}"' for p in cpp_info.includedirs or []]
             cl_flags.extend(cpp_info.cflags or [])
             cl_flags.extend(cpp_info.cxxflags or [])
+            cl_flags.extend([format_define(define) for define in cpp_info.defines or []])
 
             env = Environment()
             env.append("CL", " ".join(cl_flags))

(note that format_define() is fundamental)

@SpaceIm SpaceIm changed the title [bug] NMakeDeps: system_libs an defines of dependencies are not injected [bug] NMakeDeps: cpp_info.system_libs and cpp_info.defines of dependencies are not injected Jan 21, 2023
@memsharded memsharded added this to the 1.58 milestone Jan 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants