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

CMakeLists: Install files with MSVC #199

Closed
wants to merge 1 commit into from

Conversation

jonaski
Copy link

@jonaski jonaski commented Feb 11, 2024

I build faad2 with MSVC but it's hard-coded to turn off installation of files for MSVC. If this is still necessary, I suggest to make it a CMake option instead. I also use pkg-config on Windows.

I build faad2 with MSVC but it's hard-coded to turn off installation of files for MSVC. If this is still necessary, I suggest to make it a CMake option instead. I also use pkg-config on Windows.
@fabiangreffrath
Copy link
Collaborator

I build faad2 with MSVC but it's hard-coded to turn off installation of files for MSVC. If this is still necessary, I suggest to make it a CMake option instead. I also use pkg-config on Windows.

@eustas Any opinion?

@eustas
Copy link
Contributor

eustas commented Feb 12, 2024

I'll take a look soon.
But, isn't it better to use vcpkg with MSVC? (will check if it is up-to-date soon as well).

@eustas
Copy link
Contributor

eustas commented Feb 12, 2024

VCPKG have been updated recently... And indeed, they remove "no-install" condition. Will prepare PR for that in this repo soon.

@eustas
Copy link
Contributor

eustas commented Feb 12, 2024

LGTM

@eustas
Copy link
Contributor

eustas commented Feb 12, 2024

There might be a reason why this condition exists:

CMake Error at build/cmake_install.cmake:43 (file):
  file INSTALL cannot find "D:/a/faad2/faad2/build/Release/faad.lib": File
  exists.

@eustas
Copy link
Contributor

eustas commented Feb 12, 2024

Hmm... CMake builds "Debug" configuration, but searches artifacts in "Release".

@fabiangreffrath
Copy link
Collaborator

f3a2fc3

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.

3 participants