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

Update cmd/CMakeLists to conform with all other gz libraries #478

Merged
merged 3 commits into from
Aug 30, 2022

Conversation

azeey
Copy link
Contributor

@azeey azeey commented Aug 25, 2022

🦟 Bug fix

Summary

The updated cmake file is now similar to other gz libraries, eg. https://github.com/gazebosim/gz-transport/blob/9ba73b74f6ba9717d33429ebf93ef1500c7958f4/src/cmd/CMakeLists.txt

An outcome of updating the cmake files is that it fixes the library path obtained by cmdgui.rb such that it points to the .so file contained in the non-dev debian package.

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.

This fixes the library path obtained by cmdgui.rb such that it points to
the .so file contained in the non-dev debian package.

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
@azeey azeey requested a review from jennuine as a code owner August 25, 2022 22:14
@github-actions github-actions bot added the 🏰 citadel Ignition Citadel label Aug 25, 2022
@chapulina chapulina added the bug Something isn't working label Aug 26, 2022
Copy link
Contributor

@jennuine jennuine left a comment

Choose a reason for hiding this comment

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

ign_TEST is failing

/github/workspace/src/ign_TEST.cc:98: Failure
  Expected: (output.find("TopicEcho")) != (std::string::npos), actual: 18446744073709551615 vs 18446744073709551615
  Library error: [/github/workspace/lib/libignition-gui3.so.3.11.2] not found.
  
  /github/workspace/src/ign_TEST.cc:99: Failure
  Expected: (output.find("Publisher")) != (std::string::npos), actual: 18[4467](https://github.com/gazebosim/gz-gui/runs/8026238453?check_suite_focus=true#step:4:4475)44073709551615 vs 18446744073709551615
  Library error: [/github/workspace/lib/libignition-gui3.so.3.11.2] not found.

src/cmd/CMakeLists.txt Outdated Show resolved Hide resolved
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
@azeey
Copy link
Contributor Author

azeey commented Aug 26, 2022

ign_TEST is failing

/github/workspace/src/ign_TEST.cc:98: Failure
  Expected: (output.find("TopicEcho")) != (std::string::npos), actual: 18446744073709551615 vs 18446744073709551615
  Library error: [/github/workspace/lib/libignition-gui3.so.3.11.2] not found.
  
  /github/workspace/src/ign_TEST.cc:99: Failure
  Expected: (output.find("Publisher")) != (std::string::npos), actual: 18[4467](https://github.com/gazebosim/gz-gui/runs/8026238453?check_suite_focus=true#step:4:4475)44073709551615 vs 18446744073709551615
  Library error: [/github/workspace/lib/libignition-gui3.so.3.11.2] not found.

Fixed failing test in 4c33a15. I missed the corresponding changes in conf/CMakeLIsts.txt

@codecov
Copy link

codecov bot commented Aug 27, 2022

Codecov Report

Merging #478 (4c33a15) into ign-gui3 (d43ea6a) will not change coverage.
The diff coverage is n/a.

❗ Current head 4c33a15 differs from pull request most recent head 18082b7. Consider uploading reports for the commit 18082b7 to get more accurate results

@@            Coverage Diff            @@
##           ign-gui3     #478   +/-   ##
=========================================
  Coverage     73.51%   73.51%           
=========================================
  Files            30       30           
  Lines          3270     3270           
=========================================
  Hits           2404     2404           
  Misses          866      866           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@chapulina
Copy link
Contributor

Windows isn't happy

``
CMake Error in src/cmd/CMakeLists.txt:
Evaluation file to be written multiple times with different content. This
is generally caused by the content evaluating the configuration type,
language, or location of object files:

C:/Jenkins/workspace/ign_gui-pr-win/ws/build/ignition-gui3/test/lib/ruby/ignition/cmdgui3.rb

UNIT_ign_TEST was disabled on windows anyway. In addition, this follows
the pattern in ign-transport and ign-msgs.

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
@azeey
Copy link
Contributor Author

azeey commented Aug 27, 2022

I disabled the cmake generation for cmd in 18082b7 since UNIT_ign_TEST was disabled on windows anyway. In addition, this follows the pattern in ign-transport and ign-msgs.

@azeey
Copy link
Contributor Author

azeey commented Aug 30, 2022

INTEGRATION_scene3d failed on Github Actions for Bionic, but I see that other PRs have been merged with the same failure, so I'll go ahead and merge this.

@azeey azeey merged commit 0948063 into gazebosim:ign-gui3 Aug 30, 2022
@azeey azeey deleted the fix_ign_library_path branch August 30, 2022 18:52
azeey added a commit that referenced this pull request Aug 31, 2022
This fixes the library path obtained by cmdgui.rb such that it points to
the .so file contained in the non-dev debian pack

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
azeey added a commit to azeey/gz-gui that referenced this pull request Aug 31, 2022
…im#478)

This fixes the library path obtained by cmdgui.rb such that it points to
the .so file contained in the non-dev debian pack

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
azeey added a commit to azeey/gz-gui that referenced this pull request Aug 31, 2022
…im#478)

This fixes the library path obtained by cmdgui.rb such that it points to
the .so file contained in the non-dev debian pack

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
chapulina added a commit that referenced this pull request Sep 1, 2022
…480)

This fixes the library path obtained by cmdgui.rb such that it points to
the .so file contained in the non-dev debian pack

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>

Co-authored-by: Louise Poubel <louise@openrobotics.org>
Co-authored-by: Jenn Nguyen <jenn@openrobotics.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working 🏰 citadel Ignition Citadel
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants