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

Add visibility to ModelEditorAddEntity to fix Windows #1246

Merged
merged 1 commit into from
Dec 14, 2021

Conversation

j-rivero
Copy link
Contributor

🦟 Bug fix

Fixes problem with Windows compilation after #1231

Summary

Jenkins for ign-gazebo6 branch was not happy after #1231 on Windows.

MSVC complains about:

ComponentInspectorEditor.obj : error LNK2001: unresolved external symbol "public: class QMap<class QString,class QString> & __cdecl ignition::gazebo::gui::v6::events::ModelEditorAddEntity::Data(void)" (?Data@ModelEditorAddEntity@events@v6@gui@gazebo@ignition@@QEAAAEAV?$QMap@VQString@@v1@@@xz) [C:\Jenkins\workspace\ign_gazebo-ign-6-win\ws\build\ignition-gazebo6\src\gui\plugins\component_inspector_editor\ComponentInspectorEditor.vcxproj]
ComponentInspectorEditor.obj : error LNK2001: unresolved external symbol "public: __cdecl ignition::gazebo::gui::v6::events::ModelEditorAddEntity::ModelEditorAddEntity(class QString,class QString,unsigned __int64)" (??0ModelEditorAddEntity@events@v6@gui@gazebo@ignition@@qeaa@VQString@@0_K@Z) [C:\Jenkins\workspace\ign_gazebo-ign-6-win\ws\build\ignition-gazebo6\src\gui\plugins\component_inspector_editor\ComponentInspectorEditor.vcxproj]
ModelEditor.obj : error LNK2001: unresolved external symbol "public: unsigned __int64 __cdecl ignition::gazebo::gui::v6::events::ModelEditorAddEntity::ParentEntity(void)const " (?ParentEntity@ModelEditorAddEntity@events@v6@gui@gazebo@ignition@@QEBA_KXZ) [C:\Jenkins\workspace\ign_gazebo-ign-6-win\ws\build\ignition-gazebo6\src\gui\plugins\component_inspector_editor\ComponentInspectorEditor.vcxproj]
ModelEditor.obj : error LNK2001: unresolved external symbol "public: class QString __cdecl ignition::gazebo::gui::v6::events::ModelEditorAddEntity::EntityType(void)const " (?EntityType@ModelEditorAddEntity@events@v6@gui@gazebo@ignition@@qeba?AVQString@@xz) [C:\Jenkins\workspace\ign_gazebo-ign-6-win\ws\build\ignition-gazebo6\src\gui\plugins\component_inspector_editor\ComponentInspectorEditor.vcxproj]
ModelEditor.obj : error LNK2001: unresolved external symbol "public: class QString __cdecl ignition::gazebo::gui::v6::events::ModelEditorAddEntity::Entity(void)const " (?Entity@ModelEditorAddEntity@events@v6@gui@gazebo@ignition@@qeba?AVQString@@xz) [C:\Jenkins\workspace\ign_gazebo-ign-6-win\ws\build\ignition-gazebo6\src\gui\plugins\component_inspector_editor\ComponentInspectorEditor.vcxproj]

Seems clear to me that ModelEditorAddEntityclass added in #1231 is somehow problematic when the rest of classes are trying to link against it. I'm adding the same visibility that the rest of the header in this PR. Let's see if that is enough.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • 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

@j-rivero j-rivero requested a review from chapulina as a code owner December 13, 2021 19:52
@github-actions github-actions bot added the 🏯 fortress Ignition Fortress label Dec 13, 2021
@codecov
Copy link

codecov bot commented Dec 13, 2021

Codecov Report

Merging #1246 (1b7c17c) into ign-gazebo6 (e3d880c) will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           ign-gazebo6    #1246      +/-   ##
===============================================
- Coverage        61.96%   61.95%   -0.01%     
===============================================
  Files              276      276              
  Lines            22991    22991              
