Add support for --incompatible_compact_repo_mapping_manifest

Copybara Import from #417

BEGIN_PUBLIC
Add support for `--incompatible_compact_repo_mapping_manifest` (#417)

Closes #417
END_PUBLIC

COPYBARA_INTEGRATE_REVIEW=#417 from fmeum:compact-repo-mapping 09aaf16
PiperOrigin-RevId: 804917692
Change-Id: I2399ee18d785cfaba5cbe1d0502bb1a805fe992e

Author: fmeum

cc/runfiles/runfiles.cc

@@ -46,37 +46,17 @@ using std::vector;
 
 namespace {
 
-bool starts_with(const string& s, const char* prefix) {
-  if (!prefix || !*prefix) {
-    return true;
-  }
-  if (s.empty()) {
-    return false;
-  }
-  return s.find(prefix) == 0;
+bool starts_with(const string& s, const string& prefix) {
+  return s.compare(0, prefix.size(), prefix) == 0;
 }
 
-bool contains(const string& s, const char* substr) {
-  if (!substr || !*substr) {
-    return true;
-  }
-  if (s.empty()) {
-    return false;
-  }
+bool contains(const string& s, const string& substr) {
   return s.find(substr) != string::npos;
 }
 
 bool ends_with(const string& s, const string& suffix) {
-  if (suffix.empty()) {
-    return true;
-  }
-  if (s.empty()) {
-    return false;
-  }
-  if (suffix.size() > s.size()) {
-    return false;
-  }
-  return s.rfind(suffix) == s.size() - suffix.size();
+  return s.size() >= suffix.size() &&
+         s.compare(s.size() - suffix.size(), suffix.size(), suffix) == 0;
 }
 
 bool IsReadableFile(const string& path) {
@@ -232,13 +212,27 @@ string Runfiles::Rlocation(const string& path,
     return RlocationUnchecked(path, runfiles_map_, directory_);
   }
   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, runfiles_map_, directory_);
+  auto lookup_key = std::make_pair(target_apparent, source_repo);
+  auto lower_bound = repo_mapping_.lower_bound(lookup_key);
+  if (lower_bound->first == lookup_key) {
+    return RlocationUnchecked(lower_bound->second + path.substr(first_slash),
+                              runfiles_map_, directory_);
+  }
+  // Since the asterisk sorts before any other valid character in a repo name,
+  // the previous element may be a prefix match.
+  if (lower_bound != repo_mapping_.begin()) {
+    std::pair<string, string> key;
+    string value;
+    std::tie(key, value) = *std::prev(lower_bound);
+    if (key.first == target_apparent &&
+        ends_with(key.second, "*") &&
+        starts_with(
+          source_repo, key.second.substr( 0, key.second.size() - 1))) {
+      return RlocationUnchecked(
+        value + path.substr(first_slash), runfiles_map_, directory_);
+    }
   }
-  return RlocationUnchecked(target->second + path.substr(first_slash),
-                            runfiles_map_, directory_);
+  return RlocationUnchecked(path, runfiles_map_, directory_);
 }
 
 string Runfiles::RlocationUnchecked(const string& path,
@@ -360,12 +354,17 @@ bool ParseRepoMapping(const string& path,
       return false;
     }
 
+    // If the source repo ends with an asterisk, the line is supposed to match
+    // any name that starts with the prefix up to the asterisk. Since an
+    // asterisk sorts before any other valid character in a repo name, we can
+    // insert the line as is and search for a lower bound if we store the
+    // apparent target repo name first in the map key.
     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;
+    (*result)[std::make_pair(target_apparent, source)] = target;
     std::getline(stm, line);
     ++line_count;
   }

tests/runfiles/runfiles_test.cc

@@ -717,6 +717,40 @@ TEST_F(RunfilesTest, ManifestBasedRlocationWithRepoMapping_fromOtherRepo) {
   EXPECT_EQ(r->Rlocation("protobuf"), "");
 }
 
+TEST_F(RunfilesTest, ManifestBasedRlocationWithRepoMapping_fromExtensionRepo) {
+  string uid = LINE_AS_STRING();
+  unique_ptr<MockFile> rm(
+      MockFile::Create("foo" + uid + ".runfiles/_repo_mapping",
+                       {",config.json,config.json+1.2.3", ",my_module,_main",
+                        ",my_protobuf,protobuf+3.19.2", ",my_workspace,_main",
+                        "my_module++ext+*,my_module,my_module+",
+                        "my_module++ext+*,repo1,my_module++ext+repo1"}));
+  ASSERT_TRUE(rm != nullptr);
+  unique_ptr<MockFile> mf(MockFile::Create(
+      "foo" + uid + ".runfiles_manifest",
+      {"_repo_mapping " + rm->Path(), "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",
+       "my_module+/foo /the/path/to/my_module+/runfile",
+       "my_module++ext+repo1/foo /the/path/to/my_module++ext+repo1/runfile",
+       "repo2+/foo /the/path/to/repo2+/runfile"}));
+  ASSERT_TRUE(mf != nullptr);
+  string argv0(mf->Path().substr(
+      0, mf->Path().size() - string(".runfiles_manifest").size()));
+
+  string error;
+  unique_ptr<Runfiles> r(Runfiles::Create(argv0, /*runfiles_manifest_file=*/"",
+                                          /*runfiles_dir=*/"",
+                                          "my_module++ext+repo1", &error));
+  ASSERT_TRUE(r != nullptr);
+  EXPECT_TRUE(error.empty());
+
+  EXPECT_EQ(r->Rlocation("my_module/foo"), "/the/path/to/my_module+/runfile");
+  EXPECT_EQ(r->Rlocation("repo1/foo"), "/the/path/to/my_module++ext+repo1/runfile");
+  EXPECT_EQ(r->Rlocation("repo2+/foo"), "/the/path/to/repo2+/runfile");
+}
+
 TEST_F(RunfilesTest, DirectoryBasedRlocationWithRepoMapping_fromMain) {
   string uid = LINE_AS_STRING();
   unique_ptr<MockFile> rm(
@@ -858,6 +892,31 @@ TEST_F(RunfilesTest,
   EXPECT_EQ(r->Rlocation("config.json"), dir + "/config.json");
 }
 
+TEST_F(RunfilesTest, DirectoryBasedRlocationWithRepoMapping_fromExtensionRepo) {
+  string uid = LINE_AS_STRING();
+  unique_ptr<MockFile> rm(
+      MockFile::Create("foo" + uid + ".runfiles/_repo_mapping",
+                       {",config.json,config.json+1.2.3", ",my_module,_main",
+                        ",my_protobuf,protobuf+3.19.2", ",my_workspace,_main",
+                        "my_module++ext+*,my_module,my_module+",
+                        "my_module++ext+*,repo1,my_module++ext+repo1"}));
+  ASSERT_TRUE(rm != nullptr);
+  string dir = rm->DirName();
+  string argv0(dir.substr(0, dir.size() - string(".runfiles").size()));
+
+  string error;
+  unique_ptr<Runfiles> r(Runfiles::Create(argv0, /*runfiles_manifest_file=*/"",
+                                          /*runfiles_dir=*/"",
+                                          /*source_repository=*/"", &error));
+  r = r->WithSourceRepository("my_module++ext+repo1");
+  ASSERT_TRUE(r != nullptr);
+  EXPECT_TRUE(error.empty());
+
+  EXPECT_EQ(r->Rlocation("my_module/foo"), dir + "/my_module+/foo");
+  EXPECT_EQ(r->Rlocation("repo1/foo"), dir + "/my_module++ext+repo1/foo");
+  EXPECT_EQ(r->Rlocation("repo2+/foo"), dir + "/repo2+/foo");
+}
+
 TEST_F(RunfilesTest, InvalidRepoMapping) {
   string uid = LINE_AS_STRING();
   unique_ptr<MockFile> rm(