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

Fix set of GZ_SIM_RESOURCE_PATH in tests on Windows #190

Merged
merged 2 commits into from
Aug 13, 2024
Merged

Conversation

traversaro
Copy link
Member

@traversaro traversaro commented Aug 13, 2024

I was trying to get the tests on Windows to work, and I noticed that the modifications in https://github.com/robotology/gz-sim-yarp-plugins/pull/176/files were unix-specific, as they used : as separator for env variables that contain list of paths, while on Windows ; is used (as : could be ambigous with the driver letter separator, i.e. the lists on Windows are C:\src;C:\otherdir, not C:\src:C:\otherdir that would be ambiguous.

To fix this, I modified the CMake to use ENVIRONMENT_MODIFICATION and path_list_append command, that handles this differences between Windows and *nix automatically, see https://cmake.org/cmake/help/latest/prop_test/ENVIRONMENT_MODIFICATION.html .

To do so, I bumped the minimum CMake version to 3.22 .

Furthermore, I also added .Finalize() calls to all the fixtures that were missing them, to get rid of the "[Wrn] [D:\bld\gz-sim8_1713968558858\work\src\TestFixture.cc:208] Fixture has not been finalized, any functions you attemptedto hook into will not be run. It is recommended to call Finalize()before accessing the server. warnings.

I also removed the LIBGL_ALWAYS_SOFTWARE=1, as they are not necessary since gazebosim/gz-sim#920 has been fixed.

Fix #181 .

@traversaro
Copy link
Member Author

(there is a spurious commit, please squash and merge).

@traversaro traversaro changed the title Fix tests on Windows Fix set of GZ_SIM_RESOURCE_PATH in tests on Windows Aug 13, 2024
@traversaro
Copy link
Member Author

Even with this fix, the test still segfault on Windows, but I will investigate this once all the PR of fixes are merged.

@xela-95
Copy link
Member

xela-95 commented Aug 13, 2024

Thanks a lot @traversaro !! Do you think we should then remove also the the suggestions in the tutorial READMEs, like

If you are using Linux on WSLg and you have the following error
~~~
OGRE EXCEPTION(9:UnimplementedException): in GL3PlusTextureGpu::copyTo at ./RenderSystems/GL3Plus/src/OgreGL3PlusTextureGpu.cpp (line 685)
~~~
try forcing software rendering:
~~~
export LIBGL_ALWAYS_SOFTWARE=1
~~~
?

@traversaro
Copy link
Member Author

Yes, I think it make sense to remove also the one in the README, thanks for catching that!

@xela-95
Copy link
Member

xela-95 commented Aug 13, 2024

Rebased and squashed the fixups.

Copy link
Member

@xela-95 xela-95 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super @traversaro !! Approving

@xela-95
Copy link
Member

xela-95 commented Aug 13, 2024

Merging 🚀

@xela-95 xela-95 merged commit 56b3854 into main Aug 13, 2024
5 checks passed
@xela-95 xela-95 deleted the cleanupwin branch August 13, 2024 09:57
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

Successfully merging this pull request may close these issues.

Use CMake ENVIRONMENT_MODIFICATION property to update environment variables in tests
2 participants