===============================================
- Hits             14246    14245       -1     
- Misses            8745     8746       +1     
Impacted Files Coverage Δ
include/ignition/gazebo/gui/GuiEvents.hh 0.00% <ø> (ø)
src/Server.cc 86.14% <0.00%> (-0.61%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e3d880c...1b7c17c. Read the comment docs.

@chapulina
Copy link
Contributor

Windows CI still failed, but because of infrastructure:

CI failure

# BEGIN SECTION: get open robotics deps (ign-gazebo6.yaml) sources into the workspace
Cloning into 'gazebodistro'...
jrivero/fix_add_entity_visibility does not match ci_matching_branch/
trying to checkout branch jrivero/fix_add_entity_visibility from gazebodistro
fatal: couldn't find remote ref jrivero/fix_add_entity_visibility
error: pathspec 'jrivero/fix_add_entity_visibility' did not match any file(s) known to git
* master
...............
=== C:\Jenkins\workspace\ign_gazebo-pr-win\ws\ign-cmake (git) ===

Your branch is up to date with 'origin/ign-cmake2'.
Already on 'ign-cmake2'
=== C:\Jenkins\workspace\ign_gazebo-pr-win\ws\ign-common (git) ===

Your branch is up to date with 'origin/ign-common4'.
Already on 'ign-common4'
=== C:\Jenkins\workspace\ign_gazebo-pr-win\ws\ign-fuel-tools (git) ===

Your branch is up to date with 'origin/ign-fuel-tools7'.
Already on 'ign-fuel-tools7'
=== C:\Jenkins\workspace\ign_gazebo-pr-win\ws\ign-gazebo (git) ===
Cloning into '.'...
Updating files:  94% (1222/1292)
Updating files:  95% (1228/1292)
Updating files:  96% (1241/1292)
Updating files:  97% (1254/1292)
Updating files:  98% (1267/1292)
Updating files:  99% (1280/1292)
Updating files: 100% (1292/1292)
Updating files: 100% (1292/1292), done.
Your branch is up to date with 'origin/ign-gazebo6'.
Already on 'ign-gazebo6'
=== C:\Jenkins\workspace\ign_gazebo-pr-win\ws\ign-gui (git) ===
From https://github.com/ignitionrobotics/ign-gui
   d161798..ecb14d3  arjo/performance/MarkerManager -> origin/arjo/performance/MarkerManager
Your branch is up to date with 'origin/ign-gui6'.
Already on 'ign-gui6'
=== C:\Jenkins\workspace\ign_gazebo-pr-win\ws\ign-math (git) ===
From https://github.com/ignitionrobotics/ign-math
 * [new branch]      ahcorde/pybind11/angle -> origin/ahcorde/pybind11/angle
   ea0512b..4fdc196  drop_mwf_pimpl         -> origin/drop_mwf_pimpl
Your branch is up to date with 'origin/ign-math6'.
Already on 'ign-math6'
=== C:\Jenkins\workspace\ign_gazebo-pr-win\ws\ign-msgs (git) ===

Your branch is up to date with 'origin/ign-msgs8'.
Already on 'ign-msgs8'
=== C:\Jenkins\workspace\ign_gazebo-pr-win\ws\ign-physics (git) ===

Your branch is up to date with 'origin/ign-physics5'.
Already on 'ign-physics5'
=== C:\Jenkins\workspace\ign_gazebo-pr-win\ws\ign-plugin (git) ===

Your branch is up to date with 'origin/ign-plugin1'.
Already on 'ign-plugin1'
=== C:\Jenkins\workspace\ign_gazebo-pr-win\ws\ign-rendering (git) ===

Your branch is up to date with 'origin/ign-rendering6'.
Already on 'ign-rendering6'
=== C:\Jenkins\workspace\ign_gazebo-pr-win\ws\ign-sensors (git) ===

Your branch is up to date with 'origin/ign-sensors6'.
Already on 'ign-sensors6'
=== C:\Jenkins\workspace\ign_gazebo-pr-win\ws\ign-tools (git) ===

Your branch is up to date with 'origin/ign-tools1'.
Already on 'ign-tools1'
=== C:\Jenkins\workspace\ign_gazebo-pr-win\ws\ign-transport (git) ===
From https://github.com/ignitionrobotics/ign-transport
   ecc2da40..f771ab10  ign-transport8 -> origin/ign-transport8
Your branch is up to date with 'origin/ign-transport11'.
Already on 'ign-transport11'
=== C:\Jenkins\workspace\ign_gazebo-pr-win\ws\ign-utils (git) ===

Your branch is up to date with 'origin/ign-utils1'.
Already on 'ign-utils1'
=== C:\Jenkins\workspace\ign_gazebo-pr-win\ws\sdformat (git) ===
From https://github.com/osrf/sdformat
   2f21fb24..826e7d62  sdf12                   -> origin/sdf12
   621933c0..b69f4489  aaron/snapping-rotation-printouts -> origin/aaron/snapping-rotation-printouts
   d9b36755..25f80dc0  ahcorde/sdf_to_usd/materials -> origin/ahcorde/sdf_to_usd/materials
 * [new branch]        light-equality-operator -> origin/light-equality-operator
Your branch is behind 'origin/sdf12' by 1 commit, and can be fast-forwarded.
  (use "git pull" to update your local branch)
Already on 'sdf12'
....E............
=== .\build\ignition-gazebo6\_deps\pybind11-src (git) ===
From https://github.com/pybind/pybind11
   59aa9986..75168113  master     -> origin/master
You are not currently on a branch.
Please specify which branch you want to merge with.
See git-pull(1) for details.

    git pull <remote> <branch>
=== .\gazebodistro (git) ===
Already up to date.
=== .\ign-cmake (git) ===
Already up to date.
=== .\ign-common (git) ===
Already up to date.
=== .\ign-fuel-tools (git) ===
Already up to date.
=== .\ign-gazebo (git) ===
Already up to date.
=== .\ign-gui (git) ===
Already up to date.
=== .\ign-math (git) ===
Already up to date.
=== .\ign-msgs (git) ===
Already up to date.
=== .\ign-physics (git) ===
Already up to date.
=== .\ign-plugin (git) ===
Already up to date.
=== .\ign-rendering (git) ===
Already up to date.
=== .\ign-sensors (git) ===
Already up to date.
=== .\ign-tools (git) ===
Already up to date.
=== .\ign-transport (git) ===
Already up to date.
=== .\ign-utils (git) ===
Already up to date.
=== .\sdformat (git) ===
Updating 2f21fb24..826e7d62
Fast-forward
 include/sdf/Element.hh |   9 +++-
 include/sdf/Gui.hh     |   5 +++
 include/sdf/Scene.hh   |   5 +++
 include/sdf/Sky.hh     |   5 +++
 include/sdf/World.hh   |  16 ++++++-
 sdf/1.9/scene.sdf      |   2 +-
 src/Actor.cc           |   5 +--
 src/Actor_TEST.cc      |  10 +++--
 src/Collision.cc       |   4 +-
 src/Element.cc         |   8 ++++
 src/Element_TEST.cc    |   8 ++--
 src/Geometry.cc        |  16 +++----
 src/Gui.cc             |  13 ++++++
 src/Gui_TEST.cc        |  16 +++++++
 src/Link.cc            |  10 ++---
 src/Link_TEST.cc       |  39 +++++++++--------
 src/Model.cc           |   6 +--
 src/Model_TEST.cc      |  15 ++++---
 src/ParticleEmitter.cc |   2 +-
 src/Scene.cc           |  19 +++++++++
 src/Scene_TEST.cc      |  30 +++++++++++++
 src/Sky.cc             |  22 ++++++++++
 src/Sky_TEST.cc        |  30 +++++++++++++
 src/Visual.cc          |   4 +-
 src/World.cc           |  86 +++++++++++++++++++++++++++++++++++++
 src/World_TEST.cc      | 113 +++++++++++++++++++++++++++++++++++++++++++++++++
 src/parser.cc          |   6 +--
 27 files changed, 441 insertions(+), 63 deletions(-)
Failed in windows_library with error #1.
Build step 'Execute Windows batch command' marked build as failure
Recording test results
ERROR: Step ‘Publish JUnit test result report’ failed: Test reports were found but none of them are new. Did leafNodes run? 

@j-rivero
Copy link
Contributor Author

Windows CI still failed, but because of infrastructure:

There are vcs problems on getting sources:

=== C:\Jenkins\workspace\ign_gazebo-pr-win\ws\sdformat (git) ===
From https://github.com/osrf/sdformat
   2f21fb24..826e7d62  sdf12                   -> origin/sdf12
   621933c0..b69f4489  aaron/snapping-rotation-printouts -> origin/aaron/snapping-rotation-printouts
   d9b36755..25f80dc0  ahcorde/sdf_to_usd/materials -> origin/ahcorde/sdf_to_usd/materials
 * [new branch]        light-equality-operator -> origin/light-equality-operator
Your branch is behind 'origin/sdf12' by 1 commit, and can be fast-forwarded.
  (use "git pull" to update your local branch)
Already on 'sdf12'
....E............
=== .\build\ignition-gazebo6\_deps\pybind11-src (git) ===
From https://github.com/pybind/pybind11
   59aa9986..75168113  master     -> origin/master
You are not currently on a branch.
Please specify which branch you want to merge with.
See git-pull(1) for details.

    git pull <remote> <branch>

Somehow vcs was having problems with the pybind checkout that was under the build directory. This surprised me since the build/ directory should have been cleanup up. I think it is a bug.

@j-rivero
Copy link
Contributor Author

Windows CI is working again. I think that all the tests failing under the CI are not related to this change.

Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

Thanks!

@chapulina chapulina merged commit 0810a6b into ign-gazebo6 Dec 14, 2021
@chapulina chapulina deleted the jrivero/fix_add_entity_visibility branch December 14, 2021 17:25
@chapulina chapulina added the Windows Windows support label Dec 14, 2021
@osrf-triage
Copy link

This pull request has been mentioned on Gazebo Community. There might be relevant details there:

https://community.gazebosim.org/t/new-ignition-releases-2022-01-24-citadel-edifice-fortress/1241/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏯 fortress Ignition Fortress Windows Windows support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants