kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From danburk...@apache.org
Subject [1/4] kudu git commit: [util] update on ParseVersion()
Date Mon, 30 Jul 2018 18:46:57 GMT
Repository: kudu
Updated Branches:
  refs/heads/master 34b058b8b -> 8e40dfdb9


[util] update on ParseVersion()

This changelist updates implementation of the kudu::ParseVersion()
utility function to recognize and successfully parse version strings
like '1.8.0-cdh6.x-SNAPSHOT'.

Version::ToString() implementation changed to return
'canonical' representation of the parsed version string.  Added
corresponding unit tests.

This patch also fixes KUDU-2511.

This is a follow-up for c12e63528a2f2c265136e702a48cbc6c93b7f2fd.

Change-Id: I98b200d0529cb4471e4e855b88bfdb2ff2f346bf
Reviewed-on: http://gerrit.cloudera.org:8080/11060
Tested-by: Alexey Serbin <aserbin@cloudera.com>
Reviewed-by: Andrew Wong <awong@cloudera.com>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/7e733a73
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/7e733a73
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/7e733a73

Branch: refs/heads/master
Commit: 7e733a73652db68342859d03d02763ef5e3de79d
Parents: 34b058b
Author: Alexey Serbin <aserbin@cloudera.com>
Authored: Wed Jul 25 22:07:11 2018 -0700
Committer: Alexey Serbin <aserbin@cloudera.com>
Committed: Mon Jul 30 01:55:52 2018 +0000

----------------------------------------------------------------------
 src/kudu/util/version_util-test.cc | 88 +++++++++++++++++++++++++++------
 src/kudu/util/version_util.cc      | 29 ++++++-----
 src/kudu/util/version_util.h       | 11 +++--
 3 files changed, 98 insertions(+), 30 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/7e733a73/src/kudu/util/version_util-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/version_util-test.cc b/src/kudu/util/version_util-test.cc
index 54e8e76..3cb39b9 100644
--- a/src/kudu/util/version_util-test.cc
+++ b/src/kudu/util/version_util-test.cc
@@ -14,53 +14,109 @@
 // KIND, either express or implied.  See the License for the
 // specific language governing permissions and limitations
 // under the License.
+
 #include "kudu/util/version_util.h"
 
 #include <string>
+#include <utility>
 #include <vector>
 
 #include <gtest/gtest.h>
 
 #include "kudu/util/status.h"
 #include "kudu/util/test_macros.h"
-#include "kudu/util/test_util.h"
+#include "kudu/util/version_info.h"
 
 using std::string;
