Search for repository location in database before parsing for pkg-build

2 files changed, 90 insertions, 45 deletions

diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx
index efe7e20..5395d8d 100644
--- a/bpkg/pkg-build.cxx
+++ b/bpkg/pkg-build.cxx
@@ -1081,7 +1081,7 @@ namespace bpkg
     // Note that we consider '@' to be such a delimiter only if it comes
     // before ":/" (think a URL which could contain its own '@').
     //
-    auto location = [] (const string& arg) -> size_t
+    auto find_location = [] (const string& arg) -> size_t
     {
       using url_traits = butl::url::traits;
 
@@ -1109,6 +1109,35 @@ namespace bpkg
       return p;
     };
 
+    database db (open (c, trace)); // Also populates the system repository.
+
+    // Search for the repository location in the database before trying to
+    // parse it. Note that the straight parsing could otherwise fail, being
+    // unable to properly guess the repository type.
+    //
+    auto location = [&db] (const string& l) -> repository_location
+    {
+      using query = query<repository>;
+
+      shared_ptr<repository> r (
+        db.query_one<repository> (query::location.url == l));
+
+      if (r != nullptr)
+        return r->location;
+
+      return parse_location (l, nullopt /* type */);
+    };
+
+    // Note that the session spans all our transactions. The idea here is
+    // that selected_package objects in the build_packages list below will
+    // be cached in this session. When subsequent transactions modify any
+    // of these objects, they will modify the cached instance, which means
+    // our list will always "see" their updated state.
+    //
+    // Also note that rep_fetch() must be called in session.
+    //
+    session s;
+
     // Collect repository locations from <packages>@<location> arguments,
     // suppressing duplicates.
     //
@@ -1117,44 +1146,36 @@ namespace bpkg
     //
     strings args;
     vector<repository_location> locations;
-
-    while (a.more ())
     {
-      string arg (a.next ());
-      size_t p (location (arg));
+      transaction t (db.begin ());
 
-      if (p != string::npos)
+      while (a.more ())
       {
-        repository_location l (
-          parse_location (string (arg, p), nullopt /* type */));
+        string arg (a.next ());
+        size_t p (find_location (arg));
 
-        auto pr = [&l] (const repository_location& i) -> bool
+        if (p != string::npos)
         {
-          return i.canonical_name () == l.canonical_name ();
-        };
+          repository_location l (location (string (arg, p)));
 
-        auto i (find_if (locations.begin (), locations.end (), pr));
+          auto pr = [&l] (const repository_location& i) -> bool
+          {
+            return i.canonical_name () == l.canonical_name ();
+          };
 
-        if (i != locations.end ())
-          *i = move (l);
-        else
-          locations.push_back (move (l));
-      }
+          auto i (find_if (locations.begin (), locations.end (), pr));
 
-      args.push_back (move (arg));
-    }
+          if (i != locations.end ())
+            *i = move (l);
+          else
+            locations.push_back (move (l));
+        }
 
-    database db (open (c, trace)); // Also populates the system repository.
+        args.push_back (move (arg));
+      }
 
-    // Note that the session spans all our transactions. The idea here is
-    // that selected_package objects in the build_packages list below will
-    // be cached in this session. When subsequent transactions modify any
-    // of these objects, they will modify the cached instance, which means
-    // our list will always "see" their updated state.
-    //
-    // Also note that rep_fetch() must be called in session.
-    //
-    session s;
+      t.commit ();
+    }
 
     if (!locations.empty ())
       rep_fetch (o, c, db, locations);
@@ -1167,7 +1188,7 @@ namespace bpkg
 
       for (string& arg: args)
       {
-        size_t p (location (arg));
+        size_t p (find_location (arg));
 
         if (p == string::npos)
         {
@@ -1175,9 +1196,7 @@ namespace bpkg
           continue;
         }
 
-        repository_location l (
-          parse_location (string (arg, p), nullopt /* type */));
-
+        repository_location l (location (string (arg, p)));
         shared_ptr<repository> r (db.load<repository> (l.canonical_name ()));
 
         // If no packages are specified explicitly (the argument starts with
diff --git a/tests/pkg-build.test b/tests/pkg-build.test
index f5b3dee..6c5ceed 100644
--- a/tests/pkg-build.test
+++ b/tests/pkg-build.test
@@ -1445,6 +1445,29 @@ rep_fetch     += -d cfg --auth all --trust-yes 2>!
       error: package libbar is not found in pkg:build2.org/pkg-build/t4d or its complements
       EOE
   }
+
+  : location-search
+  :
+  : Test that the repository location is searched in the database before being
+  : parsed. The latest would fail as the repository type would be misguessed.
+  :
+  {
+    $clone_root_cfg;
+    $rep_add $src/libfoo-1.1.0 --type dir && $rep_fetch;
+
+    d = $canonicalize([dir_path] $src/libfoo-1.1.0);
+
+    $* "libfoo@$d" 2>>~"%EOE%";
+      fetching dir:$d
+      using libfoo/1.1.0 \(external\)
+      configured libfoo/1.1.0
+      %info: .+dir\\{libfoo-1.1.0.\\} is up to date%
+      updated libfoo/1.1.0
+      EOE
+
+    $pkg_disfigure libfoo 2>'disfigured libfoo/1.1.0';
+    $pkg_purge     libfoo 2>'purged libfoo/1.1.0'
+  }
 }
 
 : dir-rep
@@ -1455,19 +1478,22 @@ rep_fetch     += -d cfg --auth all --trust-yes 2>!
   : Test that dir (local) repository is preferred over non-dir local repository
   : that comes first.
   :
-  $clone_root_cfg;
-  $rep_fetch $rep/t4a;
-  $rep_add $src/libfoo-1.1.0 --type dir && $rep_fetch "dir:$src/libfoo-1.1.0";
-
-  $* libfoo 2>>~%EOE%;
-    using libfoo/1.1.0 (external)
-    configured libfoo/1.1.0
-    %info: .+dir\{libfoo-1.1.0.\} is up to date%
-    updated libfoo/1.1.0
-    EOE
+  {
+    $clone_root_cfg;
+    $rep_fetch $rep/t4a;
+    $rep_add $src/libfoo-1.1.0 --type dir;
+    $rep_fetch "dir:$canonicalize([dir_path] $src/libfoo-1.1.0)";
+
+    $* libfoo 2>>~%EOE%;
+      using libfoo/1.1.0 (external)
+      configured libfoo/1.1.0
+      %info: .+dir\{libfoo-1.1.0.\} is up to date%
+      updated libfoo/1.1.0
+      EOE
 
-  $pkg_disfigure libfoo 2>'disfigured libfoo/1.1.0';
-  $pkg_purge     libfoo 2>'purged libfoo/1.1.0'
+    $pkg_disfigure libfoo 2>'disfigured libfoo/1.1.0';
+    $pkg_purge     libfoo 2>'purged libfoo/1.1.0'
+  }
 }
 
 : git-rep