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

if libusb is used -- LGPL licensing causes static/dynamic compile flags, doc updates, etc. #37

Open
diablodale opened this issue Jan 7, 2022 · 8 comments

Comments

@diablodale
Copy link
Contributor

Introducing libusb obj/binary code into the depthai project, distributing it, etc. introduces libusb's LGPL licensing.
https://www.gnu.org/licenses/gpl-faq.html#LGPLStaticVsDynamic

I like tools that "help keep honest people honest". And to help prevent accidents.
I request tools/processes in depthai to manage the inclusion/flags of dependencies that have restrictive licensing (libusb is just the first one...)

@themarpe writes

Yeah, LGPL isn't the most open licence but will work okay as long as we note a couple of things.
There are 2 ways to go:

  • make libusb a dll (as is already on macos and Linux platforms). Windows might make this a tad harder because of the lack of rpath
  • compile depthai-core as dll with libusb static

The third option of having static libusb and static core, comes with a caveat that consuming application must then either be: provided in an object form to be able to relink another libusb or be opensource. This case should be documented so people aren't introducing licencing incompatible code into their project.
Note, we'll default to build libusb as a dll, so the change shouldn't affect anyone (with exception of Windows users regarding installing the libusb dll along their app)
https://softwareengineering.stackexchange.com/questions/312758/does-providing-object-files-satisfy-lgpl-relink-clause#312759
Core doesn't have to become LGPL licenced as there is the exemption of being able to relink with another version of libusb, which an opensource library consuming it fits into (in dll case).
Disclaimer, I'm am not a lawyer - this is my understanding from reading through various sources.

I am in aligment with that (as a non-lawyer).

Suggested work

  • adjust depthai project(s) cmake to enable those three major options. BUILD_SHARED is already avail, but there will need to be more granularity to specify which components are shared/dynamic and which are static so to comply with licensing
  • add licensing section to https://docs.luxonis.com/en/latest/ (No content exists there today)
  • depthai docs list those major three options and the cmake/compile flags to use
  • add DLL loading path management to Windows codepath to enable rpath similar behavior. I can help with this. I have mature code 8+ years that does this.

FYI, OpenCV has a cmake OPENCV_ENABLE_NONFREE. It adjusts the cmake flags/files to help devs be more honest and prevent accidential license issues.

@themarpe
Copy link
Collaborator

Thanks for creating a separate "thread" for this issue

I agree on the points - initially I intend to expose static libusb only through a specific option, so it will always require an "opt-in" behavior to have libusb statically linked.

Will also add a licensing section to the docs (CC: @Erol444 ping me once you have a slice of time available for this please)

add DLL loading path management to Windows codepath to enable rpath similar behavior. I can help with this. I have mature code 8+ years that does this.

Please do, that'd be great - right now the core is already buildable with libusb used in this manner (but static currently). The branch is xlink_device_search_improvements and changing cmake/Hunter/config.cmake
(There is also an option to build all Hunter libraries as shared, which could be used instead)

...
hunter_config(
    libusb-luxonis
    VERSION "1.0.24-cmake"
    URL "https://github.com/luxonis/libusb/archive/603ea0c74b6119c8fec25c20d4358462ee1309d4.tar.gz"
    SHA1 "46ed5287889efa4e78d23741facd70fb432d6914"
    CMAKE_ARGS
        WITH_UDEV=OFF
        BUILD_SHARED=ON # Add the following line
)

Note, the above snippet is untested

If you want, you may test out the approach there. Also, when installing core, the DLL/so will be pulled to [install]/lib/cmake/depthai/dependencies/lib/libusb/... so this might be the path to take it from for consumption reasons.

@diablodale
Copy link
Contributor Author

@themarpe hi, I see progress on above PR. Is it to a point where I can start looking into the issue of controlling where Windows looks for the libusb.dll?

Such functionality is needed by my app and others like Unreal, Unity, etc. where the main EXE is in a different path than the install location for the depthai-core DLL/plugin/app. Windows looks in a very tightly controlled series of paths (e.g. where the main EXE is located). Therefore when Windows OS searches for the libusb.dll to load (due to linking dynamically due to LGPL), Windows likely won't find libusb.dll because the OS doesn't know to look in the depthai-core install location.

I've got code that resolves that issue, but I need your stable approach/code to fully understand how depthai-core loads xlink loads libusb. :-)

@themarpe
Copy link
Collaborator

themarpe commented May 6, 2022

