From fbcde7b760ab1eaea7180c6f347c422ed14800b0 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Sat, 22 Oct 2022 11:27:01 +0200 Subject: [PATCH 1/2] Make C++ runfiles library repo mapping aware Also removes a comment mentioning a `Create` overload that does not exist. --- tools/cpp/runfiles/runfiles_src.cc | 186 +++++++++++-- tools/cpp/runfiles/runfiles_src.h | 69 ++++- tools/cpp/runfiles/runfiles_test.cc | 405 +++++++++++++++++++++++++--- 3 files changed, 600 insertions(+), 60 deletions(-) diff --git a/tools/cpp/runfiles/runfiles_src.cc b/tools/cpp/runfiles/runfiles_src.cc index 2d3078065b5f78..aeaf119fd05257 100644 --- a/tools/cpp/runfiles/runfiles_src.cc +++ b/tools/cpp/runfiles/runfiles_src.cc @@ -96,25 +96,32 @@ bool IsDirectory(const string& path) { bool PathsFrom(const std::string& argv0, std::string runfiles_manifest_file, std::string runfiles_dir, std::string* out_manifest, - std::string* out_directory); + std::string* out_directory, std::string* out_repo_mapping); bool PathsFrom(const std::string& argv0, std::string runfiles_manifest_file, std::string runfiles_dir, std::function is_runfiles_manifest, std::function is_runfiles_directory, - std::string* out_manifest, std::string* out_directory); + std::function is_repo_mapping, + std::string* out_manifest, std::string* out_directory, + std::string* out_repo_mapping); bool ParseManifest(const string& path, map* result, string* error); +bool ParseRepoMapping(const string& path, + map, string>* result, + string* error); } // namespace Runfiles* Runfiles::Create(const string& argv0, const string& runfiles_manifest_file, - const string& runfiles_dir, string* error) { - string manifest, directory; + const string& runfiles_dir, + const string& source_repository, + string* error) { + string manifest, directory, repo_mapping; if (!PathsFrom(argv0, runfiles_manifest_file, runfiles_dir, &manifest, - &directory)) { + &directory, &repo_mapping)) { if (error) { std::ostringstream err; err << "ERROR: " << __FILE__ << "(" << __LINE__ @@ -124,7 +131,7 @@ Runfiles* Runfiles::Create(const string& argv0, return nullptr; } - const vector > envvars = { + vector > envvars = { {"RUNFILES_MANIFEST_FILE", manifest}, {"RUNFILES_DIR", directory}, // TODO(laszlocsomor): remove JAVA_RUNFILES once the Java launcher can @@ -138,8 +145,16 @@ Runfiles* Runfiles::Create(const string& argv0, } } + map, string> mapping; + if (!repo_mapping.empty()) { + if (!ParseRepoMapping(repo_mapping, &mapping, error)) { + return nullptr; + } + } + return new Runfiles(std::move(runfiles), std::move(directory), - std::move(envvars)); + std::move(mapping), std::move(envvars), + string(source_repository)); } bool IsAbsolute(const string& path) { @@ -148,7 +163,7 @@ bool IsAbsolute(const string& path) { } char c = path.front(); return (c == '/' && (path.size() < 2 || path[1] != '/')) || - (path.size() >= 3 && + (path.size() >= 3 && ((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z')) && path[1] == ':' && (path[2] == '\\' || path[2] == '/')); } @@ -169,6 +184,11 @@ string GetEnv(const string& key) { } string Runfiles::Rlocation(const string& path) const { + return Rlocation(path, source_repository_); +} + +string Runfiles::Rlocation(const string& path, + const string& source_repo) const { if (path.empty() || starts_with(path, "../") || contains(path, "/..") || starts_with(path, "./") || contains(path, "/./") || ends_with(path, "/.") || contains(path, "//")) { @@ -177,6 +197,24 @@ string Runfiles::Rlocation(const string& path) const { if (IsAbsolute(path)) { return path; } + + if (repo_mapping_.empty()) { + return RlocationUnchecked(path); + } + string::size_type first_slash = path.find_first_of('/'); + if (first_slash == string::npos) { + return RlocationUnchecked(path); + } + string target_apparent = path.substr(0, first_slash); + auto + target = repo_mapping_.find(std::make_pair(source_repo, target_apparent)); + if (target == repo_mapping_.cend()) { + return RlocationUnchecked(path); + } + return RlocationUnchecked(target->second + path.substr(first_slash)); +} + +string Runfiles::RlocationUnchecked(const string& path) const { const auto exact_match = runfiles_map_.find(path); if (exact_match != runfiles_map_.end()) { return exact_match->second; @@ -188,7 +226,7 @@ string Runfiles::Rlocation(const string& path) const { // prefix to the looked up path. std::size_t prefix_end = path.size(); while ((prefix_end = path.find_last_of('/', prefix_end - 1)) != - string::npos) { + string::npos) { const string prefix = path.substr(0, prefix_end); const auto prefix_match = runfiles_map_.find(prefix); if (prefix_match != runfiles_map_.end()) { @@ -238,48 +276,138 @@ bool ParseManifest(const string& path, map* result, return true; } +bool ParseRepoMapping(const string& path, + map, string>* result, + string* error) { + std::ifstream stm(path); + if (!stm.is_open()) { + if (error) { + std::ostringstream err; + err << "ERROR: " << __FILE__ << "(" << __LINE__ + << "): cannot open repository mapping \"" << path << "\""; + *error = err.str(); + } + return false; + } + string line; + std::getline(stm, line); + size_t line_count = 1; + while (!line.empty()) { + string::size_type first_comma = line.find_first_of(','); + if (first_comma == string::npos) { + if (error) { + std::ostringstream err; + err << "ERROR: " << __FILE__ << "(" << __LINE__ + << "): bad repository mapping entry in \"" << path << "\" line #" + << line_count << ": \"" << line << "\""; + *error = err.str(); + } + return false; + } + string::size_type second_comma = line.find_first_of(',', first_comma + 1); + if (second_comma == string::npos) { + if (error) { + std::ostringstream err; + err << "ERROR: " << __FILE__ << "(" << __LINE__ + << "): bad repository mapping entry in \"" << path << "\" line #" + << line_count << ": \"" << line << "\""; + *error = err.str(); + } + return false; + } + + string source = line.substr(0, first_comma); + string target_apparent = + line.substr(first_comma + 1, second_comma - (first_comma + 1)); + string target = line.substr(second_comma + 1); + + (*result)[std::make_pair(source, target_apparent)] = target; + std::getline(stm, line); + ++line_count; + } + return true; +} + } // namespace namespace testing { -bool TestOnly_PathsFrom(const string& argv0, string mf, string dir, +bool TestOnly_PathsFrom(const string& argv0, + string mf, + string dir, function is_runfiles_manifest, function is_runfiles_directory, - string* out_manifest, string* out_directory) { - return PathsFrom(argv0, mf, dir, is_runfiles_manifest, is_runfiles_directory, - out_manifest, out_directory); + function is_repo_mapping, + string* out_manifest, + string* out_directory, + string* out_repo_mapping) { + return PathsFrom(argv0, + mf, + dir, + is_runfiles_manifest, + is_runfiles_directory, + is_repo_mapping, + out_manifest, + out_directory, + out_repo_mapping); } bool TestOnly_IsAbsolute(const string& path) { return IsAbsolute(path); } } // namespace testing -Runfiles* Runfiles::Create(const string& argv0, string* error) { +Runfiles* Runfiles::Create(const std::string& argv0, + const std::string& runfiles_manifest_file, + const std::string& runfiles_dir, + std::string* error) { + return Runfiles::Create(argv0, + runfiles_manifest_file, + runfiles_dir, + "", + error); +} + +Runfiles* Runfiles::Create(const string& argv0, + const string& source_repository, + string* error) { return Runfiles::Create(argv0, GetEnv("RUNFILES_MANIFEST_FILE"), - GetEnv("RUNFILES_DIR"), error); + GetEnv("RUNFILES_DIR"), source_repository, error); } -Runfiles* Runfiles::CreateForTest(std::string* error) { +Runfiles* Runfiles::Create(const string& argv0, string* error) { + return Runfiles::Create(argv0, "", error); +} + +Runfiles* Runfiles::CreateForTest(const string& source_repository, + std::string* error) { return Runfiles::Create(std::string(), GetEnv("RUNFILES_MANIFEST_FILE"), - GetEnv("TEST_SRCDIR"), error); + GetEnv("TEST_SRCDIR"), source_repository, error); +} + +Runfiles* Runfiles::CreateForTest(std::string* error) { + return Runfiles::CreateForTest("", error); } namespace { bool PathsFrom(const string& argv0, string mf, string dir, string* out_manifest, - string* out_directory) { + string* out_directory, string* out_repo_mapping) { return PathsFrom(argv0, mf, dir, [](const string& path) { return IsReadableFile(path); }, [](const string& path) { return IsDirectory(path); }, - out_manifest, out_directory); + [](const string& path) { return IsReadableFile(path); }, + out_manifest, out_directory, out_repo_mapping); } bool PathsFrom(const string& argv0, string mf, string dir, function is_runfiles_manifest, function is_runfiles_directory, - string* out_manifest, string* out_directory) { + function is_repo_mapping, + string* out_manifest, string* out_directory, + string* out_repo_mapping) { out_manifest->clear(); out_directory->clear(); + out_repo_mapping->clear(); bool mfValid = is_runfiles_manifest(mf); bool dirValid = is_runfiles_directory(dir); @@ -315,6 +443,20 @@ bool PathsFrom(const string& argv0, string mf, string dir, dirValid = is_runfiles_directory(dir); } + string rm; + bool rmValid = false; + + if (dirValid && ends_with(dir, ".runfiles")) { + rm = dir.substr(0, dir.size() - 9) + ".repo_mapping"; + rmValid = is_repo_mapping(rm); + } + + if (!rmValid && mfValid && (ends_with(mf, ".runfiles_manifest") || + ends_with(mf, ".runfiles/MANIFEST"))) { + rm = mf.substr(0, mf.size() - 18) + ".repo_mapping"; + rmValid = is_repo_mapping(rm); + } + if (mfValid) { *out_manifest = mf; } @@ -323,6 +465,10 @@ bool PathsFrom(const string& argv0, string mf, string dir, *out_directory = dir; } + if (rmValid) { + *out_repo_mapping = rm; + } + return true; } diff --git a/tools/cpp/runfiles/runfiles_src.h b/tools/cpp/runfiles/runfiles_src.h index 0929b4594fee82..37678566067864 100644 --- a/tools/cpp/runfiles/runfiles_src.h +++ b/tools/cpp/runfiles/runfiles_src.h @@ -34,12 +34,11 @@ // int main(int argc, char** argv) { // std::string error; // std::unique_ptr runfiles( -// Runfiles::Create(argv[0], &error)); +// Runfiles::Create(argv[0], BAZEL_CURRENT_REPOSITORY, &error)); // // // Important: -// // If this is a test, use Runfiles::CreateForTest(&error). -// // Otherwise, if you don't have the value for argv[0] for whatever -// // reason, then use Runfiles::Create(&error). +// // If this is a test, use +// // Runfiles::CreateForTest(BAZEL_CURRENT_REPOSITORY, &error). // // if (runfiles == nullptr) { // ... // error handling @@ -58,7 +57,8 @@ // To start child processes that also need runfiles, you need to set the right // environment variables for them: // -// std::unique_ptr runfiles(Runfiles::Create(argv[0], &error)); +// std::unique_ptr runfiles(Runfiles::Create( +// argv[0], BAZEL_CURRENT_REPOSITORY, &error)); // // std::string path = runfiles->Rlocation("path/to/binary")); // if (!path.empty()) { @@ -102,7 +102,12 @@ class Runfiles { // // This method looks at the RUNFILES_MANIFEST_FILE and TEST_SRCDIR // environment variables. + // + // If source_repository is not provided, it defaults to the main repository + // (also known as the workspace). static Runfiles* CreateForTest(std::string* error = nullptr); + static Runfiles* CreateForTest(const std::string& source_repository, + std::string* error = nullptr); // Returns a new `Runfiles` instance. // @@ -116,7 +121,13 @@ class Runfiles { // environment variables. If either is empty, the method looks for the // manifest or directory using the other environment variable, or using argv0 // (unless it's empty). + // + // If source_repository is not provided, it defaults to the main repository + // (also known as the workspace). + static Runfiles* Create(const std::string& argv0, + std::string* error = nullptr); static Runfiles* Create(const std::string& argv0, + const std::string& source_repository, std::string* error = nullptr); // Returns a new `Runfiles` instance. @@ -133,6 +144,11 @@ class Runfiles { const std::string& runfiles_manifest_file, const std::string& runfiles_dir, std::string* error = nullptr); + static Runfiles* Create(const std::string& argv0, + const std::string& runfiles_manifest_file, + const std::string& runfiles_dir, + const std::string& source_repository, + std::string* error = nullptr); // Returns the runtime path of a runfile. // @@ -146,10 +162,14 @@ class Runfiles { // Args: // path: runfiles-root-relative path of the runfile; must not be empty and // must not contain uplevel references. + // source_repository: if provided, overrides the source repository set when + // this Runfiles instance was created. // Returns: // the path to the runfile, which the caller should check for existence, or // an empty string if the method doesn't know about this runfile std::string Rlocation(const std::string& path) const; + std::string Rlocation(const std::string& path, + const std::string& source_repository) const; // Returns environment variables for subprocesses. // @@ -160,13 +180,35 @@ class Runfiles { return envvars_; } + // Returns a new Runfiles instance that by default uses the provided source + // repository as a default for all calls to Rlocation. + // + // The current instance remains valid. + Runfiles* WithSourceRepository(const std::string& source_repository) const { + std::map runfiles_map(runfiles_map_); + std::string directory(directory_); + std::map, std::string> + repo_mapping(repo_mapping_); + std::vector > envvars(envvars_); + auto* new_runfiles = + new Runfiles(std::move(runfiles_map), std::move(directory), + std::move(repo_mapping), std::move(envvars), + std::string(source_repository)); + return new_runfiles; + } + private: - Runfiles(const std::map&& runfiles_map, - const std::string&& directory, - const std::vector >&& envvars) + Runfiles(std::map&& runfiles_map, + std::string&& directory, + std::map, + std::string>&& repo_mapping, + std::vector >&& envvars, + std::string&& source_repository_) : runfiles_map_(std::move(runfiles_map)), directory_(std::move(directory)), - envvars_(std::move(envvars)) {} + repo_mapping_(std::move(repo_mapping)), + envvars_(std::move(envvars)), + source_repository_(std::move(source_repository_)) {} Runfiles(const Runfiles&) = delete; Runfiles(Runfiles&&) = delete; Runfiles& operator=(const Runfiles&) = delete; @@ -174,7 +216,12 @@ class Runfiles { const std::map runfiles_map_; const std::string directory_; + const std::map, std::string> + repo_mapping_; const std::vector > envvars_; + const std::string source_repository_; + + std::string RlocationUnchecked(const std::string& path) const; }; // The "testing" namespace contains functions that allow unit testing the code. @@ -204,7 +251,9 @@ bool TestOnly_PathsFrom( std::string runfiles_dir, std::function is_runfiles_manifest, std::function is_runfiles_directory, - std::string* out_manifest, std::string* out_directory); + std::function is_repo_mapping, + std::string* out_manifest, std::string* out_directory, + std::string* out_repo_mapping); // For testing only. // Returns true if `path` is an absolute Unix or Windows path. diff --git a/tools/cpp/runfiles/runfiles_test.cc b/tools/cpp/runfiles/runfiles_test.cc index cbe2952c1904cd..88eb0c02f39417 100644 --- a/tools/cpp/runfiles/runfiles_test.cc +++ b/tools/cpp/runfiles/runfiles_test.cc @@ -488,96 +488,441 @@ TEST_F(RunfilesTest, IsAbsolute) { } TEST_F(RunfilesTest, PathsFromEnvVars) { - string mf, dir; + string mf, dir, rm; // Both envvars have a valid value. EXPECT_TRUE(TestOnly_PathsFrom( - "argv0", "mock1/MANIFEST", "mock2", - [](const string& path) { return path == "mock1/MANIFEST"; }, - [](const string& path) { return path == "mock2"; }, &mf, &dir)); - EXPECT_EQ(mf, "mock1/MANIFEST"); - EXPECT_EQ(dir, "mock2"); + "argv0", + "mock1.runfiles/MANIFEST", + "mock2.runfiles", + [](const string& path) { return path == "mock1.runfiles/MANIFEST"; }, + [](const string& path) { return path == "mock2.runfiles"; }, + [](const string& path) { return path == "mock2.repo_mapping"; }, + &mf, + &dir, + &rm)); + EXPECT_EQ(mf, "mock1.runfiles/MANIFEST"); + EXPECT_EQ(dir, "mock2.runfiles"); + EXPECT_EQ(rm, "mock2.repo_mapping"); // RUNFILES_MANIFEST_FILE is invalid but RUNFILES_DIR is good and there's a // runfiles manifest in the runfiles directory. EXPECT_TRUE(TestOnly_PathsFrom( - "argv0", "mock1/MANIFEST", "mock2", - [](const string& path) { return path == "mock2/MANIFEST"; }, - [](const string& path) { return path == "mock2"; }, &mf, &dir)); - EXPECT_EQ(mf, "mock2/MANIFEST"); - EXPECT_EQ(dir, "mock2"); + "argv0", + "mock1.runfiles/MANIFEST", + "mock2.runfiles", + [](const string& path) { return path == "mock2.runfiles/MANIFEST"; }, + [](const string& path) { return path == "mock2.runfiles"; }, + [](const string& path) { return path == "mock2.repo_mapping"; }, + &mf, + &dir, + &rm)); + EXPECT_EQ(mf, "mock2.runfiles/MANIFEST"); + EXPECT_EQ(dir, "mock2.runfiles"); + EXPECT_EQ(rm, "mock2.repo_mapping"); // RUNFILES_MANIFEST_FILE is invalid but RUNFILES_DIR is good, but there's no // runfiles manifest in the runfiles directory. EXPECT_TRUE(TestOnly_PathsFrom( - "argv0", "mock1/MANIFEST", "mock2", + "argv0", + "mock1.runfiles/MANIFEST", + "mock2.runfiles", [](const string& path) { return false; }, - [](const string& path) { return path == "mock2"; }, &mf, &dir)); + [](const string& path) { return path == "mock2.runfiles"; }, + [](const string& path) { return path == "mock2.repo_mapping"; }, + &mf, + &dir, + &rm)); EXPECT_EQ(mf, ""); - EXPECT_EQ(dir, "mock2"); + EXPECT_EQ(dir, "mock2.runfiles"); + EXPECT_EQ(rm, "mock2.repo_mapping"); // RUNFILES_DIR is invalid but RUNFILES_MANIFEST_FILE is good, and it is in // a valid-looking runfiles directory. EXPECT_TRUE(TestOnly_PathsFrom( - "argv0", "mock1/MANIFEST", "mock2", - [](const string& path) { return path == "mock1/MANIFEST"; }, - [](const string& path) { return path == "mock1"; }, &mf, &dir)); - EXPECT_EQ(mf, "mock1/MANIFEST"); - EXPECT_EQ(dir, "mock1"); + "argv0", + "mock1.runfiles/MANIFEST", + "mock2", + [](const string& path) { return path == "mock1.runfiles/MANIFEST"; }, + [](const string& path) { return path == "mock1.runfiles"; }, + [](const string& path) { return path == "mock1.repo_mapping"; }, + &mf, + &dir, + &rm)); + EXPECT_EQ(mf, "mock1.runfiles/MANIFEST"); + EXPECT_EQ(dir, "mock1.runfiles"); + EXPECT_EQ(rm, "mock1.repo_mapping"); // RUNFILES_DIR is invalid but RUNFILES_MANIFEST_FILE is good, but it is not // in any valid-looking runfiles directory. EXPECT_TRUE(TestOnly_PathsFrom( "argv0", "mock1/MANIFEST", "mock2", [](const string& path) { return path == "mock1/MANIFEST"; }, - [](const string& path) { return false; }, &mf, &dir)); + [](const string& path) { return false; }, + [](const string& path) { return true; }, &mf, &dir, &rm)); EXPECT_EQ(mf, "mock1/MANIFEST"); EXPECT_EQ(dir, ""); + EXPECT_EQ(rm, ""); // Both envvars are invalid, but there's a manifest in a runfiles directory // next to argv0, however there's no other content in the runfiles directory. EXPECT_TRUE(TestOnly_PathsFrom( - "argv0", "mock1/MANIFEST", "mock2", + "argv0", + "mock1/MANIFEST", + "mock2", [](const string& path) { return path == "argv0.runfiles/MANIFEST"; }, - [](const string& path) { return false; }, &mf, &dir)); + [](const string& path) { return false; }, + [](const string& path) { return path == "argv0.repo_mapping"; }, + &mf, + &dir, + &rm)); EXPECT_EQ(mf, "argv0.runfiles/MANIFEST"); EXPECT_EQ(dir, ""); + EXPECT_EQ(rm, "argv0.repo_mapping"); // Both envvars are invalid, but there's a manifest next to argv0. There's // no runfiles tree anywhere. EXPECT_TRUE(TestOnly_PathsFrom( - "argv0", "mock1/MANIFEST", "mock2", + "argv0", + "mock1/MANIFEST", + "mock2", [](const string& path) { return path == "argv0.runfiles_manifest"; }, - [](const string& path) { return false; }, &mf, &dir)); + [](const string& path) { return false; }, + [](const string& path) { return path == "argv0.repo_mapping"; }, + &mf, + &dir, + &rm)); EXPECT_EQ(mf, "argv0.runfiles_manifest"); EXPECT_EQ(dir, ""); + EXPECT_EQ(rm, "argv0.repo_mapping"); // Both envvars are invalid, but there's a valid manifest next to argv0, and a // valid runfiles directory (without a manifest in it). EXPECT_TRUE(TestOnly_PathsFrom( - "argv0", "mock1/MANIFEST", "mock2", + "argv0", + "mock1/MANIFEST", + "mock2", [](const string& path) { return path == "argv0.runfiles_manifest"; }, - [](const string& path) { return path == "argv0.runfiles"; }, &mf, &dir)); + [](const string& path) { return path == "argv0.runfiles"; }, + [](const string& path) { return path == "argv0.repo_mapping"; }, + &mf, + &dir, + &rm)); EXPECT_EQ(mf, "argv0.runfiles_manifest"); EXPECT_EQ(dir, "argv0.runfiles"); + EXPECT_EQ(rm, "argv0.repo_mapping"); // Both envvars are invalid, but there's a valid runfiles directory next to // argv0, though no manifest in it. EXPECT_TRUE(TestOnly_PathsFrom( - "argv0", "mock1/MANIFEST", "mock2", + "argv0", + "mock1/MANIFEST", + "mock2", [](const string& path) { return false; }, - [](const string& path) { return path == "argv0.runfiles"; }, &mf, &dir)); + [](const string& path) { return path == "argv0.runfiles"; }, + [](const string& path) { return path == "argv0.repo_mapping"; }, + &mf, + &dir, + &rm)); EXPECT_EQ(mf, ""); EXPECT_EQ(dir, "argv0.runfiles"); + EXPECT_EQ(rm, "argv0.repo_mapping"); // Both envvars are invalid, but there's a valid runfiles directory next to // argv0 with a valid manifest in it. EXPECT_TRUE(TestOnly_PathsFrom( - "argv0", "mock1/MANIFEST", "mock2", + "argv0", + "mock1/MANIFEST", + "mock2", [](const string& path) { return path == "argv0.runfiles/MANIFEST"; }, - [](const string& path) { return path == "argv0.runfiles"; }, &mf, &dir)); + [](const string& path) { return path == "argv0.runfiles"; }, + [](const string& path) { return path == "argv0.repo_mapping"; }, + &mf, + &dir, + &rm)); EXPECT_EQ(mf, "argv0.runfiles/MANIFEST"); EXPECT_EQ(dir, "argv0.runfiles"); + EXPECT_EQ(rm, "argv0.repo_mapping"); +} + +TEST_F(RunfilesTest, ManifestBasedRlocationWithRepoMapping_fromMain) { + string uid = LINE_AS_STRING(); + unique_ptr mf(MockFile::Create( + "foo" + uid + ".runfiles_manifest", { + "config.json /etc/config.json", + "protobuf~3.19.2/foo/runfile C:/Actual Path\\protobuf\\runfile", + "_main/bar/runfile /the/path/./to/other//other runfile.txt", + "protobuf~3.19.2/bar/dir E:\\Actual Path\\Directory"})); + EXPECT_TRUE(mf != nullptr); + unique_ptr rm(MockFile::Create( + "foo" + uid + ".repo_mapping", { + ",my_module,_main", + ",my_protobuf,protobuf~3.19.2", + ",my_workspace,_main", + "protobuf~3.19.2,protobuf,protobuf~3.19.2"})); + EXPECT_TRUE(rm != nullptr); + string argv0(mf->Path().substr( + 0, mf->Path().size() - string(".runfiles_manifest").size())); + + string error; + unique_ptr r(Runfiles::Create(argv0, "", "", "", &error)); + ASSERT_NE(r, nullptr); + EXPECT_TRUE(error.empty()); + + EXPECT_EQ(r->Rlocation("my_module/bar/runfile"), + "/the/path/./to/other//other runfile.txt"); + EXPECT_EQ(r->Rlocation("my_workspace/bar/runfile"), + "/the/path/./to/other//other runfile.txt"); + EXPECT_EQ(r->Rlocation("my_protobuf/foo/runfile"), + "C:/Actual Path\\protobuf\\runfile"); + EXPECT_EQ(r->Rlocation("my_protobuf/bar/dir"), "E:\\Actual Path\\Directory"); + EXPECT_EQ(r->Rlocation("my_protobuf/bar/dir/file"), + "E:\\Actual Path\\Directory/file"); + EXPECT_EQ(r->Rlocation("my_protobuf/bar/dir/de eply/nes ted/fi~le"), + "E:\\Actual Path\\Directory/de eply/nes ted/fi~le"); + + EXPECT_EQ(r->Rlocation("protobuf/foo/runfile"), ""); + EXPECT_EQ(r->Rlocation("protobuf/bar/dir"), ""); + EXPECT_EQ(r->Rlocation("protobuf/bar/dir/file"), ""); + EXPECT_EQ(r->Rlocation("protobuf/bar/dir/dir/de eply/nes ted/fi~le"), ""); + + EXPECT_EQ(r->Rlocation("_main/bar/runfile"), + "/the/path/./to/other//other runfile.txt"); + EXPECT_EQ(r->Rlocation("protobuf~3.19.2/foo/runfile"), + "C:/Actual Path\\protobuf\\runfile"); + EXPECT_EQ(r->Rlocation("protobuf~3.19.2/bar/dir"), + "E:\\Actual Path\\Directory"); + EXPECT_EQ(r->Rlocation("protobuf~3.19.2/bar/dir/file"), + "E:\\Actual Path\\Directory/file"); + EXPECT_EQ(r->Rlocation("protobuf~3.19.2/bar/dir/de eply/nes ted/fi~le"), + "E:\\Actual Path\\Directory/de eply/nes ted/fi~le"); + + EXPECT_EQ(r->Rlocation("config.json"), "/etc/config.json"); + EXPECT_EQ(r->Rlocation("_main"), ""); + EXPECT_EQ(r->Rlocation("my_module"), ""); + EXPECT_EQ(r->Rlocation("protobuf"), ""); +} + +TEST_F(RunfilesTest, ManifestBasedRlocationWithRepoMapping_fromOtherRepo) { + string uid = LINE_AS_STRING(); + unique_ptr mf(MockFile::Create( + "foo" + uid + ".runfiles_manifest", { + "config.json /etc/config.json", + "protobuf~3.19.2/foo/runfile C:/Actual Path\\protobuf\\runfile", + "_main/bar/runfile /the/path/./to/other//other runfile.txt", + "protobuf~3.19.2/bar/dir E:\\Actual Path\\Directory"})); + EXPECT_TRUE(mf != nullptr); + unique_ptr rm(MockFile::Create( + "foo" + uid + ".repo_mapping", { + ",my_module,_main", + ",my_protobuf,protobuf~3.19.2", + ",my_workspace,_main", + "protobuf~3.19.2,protobuf,protobuf~3.19.2"})); + EXPECT_TRUE(rm != nullptr); + string argv0(mf->Path().substr( + 0, mf->Path().size() - string(".runfiles_manifest").size())); + + string error; + unique_ptr + r(Runfiles::Create(argv0, "", "", "protobuf~3.19.2", &error)); + ASSERT_NE(r, nullptr); + EXPECT_TRUE(error.empty()); + + EXPECT_EQ(r->Rlocation("protobuf/foo/runfile"), + "C:/Actual Path\\protobuf\\runfile"); + EXPECT_EQ(r->Rlocation("protobuf/bar/dir"), "E:\\Actual Path\\Directory"); + EXPECT_EQ(r->Rlocation("protobuf/bar/dir/file"), + "E:\\Actual Path\\Directory/file"); + EXPECT_EQ(r->Rlocation("protobuf/bar/dir/de eply/nes ted/fi~le"), + "E:\\Actual Path\\Directory/de eply/nes ted/fi~le"); + + EXPECT_EQ(r->Rlocation("my_module/bar/runfile"), ""); + EXPECT_EQ(r->Rlocation("my_protobuf/foo/runfile"), ""); + EXPECT_EQ(r->Rlocation("my_protobuf/bar/dir"), ""); + EXPECT_EQ(r->Rlocation("my_protobuf/bar/dir/file"), ""); + EXPECT_EQ(r->Rlocation("my_protobuf/bar/dir/de eply/nes ted/fi~le"), ""); + + EXPECT_EQ(r->Rlocation("_main/bar/runfile"), + "/the/path/./to/other//other runfile.txt"); + EXPECT_EQ(r->Rlocation("protobuf~3.19.2/foo/runfile"), + "C:/Actual Path\\protobuf\\runfile"); + EXPECT_EQ(r->Rlocation("protobuf~3.19.2/bar/dir"), + "E:\\Actual Path\\Directory"); + EXPECT_EQ(r->Rlocation("protobuf~3.19.2/bar/dir/file"), + "E:\\Actual Path\\Directory/file"); + EXPECT_EQ(r->Rlocation("protobuf~3.19.2/bar/dir/de eply/nes ted/fi~le"), + "E:\\Actual Path\\Directory/de eply/nes ted/fi~le"); + + EXPECT_EQ(r->Rlocation("config.json"), "/etc/config.json"); + EXPECT_EQ(r->Rlocation("_main"), ""); + EXPECT_EQ(r->Rlocation("my_module"), ""); + EXPECT_EQ(r->Rlocation("protobuf"), ""); +} + +TEST_F(RunfilesTest, DirectoryBasedRlocationWithRepoMapping_fromMain) { + string uid = LINE_AS_STRING(); + unique_ptr + dir_marker(MockFile::Create("foo" + uid + ".runfiles/marker"), {}); + EXPECT_TRUE(dir_marker != nullptr); + string dir = dir_marker->DirName(); + unique_ptr rm(MockFile::Create( + "foo" + uid + ".repo_mapping", { + ",my_module,_main", + ",my_protobuf,protobuf~3.19.2", + ",my_workspace,_main", + "protobuf~3.19.2,protobuf,protobuf~3.19.2"})); + EXPECT_TRUE(rm != nullptr); + string argv0(rm->Path().substr( + 0, rm->Path().size() - string(".repo_mapping").size())); + + string error; + unique_ptr r(Runfiles::Create(argv0, "", "", "", &error)); + ASSERT_NE(r, nullptr); + EXPECT_TRUE(error.empty()); + + EXPECT_EQ(r->Rlocation("my_module/bar/runfile"), dir + "/_main/bar/runfile"); + EXPECT_EQ(r->Rlocation("my_workspace/bar/runfile"), + dir + "/_main/bar/runfile"); + EXPECT_EQ(r->Rlocation("my_protobuf/foo/runfile"), + dir + "/protobuf~3.19.2/foo/runfile"); + EXPECT_EQ(r->Rlocation("my_protobuf/bar/dir"), + dir + "/protobuf~3.19.2/bar/dir"); + EXPECT_EQ(r->Rlocation("my_protobuf/bar/dir/file"), + dir + "/protobuf~3.19.2/bar/dir/file"); + EXPECT_EQ(r->Rlocation("my_protobuf/bar/dir/de eply/nes ted/fi~le"), + dir + "/protobuf~3.19.2/bar/dir/de eply/nes ted/fi~le"); + + EXPECT_EQ(r->Rlocation("protobuf/foo/runfile"), + dir + "/protobuf/foo/runfile"); + EXPECT_EQ(r->Rlocation("protobuf/bar/dir/dir/de eply/nes ted/fi~le"), + dir + "/protobuf/bar/dir/dir/de eply/nes ted/fi~le"); + + EXPECT_EQ(r->Rlocation("_main/bar/runfile"), dir + "/_main/bar/runfile"); + EXPECT_EQ(r->Rlocation("protobuf~3.19.2/foo/runfile"), + dir + "/protobuf~3.19.2/foo/runfile"); + EXPECT_EQ(r->Rlocation("protobuf~3.19.2/bar/dir"), + dir + "/protobuf~3.19.2/bar/dir"); + EXPECT_EQ(r->Rlocation("protobuf~3.19.2/bar/dir/file"), + dir + "/protobuf~3.19.2/bar/dir/file"); + EXPECT_EQ(r->Rlocation("protobuf~3.19.2/bar/dir/de eply/nes ted/fi~le"), + dir + "/protobuf~3.19.2/bar/dir/de eply/nes ted/fi~le"); + + EXPECT_EQ(r->Rlocation("config.json"), dir + "/config.json"); +} + +TEST_F(RunfilesTest, DirectoryBasedRlocationWithRepoMapping_fromOtherRepo) { + string uid = LINE_AS_STRING(); + unique_ptr + dir_marker(MockFile::Create("foo" + uid + ".runfiles/marker"), {}); + EXPECT_TRUE(dir_marker != nullptr); + string dir = dir_marker->DirName(); + unique_ptr rm(MockFile::Create( + "foo" + uid + ".repo_mapping", { + ",my_module,_main", + ",my_protobuf,protobuf~3.19.2", + ",my_workspace,_main", + "protobuf~3.19.2,protobuf,protobuf~3.19.2"})); + EXPECT_TRUE(rm != nullptr); + string argv0(rm->Path().substr( + 0, rm->Path().size() - string(".repo_mapping").size())); + + string error; + unique_ptr + r(Runfiles::Create(argv0, "", "", "protobuf~3.19.2", &error)); + ASSERT_NE(r, nullptr); + EXPECT_TRUE(error.empty()); + + EXPECT_EQ(r->Rlocation("protobuf/foo/runfile"), + dir + "/protobuf~3.19.2/foo/runfile"); + EXPECT_EQ(r->Rlocation("protobuf/bar/dir"), dir + "/protobuf~3.19.2/bar/dir"); + EXPECT_EQ(r->Rlocation("protobuf/bar/dir/file"), + dir + "/protobuf~3.19.2/bar/dir/file"); + EXPECT_EQ(r->Rlocation("protobuf/bar/dir/de eply/nes ted/fi~le"), + dir + "/protobuf~3.19.2/bar/dir/de eply/nes ted/fi~le"); + + EXPECT_EQ(r->Rlocation("my_module/bar/runfile"), + dir + "/my_module/bar/runfile"); + EXPECT_EQ(r->Rlocation("my_protobuf/bar/dir/de eply/nes ted/fi~le"), + dir + "/my_protobuf/bar/dir/de eply/nes ted/fi~le"); + + EXPECT_EQ(r->Rlocation("_main/bar/runfile"), dir + "/_main/bar/runfile"); + EXPECT_EQ(r->Rlocation("protobuf~3.19.2/foo/runfile"), + dir + "/protobuf~3.19.2/foo/runfile"); + EXPECT_EQ(r->Rlocation("protobuf~3.19.2/bar/dir"), + dir + "/protobuf~3.19.2/bar/dir"); + EXPECT_EQ(r->Rlocation("protobuf~3.19.2/bar/dir/file"), + dir + "/protobuf~3.19.2/bar/dir/file"); + EXPECT_EQ(r->Rlocation("protobuf~3.19.2/bar/dir/de eply/nes ted/fi~le"), + dir + "/protobuf~3.19.2/bar/dir/de eply/nes ted/fi~le"); + + EXPECT_EQ(r->Rlocation("config.json"), dir + "/config.json"); +} + +TEST_F(RunfilesTest, + DirectoryBasedRlocationWithRepoMapping_fromOtherRepo_withSourceRepo) { + string uid = LINE_AS_STRING(); + unique_ptr + dir_marker(MockFile::Create("foo" + uid + ".runfiles/marker"), {}); + EXPECT_TRUE(dir_marker != nullptr); + string dir = dir_marker->DirName(); + unique_ptr rm(MockFile::Create( + "foo" + uid + ".repo_mapping", { + ",my_module,_main", + ",my_protobuf,protobuf~3.19.2", + ",my_workspace,_main", + "protobuf~3.19.2,protobuf,protobuf~3.19.2"})); + EXPECT_TRUE(rm != nullptr); + string argv0(rm->Path().substr( + 0, rm->Path().size() - string(".repo_mapping").size())); + + string error; + unique_ptr r(Runfiles::Create(argv0, "", "", "", &error)); + r.reset(r->WithSourceRepository("protobuf~3.19.2")); + ASSERT_NE(r, nullptr); + EXPECT_TRUE(error.empty()); + + EXPECT_EQ(r->Rlocation("protobuf/foo/runfile"), + dir + "/protobuf~3.19.2/foo/runfile"); + EXPECT_EQ(r->Rlocation("protobuf/bar/dir"), dir + "/protobuf~3.19.2/bar/dir"); + EXPECT_EQ(r->Rlocation("protobuf/bar/dir/file"), + dir + "/protobuf~3.19.2/bar/dir/file"); + EXPECT_EQ(r->Rlocation("protobuf/bar/dir/de eply/nes ted/fi~le"), + dir + "/protobuf~3.19.2/bar/dir/de eply/nes ted/fi~le"); + + EXPECT_EQ(r->Rlocation("my_module/bar/runfile"), + dir + "/my_module/bar/runfile"); + EXPECT_EQ(r->Rlocation("my_protobuf/bar/dir/de eply/nes ted/fi~le"), + dir + "/my_protobuf/bar/dir/de eply/nes ted/fi~le"); + + EXPECT_EQ(r->Rlocation("_main/bar/runfile"), dir + "/_main/bar/runfile"); + EXPECT_EQ(r->Rlocation("protobuf~3.19.2/foo/runfile"), + dir + "/protobuf~3.19.2/foo/runfile"); + EXPECT_EQ(r->Rlocation("protobuf~3.19.2/bar/dir"), + dir + "/protobuf~3.19.2/bar/dir"); + EXPECT_EQ(r->Rlocation("protobuf~3.19.2/bar/dir/file"), + dir + "/protobuf~3.19.2/bar/dir/file"); + EXPECT_EQ(r->Rlocation("protobuf~3.19.2/bar/dir/de eply/nes ted/fi~le"), + dir + "/protobuf~3.19.2/bar/dir/de eply/nes ted/fi~le"); + + EXPECT_EQ(r->Rlocation("config.json"), dir + "/config.json"); +} + +TEST_F(RunfilesTest, InvalidRepoMapping) { + string uid = LINE_AS_STRING(); + unique_ptr + dir_marker(MockFile::Create("foo" + uid + ".runfiles/marker"), {}); + EXPECT_TRUE(dir_marker != nullptr); + string dir = dir_marker->DirName(); + unique_ptr + rm(MockFile::Create("foo" + uid + ".repo_mapping", {"a,b"})); + EXPECT_TRUE(rm != nullptr); + string argv0( + rm->Path().substr(0, rm->Path().size() - string(".repo_mapping").size())); + + string error; + unique_ptr r(Runfiles::Create(argv0, "", "", "", &error)); + ASSERT_EQ(r, nullptr); + EXPECT_TRUE(error.find("bad repository mapping") != string::npos); } } // namespace From 80074c4998417a8a0d59a2746e365512df2deada Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Wed, 2 Nov 2022 21:44:17 +0100 Subject: [PATCH 2/2] Address review comments --- tools/cpp/runfiles/runfiles_src.h | 29 +++++++++++++---------------- tools/cpp/runfiles/runfiles_test.cc | 2 +- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/tools/cpp/runfiles/runfiles_src.h b/tools/cpp/runfiles/runfiles_src.h index 37678566067864..dc960b2ebf8c64 100644 --- a/tools/cpp/runfiles/runfiles_src.h +++ b/tools/cpp/runfiles/runfiles_src.h @@ -184,26 +184,23 @@ class Runfiles { // repository as a default for all calls to Rlocation. // // The current instance remains valid. - Runfiles* WithSourceRepository(const std::string& source_repository) const { - std::map runfiles_map(runfiles_map_); - std::string directory(directory_); - std::map, std::string> - repo_mapping(repo_mapping_); - std::vector > envvars(envvars_); - auto* new_runfiles = - new Runfiles(std::move(runfiles_map), std::move(directory), - std::move(repo_mapping), std::move(envvars), - std::string(source_repository)); - return new_runfiles; + std::unique_ptr WithSourceRepository( + const std::string& source_repository) const { + return std::unique_ptr( + new Runfiles(runfiles_map_, + directory_, + repo_mapping_, + envvars_, + source_repository)); } private: - Runfiles(std::map&& runfiles_map, - std::string&& directory, + Runfiles(std::map runfiles_map, + std::string directory, std::map, - std::string>&& repo_mapping, - std::vector >&& envvars, - std::string&& source_repository_) + std::string> repo_mapping, + std::vector > envvars, + std::string source_repository_) : runfiles_map_(std::move(runfiles_map)), directory_(std::move(directory)), repo_mapping_(std::move(repo_mapping)), diff --git a/tools/cpp/runfiles/runfiles_test.cc b/tools/cpp/runfiles/runfiles_test.cc index 88eb0c02f39417..15568ac366793e 100644 --- a/tools/cpp/runfiles/runfiles_test.cc +++ b/tools/cpp/runfiles/runfiles_test.cc @@ -877,7 +877,7 @@ TEST_F(RunfilesTest, string error; unique_ptr r(Runfiles::Create(argv0, "", "", "", &error)); - r.reset(r->WithSourceRepository("protobuf~3.19.2")); + r = r->WithSourceRepository("protobuf~3.19.2"); ASSERT_NE(r, nullptr); EXPECT_TRUE(error.empty());