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

Fix library install directories on Windows #634

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sloede
Copy link

@sloede sloede commented Dec 31, 2023

Currently, OpenFHE installs all library products in the lib directory. AFAIK, this is wrong (or at least bad practice) on Windows, where the .dll files belong into the bin directory.

This PR remedies this by configuring the library installation in CMake to do The Right Thing:

  • On *nix systems, all library products go to lib
  • On Windows systems (including Cygwin and MinGW), static libraries and .dll.a or .lib libraries go to lib, while the dynamic libraries go to bin

@sloede
Copy link
Author

sloede commented Jan 5, 2024

From #633 (comment):

At the moment I am not sure how we are going to proceed with PR #634. I would need to discuss this with my colleague(s) first.

@dsuponitskiy Sure, let me know if I can be of any help. Right now, I don't see any downsides to the proposed change, since it essentially leaves all non-DLL systems unchanged and fixes only the install locations for DLL systems.

Note that the proposed change is also the default for the currently required minimum CMake version 3.5.2, while at least since CMake version 3.14 it is possible to change the install location ad hoc during configure time using the CMAKE_INSTALL_XXX variables.

@yspolyakov yspolyakov marked this pull request as draft January 16, 2024 20:13
@sloede
Copy link
Author

sloede commented Mar 6, 2024

@dsuponitskiy Are there any updates on this issue?

@arcturusannamalai
Copy link

@sloede - you may want to change your code to be conditional to WINDOWS
e.g. see: https://stackoverflow.com/questions/9160335/os-specific-instructions-in-cmake-how-to

if(MSVC OR MSYS OR MINGW)
    # for detecting Windows compilers
endif()

@sloede
Copy link
Author

sloede commented Apr 15, 2024

you may want to change your code to be conditional to WINDOWS

I do not think that's necessary. The proposed parameter settings should work for all Unix-like and Windows-like systems. In fact, I think it has been the default for a number of CMake releases now.

@arcturusannamalai
Copy link

you may want to change your code to be conditional to WINDOWS

I do not think that's necessary. The proposed parameter settings should work for all Unix-like and Windows-like systems. In fact, I think it has been the default for a number of CMake releases now.

good to know; thanks

@sloede
Copy link
Author

sloede commented Apr 20, 2024

@yspolyakov are there any updates on this issue? I'd be happy to update the PR such that it can be merged eventually (I don't think there is much to it anyways)

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.

None yet

2 participants