Add support for --keep-out option

8 files changed, 125 insertions, 22 deletions

diff --git a/bpkg/pkg-build.cli b/bpkg/pkg-build.cli
index a4d49ef..4bc55a7 100644
--- a/bpkg/pkg-build.cli
+++ b/bpkg/pkg-build.cli
@@ -142,6 +142,12 @@ namespace bpkg
        anything."
     }
 
+    bool --keep-out
+    {
+      "Keep output directories of external packages between upgrades and
+       downgrades. Refer to \l{bpkg-pkg-disfigure(1)} for details."
+    }
+
     bool --fetch-shallow
     {
       "Do not re-fetch complement and prerequisite repositories. Refer to
diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx
index 8f80ff5..a8e90aa 100644
--- a/bpkg/pkg-build.cxx
+++ b/bpkg/pkg-build.cxx
@@ -175,6 +175,11 @@ namespace bpkg
       ? pkg_verify (options, a->absolute () ? *a : cd / *a, true)
       : pkg_verify (d->absolute () ? *d : cd / *d, true));
 
+    // Copy the possibly fixed up version from the selected package.
+    //
+    if (sp->external ())
+      m.version = sp->version;
+
     return make_pair (make_shared<available_package> (move (m)), move (ar));
   }
 
@@ -243,6 +248,12 @@ namespace bpkg
     //
     bool system;
 
+    // If the flag is set and the external package is being replaced with an
+    // external one, then keep its output directory between upgrades and
+    // downgrades.
+    //
+    bool keep_out;
+
     const version&
     available_version () const
     {
@@ -438,6 +449,11 @@ namespace bpkg
             (p2->hold_version && *p2->hold_version > *p1->hold_version))
           p1->hold_version = p2->hold_version;
 
+        // Save the 'keep output directory' flag if specified by the user.
+        //
+        if (p2->user_selection () && p2->keep_out)
+          p1->keep_out = true;
+
         // Note that we don't copy the build_package::system flag. If it was
         // set from the command line ("strong system") then we will also have
         // the '== 0' constraint which means that this build_package object
@@ -673,6 +689,7 @@ namespace bpkg
           nullopt,      // Hold version.
           {},           // Constraints.
           system,       // System.
+          false,        // Keep output directory.
           {name},       // Required by.
           false};       // Reconfigure.
 
@@ -1029,6 +1046,7 @@ namespace bpkg
                 nullopt,        // Hold version.
                 {},             // Constraints.
                 system,
+                false,          // Keep output directory.
                 {n},            // Required by.
                 true}           // Reconfigure.
             }).first;
@@ -1435,6 +1453,14 @@ namespace bpkg
 
                 package_manifest m (pkg_verify (d, true, diag));
 
+                // Fix-up the package version to properly decide if we need to
+                // upgrade/downgrade the package.
+                //
+                optional<version> pv (package_version (o, d));
+
+                if (pv)
+                  m.version = move (*pv);
+
                 // This is a package directory (note that we shouldn't throw
                 // failed from here on).
                 //
@@ -1638,18 +1664,27 @@ namespace bpkg
         if (v.empty () && sys)
           v = wildcard_version;
 
+        // We will keep the output directory only if the external package is
+        // replaced with an external one. Note, however, that at this stage
+        // the available package is not settled down yet, as we still need to
+        // satisfy all the constraints. Thus the available package check is
+        // postponed until the package disfiguring.
+        //
+        bool keep_out (o.keep_out () && sp->external ());
+
         // Finally add this package to the list.
         //
         build_package p {
           move (sp),
           move (ap),
           move (ar),
-          true,        // Hold package.
-          !v.empty (), // Hold version.
-          {},          // Constraints.
+          true,          // Hold package.
+          !v.empty (),   // Hold version.
+          {},            // Constraints.
           sys,
-          {""},        // Required by (command line).
-          false};      // Reconfigure.
+          keep_out,
+          {""},          // Required by (command line).
+          false};        // Reconfigure.
 
         l4 ([&]{trace << "collect " << p.available_name ();});
 
@@ -1890,7 +1925,37 @@ namespace bpkg
         }
       }
 
-      pkg_disfigure (c, o, t, sp); // Commits the transaction.
+      // Reset the flag if the package being unpacked is not an external one.
+      //
+      if (p.keep_out)
+      {
+        const shared_ptr<available_package>& ap (p.available);
+        const package_location& pl (ap->locations[0]);
+
+        if (pl.repository.object_id () == "") // Special root.
+          p.keep_out = !exists (pl.location); // Directory case.
+        else
+        {
+          p.keep_out = false;
+
+          // See if the package comes from the directory-based repository, and
+          // so is external.
+          //
+          // Note that such repositories are always preferred over others (see
+          // below).
+          //
+          for (const package_location& l: ap->locations)
+          {
+            if (l.repository.load ()->location.directory_based ())
+            {
+              p.keep_out = true;
+              break;
+            }
+          }
+        }
+      }
+
+      pkg_disfigure (c, o, t, sp, !p.keep_out); // Commits the transaction.
       assert (sp->state == package_state::unpacked ||
               sp->state == package_state::transient);
 
@@ -2092,6 +2157,9 @@ namespace bpkg
           {
             transaction t (db.begin ());
             sp = pkg_unpack (o, c, t, ap->id.name); // Commits the transaction.
+
+            if (verb)
+              text << "unpacked " << *sp;
           }
           else
           {
@@ -2105,12 +2173,12 @@ namespace bpkg
                              path_cast<dir_path> (pl.location),
                              true,   // Replace.
                              false); // Don't purge; commits the transaction.
+
+            if (verb)
+              text << "using " << *sp << " (external)";
           }
 
           assert (sp->state == package_state::unpacked);
-
-          if (verb)
-            text << "unpacked " << *sp;
         }
 
         break; // Get out from the breakout loop.
diff --git a/bpkg/pkg-configure.cxx b/bpkg/pkg-configure.cxx
index aaa426f..19b64d9 100644
--- a/bpkg/pkg-configure.cxx
+++ b/bpkg/pkg-configure.cxx
@@ -175,7 +175,9 @@ namespace bpkg
       p->out_root = out_root.leaf ();
       p->state = package_state::broken;
 
-      pkg_disfigure (c, o, t, p); // Commits the transaction.
+      // Commits the transaction.
+      //
+      pkg_disfigure (c, o, t, p, true /* clean */);
       throw;
     }
 
diff --git a/bpkg/pkg-disfigure.cli b/bpkg/pkg-disfigure.cli
index e7708e0..34e7ad2 100644
--- a/bpkg/pkg-disfigure.cli
+++ b/bpkg/pkg-disfigure.cli
@@ -22,11 +22,20 @@ namespace bpkg
      The \cb{pkg-disfigure} command disfigures the previously configured
      (via \l{bpkg-pkg-build(1)} or \l{bpkg-pkg-configure(1)}) package. A
      source code package is returned to the \cb{unpacked} state. A system
-     package is removed from the configuration."
+     package is removed from the configuration.
+
+     By default \cb{pkg-disfigure} will also clean the package's output
+     directory. This behavior can be suppressed with the \cb{--keep-out}
+     option, for example, if the package is to be reconfigured."
   }
 
   class pkg_disfigure_options: configuration_options
   {
     "\h|PKG-DISFIGURE OPTIONS|"
+
+    bool --keep-out
+    {
+      "Don't clean the package's output directory."
+    }
   };
 }
