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

improve CMake regex to detect messages without filename:linenumber #169

Merged
merged 1 commit into from
Jan 23, 2018

Conversation

dirk-thomas
Copy link
Member

Addresses ros2/build_farmer#73.

The following messages are being ignored since we currently pass some CMake variables to all packages even though they are only expected by some packages:

Manually-specified variables were not used by the project: ...

The change has already been deployed on ci.ros2.org.

@@ -3,17 +3,23 @@
<groovyParsers>
<hudson.plugins.warnings.GroovyParser>
<name>CMake</name>
<regexp>^CMake (Deprecation Warning|Warning|Warning \(dev\)|Error) at (.+):(\d+)( \((.+)\))?:\n( .+\n)?(.+\n( .+\n)*)?</regexp>
<regexp>^CMake (Deprecation Warning|Warning|Warning \(dev\)|Error)( at (.+):(\d+)( \((.+)\))?)?:\n(?! Manually-specified variables were not used by the project:)( .+\n)?(.+\n( .+\n)*)?</regexp>
Copy link
Contributor

Choose a reason for hiding this comment

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

We should consider updating the regexp to use named capture groups in a follow-up pull request to avoid the index shuffling below.

@nuclearsandwich
Copy link
Contributor

This change will need to be manually propagated to buildfarm master hosts since re-running puppet on them is not safe (#160).

@nuclearsandwich
Copy link
Contributor

nuclearsandwich commented Jan 23, 2018

@dirk-thomas am I correct in thinking we want to deploy this on build.ros.org and build.ros2.org as well?

@mikaelarguedas
Copy link
Contributor

we want to deploy this on build.ros.org and build.ros2.org as well?

A bunch of dev jobs may become unstable with that change, should we notify ROS maintainers before deploying ?

Is any part of the infrastructure using this branch or can it be safely deleted?

@nuclearsandwich nuclearsandwich deleted the cmake_msgs_without_location branch January 23, 2018 01:06
@dirk-thomas
Copy link
Member Author

am I correct in thinking we want to deploy this on build.ros.org and build.ros2.org as well?

Yes.

A bunch of dev jobs may become unstable with that change, should we notify ROS maintainers before deploying ?

I wouldn't announce it specifically. We announced it for Kinetic. That the regex missed some is a fix which I would say is fine to deploy.

@dirk-thomas
Copy link
Member Author

@nuclearsandwich What is the timeline to deploy this change?

@nuclearsandwich
Copy link
Contributor

@nuclearsandwich What is the timeline to deploy this change?

If there's team consensus that it doesn't need an announcement, I'll deploy it this morning.

@mikaelarguedas
Copy link
Contributor

fine by me 👍

@nuclearsandwich
Copy link
Contributor

Updated on build.ros2.org and build.ros.org.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants