Implement --replace|-r mode for pkg-fetch

16 files changed, 171 insertions, 61 deletions

diff --git a/bpkg/pkg-fetch-options.cli b/bpkg/pkg-fetch-options.cli
index afd75fb..4cfeb35 100644
--- a/bpkg/pkg-fetch-options.cli
+++ b/bpkg/pkg-fetch-options.cli
@@ -16,12 +16,17 @@ bpkg pkg-fetch [<options>] (<pkg>/<ver>)|(-e <file>)"
 
 The \cb{pkg-fetch} command fetches the archive for the specified package
 name and version from one of the configuration's repositories. If the
-\cb{-e|--existing} option is used, then instead of the name and version
-argument, \cb{pkg-fetch} expects a local path to the existing package
-archive. In this case, \cb{bpkg} will use the archive in place, without
-copying it to the configuration or package cache directories. Also,
-unless the \cb{-p|--purge} option is specified, \cb{bpkg} will not
-attempt to remove the archive when the package is purged with the
+\cb{-r|--replace} option is specified, then \cb{pkg-fetch} will replace the
+archive of a package that is already in the \cb{fetched} state. Otherwise,
+\cb{pkg-fetch} expects the package to not exist in the configuration's
+database.
+
+If the \cb{-e|--existing} option is used, then instead of the name and
+version argument, \cb{pkg-fetch} expects a local path to the existing
+package archive. In this case, \cb{bpkg} will use the archive in place,
+without copying it to the configuration or package cache directories.
+Also, unless the \cb{-p|--purge} option is specified, \cb{bpkg} will
+not attempt to remove the archive when the package is purged with the
 \cb{pkg-purge} command."
 */
 
