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

Followup from PR #40 #44

Open
JohannesKauffmann opened this issue Sep 28, 2022 · 2 comments
Open

Followup from PR #40 #44

JohannesKauffmann opened this issue Sep 28, 2022 · 2 comments

Comments

@JohannesKauffmann
Copy link
Contributor

JohannesKauffmann commented Sep 28, 2022

As a followup from PR #40, I wanted to at least document a few issues I found when building FluidLite on Windows with CMake.

Originally, my PR touched some code that was part of the public API. Then I wanted to test those changes. What better way to hack up the examples? Well, that's where the fun begins.

I built the library itself as follows in an x64 dev prompt:

mkdir build && cd build
cmake ..
cmake --build . --parallel

That worked fine, so I thought it'd be possible to do the same for the examples.

cd ..\examples && mkdir build && cd build
cmake ..

... and then disaster struck:

-- The C compiler identification is MSVC 19.32.31332.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.32.31326/bin/Hostx64/x64/cl.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done
CMake Error at CMakeLists.txt:12 (add_subdirectory):
  add_subdirectory given source
  "C:/Users/user/FluidLite/example/FluidLite" which is not
  an existing directory.


CMake Error at CMakeLists.txt:13 (include):
  include could not find requested file:

    C:/Users/user/FluidLite/example/FluidLite/aliases.cmake


-- Configuring incomplete, errors occurred!
See also "C:/Users/user/FluidLite/example/build/CMakeFiles/CMakeOutput.log".

So the issue is that examples/FluidLite is a (Unix) symbolic link. Those don't work on Windows. When checked out on my machine, it is a text file with two dots as content.

I first tried to patch the CMakeLists.txt in examples to simply include the parent directory:

-add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/FluidLite)
+add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/..)

... but that isn't allowed in CMake:

CMake Error at CMakeLists.txt:12 (add_subdirectory):
  add_subdirectory not given a binary directory but the given source
  directory "C:/Users/user/FluidLite" is not a subdirectory
  of "C:/Users/user/FluidLite/example".  When specifying an
  out-of-tree source a binary directory must be explicitly specified.

So as a hack, I copied the entire FluidLite directory to the examples directory, which did work in the end. But before that, I think I also tried to create a symlink in an admin cmd prompt (symlinks do exist on Windows!). I don't think it worked in the end.


I think a proper solution would be to add_subdirectory() the examples folder from the main CMakeLists.txt, and then have the examples linked against the very same CMake target that you export to real consumers. This should work no problem, as the main CMakeLists.txt already makes use of CMake's BUILD_INTERFACE and INSTALL_INTERFACE for fine-grained control over what gets exported.

You might also (for a future CI?) want to enable building this example by default (option(BUILD_EXAMPLES ON)), as consumers such as distro packagers (or VLC) will turn off everything unnecessary anyway. This ensures that the examples stay buildable.

@pedrolcl
Copy link
Contributor

the issue is that examples/FluidLite is a (Unix) symbolic link. Those don't work on Windows. When checked out on my machine, it is a text file with two dots as content.

Windows supports symlinks in Vista/7/8/10 when running mklink as administrator. Since Windows 10 Creators, if you enable the "developer mode" in Windows settings, you can make symlinks as ordinary user as well.

Git for Windows supports symlinks since v2.17.1 if you install it with the right option enabled. It also has a configuration option to create symlinks by default when cloning a repository with symlinks:

$ git config --global core.symlinks true

Once the developer mode and the above options are enabled, cloning the FluidLite repository with git for windows a suitable native symlink is created in the example/ directory, like on other operating systems.

I think a proper solution would be to add_subdirectory() the examples folder from the main CMakeLists.txt

I disagree. The example is not provided as trivial fast food. It is provided as a learning tool (and learning requires some effort and attitude). In your case, you have already learned something, and could have learned even more about your operating system of choice. You may still do so.

I could agree to add a little note to the readme file, warning future Windows newbies with something like this:
"Beware! this repository contains symlinks. If you are a Windows user and this is new for you, please learn about this feature in Windows and git for windows."

@JohannesKauffmann
Copy link
Contributor Author

I could certainly see why you'd like symlinks, as indeed they do work with current Windows versions. However, I think it misses the point.

  • Developer mode is not enabled by default. The only reason I have it enabled, is because I did some work on UWP apps sometime ago.
  • What about Windows 8.1, where one cannot enable them without administrator access?
  • I don't know if the symlink option you are referring to is enabled by default during a Git for Windows install. If it isn't, that's an extra hoop that users have to jumpp through.
  • What about alternative Git clients, which are ubiquitous on Windows? Think of GitHub Desktop, GitKraken and the likes.

IMO, the examples should work out of the box when cloning the repository without fiddling with a dozen settings first. They should be about demonstrating the capabilities of the library and be a starting point for users, not learning about their OS.

And you still don't have broken build detection for the examples when they are not build by default.

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