diff --git a/bpkg/pkg-disfigure.cxx b/bpkg/pkg-disfigure.cxx
index 4c32d53..9153a6a 100644
--- a/bpkg/pkg-disfigure.cxx
+++ b/bpkg/pkg-disfigure.cxx
@@ -18,7 +18,8 @@ namespace bpkg
   pkg_disfigure (const dir_path& c,
                  const common_options& o,
                  transaction& t,
-                 const shared_ptr<selected_package>& p)
+                 const shared_ptr<selected_package>& p,
+                 bool clean)
   {
     assert (p->state == package_state::configured ||
             p->state == package_state::broken);
@@ -91,7 +92,10 @@ namespace bpkg
 
     if (p->state == package_state::configured)
     {
-      bspec = "clean('" + rep + "') " "disfigure('" + rep + "')";
+      if (clean)
+        bspec = "clean('" + rep + "') ";
+
+      bspec += "disfigure('" + rep + "')";
     }
     else
     {
@@ -113,11 +117,23 @@ namespace bpkg
     try
     {
       if (exists (out_root))
-        run_b (o, c, bspec, true); // Run quiet.
+      {
+        // Note that for external packages this is just the output directory.
+        // It is also possible that the buildfiles in the source directory
+        // have changed in a way that they don't clean everything. So in this
+        // case we just remove the output directory manually rather then
+        // running 'b clean disfigure'.
+        //
+        if (clean && p->external ())
+          rm_r (out_root);
+        else
+          run_b (o, c, bspec, true); // Run quiet.
+      }
 
-      // Make sure the out directory is gone unless it is the same as src.
+      // Make sure the out directory is gone unless it is the same as src, or
+      // we didn't clean it.
       //
-      if (out_root != src_root && exists (out_root))
+      if (out_root != src_root && clean && exists (out_root))
         fail << "package output directory " << out_root << " still exists";
     }
     catch (const failed&)
@@ -167,7 +183,7 @@ namespace bpkg
       fail << "package " << n << " is " << p->state <<
         info << "expected it to be configured";
 
-    pkg_disfigure (c, o, t, p); // Commits the transaction.
+    pkg_disfigure (c, o, t, p, !o.keep_out ()); // Commits the transaction.
 
     assert (p->state == package_state::unpacked ||
             p->state == package_state::transient);
diff --git a/bpkg/pkg-disfigure.hxx b/bpkg/pkg-disfigure.hxx
index 920fc7c..d475f03 100644
--- a/bpkg/pkg-disfigure.hxx
+++ b/bpkg/pkg-disfigure.hxx
@@ -26,7 +26,8 @@ namespace bpkg
   pkg_disfigure (const dir_path& configuration,
                  const common_options&,
                  transaction&,
-                 const shared_ptr<selected_package>&);
+                 const shared_ptr<selected_package>&,
+                 bool clean);
 }
 
 #endif // BPKG_PKG_DISFIGURE_HXX
diff --git a/bpkg/pkg-drop.cxx b/bpkg/pkg-drop.cxx
index 4309b2e..6e28c37 100644
--- a/bpkg/pkg-drop.cxx
+++ b/bpkg/pkg-drop.cxx
@@ -354,7 +354,10 @@ namespace bpkg
       // leave the configuration in a valid state.
       //
       transaction t (db.begin ());
-      pkg_disfigure (c, o, t, p); // Commits the transaction.
+
+      // Commits the transaction.
+      //
+      pkg_disfigure (c, o, t, p, true /* clean */);
 
       assert (p->state == package_state::unpacked ||
               p->state == package_state::transient);
diff --git a/bpkg/pkg-unpack.cli b/bpkg/pkg-unpack.cli
index 56682d9..dc3a1c3 100644
--- a/bpkg/pkg-unpack.cli
+++ b/bpkg/pkg-unpack.cli
@@ -50,9 +50,7 @@ namespace bpkg
      example, during development). To support this, \cb{bpkg} implements the
      \i{package iteration} mechanism which may result in iteration numbers to
      be shown as part of the package version, for example, \cb{1.2.3#1} (see
-     \l{bpkg#package-version Package Version}). Finally, it is possible to
-     replace one external package version with another while preserving its
-     output directory by specifying the \cb{--keep-out} option."
+     \l{bpkg#package-version Package Version})."
   }
 
   class pkg_unpack_options: configuration_options