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

Put glfw under subrepo #21

Closed
FunMiles opened this issue Apr 6, 2020 · 8 comments
Closed

Put glfw under subrepo #21

FunMiles opened this issue Apr 6, 2020 · 8 comments

Comments

@FunMiles
Copy link
Contributor

FunMiles commented Apr 6, 2020

I am suggesting to put glfw under subrepo. This follows a short discussion at the end of the MacOS issue (and thanks, @andy-thomason for taking that PR in).

Following the discussion of subrepo in #19 :

I have a slight disagreement with @andy-thomason and @lhog view/comments on subrepo. One of the significant benefits of subrepo in this case is that users of Vookoo and in most cases developers as well, can ignore the existence of the subrepo.
Let's say we put glfw3 under subrepo, appart from having the source code clone, all it does is add a file '.gitrepo' in the external/GLFW directory. Cloning the complete vookoo with the glfw3 will download the complete vookoo code, including GLFW transparently. No need to install gitrepo for that. Unless you need to update the version of glfw3 or want to push a change to glfw3, you don't even have to know about subrepo. You don't have to have the subrepo software installed. I think that is one of the greatest strenghts of subrepo vs submodule.
So, @lhog, I think end-user experience is unaffected.

I will create a version/PR for it.
PS:
An auxiliary result of cloning glfw3 into external is that a CMakeLists in the main directory, (which can then install the header-only library vookoo library) is needed because one cannot use add_subdirectory to a sibling if it is not binary.

@FunMiles
Copy link
Contributor Author

FunMiles commented Apr 6, 2020

I have created a first basic version with glfw clone into the external directory.
Can you check it out on windows, @lhog ? I have checked it also works on my Linux box.
https://github.com/FunMiles/Vookoo. Branch glfw-subrepo

The changes also address #16

@andy-thomason
Copy link
Owner

Bear in mind that GLFW only exists to run the examples and is not a dependency of Vookoo itself (vku.hpp) which is dependant only on the Vulkan headers. However, we need an example framework to get us started - if only to create windows on three disparate platforms. I am not averse to using subrepos however, but we must document them carefully so that a new user who wants to run the examples will know how.

@FunMiles
Copy link
Contributor Author

FunMiles commented Apr 6, 2020

@andy-thomason So far, my CMakeLists.txt in the main directory only supports the building of the examples. I intend to add an install capability, but perhaps as a separate pull request, as to keep issues well separated. In the install capability, it will create cmake files. The cmake file would allow to use find_package(vookoo) and they could have two components, the base one for vku.hpp and the framework one for vku_framework.hpp. The first one would add no dependency, but the second could carry with it the glfw dependency. Maybe we can have a discussion about this in a separate issue about installation. I can also amend the README.md to cover the subject of the examples. It already does say clearly that it is an include-only library.

In the mean time, do you think the building of the examples should be under a CMake option? Should it be on or off by default?

@lhog
Copy link
Collaborator

lhog commented Apr 6, 2020

Didn't work for me out of the box:
image

@FunMiles
Copy link
Contributor Author

FunMiles commented Apr 6, 2020

@lhog, I rebooted my linux machine under windows and it works without trouble. Did you, by any chance, keep your build to point to the example as the CMake root directory? You need to select the main directory as the location of the CMakeLists.

@lhog
Copy link
Collaborator

lhog commented Apr 6, 2020

@FunMiles yep, apologies, I missed the fact you added another CMakeLists.
Works for me now too.

@FunMiles
Copy link
Contributor Author

FunMiles commented Apr 6, 2020

Great. I'm creating the pull request.

@FunMiles
Copy link
Contributor Author

FunMiles commented Apr 7, 2020

The PR has been merged. I'm closing this issue.

@FunMiles FunMiles closed this as completed Apr 7, 2020
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

3 participants