Skip to content

Commit

Permalink
Move path-manipulation functions to own library file.
Browse files Browse the repository at this point in the history
Leave functions that make file accesses in the file library, and general blaze utilities in the blaze_util file, but move the functions that boil down to string manipulation and path formatting to their own file. (With the exception of getCWD, since absolute path syntax is relevant here.)

Doing this largely to consolidate all Windows path control into a single place, so that it's easier to notice inconsistencies. For instance, ConvertPath currently makes Windows paths absolute, but not Posix paths, and MakeAbsolute relies on this behavior. In addition, JoinPath assumes Posix path syntax, which leads to some odd looking paths. These will be fixed in a followup change.

(Found these issues while working on bazelbuild#4502, trying to fix the windows-specific system bazelrc.)

RELNOTES: None.
PiperOrigin-RevId: 199368226
  • Loading branch information
cvcal authored and George Gensure committed Oct 18, 2018
1 parent ec15f38 commit 737ada3
Show file tree
Hide file tree
Showing 45 changed files with 1,303 additions and 971 deletions.
19 changes: 11 additions & 8 deletions src/main/cpp/blaze.cc
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@
#include "src/main/cpp/util/file.h"
#include "src/main/cpp/util/logging.h"
#include "src/main/cpp/util/numbers.h"
#include "src/main/cpp/util/path.h"
#include "src/main/cpp/util/path_platform.h"
#include "src/main/cpp/util/port.h"
#include "src/main/cpp/util/strings.h"
#include "src/main/cpp/workspace_layout.h"
Expand Down Expand Up @@ -412,7 +414,8 @@ static vector<string> GetArgumentArray(

result.push_back("-XX:+HeapDumpOnOutOfMemoryError");
string heap_crash_path = globals->options->output_base;
result.push_back("-XX:HeapDumpPath=" + blaze::PathAsJvmFlag(heap_crash_path));
result.push_back("-XX:HeapDumpPath=" +
blaze_util::PathAsJvmFlag(heap_crash_path));

result.push_back("-Xverify:none");

Expand Down Expand Up @@ -442,7 +445,7 @@ static vector<string> GetArgumentArray(
bool first = true;
for (const auto &it : globals->extracted_binaries) {
if (IsSharedLibrary(it)) {
string libpath(blaze::PathAsJvmFlag(
string libpath(blaze_util::PathAsJvmFlag(
blaze_util::JoinPath(real_install_dir, blaze_util::Dirname(it))));
// Only add the library path if it's not added yet.
if (java_library_paths.find(libpath) == java_library_paths.end()) {
Expand Down Expand Up @@ -497,14 +500,14 @@ static vector<string> GetArgumentArray(
ToString(globals->options->connect_timeout_secs));

result.push_back("--output_user_root=" +
blaze::ConvertPath(globals->options->output_user_root));
blaze_util::ConvertPath(globals->options->output_user_root));
result.push_back("--install_base=" +
blaze::ConvertPath(globals->options->install_base));
blaze_util::ConvertPath(globals->options->install_base));
result.push_back("--install_md5=" + globals->install_md5);
result.push_back("--output_base=" +
blaze::ConvertPath(globals->options->output_base));
blaze_util::ConvertPath(globals->options->output_base));
result.push_back("--workspace_directory=" +
blaze::ConvertPath(globals->workspace));
blaze_util::ConvertPath(globals->workspace));
result.push_back("--default_system_javabase=" + GetSystemJavabase());

if (!globals->options->server_jvm_out.empty()) {
Expand Down Expand Up @@ -1170,8 +1173,8 @@ static void EnsureCorrectRunningVersion(BlazeServer *server) {
string prev_installation;
bool ok =
blaze_util::ReadDirectorySymlink(installation_path, &prev_installation);
if (!ok || !CompareAbsolutePaths(prev_installation,
globals->options->install_base)) {
if (!ok || !blaze_util::CompareAbsolutePaths(
prev_installation, globals->options->install_base)) {
if (server->Connected()) {
BAZEL_LOG(INFO)
<< "Killing running server because it is using another version of "
Expand Down
12 changes: 0 additions & 12 deletions src/main/cpp/blaze_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,6 @@ const unsigned int kPostShutdownGracePeriodSeconds = 60;

const unsigned int kPostKillGracePeriodSeconds = 10;

string MakeAbsolute(const string &p) {
string path = ConvertPath(p);
if (path.empty()) {
return blaze_util::GetCwd();
}
if (blaze_util::IsDevNull(path.c_str()) || blaze_util::IsAbsolute(path)) {
return path;
}

return blaze_util::JoinPath(blaze_util::GetCwd(), path);
}

const char* GetUnaryOption(const char *arg,
const char *next_arg,
const char *key) {
Expand Down
9 changes: 0 additions & 9 deletions src/main/cpp/blaze_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,6 @@ namespace blaze {

extern const char kServerPidFile[];

// Returns the given path in absolute form. Does not change paths that are
// already absolute.
//
// If called from working directory "/bar":
// MakeAbsolute("foo") --> "/bar/foo"
// MakeAbsolute("/foo") ---> "/foo"
// MakeAbsolute("C:/foo") ---> "C:/foo"
std::string MakeAbsolute(const std::string &path);

// If 'arg' matches 'key=value', returns address of 'value'.
// If it matches 'key' alone, returns address of next_arg.
// Returns NULL otherwise.
Expand Down
1 change: 1 addition & 0 deletions src/main/cpp/blaze_util_linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "src/main/cpp/util/exit_code.h"
#include "src/main/cpp/util/file.h"
#include "src/main/cpp/util/logging.h"
#include "src/main/cpp/util/path.h"
#include "src/main/cpp/util/port.h"
#include "src/main/cpp/util/strings.h"

Expand Down
16 changes: 0 additions & 16 deletions src/main/cpp/blaze_util_platform.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,16 +119,6 @@ int ExecuteDaemon(const std::string& exe,
const std::string& server_dir,
BlazeServerStartup** server_startup);

// Convert a path from Bazel internal form to underlying OS form.
// On Unixes this is an identity operation.
// On Windows, Bazel internal form is cygwin path, and underlying OS form
// is Windows path.
std::string ConvertPath(const std::string& path);

// Converts `path` to a string that's safe to pass as path in a JVM flag.
// See https://github.com/bazelbuild/bazel/issues/2576
std::string PathAsJvmFlag(const std::string& path);

// A character used to separate paths in a list.
extern const char kListSeparator;

Expand All @@ -137,12 +127,6 @@ extern const char kListSeparator;
// Implemented via junctions on Windows.
bool SymlinkDirectories(const std::string& target, const std::string& link);

// Compares two absolute paths. Necessary because the same path can have
// multiple different names under msys2: "C:\foo\bar" or "C:/foo/bar"
// (Windows-style) and "/c/foo/bar" (msys2 style). Returns if the paths are
// equal.
bool CompareAbsolutePaths(const std::string& a, const std::string& b);

struct BlazeLock {
#if defined(COMPILER_MSVC) || defined(__CYGWIN__)
/* HANDLE */ void* handle;
Expand Down
13 changes: 4 additions & 9 deletions src/main/cpp/blaze_util_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.

#include "src/main/cpp/blaze_util_platform.h"

#define _WITH_DPRINTF
#include <dirent.h>
#include <errno.h>
Expand All @@ -36,7 +38,6 @@
#include <cinttypes>

#include "src/main/cpp/blaze_util.h"
#include "src/main/cpp/blaze_util_platform.h"
#include "src/main/cpp/global_variables.h"
#include "src/main/cpp/startup_options.h"
#include "src/main/cpp/util/errors.h"
Expand All @@ -45,6 +46,8 @@
#include "src/main/cpp/util/logging.h"
#include "src/main/cpp/util/md5.h"
#include "src/main/cpp/util/numbers.h"
#include "src/main/cpp/util/path.h"
#include "src/main/cpp/util/path_platform.h"

namespace blaze {

Expand Down Expand Up @@ -173,10 +176,6 @@ void ExecuteProgram(const string& exe, const vector<string>& args_vector) {
execv(exe.c_str(), const_cast<char**>(argv));
}

std::string ConvertPath(const std::string &path) { return path; }

std::string PathAsJvmFlag(const std::string& path) { return path; }

const char kListSeparator = ':';

bool SymlinkDirectories(const string &target, const string &link) {
Expand Down Expand Up @@ -403,10 +402,6 @@ int ExecuteDaemon(const string& exe,
}
}

bool CompareAbsolutePaths(const string& a, const string& b) {
return a == b;
}

string GetHashedBaseDir(const string& root, const string& hashable) {
unsigned char buf[blaze_util::Md5Digest::kDigestLength];
blaze_util::Md5Digest digest;
Expand Down
38 changes: 4 additions & 34 deletions src/main/cpp/blaze_util_windows.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.

#include "src/main/cpp/blaze_util_platform.h"

#include <fcntl.h>
#include <stdarg.h> // va_start, va_end, va_list

Expand All @@ -33,7 +35,6 @@
#include <vector>

#include "src/main/cpp/blaze_util.h"
#include "src/main/cpp/blaze_util_platform.h"
#include "src/main/cpp/global_variables.h"
#include "src/main/cpp/startup_options.h"
#include "src/main/cpp/util/errors.h"
Expand All @@ -43,6 +44,8 @@
#include "src/main/cpp/util/logging.h"
#include "src/main/cpp/util/md5.h"
#include "src/main/cpp/util/numbers.h"
#include "src/main/cpp/util/path.h"
#include "src/main/cpp/util/path_platform.h"
#include "src/main/cpp/util/strings.h"
#include "src/main/native/windows/file.h"
#include "src/main/native/windows/util.h"
Expand Down Expand Up @@ -652,36 +655,6 @@ void ExecuteProgram(const string& exe, const std::vector<string>& args_vector) {

const char kListSeparator = ';';

string PathAsJvmFlag(const string& path) {
string spath;
string error;
if (!blaze_util::AsShortWindowsPath(path, &spath, &error)) {
BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
<< "PathAsJvmFlag(" << path
<< "): AsShortWindowsPath failed: " << error;
}
// Convert backslashes to forward slashes, in order to avoid the JVM parsing
// Windows paths as if they contained escaped characters.
// See https://github.com/bazelbuild/bazel/issues/2576
std::replace(spath.begin(), spath.end(), '\\', '/');
return spath;
}

string ConvertPath(const string& path) {
// The path may not be Windows-style and may not be normalized, so convert it.
wstring wpath;
string error;
if (!blaze_util::AsAbsoluteWindowsPath(path, &wpath, &error)) {
BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
<< "ConvertPath(" << path
<< "): AsAbsoluteWindowsPath failed: " << error;
}
std::transform(wpath.begin(), wpath.end(), wpath.begin(), ::towlower);
return string(blaze_util::WstringToCstring(
blaze_util::RemoveUncPrefixMaybe(wpath.c_str()))
.get());
}

bool SymlinkDirectories(const string &posix_target, const string &posix_name) {
wstring name;
wstring target;
Expand All @@ -708,9 +681,6 @@ bool SymlinkDirectories(const string &posix_target, const string &posix_name) {
return true;
}

bool CompareAbsolutePaths(const string& a, const string& b) {
return ConvertPath(a) == ConvertPath(b);
}

#ifndef STILL_ACTIVE
#define STILL_ACTIVE (259) // From MSDN about GetExitCodeProcess.
Expand Down
10 changes: 6 additions & 4 deletions src/main/cpp/option_processor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
#include "src/main/cpp/blaze_util_platform.h"
#include "src/main/cpp/util/file.h"
#include "src/main/cpp/util/logging.h"
#include "src/main/cpp/util/path.h"
#include "src/main/cpp/util/path_platform.h"
#include "src/main/cpp/util/strings.h"
#include "src/main/cpp/workspace_layout.h"

Expand Down Expand Up @@ -136,7 +138,7 @@ blaze_exit_code::ExitCode OptionProcessor::FindUserBlazerc(
"." + parsed_startup_options_->GetLowercaseProductName() + "rc";

if (cmd_line_rc_file != nullptr) {
string rcFile = MakeAbsolute(cmd_line_rc_file);
string rcFile = blaze_util::MakeAbsolute(cmd_line_rc_file);
if (!blaze_util::CanReadFile(rcFile)) {
blaze_util::StringPrintf(error,
"Error: Unable to read %s file '%s'.", rc_basename.c_str(),
Expand Down Expand Up @@ -424,7 +426,7 @@ static void PreprocessEnvString(string* env_str) {
} else if (name == "TMP") {
// A valid Windows path "c:/foo" is also a valid Unix path list of
// ["c", "/foo"] so must use ConvertPath here. See GitHub issue #1684.
env_str->assign("TMP=" + ConvertPath(env_str->substr(pos + 1)));
env_str->assign("TMP=" + blaze_util::ConvertPath(env_str->substr(pos + 1)));
}
}

Expand Down Expand Up @@ -477,7 +479,7 @@ std::vector<std::string> OptionProcessor::GetBlazercAndEnvCommandArgs(
// from multiple places.
if (rcfile_indexes.find(source_path) != rcfile_indexes.end()) continue;

result.push_back("--rc_source=" + blaze::ConvertPath(source_path));
result.push_back("--rc_source=" + blaze_util::ConvertPath(source_path));
rcfile_indexes[source_path] = cur_index;
cur_index++;
}
Expand All @@ -503,7 +505,7 @@ std::vector<std::string> OptionProcessor::GetBlazercAndEnvCommandArgs(
for (const string& env_var : env) {
result.push_back("--client_env=" + env_var);
}
result.push_back("--client_cwd=" + blaze::ConvertPath(cwd));
result.push_back("--client_cwd=" + blaze_util::ConvertPath(cwd));
return result;
}

Expand Down
20 changes: 11 additions & 9 deletions src/main/cpp/startup_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
#include "src/main/cpp/util/file.h"
#include "src/main/cpp/util/logging.h"
#include "src/main/cpp/util/numbers.h"
#include "src/main/cpp/util/path.h"
#include "src/main/cpp/util/path_platform.h"
#include "src/main/cpp/util/strings.h"
#include "src/main/cpp/workspace_layout.h"

Expand Down Expand Up @@ -93,7 +95,7 @@ StartupOptions::StartupOptions(const string &product_name,
original_startup_options_(std::vector<RcStartupFlag>()) {
bool testing = !blaze::GetEnv("TEST_TMPDIR").empty();
if (testing) {
output_root = MakeAbsolute(blaze::GetEnv("TEST_TMPDIR"));
output_root = blaze_util::MakeAbsolute(blaze::GetEnv("TEST_TMPDIR"));
max_idle_secs = 15;
BAZEL_LOG(USER) << "$TEST_TMPDIR defined: output root default is '"
<< output_root << "' and max_idle_secs default is '"
Expand Down Expand Up @@ -187,19 +189,19 @@ blaze_exit_code::ExitCode StartupOptions::ProcessArg(
const char* value = NULL;

if ((value = GetUnaryOption(arg, next_arg, "--output_base")) != NULL) {
output_base = MakeAbsolute(value);
output_base = blaze_util::MakeAbsolute(value);
option_sources["output_base"] = rcfile;
} else if ((value = GetUnaryOption(arg, next_arg,
"--install_base")) != NULL) {
install_base = MakeAbsolute(value);
install_base = blaze_util::MakeAbsolute(value);
option_sources["install_base"] = rcfile;
} else if ((value = GetUnaryOption(arg, next_arg,
"--output_user_root")) != NULL) {
output_user_root = MakeAbsolute(value);
output_user_root = blaze_util::MakeAbsolute(value);
option_sources["output_user_root"] = rcfile;
} else if ((value = GetUnaryOption(arg, next_arg,
"--server_jvm_out")) != NULL) {
server_jvm_out = MakeAbsolute(value);
server_jvm_out = blaze_util::MakeAbsolute(value);
option_sources["server_jvm_out"] = rcfile;
} else if (GetNullaryOption(arg, "--deep_execroot")) {
deep_execroot = true;
Expand All @@ -221,7 +223,7 @@ blaze_exit_code::ExitCode StartupOptions::ProcessArg(
"--host_javabase")) != NULL) {
// TODO(bazel-team): Consider examining the javabase and re-execing in case
// of architecture mismatch.
host_javabase = MakeAbsolute(value);
host_javabase = blaze_util::MakeAbsolute(value);
option_sources["host_javabase"] = rcfile;
} else if ((value = GetUnaryOption(arg, next_arg, "--host_jvm_args")) !=
NULL) {
Expand Down Expand Up @@ -486,8 +488,8 @@ void StartupOptions::AddJVMArgumentSuffix(const string &real_install_dir,
const string &jar_path,
std::vector<string> *result) const {
result->push_back("-jar");
result->push_back(
blaze::PathAsJvmFlag(blaze_util::JoinPath(real_install_dir, jar_path)));
result->push_back(blaze_util::PathAsJvmFlag(
blaze_util::JoinPath(real_install_dir, jar_path)));
}

blaze_exit_code::ExitCode StartupOptions::AddJVMArguments(
Expand All @@ -502,7 +504,7 @@ void StartupOptions::AddJVMLoggingArguments(std::vector<string> *result) const {
const string propFile =
blaze_util::JoinPath(output_base, "javalog.properties");
string java_log(
blaze::PathAsJvmFlag(blaze_util::JoinPath(output_base, "java.log")));
blaze_util::PathAsJvmFlag(blaze_util::JoinPath(output_base, "java.log")));
if (!blaze_util::WriteFile("handlers=java.util.logging.FileHandler\n"
".level=INFO\n"
"java.util.logging.FileHandler.level=INFO\n"
Expand Down
Loading

0 comments on commit 737ada3

Please sign in to comment.