kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a...@apache.org
Subject [kudu] branch master updated: version_util: allow period as delimiter in extra and tighten version syntax
Date Wed, 24 Jul 2019 03:23:52 GMT
This is an automated email from the ASF dual-hosted git repository.

adar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git


The following commit(s) were added to refs/heads/master by this push:
     new 7a8d3aa  version_util: allow period as delimiter in extra and tighten version syntax
7a8d3aa is described below

commit 7a8d3aa2164c9cc2955438347ecae9abcb09ccb5
Author: Adar Dembo <adar@cloudera.com>
AuthorDate: Tue Jul 23 00:34:36 2019 -0700

    version_util: allow period as delimiter in extra and tighten version syntax
    
    This will permit versions like <major>.<minor>.<maintenance>.<vendor_ver>,
    a common pattern used by at least one Kudu vendor. The change shouldn't have
    any tangible effect upstream as we've always ignored the value of 'extra'.
    
    The string splitting wasn't cutting it for me so I rewrote the parsing to
    use POSIX extended regexes. The major side effect is that we're no longer
    loosey goosey with components (i.e. "1. 2  .  3" or "+1.+2.+3"); each must
    be a subsequence of nothing but digits. I don't think anyone was relying on
    this excessive permissiveness and I find the new code to be more readable.
    
    Change-Id: I5502efa8ba378d96432e530366e74a82c83621f7
    Reviewed-on: http://gerrit.cloudera.org:8080/13897
    Reviewed-by: Alexey Serbin <aserbin@cloudera.com>
    Reviewed-by: Andrew Wong <awong@cloudera.com>
    Tested-by: Kudu Jenkins
---
 src/kudu/util/version_util-test.cc | 15 ++++++++---
 src/kudu/util/version_util.cc      | 51 ++++++++++++++++++++++++--------------
 src/kudu/util/version_util.h       |  4 +--
 3 files changed, 46 insertions(+), 24 deletions(-)

