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

More consistently check for WIN32 instead of MSVC in CMake files #5183

Merged
merged 1 commit into from
Apr 16, 2022

Conversation

giordano
Copy link
Contributor

@giordano giordano commented Apr 9, 2022

What does this implement/fix? Explain your changes.

With this change I was able to cross compile for Windows by using MinGW.

Any other comments?

I'm not changing them, but as far as I can tell the ifs at

if(RDK_URF_LIBS)
list(APPEND LIBS_TO_USE RingDecomposerLib_static)
endif()
and
if(RDK_URF_LIBS)
list(APPEND LIBS_TO_USE RingDecomposerLib)
endif()
are completely useless: RDK_URF_LIBS is always true, so these ifs are always hit, and RingDecomposerLib is already unconditionally added to LIBS_TO_USE at
SmilesParse_static GraphMol_static RingDecomposerLib_static RDGeometryLib_static RDGeneral_static )
and
SmilesParse GraphMol RingDecomposerLib RDGeometryLib RDGeneral )

With this change I was able to cross compile for Windows by using MinGW.
Copy link
Member

@greglandrum greglandrum left a comment

Choose a reason for hiding this comment

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

LGTM

@greglandrum greglandrum added bug infrastructure build infrastructure and the like labels Apr 16, 2022
@greglandrum greglandrum added this to the 2022_03_2 milestone Apr 16, 2022
@greglandrum
Copy link
Member

Thanks @giordano !

@greglandrum greglandrum merged commit 66476f0 into rdkit:master Apr 16, 2022
@giordano giordano deleted the mg/cmake-win32 branch April 16, 2022 06:04
greglandrum pushed a commit that referenced this pull request Apr 25, 2022
…5183)

With this change I was able to cross compile for Windows by using MinGW.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug infrastructure build infrastructure and the like
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants