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

Restore <autowiring/signal.h> #1008

Merged
merged 4 commits into from Oct 18, 2016
Merged

Restore <autowiring/signal.h> #1008

merged 4 commits into from Oct 18, 2016

Conversation

jdonald
Copy link
Contributor

@jdonald jdonald commented Oct 12, 2016

While <signal.h> can be confusing as it shares its name with a standard UNIX header, it's explicit enough when customers #include <autowiring/signal.h>

We have good reason for calling it <autowiring/signal.h> as outlined in #851 when this was first added. Furthermore, I don't want to make a breaking change forcing us to autowiring-1.1.

The problem we had to work around in #998 is properly dealt with by not adding all the dangerous include paths.

@jdonald jdonald added the bug label Oct 12, 2016
@jdonald jdonald added this to the v1.0.4 milestone Oct 12, 2016
@hham hham self-assigned this Oct 17, 2016
@@ -244,7 +245,7 @@ target_include_directories(
"${PROJECT_SOURCE_DIR}/contrib/autoboost"
PUBLIC
"$<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/src>"
"$<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}>"
"$<BUILD_INTERFACE:${PROJECT_BUILD_DIR}/src>"
Copy link
Contributor

Choose a reason for hiding this comment

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

This causes a build error, "AutowiringVersion.h: No such file or directory."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's passing on Travis though . I have tested both in-source and out-of-source builds.

What are the steps to reproduce the error you're seeing?

Copy link
Contributor

Choose a reason for hiding this comment

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

I made autowiring-build directory and ccmake ../autowiring inside the directory. PROJECT_BUILD_DIR is null in my Mac and Linux box.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Hyoung and I figured it out it was working on Travis only because that tests in-source builds.

@hham
Copy link
Contributor

hham commented Oct 18, 2016

Build fails. It seems to be from travis setting. My local build is successful.

@jdonald
Copy link
Contributor Author

jdonald commented Oct 18, 2016

I've pushed a new fix that seems to work for Travis's in-source build. Please try it out on your out-of-source build environment.

@hham
Copy link
Contributor

hham commented Oct 18, 2016

Builds fine in my local machine and travis.

@hham hham merged commit 6bde44e into master Oct 18, 2016
@hham hham deleted the feature-signal-header branch October 18, 2016 03:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants