You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by GitBox <gi...@apache.org> on 2021/01/29 14:19:01 UTC
[GitHub] [qpid-dispatch] jiridanek opened a new pull request #1006: DISPATCH-1870 Do not test gRPC if HTTP/2 support was not built
jiridanek opened a new pull request #1006:
URL: https://github.com/apache/qpid-dispatch/pull/1006
CC @fgiorgetti
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] ganeshmurthy commented on a change in pull request #1006: DISPATCH-1870 Do not test gRPC if HTTP/2 support was not built
Posted by GitBox <gi...@apache.org>.
ganeshmurthy commented on a change in pull request #1006:
URL: https://github.com/apache/qpid-dispatch/pull/1006#discussion_r566863313
##########
File path: CMakeLists.txt
##########
@@ -65,6 +65,8 @@ message(STATUS "Found Proton: ${Proton_LIBRARIES} (found version \"${Proton_VERS
# http2 support is optional
find_package(libnghttp2 1.4.0)
+CMAKE_DEPENDENT_OPTION(USE_LIBNGHTTP2 "Use libnghttp2 for HTTP/2 support" ON
+ "libnghttp2_FOUND" OFF)
Review comment:
Can you please elaborate on why this change is needed ? Thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] jiridanek commented on a change in pull request #1006: DISPATCH-1870 Do not test gRPC if HTTP/2 support was not built
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #1006:
URL: https://github.com/apache/qpid-dispatch/pull/1006#discussion_r566865867
##########
File path: CMakeLists.txt
##########
@@ -65,6 +65,8 @@ message(STATUS "Found Proton: ${Proton_LIBRARIES} (found version \"${Proton_VERS
# http2 support is optional
find_package(libnghttp2 1.4.0)
+CMAKE_DEPENDENT_OPTION(USE_LIBNGHTTP2 "Use libnghttp2 for HTTP/2 support" ON
+ "libnghttp2_FOUND" OFF)
Review comment:
OK, new PR, new Jira.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] jiridanek commented on a change in pull request #1006: DISPATCH-1870 Do not test gRPC if HTTP/2 support was not built
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #1006:
URL: https://github.com/apache/qpid-dispatch/pull/1006#discussion_r566853408
##########
File path: CMakeLists.txt
##########
@@ -65,6 +65,8 @@ message(STATUS "Found Proton: ${Proton_LIBRARIES} (found version \"${Proton_VERS
# http2 support is optional
find_package(libnghttp2 1.4.0)
+CMAKE_DEPENDENT_OPTION(USE_LIBNGHTTP2 "Use libnghttp2 for HTTP/2 support" ON
+ "libnghttp2_FOUND" OFF)
Review comment:
@ganeshmurthy I think this is worthwhile change. It is the same pattern that is used for libwebsockets defined below this.
The name of the variable could be better, maybe? Something like `QD_USE_LIBNGHTTP2` or maybe `QD_ENABLE_HTTP2`? But then i'd have to rename the other one too, and would have to create alias for backwards compatibility....
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] ganeshmurthy commented on a change in pull request #1006: DISPATCH-1870 Do not test gRPC if HTTP/2 support was not built
Posted by GitBox <gi...@apache.org>.
ganeshmurthy commented on a change in pull request #1006:
URL: https://github.com/apache/qpid-dispatch/pull/1006#discussion_r566863313
##########
File path: CMakeLists.txt
##########
@@ -65,6 +65,8 @@ message(STATUS "Found Proton: ${Proton_LIBRARIES} (found version \"${Proton_VERS
# http2 support is optional
find_package(libnghttp2 1.4.0)
+CMAKE_DEPENDENT_OPTION(USE_LIBNGHTTP2 "Use libnghttp2 for HTTP/2 support" ON
+ "libnghttp2_FOUND" OFF)
Review comment:
Can you please elaborate on why this change is needed ? And if we are going to do this, I prefer we tackle it in a new JIRA which talks about adding USE_LIBNGHTTP2. Thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] jiridanek merged pull request #1006: DISPATCH-1870 Do not test gRPC if HTTP/2 support was not built
Posted by GitBox <gi...@apache.org>.
jiridanek merged pull request #1006:
URL: https://github.com/apache/qpid-dispatch/pull/1006
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] jiridanek commented on a change in pull request #1006: DISPATCH-1870 Do not test gRPC if HTTP/2 support was not built
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #1006:
URL: https://github.com/apache/qpid-dispatch/pull/1006#discussion_r566865867
##########
File path: CMakeLists.txt
##########
@@ -65,6 +65,8 @@ message(STATUS "Found Proton: ${Proton_LIBRARIES} (found version \"${Proton_VERS
# http2 support is optional
find_package(libnghttp2 1.4.0)
+CMAKE_DEPENDENT_OPTION(USE_LIBNGHTTP2 "Use libnghttp2 for HTTP/2 support" ON
+ "libnghttp2_FOUND" OFF)
Review comment:
OK, new PR, new Jira. Edit: https://issues.apache.org/jira/browse/DISPATCH-1939
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] jiridanek commented on a change in pull request #1006: DISPATCH-1870 Do not test gRPC if HTTP/2 support was not built
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #1006:
URL: https://github.com/apache/qpid-dispatch/pull/1006#discussion_r566865772
##########
File path: CMakeLists.txt
##########
@@ -65,6 +65,8 @@ message(STATUS "Found Proton: ${Proton_LIBRARIES} (found version \"${Proton_VERS
# http2 support is optional
find_package(libnghttp2 1.4.0)
+CMAKE_DEPENDENT_OPTION(USE_LIBNGHTTP2 "Use libnghttp2 for HTTP/2 support" ON
+ "libnghttp2_FOUND" OFF)
Review comment:
It is the same pattern that is used for libwebsockets defined below this. It is not needed, but I think that Alan did nice work about CMake definitions for libwebsockets.
The reason for not using `libnghttp2_FOUND` everywhere in the CMakeLists.txt is that "the decision to use the library in compiling dispatch does not only depend on whether the library is present, but the user should also have the option to choose whether the library should be used".
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org