Hi @diablodale
It is - some initial tries/cases were made already over at depthai-core & depthai-python. (luxonis/depthai-core#410 & luxonis/depthai-python#574)

Would be awesome to get your input and approach on these

Also, by default a shared library is built, so LGPL details can be left as is. In some cases it'd be nicer to have it be a static library, but I think this should suffice for now

@diablodale
Copy link
Contributor Author

diablodale commented May 6, 2022

I quick read luxonis/depthai-core#410
Three questions please...

  1. Is there anything else in the core sdk that is related to libusb dll path/loading beyond these two?
  • hunter/config.cmake with BUILD_SHARED_LIBS=ON. This suggests to me that libusb will always be compiled as a DLL and this is the one-and-only officially tested+supported configuration. True?
  • examples/cmakelists.txt prepending "PATH=${HUNTER_INSTALL_PREFIX}/bin to the system PATH. I think this is kinda unsafe for anything outside quick tests and I would not recommend you document that as the approach for users of the SDK. There are better ways to do this.
  1. Is XLink the only code that directly calls libusb APIs? I searched for libusb_init and only found it in XLink. And from what I can discern, libusb_init() must always be called first...before any other libusb api.

  2. On Windows...what is the officially supported combination of static and shared?

main app depthai-core xlink libusb notes
dll dll static lib dll I think my app would be this line
dll  dll python scripts run under python.exe, so I suspect your python support is probably a dll
 ? ? what other combinations?  

@themarpe
Copy link
Collaborator

themarpe commented May 7, 2022

@diablodale

hunter/config.cmake with BUILD_SHARED_LIBS=ON. This suggests to me that libusb will always be compiled as a DLL and this is the one-and-only officially tested+supported configuration. True?

Correct - if static is specified, everything should work as is as well, but for licensing reasons, we'll leave it shared by default.

examples/cmakelists.txt prepending "PATH=${HUNTER_INSTALL_PREFIX}/bin to the system PATH. I think this is kinda unsafe for anything outside quick tests and I would not recommend you document that as the approach for users of the SDK. There are better ways to do this.

I agree, it puts other things accessible and before any other PATH directories as well. This is mostly an "just running tests" approach - I'm up for change

For core (c++), no. For bindings, there is an explicit copy of the dll to the output directory as the mechanism by which CPython loads shared libs doesn't allow searching through PATH, etc...
There is also tests/integration which would need a better approach. (So 3rdparty integration, with either add_subdirectory or find_package approach). Right now it kinda relies on either copying the libusbs target dll or the hunter/bin.

Is XLink the only code that directly calls libusb APIs? I searched for libusb_init and only found it in XLink. And from what I can discern, libusb_init() must always be called first...before any other libusb api.

Correct - its essentially private dependency of XLink (could be made private). libusb_init() is done at XLinkInitialize stage as well before other functions are called.

Main app aside (either exe/dll), core can be both static or shared /w XLink and other dependencies all being static (for now, could be changed, but we'd have to deal with those dlls as well) and having libusb be only a shared library (if its decided to be static, then it would be supported as is already).

And yes for python bindings, those are bindings:dll, core,et al:static, libusb:dll.

@diablodale
Copy link
Contributor Author

diablodale commented May 7, 2022

I have a working prototype. Would appreciate feedback.

xlink

  • pr add control of libusb DLL on Windows #48
  • uses Windows delay-load https://docs.microsoft.com/en-us/cpp/build/reference/linker-support-for-delay-loaded-dlls?view=msvc-170
  • needs below small changes to depthai-core cmake
  • briefly tested with xlink test exe, a few depthai-core test exe's, and my app
  • added error handling and error returns. I believe this is needed to successfully bubble up errors from xlink and libusb so they eventually cascade back and are thrown as exceptions to apps. I tested this with my app and it nicely bubbled up to src\utility\Initialization.cpp line 101 and threw on 103 which my app successfully caught and reported to the user.
  • I've still got debug logs, some comments and questions in the code. Will remove/clean as I get feedback.

depthai-core

  • pr change XLink to private linking into depthai-core depthai-core#473
  • changed XLink to public includes (for that one xlink public header) and private linking (so that delayload params and duplicate linking doesn't propagate to downstream apps)
  • briefly tested shared lib (dll) depthai-core linked to xlink from hunter (standard archive) and local (with above changes)

@diablodale
Copy link
Contributor Author

made PRs to Luxonis PR branches and updated post above with links

@themarpe
Copy link
Collaborator

themarpe commented May 8, 2022

Thanks - left feedback in appropriate PRs.

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

No branches or pull requests

2 participants