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.

Sign up for GitHub

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

Jump to bottom

Add CMake warnings to Windows/OsX jobs #510

Merged
merged 3 commits into from
Oct 7, 2021
Merged

Add CMake warnings to Windows/OsX jobs #510

merged 3 commits into from
Oct 7, 2021

Conversation

j-rivero
Copy link
Contributor

@j-rivero j-rivero commented Sep 9, 2021

closes #509

Tested locally, testing it in the buildfarm will enable the warnings for all Gazebo/Ignition jobs at once, don't want to have it uncontrolled for running CI at the moment.

I expect a good bunch of unstable builds after the merge.

@j-rivero
Copy link
Contributor Author

j-rivero commented Oct 6, 2021

Before enabling this, as discussed offline, we would need a way to opt-in/opt-out the different libraries to avoid multiples warnings to be land at once.

@j-rivero
Copy link
Contributor Author

j-rivero commented Oct 6, 2021

Before enabling this, as discussed offline, we would need a way to opt-in/opt-out the different libraries to avoid multiples warnings to be land at once.

Implemented in 0bd80fa, only enabled for ignition-cmake. In manual job test generation on my machine:

release-tools2/jenkins-scripts/dsl on  warns_osx_win [?] ❯ grep -i warnings.cmake *brew*
ignition_cmake-ci-ign-cmake2-homebrew-amd64.xml:                <io.jenkins.plugins.analysis.warnings.Cmake>
ignition_cmake-ci-ign-cmake2-homebrew-amd64.xml:                </io.jenkins.plugins.analysis.warnings.Cmake>
ignition_cmake-ci-main-homebrew-amd64.xml:                <io.jenkins.plugins.analysis.warnings.Cmake>
ignition_cmake-ci-main-homebrew-amd64.xml:                </io.jenkins.plugins.analysis.warnings.Cmake>
ignition_cmake-ci-pr_any-homebrew-amd64.xml:                <io.jenkins.plugins.analysis.warnings.Cmake>
ignition_cmake-ci-pr_any-homebrew-amd64.xml:                </io.jenkins.plugins.analysis.warnings.Cmake>
release-tools2/jenkins-scripts/dsl on  warns_osx_win [?] ❯ grep -i warnings.cmake *-win*
ignition_cmake-ci-ign-cmake2-windows7-amd64.xml:                <io.jenkins.plugins.analysis.warnings.Cmake>
ignition_cmake-ci-ign-cmake2-windows7-amd64.xml:                </io.jenkins.plugins.analysis.warnings.Cmake>
ignition_cmake-ci-main-windows7-amd64.xml:                <io.jenkins.plugins.analysis.warnings.Cmake>
ignition_cmake-ci-main-windows7-amd64.xml:                </io.jenkins.plugins.analysis.warnings.Cmake>
ignition_cmake-ci-pr_any-windows7-amd64.xml:                <io.jenkins.plugins.analysis.warnings.Cmake>
ignition_cmake-ci-pr_any-windows7-amd64.xml:                </io.jenkins.plugins.analysis.warnings.Cmake>

chapulina
chapulina approved these changes Oct 6, 2021
View reviewed changes
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.

Greeat! Thanks, Jose!

@chapulina
Copy link
Contributor

Let's ticket an issue tracking the status of all other libraries, so we remember to enable them

@j-rivero j-rivero mentioned this pull request Oct 6, 2021
Open
16 tasks
@scpeters
Copy link
Contributor

scpeters commented Oct 6, 2021

testing deployment:

@scpeters
Copy link
Contributor

scpeters commented Oct 6, 2021

looks good: https://build.osrfoundation.org/job/ignition_cmake-ci-ign-cmake2-homebrew-amd64/98/cmake/

scpeters
scpeters requested changes Oct 6, 2021
View reviewed changes
Copy link
Contributor

@scpeters scpeters left a comment

Choose a reason for hiding this comment

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

actually the windows build has cmake warnings:

Build Status https://build.osrfoundation.org/job/ignition_cmake-ci-ign-cmake2-windows7-amd64/94/

@scpeters
Copy link
Contributor

scpeters commented Oct 6, 2021

should we just pick a different package that doesn't have any cmake warnings to use in this PR?

@scpeters
Copy link
Contributor

scpeters commented Oct 6, 2021

actually the windows build has cmake warnings:

Build Status https://build.osrfoundation.org/job/ignition_cmake-ci-ign-cmake2-windows7-amd64/94/

CMake Warning in C:/Jenkins/workspace/ignition_cmake-ci-ign-cmake2-windows7-amd64/ws/ign-cmake/build/examples/use_component_depsC_RelWithDebInfo-prefix/src/use_component_depsC_RelWithDebInfo-build/CMakeFiles/CMakeTmp/CMakeLists.txt:
  The object file directory

    C:/Jenkins/workspace/ignition_cmake-ci-ign-cmake2-windows7-amd64/ws/ign-cmake/build/examples/use_component_depsC_RelWithDebInfo-prefix/src/use_component_depsC_RelWithDebInfo-build/CMakeFiles/CMakeTmp/CMakeFiles/cmTC_22875.dir/

  has 226 characters.  The maximum full path to an object file is 250
  characters (see CMAKE_OBJECT_PATH_MAX).  Object file

    CMakeCXXCompilerABI.cpp.obj

  cannot be safely placed under this directory.  The build may not work
  correctly.

I think we could fix it by switching to colcon for the build script, but for now I would see if we can find a package without cmake warnings

@scpeters
Copy link
Contributor

scpeters commented Oct 6, 2021

should we just pick a different package that doesn't have any cmake warnings to use in this PR?

I suggest ign-plugin

@scpeters
Enable warnings for ign-plugin, not ign-cmake
566028a 
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@scpeters
Copy link
Contributor

scpeters commented Oct 7, 2021

should we just pick a different package that doesn't have any cmake warnings to use in this PR?

I suggest ign-plugin

done in 566028a

@scpeters
Copy link
Contributor

scpeters commented Oct 7, 2021 •
edited
Loading

testing

@j-rivero
Copy link
Contributor Author

j-rivero commented Oct 7, 2021

Thanks @scpeters !

@j-rivero j-rivero merged commit 8016cec into master Oct 7, 2021
@j-rivero j-rivero deleted the warns_osx_win branch October 7, 2021 21:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scpeters scpeters scpeters approved these changes

@chapulina chapulina chapulina approved these changes

@nuclearsandwich nuclearsandwich Awaiting requested review from nuclearsandwich

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CMake warnings should turn macOS and Windows builds yellow
4 participants
@j-rivero @chapulina @scpeters @nuclearsandwich