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

Jump to bottom

Force _GNU_SOURCE if glibc is used. #267

Merged
merged 5 commits into from Jul 20, 2020
Merged

Force _GNU_SOURCE if glibc is used. #267

merged 5 commits into from Jul 20, 2020

Conversation

hidmic
Copy link
Contributor

@hidmic hidmic commented Jul 16, 2020

Precisely what the title says. Connected to #265.

@hidmic
Force _GNU_SOURCE if glibc is used.
11696e4 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic hidmic mentioned this pull request Jul 16, 2020
Merged
@hidmic
Please lint_cmake
b20b46d 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
clalancette
clalancette reviewed Jul 17, 2020
View reviewed changes
src/process.c Outdated Show resolved Hide resolved
clalancette
clalancette requested changes Jul 17, 2020
View reviewed changes
Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

Two minor things to fix, but otherwise looking good to me.

src/process.c Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
@hidmic
Drop _GNU_SOURCE define in process.c
6bb8833 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic
Rename glibc checking cmake function.
b86881e 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic
Copy link
Contributor Author

hidmic commented Jul 17, 2020 •
edited

Running CI up to rclcpp and rclpy:

  • Linux using gcc Build Status
  • Linux using clang Build Status (unrelated warnings)
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

clalancette
clalancette approved these changes Jul 17, 2020
View reviewed changes
Copy link
Contributor

@clalancette clalancette left a comment •
edited

Choose a reason for hiding this comment

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

Looks good to me with green CI. I will suggest running CI against everything that uses src/process.c APIs, just to ensure that there is no downstream breakage. I doubt it, but good to check :).

@hidmic
Do not check for glibc on Mac OS X.
ea6b5c7 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic
Copy link
Contributor Author

hidmic commented Jul 20, 2020

Ok, all's green. @clalancette @wjwwood anything else?

@clalancette
Copy link
Contributor

Looks good to me.

@hidmic
Copy link
Contributor Author

hidmic commented Jul 20, 2020

Ok, cool. Going in!

@hidmic hidmic merged commit 9d772a9 into master Jul 20, 2020
@delete-merged-branch delete-merged-branch bot deleted the hidmic/use-gnu-on-unix branch July 20, 2020 20:57
@allsey87 allsey87 mentioned this pull request Aug 10, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wjwwood wjwwood wjwwood approved these changes

@clalancette clalancette clalancette approved these changes

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

Successfully merging this pull request may close these issues.

None yet
3 participants
@hidmic @clalancette @wjwwood