You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by be...@apache.org on 2015/02/11 08:32:30 UTC
[2/2] mesos git commit: Parse file:// arguments in Stout's flags.
Parse file:// arguments in Stout's flags.
More cases kept being added which dealt with 'file://' urls, parsing
them, reading from the file, and then handling the argument as though
the file:// wasn't there.
We now always read an argument from a file in the argument parser if
it begins with 'file://'. If you want to pass an argument which is a
filename, then just use the path directly.
Adds a new fetch<> step to parsing an argument which occurs before
parse<>. This fetch can perform actions such as reading a file from
the filesystem, or fetching a resource from a URL.
This is added to alleviate the need to keep adding more code to deal
with 'file://' URLs in each new case where someone needs one.
The patchset introduces no outside-visible argument changes, but it
does require developers who want to specify 'give me a path' to ask
for that path using the new 'Path' type rather than a string, since
for 'file://' URLs, we want to keep the filename.
All existing instances have been modified.
Specifying reading JSON from a file using an absolute path rather than
a file:// URI has been deprecated, and will now log a deprecation
message when used.
>From a quick pass on JIRA (first page of results in JIRA searching for
files): This would have prevented: MESOS-1368, MESOS-1635, MESOS-1590
And simplifies supporting: MESOS-18
Review: https://reviews.apache.org/r/30194
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/1fa5d5b1
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/1fa5d5b1
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/1fa5d5b1
Branch: refs/heads/master
Commit: 1fa5d5b1de304bcb90638a889677e53756152c6b
Parents: 5454565
Author: Cody Maloney <co...@mesosphere.io>
Authored: Tue Feb 10 23:13:43 2015 -0800
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Tue Feb 10 23:32:13 2015 -0800
----------------------------------------------------------------------
.../3rdparty/stout/include/Makefile.am | 1 +
.../stout/include/stout/flags/fetch.hpp | 70 ++++++++++++++++++++
.../stout/include/stout/flags/flags.hpp | 10 +--
.../stout/include/stout/flags/parse.hpp | 37 ++++++-----
.../3rdparty/stout/include/stout/path.hpp | 23 ++++++-
.../3rdparty/stout/tests/flags_tests.cpp | 24 +++++++
6 files changed, 144 insertions(+), 21 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/1fa5d5b1/3rdparty/libprocess/3rdparty/stout/include/Makefile.am
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/Makefile.am b/3rdparty/libprocess/3rdparty/stout/include/Makefile.am
index dc3ef53..8f6ac15 100644
--- a/3rdparty/libprocess/3rdparty/stout/include/Makefile.am
+++ b/3rdparty/libprocess/3rdparty/stout/include/Makefile.am
@@ -11,6 +11,7 @@ nobase_include_HEADERS = \
stout/exit.hpp \
stout/fatal.hpp \
stout/flags.hpp \
+ stout/flags/fetch.hpp \
stout/flags/flag.hpp \
stout/flags/flags.hpp \
stout/flags/loader.hpp \
http://git-wip-us.apache.org/repos/asf/mesos/blob/1fa5d5b1/3rdparty/libprocess/3rdparty/stout/include/stout/flags/fetch.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/flags/fetch.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/flags/fetch.hpp
new file mode 100644
index 0000000..1df982c
--- /dev/null
+++ b/3rdparty/libprocess/3rdparty/stout/include/stout/flags/fetch.hpp
@@ -0,0 +1,70 @@
+/**
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#ifndef __STOUT_FLAGS_FETCH_HPP__
+#define __STOUT_FLAGS_FETCH_HPP__
+
+#include <sstream> // For istringstream.
+#include <string>
+
+#include <stout/duration.hpp>
+#include <stout/error.hpp>
+#include <stout/json.hpp>
+#include <stout/path.hpp>
+#include <stout/strings.hpp>
+#include <stout/try.hpp>
+
+#include <stout/flags/parse.hpp>
+
+#include <stout/os/read.hpp>
+
+namespace flags {
+
+// Allow the value for a flag to be fetched/resolved from a URL such
+// as file://foo/bar/baz. Use template specialization to add a custom
+// fetcher for a given type, such as Path from <stout/path.hpp> Path
+// has a custom fetcher so that it returns the path given rather than
+// the value read from that path.
+template <typename T>
+Try<T> fetch(const std::string& value)
+{
+ // If the flag value corresponds to a file indicated by file://
+ // fetch and then parse the contents of that file.
+ //
+ // TODO(cmaloney): Introduce fetching for things beyond just file://
+ // such as http:// as well!
+ if (strings::startsWith(value, "file://")) {
+ const std::string& path = value.substr(7);
+
+ Try<std::string> read = os::read(path);
+ if (read.isError()) {
+ return Error("Error reading file '" + path + "': " + read.error());
+ }
+
+ return parse<T>(read.get());
+ }
+
+ return parse<T>(value);
+}
+
+
+template <>
+inline Try<Path> fetch(const std::string& value)
+{
+ // Explicitly skip fetching the file if this is for a Path flag!
+ return parse<Path>(value);
+}
+
+} // namespace flags {
+
+#endif // __STOUT_FLAGS_FETCH_HPP__
http://git-wip-us.apache.org/repos/asf/mesos/blob/1fa5d5b1/3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp
index f4b7a95..aedb6ab 100644
--- a/3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp
+++ b/3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp
@@ -31,10 +31,10 @@
#include <stout/strings.hpp>
#include <stout/try.hpp>
+#include <stout/flags/fetch.hpp>
#include <stout/flags/flag.hpp>
#include <stout/flags/loader.hpp>
#include <stout/flags/stringifier.hpp>
-#include <stout/flags/parse.hpp>
namespace flags {
@@ -171,7 +171,7 @@ void FlagsBase::add(
&Loader<T1>::load,
t1,
lambda::function<Try<T1>(const std::string&)>(
- lambda::bind(&parse<T1>, lambda::_1)),
+ lambda::bind(&fetch<T1>, lambda::_1)),
name,
lambda::_2); // Use _2 because ignore FlagsBase*.
flag.stringify = lambda::bind(&Stringifier<T1>, t1);
@@ -201,7 +201,7 @@ void FlagsBase::add(
&OptionLoader<T>::load,
option,
lambda::function<Try<T>(const std::string&)>(
- lambda::bind(&parse<T>, lambda::_1)),
+ lambda::bind(&fetch<T>, lambda::_1)),
name,
lambda::_2); // Use _2 because ignore FlagsBase*.
flag.stringify = lambda::bind(&OptionStringifier<T>, option);
@@ -233,7 +233,7 @@ void FlagsBase::add(
lambda::_1,
t1,
lambda::function<Try<T1>(const std::string&)>(
- lambda::bind(&parse<T1>, lambda::_1)),
+ lambda::bind(&fetch<T1>, lambda::_1)),
name,
lambda::_2);
flag.stringify = lambda::bind(
@@ -272,7 +272,7 @@ void FlagsBase::add(
lambda::_1,
option,
lambda::function<Try<T>(const std::string&)>(
- lambda::bind(&parse<T>, lambda::_1)),
+ lambda::bind(&fetch<T>, lambda::_1)),
name,
lambda::_2);
flag.stringify = lambda::bind(
http://git-wip-us.apache.org/repos/asf/mesos/blob/1fa5d5b1/3rdparty/libprocess/3rdparty/stout/include/stout/flags/parse.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/flags/parse.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/flags/parse.hpp
index 1209469..e34ed78 100644
--- a/3rdparty/libprocess/3rdparty/stout/include/stout/flags/parse.hpp
+++ b/3rdparty/libprocess/3rdparty/stout/include/stout/flags/parse.hpp
@@ -76,28 +76,35 @@ inline Try<Bytes> parse(const std::string& value)
template <>
inline Try<JSON::Object> parse(const std::string& value)
{
- // If the flag value corresponds to a file parse the contents of the
- // file as JSON.
- // TODO(vinod): We do not support relative paths because it is
- // tricky to figure out if a flag value corresponds to a relative
- // path or a JSON string. For example, "{", " {" and " \n {" are
- // all valid prefixes of a JSON string.
- if (strings::startsWith(value, "/") ||
- strings::startsWith(value, "file://")) {
- const std::string& path =
- strings::remove(value, "file://", strings::PREFIX);
-
- Try<std::string> read = os::read(path);
+ // A value that already starts with 'file://' will properly be
+ // loaded from the file and put into 'value' but if it starts with
+ // '/' we need to explicitly handle it for backwards compatibility
+ // reasons (because we used to handle it before we introduced the
+ // 'fetch' mechanism for flags that first fetches the data from URIs
+ // such as 'file://').
+ if (strings::startsWith(value, "/")) {
+ LOG(WARNING) << "Specifying a absolute filename to read a command line "
+ "option out of without using 'file:// is deprecated and "
+ "will be removed in a future release. Simply adding "
+ "'file://' to the beginning of the path should eliminate "
+ "this warning.";
+
+ Try<std::string> read = os::read(value);
if (read.isError()) {
- return Error("Error reading file '" + path + "': " + read.error());
+ return Error("Error reading file '" + value + "': " + read.error());
}
-
return JSON::parse<JSON::Object>(read.get());
}
-
return JSON::parse<JSON::Object>(value);
}
+
+template <>
+inline Try<Path> parse(const std::string& value)
+{
+ return Path(value);
+}
+
} // namespace flags {
#endif // __STOUT_FLAGS_PARSE_HPP__
http://git-wip-us.apache.org/repos/asf/mesos/blob/1fa5d5b1/3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp
index 9f8f520..d4df650 100644
--- a/3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp
+++ b/3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp
@@ -15,10 +15,31 @@
#define __STOUT_PATH_HPP__
#include <string>
-#include <vector>
#include <stout/strings.hpp>
+// Basic abstraction for representing a path in a filesystem.
+class Path
+{
+public:
+ explicit Path(const std::string& path)
+ : value(strings::remove(path, "file://", strings::PREFIX)) {}
+
+ // TODO(cmaloney): Add more useful operations such as 'absolute()'
+ // and 'basename', and 'dirname', etc.
+
+ const std::string value;
+};
+
+
+inline std::ostream& operator << (
+ std::ostream& stream,
+ const Path& path)
+{
+ return stream << path.value;
+}
+
+
namespace path {
inline std::string join(const std::string& path1, const std::string& path2)
http://git-wip-us.apache.org/repos/asf/mesos/blob/1fa5d5b1/3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp b/3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp
index 3b60ff8..0028119 100644
--- a/3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp
+++ b/3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp
@@ -588,3 +588,27 @@ TEST_F(FlagsFileTest, JSONFile)
ASSERT_SOME_EQ(object, json);
}
+
+
+TEST_F(FlagsFileTest, FilePrefix)
+{
+ Flags<TestFlags> flags;
+
+ Option<std::string> something;
+
+ flags.add(&something,
+ "something",
+ "arg to be loaded from file");
+
+ // Write the JSON to a file.
+ const string& file = path::join(os::getcwd(), "file");
+ ASSERT_SOME(os::write(file, "testing"));
+
+ // Read the JSON from the file.
+ map<string, Option<string> > values;
+ values["something"] = Some("file://" + file);
+
+ ASSERT_SOME(flags.load(values));
+
+ ASSERT_SOME_EQ("testing", something);
+}