You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ne...@apache.org on 2017/05/10 04:09:29 UTC
[1/2] mesos git commit: Allowed leading zeros in input to stout's
Version parser.
Repository: mesos
Updated Branches:
refs/heads/master 1bdb1afef -> 002fbaef7
Allowed leading zeros in input to stout's Version parser.
The original behavior was to allow leading zeros. When Version was
enhanced with more complete support for SemVer [1], it was also changed
to reject leading zeros in numeric components (version numbers and
prerelease identifiers). Although this change was consistent with the
SemVer spec (which mandates that such version strings "MUST" be
considered invalid), it breaks compatibility with the versions used by
some common software packages (e.g., Docker).
This commit reverts the change in behavior, so that leading zeros are
once again allowed. In the future, we might consider adding a "strict"
mode to the Version parser, and/or supporting an "options" scheme that
would allow the user to customize the version parser's behavior.
[1] 287556284d76e03c11cff3fc076224fe966096e0
Review: https://reviews.apache.org/r/59105/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/0cebb125
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/0cebb125
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/0cebb125
Branch: refs/heads/master
Commit: 0cebb12535a64da78643987cf876e89eb5f178c3
Parents: 1bdb1af
Author: Neil Conway <ne...@gmail.com>
Authored: Tue May 9 17:50:11 2017 -0700
Committer: Neil Conway <ne...@gmail.com>
Committed: Tue May 9 21:09:15 2017 -0700
----------------------------------------------------------------------
3rdparty/stout/include/stout/version.hpp | 63 ++++++++-------------------
3rdparty/stout/tests/version_tests.cpp | 14 +++---
2 files changed, 26 insertions(+), 51 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/0cebb125/3rdparty/stout/include/stout/version.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/version.hpp b/3rdparty/stout/include/stout/version.hpp
index 4be7222..bff6e4c 100644
--- a/3rdparty/stout/include/stout/version.hpp
+++ b/3rdparty/stout/include/stout/version.hpp
@@ -30,7 +30,14 @@
// This class provides convenience routines for working with version
// numbers. We support the SemVer 2.0.0 (http://semver.org) format,
-// with minor extensions.
+// with two differences:
+//
+// (1) Numeric components with leading zeros are allowed.
+//
+// (2) Missing version components are allowed and treated as zero.
+//
+// TODO(neilc): Consider providing a "strict" variant that does not
+// allow these extensions.
//
// Ideally, the version components would be called simply "major",
// "minor", and "patch". However, GNU libstdc++ already defines these
@@ -42,11 +49,6 @@ struct Version
// <major>[.<minor>[.<patch>]][-prerelease][+build]
//
// Missing `minor` or `patch` components are treated as zero.
- // Allowing some version numbers to be omitted is an extension to
- // SemVer, albeit one that several other SemVer libraries implement.
- //
- // TODO(neilc): Consider providing a "strict" variant that does not
- // allow version numbers to be omitted.
//
// `prerelease` is a prerelease label (e.g., "beta", "rc.1");
// `build` is a build metadata label. Both `prerelease` and `build`
@@ -69,9 +71,7 @@ struct Version
if (buildParts.size() == 2) {
const std::string& buildString = buildParts.back();
- // NOTE: Build metadata identifiers can contain leading zeros
- // (unlike numeric prerelease identifiers; see below).
- Try<std::vector<std::string>> parsed = parseLabel(buildString, false);
+ Try<std::vector<std::string>> parsed = parseLabel(buildString);
if (parsed.isError()) {
return Error("Invalid build label: " + parsed.error());
}
@@ -92,8 +92,7 @@ struct Version
if (prereleaseParts.size() == 2) {
const std::string& prereleaseString = prereleaseParts.back();
- // Prerelease identifiers cannot contain leading zeros.
- Try<std::vector<std::string>> parsed = parseLabel(prereleaseString, true);
+ Try<std::vector<std::string>> parsed = parseLabel(prereleaseString);
if (parsed.isError()) {
return Error("Invalid prerelease label: " + parsed.error());
}
@@ -121,11 +120,6 @@ struct Version
": " + result.error());
}
- if (hasLeadingZero(numericComponents[i])) {
- return Error("Invalid version component '" + numericComponents[i] + "'"
- ": cannot contain leading zero");
- }
-
versionNumbers[i] = result.get();
}
@@ -153,11 +147,11 @@ struct Version
// valid prerelease and build identifiers.
foreach (const std::string& identifier, prerelease) {
- CHECK_NONE(validateIdentifier(identifier, true));
+ CHECK_NONE(validateIdentifier(identifier));
}
foreach (const std::string& identifier, build) {
- CHECK_NONE(validateIdentifier(identifier, false));
+ CHECK_NONE(validateIdentifier(identifier));
}
}
@@ -290,10 +284,9 @@ struct Version
private:
// Check that a string contains a valid identifier. An identifier is
// a non-empty string; each character must be an ASCII alphanumeric
- // or hyphen.
- static Option<Error> validateIdentifier(
- const std::string& identifier,
- bool rejectLeadingZero)
+ // or hyphen. We allow leading zeros in numeric identifiers, which
+ // inconsistent with the SemVer spec.
+ static Option<Error> validateIdentifier(const std::string& identifier)
{
if (identifier.empty()) {
return Error("Empty identifier");
@@ -311,23 +304,13 @@ private:
"'" + stringify(*firstInvalid) + "'");
}
- // If requested, disallow identifiers that contain a leading
- // zero. Note that this only applies to numeric identifiers, and
- // that zero-valued identifiers are allowed.
- if (rejectLeadingZero && hasLeadingZero(identifier)) {
- return Error("Identifier contains leading zero");
- }
-
return None();
}
// Parse a string containing a series of dot-separated identifiers
// into a vector of strings; each element of the vector contains a
- // single identifier. If `rejectLeadingZeros` is true, we reject any
- // numeric identifier in the label that contains a leading zero.
- static Try<std::vector<std::string>> parseLabel(
- const std::string& label,
- bool rejectLeadingZeros)
+ // single identifier.
+ static Try<std::vector<std::string>> parseLabel(const std::string& label)
{
if (label.empty()) {
return Error("Empty label");
@@ -336,7 +319,7 @@ private:
std::vector<std::string> identifiers = strings::split(label, ".");
foreach (const std::string& identifier, identifiers) {
- Option<Error> error = validateIdentifier(identifier, rejectLeadingZeros);
+ Option<Error> error = validateIdentifier(identifier);
if (error.isSome()) {
return error.get();
}
@@ -345,16 +328,6 @@ private:
return identifiers;
}
- // Checks whether the input string is numeric and contains a leading
- // zero. Note that "0" by itself is not considered a "leading zero".
- static bool hasLeadingZero(const std::string& identifier) {
- Try<uint32_t> numericIdentifier = parseNumericIdentifier(identifier);
-
- return numericIdentifier.isSome() &&
- numericIdentifier.get() != 0 &&
- strings::startsWith(identifier, '0');
- }
-
// Try to parse the given string as a numeric identifier. According
// to the SemVer spec, identifiers that begin with hyphens are
// considered non-numeric.
http://git-wip-us.apache.org/repos/asf/mesos/blob/0cebb125/3rdparty/stout/tests/version_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/tests/version_tests.cpp b/3rdparty/stout/tests/version_tests.cpp
index bce185e..a532d0c 100644
--- a/3rdparty/stout/tests/version_tests.cpp
+++ b/3rdparty/stout/tests/version_tests.cpp
@@ -124,7 +124,13 @@ TEST(VersionTest, ParseValid)
{"1-2-3.4+5.6-7",
{Version(1, 0, 0, {"2-3", "4"}, {"5", "6-7"}), "1.0.0-2-3.4+5.6-7"}},
{"1-2.-3+4.-5",
- {Version(1, 0, 0, {"2", "-3"}, {"4", "-5"}), "1.0.0-2.-3+4.-5"}}
+ {Version(1, 0, 0, {"2", "-3"}, {"4", "-5"}), "1.0.0-2.-3+4.-5"}},
+ // Allow leading zeros: in violation of the SemVer spec, but we
+ // tolerate it for compatibility with common practice (e.g., Docker).
+ {"01.2.3", {Version(1, 2, 3), "1.2.3"}},
+ {"1.02.3", {Version(1, 2, 3), "1.2.3"}},
+ {"1.2.03", {Version(1, 2, 3), "1.2.3"}},
+ {"1.2.3-alpha.001", {Version(1, 2, 3, {"alpha", "001"}), "1.2.3-alpha.001"}}
};
foreachpair (const string& input, const ExpectedValue& expected, testCases) {
@@ -152,11 +158,8 @@ TEST(VersionTest, ParseInvalid)
".1.2",
"0.1.-2",
"0.-1.2",
- "0.1.2.3",
+ "1.2.3.4",
"-1.1.2",
- "01.2.3",
- "1.02.3",
- "1.2.03",
"1.1.2-",
"1.1.2+",
"1.1.2-+",
@@ -169,7 +172,6 @@ TEST(VersionTest, ParseInvalid)
"1.1.2+.foo",
"1.1.2-al^pha",
"1.1.2+exp;",
- "1.1.2-alpha.001",
"-foo",
"+foo",
u8"1.0.0-b\u00e9ta"
[2/2] mesos git commit: Cleaned up comments in stout's Version.
Posted by ne...@apache.org.
Cleaned up comments in stout's Version.
Review: https://reviews.apache.org/r/59122
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/002fbaef
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/002fbaef
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/002fbaef
Branch: refs/heads/master
Commit: 002fbaef7a7b9064a5643b692e5f5f3ce3400fe7
Parents: 0cebb12
Author: Neil Conway <ne...@gmail.com>
Authored: Tue May 9 17:52:20 2017 -0700
Committer: Neil Conway <ne...@gmail.com>
Committed: Tue May 9 21:09:22 2017 -0700
----------------------------------------------------------------------
3rdparty/stout/include/stout/version.hpp | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/002fbaef/3rdparty/stout/include/stout/version.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/version.hpp b/3rdparty/stout/include/stout/version.hpp
index bff6e4c..b396cad 100644
--- a/3rdparty/stout/include/stout/version.hpp
+++ b/3rdparty/stout/include/stout/version.hpp
@@ -38,11 +38,6 @@
//
// TODO(neilc): Consider providing a "strict" variant that does not
// allow these extensions.
-//
-// Ideally, the version components would be called simply "major",
-// "minor", and "patch". However, GNU libstdc++ already defines these
-// as macros for compatibility reasons (man 3 makedev for more
-// information) implicitly included in every compilation.
struct Version
{
// Expect the string in the following format:
@@ -55,6 +50,11 @@ struct Version
// consist of one or more dot-separated identifiers. An identifier
// is a non-empty string containing ASCII alphanumeric characters or
// hyphens.
+ //
+ // Ideally, the version components would be called simply "major",
+ // "minor", and "patch". However, GNU libstdc++ already defines
+ // these as macros for compatibility reasons (man 3 makedev for more
+ // information) implicitly included in every compilation.
static Try<Version> parse(const std::string& input)
{
// The input string consists of the numeric components, optionally