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 CMake issues #737

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix CMake issues #737

wants to merge 2 commits into from

Conversation

ikspress
Copy link
Contributor

@ikspress ikspress commented Jul 1, 2024

Based on #735.

The propose of this PR is as follows.

  • Make LibNFCConfig.cmake relocatable.
  • Use GNUInstallDirs to simplify code.
  • Add BUILD_MANUAL and INSTALL_BUNDLE for user discretion.
  • Use zip as the CPack default generator (let others use cpack -G "xxxx").
  • Rewrite the confusing FindLIBUSB.cmake.
  • Enable static/shared build selection (Enable selection of static vs shared builds #595)

The reason why I changed UTILS-SOURCES and EXAMPLES-SOURCES to SOURCES is that I was going to fix FixBundle.cmake.in but it failed serveral times and finally I gave up. :(
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR} is unneeded which is built-in default.

@ikspress ikspress force-pushed the packaging branch 2 times, most recently from e5bb5ff to e558853 Compare July 19, 2024 08:49
Copy link
Collaborator

@daixtrose daixtrose left a comment

Choose a reason for hiding this comment

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

I appreciate the effort. Feel free to ignore my comments and proceed with a merge. Since I am currently away from computers where I can test the changes, I kindly ask for other reviewers to give green light.

Generally speaking: The changes point into the right direction. Making libnfc's CMake system truly platform independent is still some work to be done. I currently take a third deep dive into Craig Scott's book which got some chapters added about release and dependency management. You might find these helpful.

CMakeLists.txt Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
ENDIF(LIBUSB_INCLUDE_DIRS AND LIBUSB_LIBRARIES)
FIND_PATH(LIBUSB_INCLUDE_DIRS NAMES usb.h lusb0_usb.h)
FIND_LIBRARY(LIBUSB_LIBRARIES NAMES libusb libusb0)
GET_FILENAME_COMPONENT(LIBUSB_LIBRARY_DIRS ${LIBUSB_LIBRARIES} PATH CACHE)
ENDIF(NOT LIBUSB_FOUND)
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about using ExternalProject_Add for libusb when everything else fails?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, okay, I will make changes for your suggestion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, libnfc uses libusb-0.1 rather than libusb-1.0. libusb-win32, a Windows library for libusb-0.1 api, is easy to compile, so I will just make it for Windows.

Co-authored-by: Markus Werle <daixtrose@users.noreply.github.com>
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

3 participants