diff --git a/src/kudu/util/version_util-test.cc b/src/kudu/util/version_util-test.cc
index 3cb39b9..d06df0d 100644
--- a/src/kudu/util/version_util-test.cc
+++ b/src/kudu/util/version_util-test.cc
@@ -45,14 +45,11 @@ TEST(VersionUtilTest, TestVersion) {
     { { "1.2.3-SNAPSHOT", 1, 2, 3, "SNAPSHOT" }, "1.2.3-SNAPSHOT" },
     { { "1.8.0-x-SNAPSHOT", 1, 8, 0, "x-SNAPSHOT" }, "1.8.0-x-SNAPSHOT" },
     { { "0.1.2-a-b-c-d", 0, 1, 2, "a-b-c-d" }, "0.1.2-a-b-c-d" },
-    { { " 0 .1 .2 -x", 0, 1, 2, "x" }, "0.1.2-x" },
-    { { " 0 . 1 .  2 -x ", 0, 1, 2, "x" }, "0.1.2-x" },
-    { { "+1.+2.+3-6", 1, 2, 3, "6" }, "1.2.3-6" },
-    { { " +1 .  +2 .   +3 -100 .. ", 1, 2, 3, "100 .." }, "1.2.3-100 .." },
     // no octals: leading zeros are just chopped off
     { { "00.01.010", 0, 1, 10, "" }, "0.1.10" },
     { { "  0.1.2----suffix  ", 0, 1, 2, "---suffix" }, "0.1.2----suffix" },
     { { "0.1.2- - -x- -y- ", 0, 1, 2, " - -x- -y-" }, "0.1.2- - -x- -y-" },
+    { { "1.11.0.7.0.0.0-SNAPSHOT", 1, 11, 0, "7.0.0.0-SNAPSHOT" }, "1.11.0-7.0.0.0-SNAPSHOT"
},
   };
 
   for (const auto& test_case : good_test_cases) {
@@ -99,6 +96,16 @@ TEST(VersionUtilTest, TestVersion) {
     "1.0.foo",
     "1.0foo.bar",
     "foo5-1.4.3",
+    "1-2-3",
+    "1.2-3",
+    "1-2.3",
+    "1-2-3-SNAPSHOT",
+    "1.2-3-SNAPSHOT",
+    "1-2.3-SNAPSHOT",
+    " 0 .1 .2 -x",
+    " 0 . 1 .  2 -x ",
+    "+1.+2.+3-6",
+    " +1 .  +2 .   +3 -100 .. ",
   };
 
   for (const auto& input_str : bad_test_cases) {
diff --git a/src/kudu/util/version_util.cc b/src/kudu/util/version_util.cc
index 6715894..d5cbdee 100644
--- a/src/kudu/util/version_util.cc
+++ b/src/kudu/util/version_util.cc
@@ -17,24 +17,22 @@
 
 #include "kudu/util/version_util.h"
 
-#include <iterator>
+#include <regex.h>
+
+#include <mutex>
 #include <string>
 #include <utility>
-#include <vector>
 
 #include <glog/logging.h>
 
-#include "kudu/gutil/strings/join.h"
+#include "kudu/gutil/macros.h"
 #include "kudu/gutil/strings/numbers.h"
-#include "kudu/gutil/strings/split.h"
 #include "kudu/gutil/strings/strip.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/util/status.h"
 
 using std::ostream;
 using std::string;
-using std::vector;
-using strings::Split;
 using strings::Substitute;
 
 namespace kudu {
@@ -58,29 +56,46 @@ ostream& operator<<(ostream& os, const Version& v) {
 
 Status ParseVersion(const string& version_str,
                     Version* v) {
-  static const char* const kDelimiter = "-";
+  static regex_t re;
+  static std::once_flag once;
+  static const char* kVersionPattern =
+      "^([[:digit:]]+)\\." // <major>.
+      "([[:digit:]]+)\\."  // <minor>.
+      "([[:digit:]]+)"     // <maintenance>
+      "([.-].*)?$";        // [<delimiter><extra>]
+  std::call_once(once, []{
+      CHECK_EQ(0, regcomp(&re, kVersionPattern, REG_EXTENDED));
+    });
 
   DCHECK(v);
   const Status invalid_ver_err =
       Status::InvalidArgument("invalid version string", version_str);
   auto v_str = version_str;
   StripWhiteSpace(&v_str);
-  const vector<string> main_and_extra = Split(v_str, kDelimiter);
-  if (main_and_extra.empty()) {
+
+  regmatch_t matches[5];
+  if (regexec(&re, v_str.c_str(), arraysize(matches), matches, 0) != 0) {
     return invalid_ver_err;
   }
-  const vector<string> maj_min_maint = Split(main_and_extra.front(), ".");
-  if (maj_min_maint.size() != 3) {
-    return invalid_ver_err;
+#define PARSE_REQUIRED_COMPONENT(idx, lhs)                              \
+  {                                                                     \
+    int i = (idx);                                                      \
+    if (matches[i].rm_so == -1 ||                                       \
+        !SimpleAtoi(v_str.substr(matches[i].rm_so,                      \
+                                 matches[i].rm_eo - matches[i].rm_so),  \
+                    (lhs))) {                                           \
+      return invalid_ver_err;                                           \
+    }                                                                   \
   }
   Version temp_v;
-  if (!SimpleAtoi(maj_min_maint[0], &temp_v.major) ||
-      !SimpleAtoi(maj_min_maint[1], &temp_v.minor) ||
-      !SimpleAtoi(maj_min_maint[2], &temp_v.maintenance)) {
-    return invalid_ver_err;
+  PARSE_REQUIRED_COMPONENT(1, &temp_v.major);
+  PARSE_REQUIRED_COMPONENT(2, &temp_v.minor);
+  PARSE_REQUIRED_COMPONENT(3, &temp_v.maintenance);
+#undef PARSE_REQUIRED_COMPONENT
+  if (matches[4].rm_so != -1) {
+    temp_v.extra = v_str.substr(matches[4].rm_so + 1, // skip the delimiter
+                                matches[4].rm_eo - (matches[4].rm_so + 1));
   }
-  temp_v.extra = JoinStringsIterator(std::next(main_and_extra.begin()),
-                                     main_and_extra.end(), kDelimiter);
   temp_v.raw_version = version_str;
   *v = std::move(temp_v);
 
diff --git a/src/kudu/util/version_util.h b/src/kudu/util/version_util.h
index 446934c..2c8a1ac 100644
--- a/src/kudu/util/version_util.h
+++ b/src/kudu/util/version_util.h
@@ -26,9 +26,9 @@ namespace kudu {
 
 // A struct representing a parsed version. Versions are expected to look like
 //
-//  <major>.<minor>.<maintenance>[-<extra>]
+//  <major>.<minor>.<maintenance>[[.-]<extra>]
 //
-// e.g. 1.6.0, 1.7.1-SNAPSHOT, 1.8.0-RC1-SNAPSHOT, etc.
+// e.g. 1.6.0, 1.7.1-SNAPSHOT, 1.8.0-RC1-SNAPSHOT, 1.11.0.7.0.0.0-SNAPSHOT, etc.
 //
 // This struct can be used with versions reported by ksck to determine if and
 // how certain tools should function depending on what versions are running in


Mime
View raw message