Fix loss of user configuration on reconfigure failure

4 files changed, 115 insertions, 28 deletions

diff --git a/bpkg/pkg-build-collect.cxx b/bpkg/pkg-build-collect.cxx
index 5485672..f0994ef 100644
--- a/bpkg/pkg-build-collect.cxx
+++ b/bpkg/pkg-build-collect.cxx
@@ -404,6 +404,8 @@ namespace bpkg
                                       ? override
                                       : available);
 
+    const shared_ptr<selected_package>& sp (selected);
+
     assert (!skeleton && ap != nullptr);
 
     package_key pk (db, ap->id.name);
@@ -430,11 +432,12 @@ namespace bpkg
 
     if (ap != nullptr)
     {
-      bool src_conf (selected != nullptr                          &&
-                     selected->state == package_state::configured &&
-                     selected->substate != package_substate::system);
+      bool src_conf (sp != nullptr                          &&
+                     sp->state == package_state::configured &&
+                     sp->substate != package_substate::system);
 
       database& pdb (db);
+      const dir_path& c (pdb.config);
 
       // If the package is being reconfigured, then specify {src,out}_root as
       // the existing source and output root directories not to create the
@@ -447,26 +450,52 @@ namespace bpkg
       // configuration? Yes we can, since load_config_flags stays 0 in this
       // case and all the variables in config.build will be ignored.
       //
-      if (src_conf && ap->version == selected->version)
+      optional<dir_path> unpacked_conf;
+
+      if (src_conf && ap->version == sp->version)
       {
-        src_root = selected->effective_src_root (pdb.config);
-        out_root = selected->effective_out_root (pdb.config);
+        src_root = sp->effective_src_root (c);
+        out_root = sp->effective_out_root (c);
       }
-      else
+      else if (sp != nullptr && sp->state == package_state::unpacked)
+      {
+        dir_path d (
+          sp->external ()
+          ? c / dir_path (sp->name.string ())
+          : c / dir_path (sp->name.string () + '-' + sp->version.string ()));
+
+        if (exists (d / alt_config_file) ||
+            exists (d / std_config_file))
+        {
+          unpacked_conf = move (d);
+
+          if (ap->version == sp->version)
+          {
+            src_root = sp->effective_src_root (c);
+            out_root = unpacked_conf;
+          }
+        }
+      }
+
+      if (!src_root)
       {
         src_root = external_dir ();
 
         if (src_root)
-          out_root = dir_path (pdb.config) /= name ().string ();
+          out_root = dir_path (c) /= name ().string ();
       }
 
       // Specify old_{src,out}_root paths and set load_config_flags if the old
       // configuration is present and is requested to be loaded.
       //
-      if (src_conf && (!disfigure || load_old_dependent_config))
+      if ((src_conf || unpacked_conf) &&
+          (!disfigure || load_old_dependent_config))
       {
-        old_src_root = selected->effective_src_root (pdb.config);
-        old_out_root = selected->effective_out_root (pdb.config);
+        old_src_root = sp->effective_src_root (c);
+
+        old_out_root = src_conf
+                       ? sp->effective_out_root (c)
+                       : move (unpacked_conf);
 
         if (!disfigure)
           load_config_flags |= package_skeleton::load_config_user;
@@ -483,7 +512,7 @@ namespace bpkg
       move (ap),
       config_vars, // @@ Maybe make optional<strings> and move?
       disfigure,
-      (selected != nullptr ? &selected->config_variables : nullptr),
+      (sp != nullptr ? &sp->config_variables : nullptr),
       move (src_root),
       move (out_root),
       move (old_src_root),
diff --git a/bpkg/pkg-configure.cxx b/bpkg/pkg-configure.cxx
index eb5b85b..6971a4e 100644
--- a/bpkg/pkg-configure.cxx
+++ b/bpkg/pkg-configure.cxx
@@ -680,6 +680,29 @@ namespace bpkg
         print_b (o, verb_b::quiet, cpr.config_variables, bspec);
       }
 
+      const small_vector<pair<const path*, const path*>, 2> cfs ({
+        {&std_config_file,   &alt_config_file},
+        {&std_src_root_file, &alt_src_root_file}});
+
+      small_vector<pair<path, string>, 2> cfg;
+
+      for (const auto& f: cfs)
+      {
+        path cf;
+
+        if (exists (cf = out_root / *f.second) ||
+            exists (cf = out_root / *f.first))
+        try
+        {
+          ifdstream ifs (cf);
+          cfg.emplace_back (move (cf), ifs.read_text ());
+        }
+        catch (const io_error& e)
+        {
+          fail << "unable to read from " << cf << ": " << e;
+        }
+      }
+
       try
       {
         // Note: no bpkg::failed should be thrown from this block.
@@ -848,25 +871,47 @@ namespace bpkg
       {
         // Assume the diagnostics has already been issued.
 
-        // If we failed to configure the package, make sure we revert
-        // it back to the unpacked state by running disfigure (it is
-        // valid to run disfigure on an un-configured build). And if
-        // disfigure fails as well, then the package will be set into
-        // the broken state.
-
-        // Indicate to pkg_disfigure() we are partially configured.
+        // If we can restore the build2 configuration, then do that and leave
+        // the package in the current (unpacked) state, assuming that the
+        // transaction will be rolled back when the failed exception is thrown.
         //
-        p->out_root = out_root.leaf ();
-        p->state = package_state::broken;
+        if (!cfg.empty ())
+        {
+          for (const auto& cf: cfg)
+          {
+            try
+            {
+              ofdstream ofs (cf.first);
+              ofs << cf.second;
+              ofs.close ();
+            }
+            catch (const io_error& e)
+            {
+              fail << "unable to write to " << cf.first << ": " << e;
+            }
+          }
+        }
+        else
+        {
+          // If we failed to configure the package, make sure we revert
+          // it back to the unpacked state by running disfigure (it is
+          // valid to run disfigure on an un-configured build). And if
+          // disfigure fails as well, then the package will be set into
+          // the broken state.
 
-        // Commits the transaction.
-        //
-        pkg_disfigure (o, db, t,
-                       p,
-                       true /* clean */,
-                       true /* disfigure */,
-                       false /* simulate */);
+          // Indicate to pkg_disfigure() we are partially configured.
+          //
+          p->out_root = out_root.leaf ();
+          p->state = package_state::broken;
 
+          // Commits the transaction.
+          //
+          pkg_disfigure (o, db, t,
+                         p,
+                         true /* clean */,
+                         true /* disfigure */,
+                         false /* simulate */);
+        }
 
         throw bpkg::failed ();
       }
diff --git a/bpkg/utility.cxx b/bpkg/utility.cxx
index d084b76..c68cab0 100644
--- a/bpkg/utility.cxx
+++ b/bpkg/utility.cxx
@@ -34,6 +34,9 @@ namespace bpkg
   const dir_path std_config_dir     (dir_path (std_build_dir) /= "config");
   const path     std_bootstrap_file (dir_path (std_build_dir) /= "bootstrap.build");
   const path     std_root_file      (dir_path (std_build_dir) /= "root.build");
+  const path     std_config_file    (dir_path (std_build_dir) /= "config.build");
+  const dir_path std_bootstrap_dir  (dir_path (std_build_dir) /= "bootstrap");
+  const path     std_src_root_file  (dir_path (std_bootstrap_dir) /= "src-root.build");
   const string   std_build_ext      ("build");
 
   // build2:
@@ -42,6 +45,9 @@ namespace bpkg
   const dir_path alt_config_dir     (dir_path (alt_build_dir) /= "config");
   const path     alt_bootstrap_file (dir_path (alt_build_dir) /= "bootstrap.build2");
   const path     alt_root_file      (dir_path (alt_build_dir) /= "root.build2");
+  const path     alt_config_file    (dir_path (alt_build_dir) /= "config.build2");
+  const dir_path alt_bootstrap_dir  (dir_path (alt_build_dir) /= "bootstrap");
+  const path     alt_src_root_file  (dir_path (alt_bootstrap_dir) /= "src-root.build2");
   const string   alt_build_ext      ("build2");
 
   const dir_path current_dir (".");
diff --git a/bpkg/utility.hxx b/bpkg/utility.hxx
index 7a51948..ef04272 100644
--- a/bpkg/utility.hxx
+++ b/bpkg/utility.hxx
@@ -94,12 +94,19 @@ namespace bpkg
   extern const dir_path std_config_dir;     // build/config/
   extern const path     std_bootstrap_file; // build/bootstrap.build
   extern const path     std_root_file;      // build/root.build
+  extern const path     std_config_file;    // build/config.build
+  extern const dir_path std_bootstrap_dir;  // build/bootstrap/
+  extern const path     std_src_root_file;  // build/bootstrap/src-root.build
   extern const string   std_build_ext;      // build
 
+
   extern const dir_path alt_build_dir;      // build2/
   extern const dir_path alt_config_dir;     // build2/config/
   extern const path     alt_bootstrap_file; // build2/bootstrap.build2
   extern const path     alt_root_file;      // build2/root.build2
+  extern const path     alt_config_file;    // build2/config.build2
+  extern const dir_path alt_bootstrap_dir;  // build2/bootstrap/
+  extern const path     alt_src_root_file;  // build2/bootstrap/src-root.build2
   extern const string   alt_build_ext;      // build2
 
   extern const dir_path current_dir;        // ./