nanogui icon indicating copy to clipboard operation
nanogui copied to clipboard

Please add the USE_EXTERNAL_GLFW cmake option

Open yurivict opened this issue 7 years ago • 7 comments

I have the external package glfw-3.2.1 installed, and nanogui should use it.

I am creating the FreeBSD package for nanogui. I will patch this for now, but please add such option.

yurivict avatar Dec 27 '17 23:12 yurivict

~This is not possible~. The GLFW objects are merged into NanoGUI. So it isn't the case that NanoGUI links against GLFW, it "is" GLFW. There's a detailed description of the implications here. But basically, if you want to link both NanoGUI and GLFW in a project, you'll get multiple definition errors. The only solution is to specify NanoGUI as "GLFW".

Does that make sense?

svenevs avatar Jan 10 '18 09:01 svenevs

Does that make sense?

Sorry, no.

How does GLFW merge into NanoGUI?

It says: "Generally speaking, it is unlikely that library Foo will provide you with mechanisms to explicitly specify where glfw comes from." glfw comes from a package. There is never a need to specify where things come from, because they are installed by packages.

So I don't understand why glfw can't be linked to NanoGUI as a shared library.

If your plan is to supply a private copy of glfw, this is an error-prone approach.

  1. First of all, why is this needed?
  2. How do you ensure that your copy doesn't conflict with system-wide copies if they have different versions?
  3. What happens if a security vulnerability is found in glfw? It can't be easily fixed.

So no, this doesn't make sense.

yurivict avatar Apr 04 '18 20:04 yurivict

Hi @yurivict,

to clarify: it was never intended that NanoGUI would become an package in any OS. Many users will likely modify NanoGUI to their needs and fork this repositor, so I don't think there is much value in having an official version in *BSD.

Regarding this specific question: NanoGUI is meant to be a small self-contained library to develop simple graphical user interfaces that can be redistributed (e.g. as binary with 1 shared library or even statically compiled). The point of merging GLFW is to make building as foolproof as possible (no external dependencies -- it is compiled as part of the CMake call).

Best, Wenzel

wjakob avatar Apr 05 '18 00:04 wjakob

@wjakob Thank you for this information.

FYI, we create ports and packages even for small pieces of software, sometimes even for libraries with only 2-3 files. The reason for something to be a port isn't only size, but also potential vulnerabilities. If some library is bundled with a lot of other packages and a vulnerability is found in it, it is extremely difficult to find all places it is bundled and patch them all. It's much easier when it is only in one port.

I created the FreeBSD port for NanoGUI after I noticed it bundled in instant-meshes.

yurivict avatar Apr 05 '18 15:04 yurivict

I can implement the following changes triggered by NANOGUI_INSTALL if desired.

  • Eigen
    • If NANOGUI_EIGEN_INCLUDE_DIR is set, use it.
    • Otherwise find_package(eigen3 REQUIRED)
  • GLFW
    • If NANOGUI_GLFW_TARGET is set, use it. This is the parent bypass discussed in #325, renamed to match eigen bypass naming.
    • Otherwise find_package(glfw3 REQUIRED)
  • GLAD
    • Continue to build statically and link.
    • Install GLAD headers in addition to NanoGUI headers.
      • #301 mentions a conan package for Glad, but I don't think its available on many platforms. Due to its generated nature.
      • Headers could be installed to include/nanogui/third_party, with a very simple NanoGUI cmake installed script that just includes nanogui and nanogui/third_party.

I think this could work. It makes it so find_package(nanogui) will work, and this repo will always be 0.1.0 (only because that's the "version" on rtd). In the future mitsuba fork could have any higher version number, so that e.g. find_package(nanogui 1.0.0 REQUIRED) will fail if installed from this repo.

So in the end everything remains the same for traditional submodule use (retaining build-level bypasses for external dependencies), and find_package(nanogui) users can choose to do that if they like. E.g. if not found, they could ExternalProject_Add I guess.

@yurivict would this make the FreeBSD package easier? The above fixes should also make it possible to install on conan (I think), just depends on eigen and glfw. I'm not sure about the GLAD approach, since its linked statically we just need the headers?

svenevs avatar Apr 05 '18 16:04 svenevs

would this make the FreeBSD package easier?

I think so, thank you.

yurivict avatar Apr 05 '18 16:04 yurivict

Cross-ref #347

svenevs avatar Jul 20 '18 17:07 svenevs