Fix for issue #16 #31
Conversation
|
Ok, I see it failed the check. Investigating... |
|
Waiting on microsoft/vcpkg/issues/1597 |
|
Ok, second attempt. let's see how Travis-Cl handles it now. |
| target_link_libraries( | ||
| Testbed | ||
| PlayRho | ||
| glfw |
There was a problem hiding this comment.
Does this addition of glfw need to instead be glfw3 perhaps?
There was a problem hiding this comment.
Or would it be better to use ${GLFW_STATIC_LIBRARIES} instead of glfw or glfw3?
There was a problem hiding this comment.
Err... or maybe ${GLFW_LIBRARY}?
There was a problem hiding this comment.
I found out that the name of the Imported Target is always glfw when the package manager is vcpkg. That code works (builds on my pc anyway). The travis-ci build should be taking the else() branch on line 82, and building in the same way as before.
There was a problem hiding this comment.
I'm new to pull request reviews as well. But I've since reverted the commit, and re-applied it. So the View changes button no longer takes me anywhere useful.
|
@NauticalMile64 Thank you doing this work. I see that 1 of the 3 checks failed for this pull request - the continuous-integration/travis-ci/pr check. The Travis-CI log shows: |
|
Ok I think I figured it out, fixing now... |
NauticalMile64
force-pushed
the
master
branch
2 times, most recently
from
05459c9
to
4911e79
Compare
|
I realized I had omitted the As a side note, it's also concerning that build 89.1 failed to build playrho. |
|
Best I can tell, you'd submitted an update and the GitHub system tried to check that it was good. Unfortunately, just before that, I'd submitted a push that broke the Travis-CI build. So the update you provided failed on Travis because of my changes. I've fixed what I'd broken since then. |
|
I see you'd closed this PR. Wasn't it a good set of code changes? I really appreciate your working on this stuff. |
|
It doesn't pass the Travis-CI build, and I can't tell why it's not working. I wanted to get it set up on my home pc, but I can't get it working without installing docker, which requires AMD-V to be activated on my machine. The only way to do this is through the BIOS, and my current motherboard (MSI Tomahawk B350) doesn't make this option available. I am unsure how to proceed. |
|
I think your last change set only didn't pass the Travis-CI build because I broke the Travis-CI build just before you submitted the PR for the tests it went through. So the Travis-CI build would have broke because I'd already broken it. I believe I need to get out of the habit of doing push's and into doing pull requests like anyone else but it may take me a while to get myself there (and break force of habit). My story is that push's and pull's don't really 100% live completely in harmony with each other on GitHub at least not with the automatic checks that are in place now. |
|
No worries Louis. I think you've developed a lot of good habits, too! I'll commit and push my changes again when I get the chance (probably Monday) to see if it works. |
|
Commenting to reopen. |
glfw is now included with the following precedence: 1. user specified values for `GLFW_INCLUDE_DIRS`, `GLFW_LIBRARY_DIRS`, `GLFW_STATIC_LIBRARIES`, and `OPENGL_INCLUDE_DIR` 2. using `find_package` 3. using `pkg-config` The `target_link_libraries` function is split for reasons mentioned in microsoft/vcpkg#1597.
|
Ok looks like it passed travis-ci finally. I had changed the order of the I'm going to reset this one more time and make one clean commit for the CMakeLists.txt file and then add an extra commit or two updating the documentation on building the Testbed. |
Description - What's this PR do?
Fixes issue #16.
Impacts/Risks of These Changes?
When
glfwhas been installed usingvcpkgthis correctly includes and linksglfw. Will behave in the original way when thefind_packagemethod fails.How should this be tested?
Generate build using CMake and build the project.