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

Fixed finding Ogre2 on Windows+Conda. #384

Merged
merged 2 commits into from
Sep 14, 2023
Merged

Conversation

peci1
Copy link
Contributor

@peci1 peci1 commented Sep 3, 2023

🦟 Bug fix

Fixes #357

Summary

This seems to do the trick on Windows+Conda (following Harmonic install tutorial). Potential for unwanted side-effects on other OSs seem small to me.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

@traversaro
Copy link
Contributor

Cool! My fear was to break vcpkg CI builds with ogre-next, but as of the comment in gazebosim/gz-rendering#897 (comment) there is not such CI, probably we are good to do, at least for me. Just a small nitpick, can you update the docs of the Find script to reflect the changes? See the docs changes in https://github.com/gazebosim/gz-cmake/pull/357/files .

Signed-off-by: Martin Pecka <[email protected]>
@peci1
Copy link
Contributor Author

peci1 commented Sep 4, 2023

but as of the comment in gazebosim/gz-rendering#897 (comment) there is not such CI, probably we are good to do, at least for me

Question is whether that is expected :) I'm not good enough in reading buildfarm logs to tell if the tests actually worked at some point or not for Windows.

can you update the docs of the Find script

Done

@traversaro
Copy link
Contributor

but as of the comment in gazebosim/gz-rendering#897 (comment) there is not such CI, probably we are good to do, at least for me

Question is whether that is expected :) I'm not good enough in reading buildfarm logs to tell if the tests actually worked at some point or not for Windows.

This is a good point. Probably @j-rivero or @azeey know more on this.

@mjcarroll
Copy link
Contributor

I'm going to try to run this through on the vcpkg version using our ci_matching_branch trick and see what happens.

mjcarroll added a commit to gazebo-tooling/gazebodistro that referenced this pull request Sep 5, 2023
Signed-off-by: Michael Carroll <[email protected]>
@mjcarroll
Copy link
Contributor

gazebosim/gz-rendering#899 🤞

@mjcarroll
Copy link
Contributor

this still doesn't seem to work with me. I have conda and ogre-next, is there something else that I'm missing here?

@traversaro
Copy link
Contributor

this still doesn't seem to work with me. I have conda and ogre-next, is there something else that I'm missing here?

ogre-next is from binaries or from source?

@mjcarroll
Copy link
Contributor

ogre-next is from binaries or from source?

Binaries, 2.3.1

@peci1
Copy link
Contributor Author

peci1 commented Sep 8, 2023

Could you put a few prints to the install/gz-cmake/../FindGzOGRE2.cmake to figure out why it isn't found?

@peci1
Copy link
Contributor Author

peci1 commented Sep 8, 2023

I have:

$ conda list ogre
# packages in environment at D:\Programy\conda-envs\ign8:
#
# Name                    Version                   Build  Channel
ogre                      1.10.12             h4c42d57_11    conda-forge
ogre-next                 2.3.1                h606bb5d_4    conda-forge

@traversaro
Copy link
Contributor

ogre-next is from binaries or from source?

Binaries, 2.3.1

Can you post your mamba list? Another possible problem could be if you are missing pkg-config.

@mjcarroll
Copy link
Contributor

Well, to make things complicated, I'm trying out pixi (no pixi list yet!). I do have ogre, ogre-next, and pkg-config installed, though. I'm going to do some advanced cmake debugging (tm) to see what's going on.

@traversaro
Copy link
Contributor

Well, to make things complicated, I'm trying out pixi (no pixi list yet!). I do have ogre, ogre-next, and pkg-config installed, though. I'm going to do some advanced cmake debugging (tm) to see what's going on.

If you have mamba or micromamba installed, you can activate the .pixi/env environment with mamba and then do mamba list. Otherwise the pixi.toml file contain almost the same info of a mamba list.

@mjcarroll
Copy link
Contributor

Okay, I have confirmed that this works for me locally and it LGTM.

@mjcarroll mjcarroll merged commit ba4b99b into gazebosim:gz-cmake3 Sep 14, 2023
4 checks passed
@azeey azeey added the beta Targeting beta release of upcoming collection label Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta Targeting beta release of upcoming collection 🌱 garden Ignition Garden 🎵 harmonic Gazebo Harmonic
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants