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

NMakeDeps: inject defines & system_libs of dependencies #12944

Merged

Conversation

SpaceIm
Copy link
Contributor

@SpaceIm SpaceIm commented Jan 21, 2023

Changelog: Fix: NMakeDeps support cpp_info.defines and cpp_info.system_libs of dependencies.
Docs: Omit

closes #12943

  • Refer to the issue that supports this Pull Request.
  • If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code.
  • I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.

@SpaceIm SpaceIm changed the title NMakedeps: inject defines & system_libs of dependencies NMakeDeps: inject defines & system_libs of dependencies Jan 21, 2023
@SpaceIm
Copy link
Contributor Author

SpaceIm commented Jan 21, 2023

I don't know how to extend current NMakeDeps test without completly changing this test. It relies on cmake_lib template which seems to be too simple for a proper functional test of a deps like generator.

Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks.

If a functional test is too complicated or requires very big changes, then an integration test (just checking the generated .bat file) is probably enough at this moment

@SpaceIm SpaceIm force-pushed the fix/nmakedeps-defines-system-libs branch from c9f31cb to a130e93 Compare January 22, 2023 01:15
@memsharded memsharded merged commit 6501a0b into conan-io:develop Jan 22, 2023
@SpaceIm SpaceIm deleted the fix/nmakedeps-defines-system-libs branch January 22, 2023 11:59
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 this pull request may close these issues.

[bug] NMakeDeps: cpp_info.system_libs and cpp_info.defines of dependencies are not injected
2 participants