+using std::pair;
 using std::vector;
 
 namespace kudu {
 
-class VersionUtilTest : public KuduTest {};
-
-TEST_F(VersionUtilTest, TestVersion) {
-  const vector<Version> good_test_cases = {
-    { "0.0.0", 0, 0, 0, "" },
-    { "1.0.0", 1, 0, 0, "" },
-    { "1.1.0", 1, 1, 0, "" },
-    { "1.1.1", 1, 1, 1, "" },
-    { "1.10.100-1000", 1, 10, 100, "1000" },
-    { "1.2.3-SNAPSHOT", 1, 2, 3, "SNAPSHOT" },
+TEST(VersionUtilTest, TestVersion) {
+  const vector<pair<Version, string>> good_test_cases = {
+    { { "0.0.0", 0, 0, 0, "" }, "0.0.0" },
+    { { "1.0.0", 1, 0, 0, "" }, "1.0.0" },
+    { { "1.1.0", 1, 1, 0, "" }, "1.1.0" },
+    { { "1.1.1", 1, 1, 1, "" }, "1.1.1" },
+    { { "1.1.1-", 1, 1, 1, "" }, "1.1.1" },
+    { { "1.1.1-0-1-2", 1, 1, 1, "0-1-2" }, "1.1.1-0-1-2" },
+    { { "1.10.100-1000.0", 1, 10, 100, "1000.0" }, "1.10.100-1000.0" },
+    { { "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-" },
   };
 
-  Version v;
   for (const auto& test_case : good_test_cases) {
-    ASSERT_OK(ParseVersion(test_case.raw_version, &v));
-    EXPECT_EQ(test_case, v);
+    const auto& version = test_case.first;
+    const auto& canonical_str = test_case.second;
+    Version v;
+    ASSERT_OK(ParseVersion(version.raw_version, &v));
+    EXPECT_EQ(version, v);
+    EXPECT_EQ(canonical_str, v.ToString());
   }
 
   const vector<string> bad_test_cases = {
     "",
+    " ",
+    "-",
+    " -",
+    "--",
+    " - - ",
+    "..",
+    "0.1",
+    "0.1.",
+    "0.1.+",
+    "+ 0.+ 1.+ 2",
+    "0.1.+-woops",
+    "0.1.2+",
+    "1..1",
+    ".0.1",
+    " . . ",
+    "-0.1.2-013",
+    "0.-1.2-013",
+    "0.1.-2-013",
+    "1000,000.2999,999.999",
+    "1e10.0.1",
+    "1e+10.0.1",
+    "1e-1.0.1",
+    "1E+1.2.3",
+    "1E-5.0.1",
     "foo",
     "foo.1.0",
+    "a.b.c",
+    "0.x1.x2",
+    "0x0.0x1.0x2",
     "1.bar.0",
     "1.0.foo",
     "1.0foo.bar",
     "foo5-1.4.3",
   };
 
-  for (const auto& test_case : bad_test_cases) {
-    ASSERT_TRUE(ParseVersion(test_case, &v).IsInvalidArgument());
+  for (const auto& input_str : bad_test_cases) {
+    Version v;
+    const auto s = ParseVersion(input_str, &v);
+    ASSERT_TRUE(s.IsInvalidArgument())
+        << s.ToString() << ": " << input_str << " ---> " <<
v.ToString();
   }
 }
 
+// Sanity check: parse current Kudu version string and make sure the 'canonical'
+// representation of the parsed version matches the 'raw' input as is.
+TEST(VersionUtilTest, ParseCurrentKuduVersionString) {
+  const auto ver_string = VersionInfo::GetShortVersionInfo();
+  Version v;
+  const auto s = ParseVersion(ver_string, &v);
+  ASSERT_TRUE(s.ok()) << s.ToString();
+  EXPECT_EQ(ver_string, v.ToString());
+}
+
 } // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/7e733a73/src/kudu/util/version_util.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/version_util.cc b/src/kudu/util/version_util.cc
index bd298f8..6715894 100644
--- a/src/kudu/util/version_util.cc
+++ b/src/kudu/util/version_util.cc
@@ -17,15 +17,17 @@
 
 #include "kudu/util/version_util.h"
 
-#include <iostream>
+#include <iterator>
 #include <string>
 #include <utility>
 #include <vector>
 
 #include <glog/logging.h>
 
+#include "kudu/gutil/strings/join.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"
 
@@ -45,7 +47,9 @@ bool Version::operator==(const Version& other) const {
 }
 
 string Version::ToString() const {
-  return raw_version;
+  return extra.empty()
+      ? Substitute("$0.$1.$2", major, minor, maintenance)
+      : Substitute("$0.$1.$2-$3", major, minor, maintenance, extra);
 }
 
 ostream& operator<<(ostream& os, const Version& v) {
@@ -54,29 +58,32 @@ ostream& operator<<(ostream& os, const Version& v) {
 
 Status ParseVersion(const string& version_str,
                     Version* v) {
-  CHECK(v);
+  static const char* const kDelimiter = "-";
+
+  DCHECK(v);
   const Status invalid_ver_err =
       Status::InvalidArgument("invalid version string", version_str);
-  Version temp_v;
-  const vector<string> main_and_extra = Split(version_str, "-");
-  if (main_and_extra.empty() || main_and_extra.size() > 2) {
+  auto v_str = version_str;
+  StripWhiteSpace(&v_str);
+  const vector<string> main_and_extra = Split(v_str, kDelimiter);
+  if (main_and_extra.empty()) {
     return invalid_ver_err;
   }
-  if (main_and_extra.size() == 2) {
-    temp_v.extra = main_and_extra[1];
-  }
-  const auto& main_ver_str = main_and_extra[0];
-  const vector<string> maj_min_maint = Split(main_ver_str, ".");
+  const vector<string> maj_min_maint = Split(main_and_extra.front(), ".");
   if (maj_min_maint.size() != 3) {
     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;
   }
+  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);
+
   return Status::OK();
 }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/7e733a73/src/kudu/util/version_util.h
----------------------------------------------------------------------
diff --git a/src/kudu/util/version_util.h b/src/kudu/util/version_util.h
index 5cde6cc..446934c 100644
--- a/src/kudu/util/version_util.h
+++ b/src/kudu/util/version_util.h
@@ -16,9 +16,10 @@
 // under the License.
 #pragma once
 
-#include <iostream>
+#include <iosfwd>
 #include <string>
 
+#include "kudu/gutil/port.h"
 #include "kudu/util/status.h"
 
 namespace kudu {
@@ -27,7 +28,7 @@ namespace kudu {
 //
 //  <major>.<minor>.<maintenance>[-<extra>]
 //
-// e.g. 1.6.0 or 1.7.1-SNAPSHOT.
+// e.g. 1.6.0, 1.7.1-SNAPSHOT, 1.8.0-RC1-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
@@ -35,6 +36,10 @@ namespace kudu {
 struct Version {
   bool operator==(const Version& other) const;
 
+  // Return 'canonical' version string, i.e. the concatenation of the version
+  // components transformed back into the string representation. The parser
+  // implementation has its quirks, so the canonical version string does not
+  // always match the raw input string.
   std::string ToString() const;
 
   // The original version string.
@@ -53,6 +58,6 @@ std::ostream& operator<<(std::ostream& os, const Version&
v);
 
 // Parse 'version_str' into 'v'. 'v' must not be null.
 Status ParseVersion(const std::string& version_str,
-                    Version* v);
+                    Version* v) WARN_UNUSED_RESULT;
 
 } // namespace kudu


Mime
View raw message