@@ -29,6 +34,11 @@ namespace bpkg
 {
   class pkg_fetch_options: configuration_options
   {
+    bool --replace|-r
+    {
+      "Replace the archive if the package is already fetched."
+    };
+
     bool --existing|-e
     {
       "Treat the argument as an existing package archive path rather than
diff --git a/bpkg/pkg-fetch.cxx b/bpkg/pkg-fetch.cxx
index daa2d5e..c8b8543 100644
--- a/bpkg/pkg-fetch.cxx
+++ b/bpkg/pkg-fetch.cxx
@@ -15,6 +15,7 @@
 #include <bpkg/diagnostics>
 #include <bpkg/manifest-utility>
 
+#include <bpkg/pkg-purge>
 #include <bpkg/pkg-verify>
 
 using namespace std;
@@ -38,6 +39,35 @@ namespace bpkg
     auto_rm arm;
     bool purge;
     repository_location rl;
+    shared_ptr<selected_package> sp;
+
+    // Check if the package already exists in this configuration and
+    // diagnose all the illegal cases. We want to do this as soon as
+    // the package name is known which happens at different times
+    // depending on whether we are dealing with an existing archive
+    // or fetching one.
+    //
+    auto check = [&o, &c, &db] (const string& n)
+      -> shared_ptr<selected_package>
+    {
+      shared_ptr<selected_package> p (db.find<selected_package> (n));
+
+      if (p == nullptr ||
+          (p->state == package_state::fetched && o.replace ()))
+        return p;
+
+      {
+        diag_record dr (error);
+
+        dr << "package " << n << " already exists in configuration " << c <<
+          info << "version: " << p->version << ", state: " << p->state;
+
+        if (p->state == package_state::fetched)
+          dr << info << "use 'pkg-fetch --replace|-r' to replace its archive";
+      }
+
+      throw failed ();
+    };
 
     if (o.existing ())
     {
@@ -71,6 +101,10 @@ namespace bpkg
         fail << "package version expected" <<
           info << "run 'bpkg help pkg-fetch' for more information";
 
+      // Check/diagnose an already existing package.
+      //
+      sp = check (n);
+
       if (db.query_value<repository_count> () == 0)
         fail << "configuration " << c << " has no repositories" <<
           info << "use 'bpkg rep-add' to add a repository";
@@ -79,18 +113,18 @@ namespace bpkg
         fail << "configuration " << c << " has no available packages" <<
           info << "use 'bpkg rep-fetch' to fetch available packages list";
 
-      shared_ptr<available_package> p (
+      shared_ptr<available_package> ap (
         db.find<available_package> (available_package_id (n, v)));
 
-      if (p == nullptr)
+      if (ap == nullptr)
         fail << "package " << n << " " << v << " is not available";
 
-      // Pick a repository. Preferring local ones over the remote seems
+      // Pick a repository. Preferring a local one over the remotes seems
       // like a sensible thing to do.
       //
-      const package_location* pl (&p->locations.front ());
+      const package_location* pl (&ap->locations.front ());
 
-      for (const package_location& l: p->locations)
+      for (const package_location& l: ap->locations)
       {
         if (!l.repository.load ()->location.remote ())
         {
@@ -116,13 +150,10 @@ namespace bpkg
     package_manifest m (pkg_verify (o, a));
     level4 ([&]{trace << m.name << " " << m.version;});
 
-    const auto& n (m.name);
-
-    // See if this package already exists in this configuration.
+    // Check/diagnose an already existing package.
     //
-    if (shared_ptr<selected_package> p = db.find<selected_package> (n))
-      fail << "package " << n << " already exists in configuration " << c <<
-        info << "version: " << p->version << ", state: " << p->state;
+    if (o.existing ())
+      sp = check (m.name);
 
     // Make the archive and configuration paths absolute and normalized.
     // If the archive is inside the configuration, use the relative path.
@@ -134,9 +165,27 @@ namespace bpkg
     if (a.sub (c))
       a = a.leaf (c);
 
-    // Add the package to the configuration.
-    //
-    shared_ptr<selected_package> p (new selected_package {
+    if (sp != nullptr)
+    {
+      // Clean up the archive we are replacing. Once this is done, there
+      // is no going back. If things go badly, we can't simply abort the
+      // transaction.
+      //
+      if (sp->purge_archive)
+        pkg_purge_archive (c, t, sp);
+
+      sp->version = move (m.version);
+      sp->repository = move (rl);
+      sp->archive = move (a);
+      sp->purge_archive = purge;
+
+      db.update (sp);
+    }
+    else
+    {
+      // Add the package to the configuration.
+      //
+      sp.reset (new selected_package {
         move (m.name),
         move (m.version),
         package_state::fetched,
@@ -146,15 +195,15 @@ namespace bpkg
         nullopt, // No source directory yet.
         false,
         nullopt, // No output directory yet.
-        {}       // No prerequisites captured yet.
-     });
+        {}});    // No prerequisites captured yet.
 
-    db.persist (p);
+      db.persist (sp);
+    }
 
     t.commit ();
     arm.cancel ();
 
     if (verb)
-      text << "fetched " << p->name << " " << p->version;
+      text << "fetched " << sp->name << " " << sp->version;
   }
 }
diff --git a/bpkg/pkg-purge b/bpkg/pkg-purge
index 7ec365d..4765e60 100644
--- a/bpkg/pkg-purge
+++ b/bpkg/pkg-purge
@@ -6,12 +6,21 @@
 #define BPKG_PKG_PURGE
 
 #include <bpkg/types>
+#include <bpkg/forward> // transaction, selected_package
 #include <bpkg/pkg-purge-options>
 
 namespace bpkg
 {
   void
   pkg_purge (const pkg_purge_options&, cli::scanner& args);
+
+  // Remove package archive. If this fails, set the package state to
+  // broken, commit the transaction, and fail.
+  //
+  void
+  pkg_purge_archive (const dir_path& configuration,
+                     transaction&,
+                     const shared_ptr<selected_package>&);
 }
 
 #endif // BPKG_PKG_PURGE
diff --git a/bpkg/pkg-purge-options.cli b/bpkg/pkg-purge-options.cli
index f5fef62..a4d7633 100644
--- a/bpkg/pkg-purge-options.cli
+++ b/bpkg/pkg-purge-options.cli
@@ -16,8 +16,8 @@ bpkg pkg-purge [<options>] <pkg>"
 
 The \cb{pkg-purge} command removes the package directory and archive
 from the filesystem and removes the package from the configuration's
-database. Only packages in the \cb{fetched} and \cb{unpacked} state
-can be purged plus broken packages if the \cb{-f|--force} options is
+database. Only packages in the \cb{fetched} and \cb{unpacked} state can
+be purged plus \cb{broken} packages if the \cb{-f|--force} options is
 specified (see this option's description for details on purging broken
 packages). If the \cb{-k|--keep} option is specified, then the package
 archive is not removed (see this option's description for details on
diff --git a/bpkg/pkg-purge.cxx b/bpkg/pkg-purge.cxx
index 5e14a2b..91c4c5b 100644
--- a/bpkg/pkg-purge.cxx
+++ b/bpkg/pkg-purge.cxx
@@ -17,6 +17,40 @@ using namespace butl;
 namespace bpkg
 {
   void
+  pkg_purge_archive (const dir_path& c,
+                     transaction& t,
+                     const shared_ptr<selected_package>& p)
+  {
+    assert (p->purge_archive && p->state != package_state::broken);
+
+    tracer trace ("pkg_purge_archive");
+
+    database& db (t.database ());
+    tracer_guard tg (db, trace);
+
+    path a (p->archive->absolute () ? *p->archive : c / *p->archive);
+
+    try
+    {
+      if (exists (a))
+        rm (a);
+
+      p->archive = nullopt;
+      p->purge_archive = false;
+    }
+    catch (const failed&)
+    {
+      p->state = package_state::broken;
+      db.update (p);
+      t.commit ();
+
+      info << "package " << p->name << " is now broken; "
+           << "use 'pkg-purge --force' to remove";
+      throw;
+    }
+  }
+
+  void
   pkg_purge (const pkg_purge_options& o, cli::scanner& args)
   {
     tracer trace ("pkg_purge");
@@ -131,31 +165,12 @@ namespace bpkg
     //
     if (p->purge_archive && !o.keep ())
     {
-      path a (p->archive->absolute () ? *p->archive : c / *p->archive);
-
       if (p->state != package_state::broken)
-      {
-        try
-        {
-          if (exists (a))
-            rm (a);
-
-          p->archive = nullopt;
-          p->purge_archive = false;
-        }
-        catch (const failed&)
-        {
-          p->state = package_state::broken;
-          db.update (p);
-          t.commit ();
-
-          info << "package " << n << " is now broken; "
-               << "use 'pkg-purge --force' to remove";
-          throw;
-        }
-      }
+        pkg_purge_archive (c, t, p);
       else
       {
+        path a (p->archive->absolute () ? *p->archive : c / *p->archive);
+
         if (exists (a))
           fail << "broken package " << n << " archive still exists" <<
             info << "remove " << a << " manually then re-run pkg-purge";
diff --git a/bpkg/test.sh b/bpkg/test.sh
index a806a25..f787baa 100755
--- a/bpkg/test.sh
+++ b/bpkg/test.sh
@@ -183,30 +183,43 @@ test rep-fetch
 ##
 ## pkg-fetch
 ##
-
+test rep-create ../tests/repository/1/fetch/t1
 test cfg-create --wipe
 
 fail pkg-fetch -e                # archive expected
 fail pkg-fetch -e ./no-such-file # archive does not exist
 
-fail pkg-fetch                  # package name expected
-fail pkg-fetch libhello         # package version expected
-fail pkg-fetch libhello/1/2/3   # invalid package version
+fail pkg-fetch                   # package name expected
+fail pkg-fetch libfoo            # package version expected
+fail pkg-fetch libfoo/1/2/3      # invalid package version
 
-fail pkg-fetch libhello/1.0.0   # no repositories
-test rep-add $rep
-fail pkg-fetch libhello/1.0.0   # no packages
+fail pkg-fetch libfoo/1.0.0      # no repositories
+test rep-add ../tests/repository/1/fetch/t1
+fail pkg-fetch libfoo/1.0.0      # no packages
 test rep-fetch
-fail pkg-fetch libhello/2+1.0.0 # not available
+fail pkg-fetch libfoo/2+1.0.0    # not available
 
 # local
 #
 test cfg-create --wipe
-test rep-add $rep
+test rep-add ../tests/repository/1/fetch/t1
 test rep-fetch
-test pkg-fetch libhello/1.0.0
-test pkg-unpack libhello
-test pkg-purge libhello
+test pkg-fetch libfoo/1.0.0
+stat libfoo/1.0.0 fetched
+fail pkg-fetch libfoo/1.0.0
+fail pkg-fetch -e ../tests/repository/1/fetch/t1/libfoo-1.0.0.tar.gz
+test pkg-purge libfoo
+test pkg-fetch -e ../tests/repository/1/fetch/t1/libfoo-1.0.0.tar.gz
+stat libfoo/1.0.0 fetched
+test pkg-unpack libfoo
+fail pkg-fetch -r libfoo/1.0.0
+fail pkg-fetch -r -e ../tests/repository/1/fetch/t1/libfoo-1.0.0.tar.gz
+test pkg-purge -k libfoo
+test pkg-fetch -r libfoo/1.1.0
+stat libfoo/1.1.0 fetched
+test pkg-fetch -r -e ../tests/repository/1/fetch/t1/libfoo-1.0.0.tar.gz
+stat libfoo/1.0.0 fetched
+test pkg-purge libfoo
 
 # remote
 #
diff --git a/tests/repository/1/fetch/libfoo-1.0.0.tar.gz b/tests/repository/1/fetch/libfoo-1.0.0.tar.gz
new file mode 100644
index 0000000..28a6a90
--- /dev/null
+++ b/tests/repository/1/fetch/libfoo-1.0.0.tar.gz
diff --git a/tests/repository/1/fetch/libfoo-1.1.0.tar.gz b/tests/repository/1/fetch/libfoo-1.1.0.tar.gz
new file mode 100644
index 0000000..e03481f
--- /dev/null
+++ b/tests/repository/1/fetch/libfoo-1.1.0.tar.gz
diff --git a/tests/repository/1/fetch/libfoo-1.1.0/build/bootstrap.build b/tests/repository/1/fetch/libfoo-1.1.0/build/bootstrap.build
new file mode 100644
index 0000000..54f267e
--- /dev/null
+++ b/tests/repository/1/fetch/libfoo-1.1.0/build/bootstrap.build
@@ -0,0 +1,2 @@
+project = fetch-libfoo
+using config
diff --git a/tests/repository/1/fetch/libfoo-1.1.0/buildfile b/tests/repository/1/fetch/libfoo-1.1.0/buildfile
new file mode 100644
index 0000000..b3ec74f
--- /dev/null
+++ b/tests/repository/1/fetch/libfoo-1.1.0/buildfile
@@ -0,0 +1 @@
+.:
diff --git a/tests/repository/1/fetch/libfoo-1.1.0/manifest b/tests/repository/1/fetch/libfoo-1.1.0/manifest
new file mode 100644
index 0000000..3453757
--- /dev/null
+++ b/tests/repository/1/fetch/libfoo-1.1.0/manifest
@@ -0,0 +1,7 @@
+: 1
+name: libfoo
+version: 1.1.0
+summary: libfoo
+license: MIT
+url: http://example.org
+email: pkg@example.org
diff --git a/tests/repository/1/fetch/repositories b/tests/repository/1/fetch/repositories
new file mode 100644
index 0000000..5b70556
--- /dev/null
+++ b/tests/repository/1/fetch/repositories
@@ -0,0 +1 @@
+: 1
diff --git a/tests/repository/1/fetch/t1/libfoo-1.0.0.tar.gz b/tests/repository/1/fetch/t1/libfoo-1.0.0.tar.gz
new file mode 120000
index 0000000..32e5a3c
--- /dev/null
+++ b/tests/repository/1/fetch/t1/libfoo-1.0.0.tar.gz
@@ -0,0 +1 @@
+../libfoo-1.0.0.tar.gz
\ No newline at end of file
diff --git a/tests/repository/1/fetch/t1/libfoo-1.1.0.tar.gz b/tests/repository/1/fetch/t1/libfoo-1.1.0.tar.gz
new file mode 120000
index 0000000..c004b2a
--- /dev/null
+++ b/tests/repository/1/fetch/t1/libfoo-1.1.0.tar.gz
@@ -0,0 +1 @@
+../libfoo-1.1.0.tar.gz
\ No newline at end of file
diff --git a/tests/repository/1/fetch/t1/repositories b/tests/repository/1/fetch/t1/repositories
new file mode 120000
index 0000000..d965b15
--- /dev/null
+++ b/tests/repository/1/fetch/t1/repositories
@@ -0,0 +1 @@
+../repositories
\ No newline at end of file
diff --git a/tests/repository/1/satisfy/libfoo-1.1.0/build/bootstrap.build b/tests/repository/1/satisfy/libfoo-1.1.0/build/bootstrap.build
index eb90fee..b24ee6a 100644
--- a/tests/repository/1/satisfy/libfoo-1.1.0/build/bootstrap.build
+++ b/tests/repository/1/satisfy/libfoo-1.1.0/build/bootstrap.build
@@ -1,2 +1,2 @@
-project = libfoo
+project = satisfy-libfoo
 using config