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 2021/07/16 19:18:13 UTC

[GitHub] [geode-native] mreddington opened a new pull request #831: GEODE-9431: Replaced std regex with Boost due to RHEL8.

mreddington opened a new pull request #831:
URL: https://github.com/apache/geode-native/pull/831


   


-- 
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 #831: GEODE-9431: Replaced std regex with Boost due to RHEL8.

Posted by GitBox <gi...@apache.org>.
pdxcodemonkey commented on a change in pull request #831:
URL: https://github.com/apache/geode-native/pull/831#discussion_r672430467



##########
File path: cppcache/include/geode/CacheableString.hpp
##########
@@ -54,11 +54,11 @@ class APACHE_GEODE_EXPORT CacheableString
       : m_str(std::move(value)), m_hashcode(0) {
     bool ascii = isAscii(m_str);
 
-    m_type = m_str.length() > (std::numeric_limits<uint16_t>::max)()
-                 ? ascii ? DSCode::CacheableASCIIStringHuge
-                         : DSCode::CacheableStringHuge
-             : ascii ? DSCode::CacheableASCIIString
-                     : DSCode::CacheableString;
+    m_type =
+        m_str.length() > (std::numeric_limits<uint16_t>::max)()
+            ? ascii ? DSCode::CacheableASCIIStringHuge
+                    : DSCode::CacheableStringHuge
+            : ascii ? DSCode::CacheableASCIIString : DSCode::CacheableString;

Review comment:
       Fixed - just another artifact of the fact that the CI version of clangformat is "special" :/




-- 
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 pull request #831: GEODE-9431: Replaced std regex with Boost due to RHEL8.

Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on pull request #831:
URL: https://github.com/apache/geode-native/pull/831#issuecomment-882667398


   > > This seems like a really big hammer for what strikes me an issue with maybe a specific RHEL C++ runtime patch or gcc patch. I didn't see any open tickets with RHEL or GNU regarding these issues in the GEODE ticket. Given that boost::regex is the basis for std::regex if we are using std::regex incorrectly resulting in different undefined behavior in RHEL then I would be just as concerned with our use of boost::regex.
   > 
   > I spent most of last week picking up and discarding various "hammers" as solutions to this problem - this was the first one that worked. Let me know what mitigations you think we need for this, I'm fine with most things. If we need/want a JIRA ticket to revert this back once it's fixed by RedHat or whomever, that's fine. I would also argue it's not really _that_ big of a hammer, since the change itself took about an hour to make and verify, and backing it out is the work of a few minutes. I'll figure out how to file the bug with RedHat and GNU today, but I don't think waiting for them to provide a fix is a viable strategy. We need to get our CI pipeline off the floor so we can proceed with dev work and know we're not further damaging the product on RHEL-8.
   
   As long as we can get issues opened with RH and and with ourselves to revert back to `std::regex` once we or RH fixes the underlying issue, I could get on board with this change.


-- 
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 pull request #831: GEODE-9431: Replaced std regex with Boost due to RHEL8.

Posted by GitBox <gi...@apache.org>.
pdxcodemonkey commented on pull request #831:
URL: https://github.com/apache/geode-native/pull/831#issuecomment-882661385


   > This seems like a really big hammer for what strikes me an issue with maybe a specific RHEL C++ runtime patch or gcc patch. I didn't see any open tickets with RHEL or GNU regarding these issues in the GEODE ticket. Given that boost::regex is the basis for std::regex if we are using std::regex incorrectly resulting in different undefined behavior in RHEL then I would be just as concerned with our use of boost::regex.
   
   I spent most of last week picking up and discarding various "hammers" as solutions to this problem - this was the first one that worked.  Let me know what mitigations you think we need for this, I'm fine with most things.  If we need/want a JIRA ticket to revert this back once it's fixed by RedHat or whomever, that's fine.  I would also argue it's not really _that_ big of a hammer, since the change itself took about an hour to make and verify, and backing it out is the work of a few minutes.  I'll figure out how to file the bug with RedHat and GNU today, but I don't think waiting for them to provide a fix is a viable strategy.  We need to get our CI pipeline off the floor so we can proceed with dev work and know we're not further damaging the product on RHEL-8.


-- 
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 pull request #831: GEODE-9431: Replaced std regex with Boost due to RHEL8.

Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on pull request #831:
URL: https://github.com/apache/geode-native/pull/831#issuecomment-882667398


   > > This seems like a really big hammer for what strikes me an issue with maybe a specific RHEL C++ runtime patch or gcc patch. I didn't see any open tickets with RHEL or GNU regarding these issues in the GEODE ticket. Given that boost::regex is the basis for std::regex if we are using std::regex incorrectly resulting in different undefined behavior in RHEL then I would be just as concerned with our use of boost::regex.
   > 
   > I spent most of last week picking up and discarding various "hammers" as solutions to this problem - this was the first one that worked. Let me know what mitigations you think we need for this, I'm fine with most things. If we need/want a JIRA ticket to revert this back once it's fixed by RedHat or whomever, that's fine. I would also argue it's not really _that_ big of a hammer, since the change itself took about an hour to make and verify, and backing it out is the work of a few minutes. I'll figure out how to file the bug with RedHat and GNU today, but I don't think waiting for them to provide a fix is a viable strategy. We need to get our CI pipeline off the floor so we can proceed with dev work and know we're not further damaging the product on RHEL-8.
   
   As long as we can get issues opened with RH and and with ourselves to revert back to `std::regex` once we or RH fixes the underlying issue, I could get on board with this change.


-- 
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 pull request #831: GEODE-9431: Replaced std regex with Boost due to RHEL8.

Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on pull request #831:
URL: https://github.com/apache/geode-native/pull/831#issuecomment-882667398


   > > This seems like a really big hammer for what strikes me an issue with maybe a specific RHEL C++ runtime patch or gcc patch. I didn't see any open tickets with RHEL or GNU regarding these issues in the GEODE ticket. Given that boost::regex is the basis for std::regex if we are using std::regex incorrectly resulting in different undefined behavior in RHEL then I would be just as concerned with our use of boost::regex.
   > 
   > I spent most of last week picking up and discarding various "hammers" as solutions to this problem - this was the first one that worked. Let me know what mitigations you think we need for this, I'm fine with most things. If we need/want a JIRA ticket to revert this back once it's fixed by RedHat or whomever, that's fine. I would also argue it's not really _that_ big of a hammer, since the change itself took about an hour to make and verify, and backing it out is the work of a few minutes. I'll figure out how to file the bug with RedHat and GNU today, but I don't think waiting for them to provide a fix is a viable strategy. We need to get our CI pipeline off the floor so we can proceed with dev work and know we're not further damaging the product on RHEL-8.
   
   As long as we can get issues opened with RH and and with ourselves to revert back to `std::regex` once we or RH fixes the underlying issue, I could get on board with this change.


-- 
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 #831: GEODE-9431: Replaced std regex with Boost due to RHEL8.

Posted by GitBox <gi...@apache.org>.
pdxcodemonkey commented on a change in pull request #831:
URL: https://github.com/apache/geode-native/pull/831#discussion_r672430467



##########
File path: cppcache/include/geode/CacheableString.hpp
##########
@@ -54,11 +54,11 @@ class APACHE_GEODE_EXPORT CacheableString
       : m_str(std::move(value)), m_hashcode(0) {
     bool ascii = isAscii(m_str);
 
-    m_type = m_str.length() > (std::numeric_limits<uint16_t>::max)()
-                 ? ascii ? DSCode::CacheableASCIIStringHuge
-                         : DSCode::CacheableStringHuge
-             : ascii ? DSCode::CacheableASCIIString
-                     : DSCode::CacheableString;
+    m_type =
+        m_str.length() > (std::numeric_limits<uint16_t>::max)()
+            ? ascii ? DSCode::CacheableASCIIStringHuge
+                    : DSCode::CacheableStringHuge
+            : ascii ? DSCode::CacheableASCIIString : DSCode::CacheableString;

Review comment:
       Fixed - just another artifact of the fact that the CI version of clangformat is "special" :/




-- 
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 #831: GEODE-9431: Replaced std regex with Boost due to RHEL8.

Posted by GitBox <gi...@apache.org>.
pdxcodemonkey commented on a change in pull request #831:
URL: https://github.com/apache/geode-native/pull/831#discussion_r672430467



##########
File path: cppcache/include/geode/CacheableString.hpp
##########
@@ -54,11 +54,11 @@ class APACHE_GEODE_EXPORT CacheableString
       : m_str(std::move(value)), m_hashcode(0) {
     bool ascii = isAscii(m_str);
 
-    m_type = m_str.length() > (std::numeric_limits<uint16_t>::max)()
-                 ? ascii ? DSCode::CacheableASCIIStringHuge
-                         : DSCode::CacheableStringHuge
-             : ascii ? DSCode::CacheableASCIIString
-                     : DSCode::CacheableString;
+    m_type =
+        m_str.length() > (std::numeric_limits<uint16_t>::max)()
+            ? ascii ? DSCode::CacheableASCIIStringHuge
+                    : DSCode::CacheableStringHuge
+            : ascii ? DSCode::CacheableASCIIString : DSCode::CacheableString;

Review comment:
       Fixed - just another artifact of the fact that the CI version of clangformat is "special" :/




-- 
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 #831: GEODE-9431: Replaced std regex with Boost due to RHEL8.

Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on a change in pull request #831:
URL: https://github.com/apache/geode-native/pull/831#discussion_r673239968



##########
File path: cppcache/test/ByteArrayFixture.cpp
##########
@@ -20,7 +20,7 @@
 ::testing::AssertionResult ByteArrayFixture::assertByteArrayEqual(
     const char* /*expectedStr*/, const char* /*bytesStr*/, const char* expected,
     const apache::geode::client::ByteArray& bytes) {
-  // One would normally just use std::regex but gcc 4.4.7 is lacking.
+  // One would normally just use boost::regex but gcc 4.4.7 is lacking.

Review comment:
       Yup




-- 
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 #831: GEODE-9431: Replaced std regex with Boost due to RHEL8.

Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on a change in pull request #831:
URL: https://github.com/apache/geode-native/pull/831#discussion_r672434300



##########
File path: cppcache/include/geode/CacheableString.hpp
##########
@@ -54,11 +54,11 @@ class APACHE_GEODE_EXPORT CacheableString
       : m_str(std::move(value)), m_hashcode(0) {
     bool ascii = isAscii(m_str);
 
-    m_type = m_str.length() > (std::numeric_limits<uint16_t>::max)()
-                 ? ascii ? DSCode::CacheableASCIIStringHuge
-                         : DSCode::CacheableStringHuge
-             : ascii ? DSCode::CacheableASCIIString
-                     : DSCode::CacheableString;
+    m_type =
+        m_str.length() > (std::numeric_limits<uint16_t>::max)()
+            ? ascii ? DSCode::CacheableASCIIStringHuge
+                    : DSCode::CacheableStringHuge
+            : ascii ? DSCode::CacheableASCIIString : DSCode::CacheableString;

Review comment:
       Did you make these changes on Windows? The CI version and macOS version seem to be in agreement. 




-- 
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 #831: GEODE-9431: Replaced std regex with Boost due to RHEL8.

Posted by GitBox <gi...@apache.org>.
pdxcodemonkey commented on a change in pull request #831:
URL: https://github.com/apache/geode-native/pull/831#discussion_r673218319



##########
File path: cppcache/test/ByteArrayFixture.cpp
##########
@@ -20,7 +20,7 @@
 ::testing::AssertionResult ByteArrayFixture::assertByteArrayEqual(
     const char* /*expectedStr*/, const char* /*bytesStr*/, const char* expected,
     const apache::geode::client::ByteArray& bytes) {
-  // One would normally just use std::regex but gcc 4.4.7 is lacking.
+  // One would normally just use boost::regex but gcc 4.4.7 is lacking.

Review comment:
       Hmmm, impenetrable test code with a somewhat cryptic comment about a compiler we no longer use.  How about we just delete the comment and move on?




-- 
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 merged pull request #831: GEODE-9431: Replaced std regex with Boost due to RHEL8.

Posted by GitBox <gi...@apache.org>.
pdxcodemonkey merged pull request #831:
URL: https://github.com/apache/geode-native/pull/831


   


-- 
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 pull request #831: GEODE-9431: Replaced std regex with Boost due to RHEL8.

Posted by GitBox <gi...@apache.org>.
pdxcodemonkey commented on pull request #831:
URL: https://github.com/apache/geode-native/pull/831#issuecomment-882661385


   > This seems like a really big hammer for what strikes me an issue with maybe a specific RHEL C++ runtime patch or gcc patch. I didn't see any open tickets with RHEL or GNU regarding these issues in the GEODE ticket. Given that boost::regex is the basis for std::regex if we are using std::regex incorrectly resulting in different undefined behavior in RHEL then I would be just as concerned with our use of boost::regex.
   
   I spent most of last week picking up and discarding various "hammers" as solutions to this problem - this was the first one that worked.  Let me know what mitigations you think we need for this, I'm fine with most things.  If we need/want a JIRA ticket to revert this back once it's fixed by RedHat or whomever, that's fine.  I would also argue it's not really _that_ big of a hammer, since the change itself took about an hour to make and verify, and backing it out is the work of a few minutes.  I'll figure out how to file the bug with RedHat and GNU today, but I don't think waiting for them to provide a fix is a viable strategy.  We need to get our CI pipeline off the floor so we can proceed with dev work and know we're not further damaging the product on RHEL-8.


-- 
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 #831: GEODE-9431: Replaced std regex with Boost due to RHEL8.

Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on a change in pull request #831:
URL: https://github.com/apache/geode-native/pull/831#discussion_r672432754



##########
File path: cppcache/test/ByteArrayFixture.cpp
##########
@@ -20,7 +20,7 @@
 ::testing::AssertionResult ByteArrayFixture::assertByteArrayEqual(
     const char* /*expectedStr*/, const char* /*bytesStr*/, const char* expected,
     const apache::geode::client::ByteArray& bytes) {
-  // One would normally just use std::regex but gcc 4.4.7 is lacking.
+  // One would normally just use boost::regex but gcc 4.4.7 is lacking.

Review comment:
       How about just resolving the issue in the comment. We don't support GCC 4.4.7 anymore.

##########
File path: cppcache/include/geode/CacheableString.hpp
##########
@@ -54,11 +54,11 @@ class APACHE_GEODE_EXPORT CacheableString
       : m_str(std::move(value)), m_hashcode(0) {
     bool ascii = isAscii(m_str);
 
-    m_type = m_str.length() > (std::numeric_limits<uint16_t>::max)()
-                 ? ascii ? DSCode::CacheableASCIIStringHuge
-                         : DSCode::CacheableStringHuge
-             : ascii ? DSCode::CacheableASCIIString
-                     : DSCode::CacheableString;
+    m_type =
+        m_str.length() > (std::numeric_limits<uint16_t>::max)()
+            ? ascii ? DSCode::CacheableASCIIStringHuge
+                    : DSCode::CacheableStringHuge
+            : ascii ? DSCode::CacheableASCIIString : DSCode::CacheableString;

Review comment:
       Did you make these changes on Windows? The CI version and macOS version seem to be in agreement. 




-- 
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 #831: GEODE-9431: Replaced std regex with Boost due to RHEL8.

Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on a change in pull request #831:
URL: https://github.com/apache/geode-native/pull/831#discussion_r672432754



##########
File path: cppcache/test/ByteArrayFixture.cpp
##########
@@ -20,7 +20,7 @@
 ::testing::AssertionResult ByteArrayFixture::assertByteArrayEqual(
     const char* /*expectedStr*/, const char* /*bytesStr*/, const char* expected,
     const apache::geode::client::ByteArray& bytes) {
-  // One would normally just use std::regex but gcc 4.4.7 is lacking.
+  // One would normally just use boost::regex but gcc 4.4.7 is lacking.

Review comment:
       How about just resolving the issue in the comment. We don't support GCC 4.4.7 anymore.

##########
File path: cppcache/include/geode/CacheableString.hpp
##########
@@ -54,11 +54,11 @@ class APACHE_GEODE_EXPORT CacheableString
       : m_str(std::move(value)), m_hashcode(0) {
     bool ascii = isAscii(m_str);
 
-    m_type = m_str.length() > (std::numeric_limits<uint16_t>::max)()
-                 ? ascii ? DSCode::CacheableASCIIStringHuge
-                         : DSCode::CacheableStringHuge
-             : ascii ? DSCode::CacheableASCIIString
-                     : DSCode::CacheableString;
+    m_type =
+        m_str.length() > (std::numeric_limits<uint16_t>::max)()
+            ? ascii ? DSCode::CacheableASCIIStringHuge
+                    : DSCode::CacheableStringHuge
+            : ascii ? DSCode::CacheableASCIIString : DSCode::CacheableString;

Review comment:
       Did you make these changes on Windows? The CI version and macOS version seem to be in agreement. 




-- 
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 #831: GEODE-9431: Replaced std regex with Boost due to RHEL8.

Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on a change in pull request #831:
URL: https://github.com/apache/geode-native/pull/831#discussion_r671514280



##########
File path: cppcache/include/geode/CacheableString.hpp
##########
@@ -54,11 +54,11 @@ class APACHE_GEODE_EXPORT CacheableString
       : m_str(std::move(value)), m_hashcode(0) {
     bool ascii = isAscii(m_str);
 
-    m_type = m_str.length() > (std::numeric_limits<uint16_t>::max)()
-                 ? ascii ? DSCode::CacheableASCIIStringHuge
-                         : DSCode::CacheableStringHuge
-             : ascii ? DSCode::CacheableASCIIString
-                     : DSCode::CacheableString;
+    m_type =
+        m_str.length() > (std::numeric_limits<uint16_t>::max)()
+            ? ascii ? DSCode::CacheableASCIIStringHuge
+                    : DSCode::CacheableStringHuge
+            : ascii ? DSCode::CacheableASCIIString : DSCode::CacheableString;

Review comment:
       What is going on with formatting changes. There are several in this PR that don't appear related to the change at hand and some that don't seem to conform to the style guide.




-- 
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 pull request #831: GEODE-9431: Replaced std regex with Boost due to RHEL8.

Posted by GitBox <gi...@apache.org>.
pdxcodemonkey commented on pull request #831:
URL: https://github.com/apache/geode-native/pull/831#issuecomment-882661385


   > This seems like a really big hammer for what strikes me an issue with maybe a specific RHEL C++ runtime patch or gcc patch. I didn't see any open tickets with RHEL or GNU regarding these issues in the GEODE ticket. Given that boost::regex is the basis for std::regex if we are using std::regex incorrectly resulting in different undefined behavior in RHEL then I would be just as concerned with our use of boost::regex.
   
   I spent most of last week picking up and discarding various "hammers" as solutions to this problem - this was the first one that worked.  Let me know what mitigations you think we need for this, I'm fine with most things.  If we need/want a JIRA ticket to revert this back once it's fixed by RedHat or whomever, that's fine.  I would also argue it's not really _that_ big of a hammer, since the change itself took about an hour to make and verify, and backing it out is the work of a few minutes.  I'll figure out how to file the bug with RedHat and GNU today, but I don't think waiting for them to provide a fix is a viable strategy.  We need to get our CI pipeline off the floor so we can proceed with dev work and know we're not further damaging the product on RHEL-8.


-- 
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 #831: GEODE-9431: Replaced std regex with Boost due to RHEL8.

Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on a change in pull request #831:
URL: https://github.com/apache/geode-native/pull/831#discussion_r672432754



##########
File path: cppcache/test/ByteArrayFixture.cpp
##########
@@ -20,7 +20,7 @@
 ::testing::AssertionResult ByteArrayFixture::assertByteArrayEqual(
     const char* /*expectedStr*/, const char* /*bytesStr*/, const char* expected,
     const apache::geode::client::ByteArray& bytes) {
-  // One would normally just use std::regex but gcc 4.4.7 is lacking.
+  // One would normally just use boost::regex but gcc 4.4.7 is lacking.

Review comment:
       How about just resolving the issue in the comment. We don't support GCC 4.4.7 anymore.




-- 
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