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 03:23:52 UTC
[kudu] branch master updated: version_util: allow period as
delimiter in extra and tighten version syntax
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 <ad...@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 <as...@cloudera.com>
Reviewed-by: Andrew Wong <aw...@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