You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by ad...@apache.org on 2019/07/24 18:16:50 UTC
[kudu] 02/02: version_util: track extra delimiter too
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
commit 7cdbac9ec54fad258f9d4d52ce7df5dde3bbb8f9
Author: Adar Dembo <ad...@cloudera.com>
AuthorDate: Wed Jul 24 10:08:09 2019 -0700
version_util: track extra delimiter too
One of the version_util tests verifies that the current version's canonical
representation matches its raw version. The test fails when the delimiter
between the maintenance and extra components is a period, because the
canonical representation builder (i.e. ToString()) assumes that the
delimiter is always a dash.
Rather than weaken the invariant enforced by the unit test, let's track the
parsed delimiter faithfully. I chose to store it separately from the extra
component itself since it didn't logically seem like part of it.
Change-Id: I0fe983e55ae9f83d075297016ccc5d50c208c549
Reviewed-on: http://gerrit.cloudera.org:8080/13911
Tested-by: Adar Dembo <ad...@cloudera.com>
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
src/kudu/util/version_util-test.cc | 29 +++++++++++++++--------------
src/kudu/util/version_util.cc | 11 ++++++++---
src/kudu/util/version_util.h | 8 ++++++++
3 files changed, 31 insertions(+), 17 deletions(-)
diff --git a/src/kudu/util/version_util-test.cc b/src/kudu/util/version_util-test.cc
index d06df0d..c92db56 100644
--- a/src/kudu/util/version_util-test.cc
+++ b/src/kudu/util/version_util-test.cc
@@ -21,6 +21,7 @@
#include <utility>
#include <vector>
+#include <boost/optional/optional.hpp>
#include <gtest/gtest.h>
#include "kudu/util/status.h"
@@ -35,21 +36,21 @@ namespace kudu {
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.0.0", 0, 0, 0, boost::none, "" }, "0.0.0" },
+ { { "1.0.0", 1, 0, 0, boost::none, "" }, "1.0.0" },
+ { { "1.1.0", 1, 1, 0, boost::none, "" }, "1.1.0" },
+ { { "1.1.1", 1, 1, 1, boost::none, "" }, "1.1.1" },
+ { { "1.1.1-", 1, 1, 1, boost::none, "" }, "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" },
// 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" },
+ { { "00.01.010", 0, 1, 10, boost::none, "" }, "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) {
diff --git a/src/kudu/util/version_util.cc b/src/kudu/util/version_util.cc
index d5cbdee..316d326 100644
--- a/src/kudu/util/version_util.cc
+++ b/src/kudu/util/version_util.cc
@@ -41,13 +41,14 @@ bool Version::operator==(const Version& other) const {
return this->major == other.major &&
this->minor == other.minor &&
this->maintenance == other.maintenance &&
+ this->extra_delimiter == other.extra_delimiter &&
this->extra == other.extra;
}
string Version::ToString() const {
return extra.empty()
? Substitute("$0.$1.$2", major, minor, maintenance)
- : Substitute("$0.$1.$2-$3", major, minor, maintenance, extra);
+ : Substitute("$0.$1.$2$3$4", major, minor, maintenance, *extra_delimiter, extra);
}
ostream& operator<<(ostream& os, const Version& v) {
@@ -93,8 +94,12 @@ Status ParseVersion(const string& version_str,
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));
+ int extra_comp_off = matches[4].rm_so + 1; // skip the delimiter
+ int extra_comp_len = matches[4].rm_eo - extra_comp_off;
+ if (extra_comp_len > 0) {
+ temp_v.extra_delimiter = v_str[extra_comp_off - 1];
+ temp_v.extra = v_str.substr(extra_comp_off, extra_comp_len);
+ }
}
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 2c8a1ac..5fcb242 100644
--- a/src/kudu/util/version_util.h
+++ b/src/kudu/util/version_util.h
@@ -19,6 +19,8 @@
#include <iosfwd>
#include <string>
+#include <boost/optional/optional.hpp>
+
#include "kudu/gutil/port.h"
#include "kudu/util/status.h"
@@ -50,6 +52,12 @@ struct Version {
int minor;
int maintenance;
+ // The character separating the maintenance version from the extra component.
+ //
+ // Even if there was a delimiter, this will be boost::none unless there was
+ // a non-empty extra component.
+ boost::optional<char> extra_delimiter;
+
// The extra component. Empty if there was no extra component.
std::string extra;
};