You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2022/02/02 20:26:46 UTC
[GitHub] [geode-native] mmartell opened a new pull request #917: GEODE-10004: support older geode versions
mmartell opened a new pull request #917:
URL: https://github.com/apache/geode-native/pull/917
This PR is to update the cmake config files for tests/javaobject to allow building javaobject.jar against geode versions that pre-date the addition of throwing AuthenticationExpiredExceptions.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode-native] mmartell commented on a change in pull request #917: GEODE-10004: support older geode versions
Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #917:
URL: https://github.com/apache/geode-native/pull/917#discussion_r805040012
##########
File path: tests/javaobject/CMakeLists.txt
##########
@@ -22,6 +22,11 @@ include(UseJava)
file(GLOB_RECURSE SOURCES "*.java")
+# Check for versions of GEODE that don't support AuthenticationExpiredException
+if (${Geode_VERSION} VERSION_LESS ${GEODE_AUTHEXPIREDEXCEPTION_VERSION})
Review comment:
It needs to be a variable that we can override on the cmake config command line, for closed source builds:
cmake ... -DGEODE_AUTHEXPIREDEXCEPTION_VERSION=9.15.0
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on a change in pull request #917: GEODE-10004: support older geode versions
Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on a change in pull request #917:
URL: https://github.com/apache/geode-native/pull/917#discussion_r804977432
##########
File path: CMakeLists.txt
##########
@@ -61,6 +61,7 @@ set(PRODUCT_BUILDDATE "1970-01-01" CACHE STRING "Product build date")
set(PRODUCT_SOURCE_REVISION "0000000000000000000000000000000000000000" CACHE STRING "Product source SHA")
set(PRODUCT_SOURCE_REPOSITORY "<unspecified>" CACHE STRING "Product source branch")
set(PRODUCT_BITS "${BUILD_BITS}bit")
+set(GEODE_AUTHEXPIREDEXCEPTION_VERSION "1.15.0" CACHE STRING "First build containing AuthenticationExpiredException")
Review comment:
Why is this a cache variable and why in the root project. This should just be a string literal in the javaobject project.
##########
File path: cmake/FindGeode.cmake
##########
@@ -73,7 +73,7 @@ if(Geode_gfsh_EXECUTABLE)
# TODO error checking
else()
if(var MATCHES "([0-9]+\\.[0-9]+\\.[0-9]+)")
- set(Geode_VERSION "${CMAKE_MATCH_1}")
+ set(Geode_VERSION ${var})
Review comment:
I agree, the group 1 has all the parts we care about in the version.
##########
File path: tests/javaobject/CMakeLists.txt
##########
@@ -22,6 +22,11 @@ include(UseJava)
file(GLOB_RECURSE SOURCES "*.java")
+# Check for versions of GEODE that don't support AuthenticationExpiredException
+if (${Geode_VERSION} VERSION_LESS ${GEODE_AUTHEXPIREDEXCEPTION_VERSION})
Review comment:
Use version literal, `"1.15.0"`, here.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode-native] mmartell merged pull request #917: GEODE-10004: support older geode versions
Posted by GitBox <gi...@apache.org>.
mmartell merged pull request #917:
URL: https://github.com/apache/geode-native/pull/917
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode-native] pdxcodemonkey commented on a change in pull request #917: GEODE-10004: support older geode versions
Posted by GitBox <gi...@apache.org>.
pdxcodemonkey commented on a change in pull request #917:
URL: https://github.com/apache/geode-native/pull/917#discussion_r806084212
##########
File path: cmake/FindGeode.cmake
##########
@@ -73,7 +73,8 @@ if(Geode_gfsh_EXECUTABLE)
# TODO error checking
else()
if(var MATCHES "([0-9]+\\.[0-9]+\\.[0-9]+)")
- set(Geode_VERSION "${CMAKE_MATCH_1}")
+ set(Geode_VERSION "${CMAKE_MATCH_1}")
+ set(Geode_VERSION_FULL ${var})
Review comment:
Unless I'm mistaken, nothing in this change depends on Geode_VERSION any more, so there should be no changes necessary in this file. Please remove, we don't want to accidentally introduce any new ways to break the build.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode-native] pdxcodemonkey commented on a change in pull request #917: GEODE-10004: support older geode versions
Posted by GitBox <gi...@apache.org>.
pdxcodemonkey commented on a change in pull request #917:
URL: https://github.com/apache/geode-native/pull/917#discussion_r806084212
##########
File path: cmake/FindGeode.cmake
##########
@@ -73,7 +73,8 @@ if(Geode_gfsh_EXECUTABLE)
# TODO error checking
else()
if(var MATCHES "([0-9]+\\.[0-9]+\\.[0-9]+)")
- set(Geode_VERSION "${CMAKE_MATCH_1}")
+ set(Geode_VERSION "${CMAKE_MATCH_1}")
+ set(Geode_VERSION_FULL ${var})
Review comment:
Unless I'm mistaken, nothing in this change depends on Geode_VERSION any more, so there should be no changes necessary in this file. Please remove, we don't want to accidentally introduce any new ways to break the build.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode-native] mmartell commented on a change in pull request #917: GEODE-10004: support older geode versions
Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #917:
URL: https://github.com/apache/geode-native/pull/917#discussion_r806070730
##########
File path: CMakeLists.txt
##########
@@ -61,6 +61,7 @@ set(PRODUCT_BUILDDATE "1970-01-01" CACHE STRING "Product build date")
set(PRODUCT_SOURCE_REVISION "0000000000000000000000000000000000000000" CACHE STRING "Product source SHA")
set(PRODUCT_SOURCE_REPOSITORY "<unspecified>" CACHE STRING "Product source branch")
set(PRODUCT_BITS "${BUILD_BITS}bit")
+set(GEODE_AUTHEXPIREDEXCEPTION_VERSION "1.15.0" CACHE STRING "First build containing AuthenticationExpiredException")
Review comment:
Removed.
##########
File path: tests/javaobject/CMakeLists.txt
##########
@@ -22,6 +22,11 @@ include(UseJava)
file(GLOB_RECURSE SOURCES "*.java")
+# Check for versions of GEODE that don't support AuthenticationExpiredException
+if (${Geode_VERSION} VERSION_LESS ${GEODE_AUTHEXPIREDEXCEPTION_VERSION})
Review comment:
No longer need to define the variable. Switched to searching for the Java class.
##########
File path: cmake/FindGeode.cmake
##########
@@ -73,7 +73,8 @@ if(Geode_gfsh_EXECUTABLE)
# TODO error checking
else()
if(var MATCHES "([0-9]+\\.[0-9]+\\.[0-9]+)")
- set(Geode_VERSION "${CMAKE_MATCH_1}")
+ set(Geode_VERSION "${CMAKE_MATCH_1}")
+ set(Geode_VERSION_FULL ${var})
Review comment:
Good catch!
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode-native] mmartell commented on a change in pull request #917: GEODE-10004: support older geode versions
Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #917:
URL: https://github.com/apache/geode-native/pull/917#discussion_r806108856
##########
File path: cmake/FindGeode.cmake
##########
@@ -73,7 +73,8 @@ if(Geode_gfsh_EXECUTABLE)
# TODO error checking
else()
if(var MATCHES "([0-9]+\\.[0-9]+\\.[0-9]+)")
- set(Geode_VERSION "${CMAKE_MATCH_1}")
+ set(Geode_VERSION "${CMAKE_MATCH_1}")
+ set(Geode_VERSION_FULL ${var})
Review comment:
Good catch!
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on a change in pull request #917: GEODE-10004: support older geode versions
Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on a change in pull request #917:
URL: https://github.com/apache/geode-native/pull/917#discussion_r803341661
##########
File path: CMakeLists.txt
##########
@@ -61,6 +61,7 @@ set(PRODUCT_BUILDDATE "1970-01-01" CACHE STRING "Product build date")
set(PRODUCT_SOURCE_REVISION "0000000000000000000000000000000000000000" CACHE STRING "Product source SHA")
set(PRODUCT_SOURCE_REPOSITORY "<unspecified>" CACHE STRING "Product source branch")
set(PRODUCT_BITS "${BUILD_BITS}bit")
+set(GEODE_AUTHEXPIREDEXCEPTION_VERSION "1.15.0-build.546" CACHE STRING "First build containing AuthenticationExpiredException")
Review comment:
Why not just test for the existence of the exception class or not and set a "has feature" type flag?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on a change in pull request #917: GEODE-10004: support older geode versions
Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on a change in pull request #917:
URL: https://github.com/apache/geode-native/pull/917#discussion_r803887440
##########
File path: CMakeLists.txt
##########
@@ -61,6 +61,7 @@ set(PRODUCT_BUILDDATE "1970-01-01" CACHE STRING "Product build date")
set(PRODUCT_SOURCE_REVISION "0000000000000000000000000000000000000000" CACHE STRING "Product source SHA")
set(PRODUCT_SOURCE_REPOSITORY "<unspecified>" CACHE STRING "Product source branch")
set(PRODUCT_BITS "${BUILD_BITS}bit")
+set(GEODE_AUTHEXPIREDEXCEPTION_VERSION "1.15.0-build.546" CACHE STRING "First build containing AuthenticationExpiredException")
Review comment:
Ok, fair enough, but why the specific build number. When building locally on your machine, since Apache doesn't do binary releases, that build number is almost always 0. Just detect "1.15.0". We don't need to handle building sources between GAs.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode-native] pdxcodemonkey commented on a change in pull request #917: GEODE-10004: support older geode versions
Posted by GitBox <gi...@apache.org>.
pdxcodemonkey commented on a change in pull request #917:
URL: https://github.com/apache/geode-native/pull/917#discussion_r804954533
##########
File path: cmake/FindGeode.cmake
##########
@@ -73,7 +73,7 @@ if(Geode_gfsh_EXECUTABLE)
# TODO error checking
else()
if(var MATCHES "([0-9]+\\.[0-9]+\\.[0-9]+)")
- set(Geode_VERSION "${CMAKE_MATCH_1}")
+ set(Geode_VERSION ${var})
Review comment:
What is this all about? I don't understand why we need a change in the Geode find module...
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode-native] mmartell commented on a change in pull request #917: GEODE-10004: support older geode versions
Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #917:
URL: https://github.com/apache/geode-native/pull/917#discussion_r805034612
##########
File path: cmake/FindGeode.cmake
##########
@@ -73,7 +73,7 @@ if(Geode_gfsh_EXECUTABLE)
# TODO error checking
else()
if(var MATCHES "([0-9]+\\.[0-9]+\\.[0-9]+)")
- set(Geode_VERSION "${CMAKE_MATCH_1}")
+ set(Geode_VERSION ${var})
Review comment:
Good catch! No longer need the build number since we're just comparing with first 3 parts.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode-native] pdxcodemonkey commented on a change in pull request #917: GEODE-10004: support older geode versions
Posted by GitBox <gi...@apache.org>.
pdxcodemonkey commented on a change in pull request #917:
URL: https://github.com/apache/geode-native/pull/917#discussion_r803874498
##########
File path: CMakeLists.txt
##########
@@ -61,6 +61,7 @@ set(PRODUCT_BUILDDATE "1970-01-01" CACHE STRING "Product build date")
set(PRODUCT_SOURCE_REVISION "0000000000000000000000000000000000000000" CACHE STRING "Product source SHA")
set(PRODUCT_SOURCE_REPOSITORY "<unspecified>" CACHE STRING "Product source branch")
set(PRODUCT_BITS "${BUILD_BITS}bit")
+set(GEODE_AUTHEXPIREDEXCEPTION_VERSION "1.15.0-build.546" CACHE STRING "First build containing AuthenticationExpiredException")
Review comment:
Just because that's a more complex fix, and neither of us really have a clue how to implement it using cmake. Like how would we do this, go dump the class list from a jar file after we find Geode? Which of the umpteen Geode jar files should we look in? How do we run the jar tool at this stage via cmake? After we do or do not find the exception class, how do we put that back into a cmake variable we can use to conditionally compile our Java?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode-native] mmartell merged pull request #917: GEODE-10004: support older geode versions
Posted by GitBox <gi...@apache.org>.
mmartell merged pull request #917:
URL: https://github.com/apache/geode-native/pull/917
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on a change in pull request #917: GEODE-10004: support older geode versions
Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on a change in pull request #917:
URL: https://github.com/apache/geode-native/pull/917#discussion_r803887968
##########
File path: CMakeLists.txt
##########
@@ -61,6 +61,7 @@ set(PRODUCT_BUILDDATE "1970-01-01" CACHE STRING "Product build date")
set(PRODUCT_SOURCE_REVISION "0000000000000000000000000000000000000000" CACHE STRING "Product source SHA")
set(PRODUCT_SOURCE_REPOSITORY "<unspecified>" CACHE STRING "Product source branch")
set(PRODUCT_BITS "${BUILD_BITS}bit")
+set(GEODE_AUTHEXPIREDEXCEPTION_VERSION "1.15.0-build.546" CACHE STRING "First build containing AuthenticationExpiredException")
Review comment:
Also the GA release ends up with build 0 too.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode-native] mmartell commented on a change in pull request #917: GEODE-10004: support older geode versions
Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #917:
URL: https://github.com/apache/geode-native/pull/917#discussion_r804380249
##########
File path: CMakeLists.txt
##########
@@ -61,6 +61,7 @@ set(PRODUCT_BUILDDATE "1970-01-01" CACHE STRING "Product build date")
set(PRODUCT_SOURCE_REVISION "0000000000000000000000000000000000000000" CACHE STRING "Product source SHA")
set(PRODUCT_SOURCE_REPOSITORY "<unspecified>" CACHE STRING "Product source branch")
set(PRODUCT_BITS "${BUILD_BITS}bit")
+set(GEODE_AUTHEXPIREDEXCEPTION_VERSION "1.15.0-build.546" CACHE STRING "First build containing AuthenticationExpiredException")
Review comment:
Good suggestion. Makes sense to use 1.15.0.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode-native] mmartell commented on a change in pull request #917: GEODE-10004: support older geode versions
Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #917:
URL: https://github.com/apache/geode-native/pull/917#discussion_r806070730
##########
File path: CMakeLists.txt
##########
@@ -61,6 +61,7 @@ set(PRODUCT_BUILDDATE "1970-01-01" CACHE STRING "Product build date")
set(PRODUCT_SOURCE_REVISION "0000000000000000000000000000000000000000" CACHE STRING "Product source SHA")
set(PRODUCT_SOURCE_REPOSITORY "<unspecified>" CACHE STRING "Product source branch")
set(PRODUCT_BITS "${BUILD_BITS}bit")
+set(GEODE_AUTHEXPIREDEXCEPTION_VERSION "1.15.0" CACHE STRING "First build containing AuthenticationExpiredException")
Review comment:
Removed.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode-native] mmartell commented on a change in pull request #917: GEODE-10004: support older geode versions
Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #917:
URL: https://github.com/apache/geode-native/pull/917#discussion_r805041305
##########
File path: CMakeLists.txt
##########
@@ -61,6 +61,7 @@ set(PRODUCT_BUILDDATE "1970-01-01" CACHE STRING "Product build date")
set(PRODUCT_SOURCE_REVISION "0000000000000000000000000000000000000000" CACHE STRING "Product source SHA")
set(PRODUCT_SOURCE_REPOSITORY "<unspecified>" CACHE STRING "Product source branch")
set(PRODUCT_BITS "${BUILD_BITS}bit")
+set(GEODE_AUTHEXPIREDEXCEPTION_VERSION "1.15.0" CACHE STRING "First build containing AuthenticationExpiredException")
Review comment:
Seems reasonable to put it in the cmake config file for the project it applies to.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode-native] pdxcodemonkey commented on a change in pull request #917: GEODE-10004: support older geode versions
Posted by GitBox <gi...@apache.org>.
pdxcodemonkey commented on a change in pull request #917:
URL: https://github.com/apache/geode-native/pull/917#discussion_r805041043
##########
File path: CMakeLists.txt
##########
@@ -61,6 +61,7 @@ set(PRODUCT_BUILDDATE "1970-01-01" CACHE STRING "Product build date")
set(PRODUCT_SOURCE_REVISION "0000000000000000000000000000000000000000" CACHE STRING "Product source SHA")
set(PRODUCT_SOURCE_REPOSITORY "<unspecified>" CACHE STRING "Product source branch")
set(PRODUCT_BITS "${BUILD_BITS}bit")
+set(GEODE_AUTHEXPIREDEXCEPTION_VERSION "1.15.0" CACHE STRING "First build containing AuthenticationExpiredException")
Review comment:
See Martell's comment - this is a string variable because there may be different version(s) to test against, depending on which product is being built.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode-native] mmartell commented on a change in pull request #917: GEODE-10004: support older geode versions
Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #917:
URL: https://github.com/apache/geode-native/pull/917#discussion_r806072155
##########
File path: tests/javaobject/CMakeLists.txt
##########
@@ -22,6 +22,11 @@ include(UseJava)
file(GLOB_RECURSE SOURCES "*.java")
+# Check for versions of GEODE that don't support AuthenticationExpiredException
+if (${Geode_VERSION} VERSION_LESS ${GEODE_AUTHEXPIREDEXCEPTION_VERSION})
Review comment:
No longer need to define the variable. Switched to searching for the Java class.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org