Add support for --force option in bdep-release and bdep-publish

7 files changed, 218 insertions, 66 deletions

diff --git a/bdep/ci.cxx b/bdep/ci.cxx
index 9298ef8..304818d 100644
--- a/bdep/ci.cxx
+++ b/bdep/ci.cxx
@@ -55,6 +55,10 @@ namespace bdep
 
       commit = move (s.commit);
 
+      // Note: not forcible. The use case could be to CI some commit from the
+      // past. But in this case we also won't have upstream. So maybe it will
+      // be better to invent the --commit option or some such.
+      //
       if (s.branch.empty ())
         fail << "project directory is in the detached HEAD state" <<
           info << "run 'git status' for details";
@@ -70,9 +74,15 @@ namespace bdep
       size_t p (path::traits::rfind_separator (s.upstream));
       branch = p != string::npos ? string (s.upstream, p + 1) : s.upstream;
 
+      // Note: not forcible (for now). While the use case is valid, the
+      // current and committed package versions are likely to differ (in
+      // snapshot id). Obtaining the committed versions feels too hairy for
+      // now.
+      //
       if (s.staged || s.unstaged)
         fail << "project directory has uncommitted changes" <<
-          info << "run 'git status' for details";
+          info << "run 'git status' for details" <<
+          info << "use 'git stash' to temporarily hide the changes";
 
       // We definitely don't want to be ahead (upstream doesn't have this
       // commit) but there doesn't seem be anything wrong with being behind.
diff --git a/bdep/project.cxx b/bdep/project.cxx
index 8092867..cf79e41 100644
--- a/bdep/project.cxx
+++ b/bdep/project.cxx
@@ -13,6 +13,7 @@
 #include <bdep/diagnostics.hxx>
 
 using namespace std;
+using namespace butl;
 
 namespace bdep
 {
@@ -384,50 +385,66 @@ namespace bdep
     }
   }
 
-  standard_version
-  package_version (const common_options& o,
-                   const dir_path& cfg,
-                   const package_name& p)
+  // Obtain build2 project info for package source or output directories.
+  //
+  static b_project_info
+  package_info (const common_options& o, const dir_path& d)
   {
-    using namespace butl;
-
-    // We could have used bpkg-pkg-status but then we would have to deal with
-    // iterations. So we use the build system's info meta-operation directly.
-    //
     try
     {
-      // Note: the package directory inside the configuration is a bit of an
-      // assumption.
-      //
-      b_project_info pi (
-        b_info ((dir_path (cfg) /= p.string ()),
-                verb,
-                [] (const char* const args[], size_t n)
-                {
-                  if (verb >= 3)
-                    print_process (args, n);
-                },
-                path (name_b (o)),
-                exec_dir,
-                o.build_option ()));
-
-      if (pi.version.empty ())
-        fail << "empty version for package " << p;
-
-      // Verify the name for good measure.
-      //
-      if (pi.project != p)
-        fail << "name mismatch for package " << p;
-
-      return move (pi.version);
+      return b_info (d,
+                     verb,
+                     [] (const char* const args[], size_t n)
+                     {
+                       if (verb >= 3)
+                         print_process (args, n);
+                     },
+                     path (name_b (o)),
+                     exec_dir,
+                     o.build_option ());
     }
     catch (const b_error& e)
     {
       if (e.normal ())
         throw failed (); // Assume the build2 process issued diagnostics.
 
-      fail << "unable to obtain package " << p << " project info: " << e
-           << endf;
+      fail << "unable to obtain project info for package directory " << d
+           << ": " << e << endf;
     }
   }
+
+  standard_version
+  package_version (const common_options& o, const dir_path& d)
+  {
+    b_project_info pi (package_info (o, d));
+
+    if (pi.version.empty ())
+      fail << "empty version for package directory " << d;
+
+    return move (pi.version);
+  }
+
+  standard_version
+  package_version (const common_options& o,
+                   const dir_path& cfg,
+                   const package_name& p)
+  {
+    // We could have used bpkg-pkg-status but then we would have to deal with
+    // iterations. So we use the build system's info meta-operation directly.
+    //
+    // Note: the package directory inside the configuration is a bit of an
+    // assumption.
+    //
+    b_project_info pi (package_info (o, (dir_path (cfg) /= p.string ())));
+
+    if (pi.version.empty ())
+      fail << "empty version for package " << p;
+
+    // Verify the name for good measure.
+    //
+    if (pi.project != p)
+      fail << "name mismatch for package " << p;
+
+    return move (pi.version);
+  }
 }
diff --git a/bdep/project.hxx b/bdep/project.hxx
index 47373b0..de39ee5 100644
--- a/bdep/project.hxx
+++ b/bdep/project.hxx
@@ -236,9 +236,13 @@ namespace bdep
   void
   verify_project_packages (const project_packages&, const configurations&);
 
-  // Determine the version of a package in the specified configuration.
+  // Determine the version of a package in the specified package (first
+  // version) or configuration (second version) directory.
   //
   standard_version
+  package_version (const common_options&, const dir_path& pkg);
+
+  standard_version
   package_version (const common_options&,
                    const dir_path& cfg,
                    const package_name&);
diff --git a/bdep/publish.cli b/bdep/publish.cli
index 1c05cd0..cba7436 100644
--- a/bdep/publish.cli
+++ b/bdep/publish.cli
@@ -2,6 +2,8 @@
 // copyright : Copyright (c) 2014-2019 Code Synthesis Ltd
 // license   : MIT; see accompanying LICENSE file
 
+include <set>;
+
 include <bdep/project.cli>;
 
 "\section=1"
@@ -125,6 +127,13 @@ namespace bdep
        for details."
     }
 
+    std::set<string> --force
+    {
+      "<check>",
+      "Force publishing, disabling the specified check. Repeat this option to
+       disable multiple checks."
+    }
+
     string --simulate
     {
       "<outcome>",
diff --git a/bdep/publish.cxx b/bdep/publish.cxx
index 3c64706..c97c652 100644
--- a/bdep/publish.cxx
+++ b/bdep/publish.cxx
@@ -120,6 +120,27 @@ namespace bdep
     };
     vector<package> pkgs;
 
+    // If the project is under recognized VCS, it feels right to fail if it is
+    // uncommitted to decrease chances of publishing package modifications
+    // under the same version. It still make sense to allow submitting
+    // uncommitted packages if requested explicitly, for whatever reason.
+    //
+    bool git_repo (git_repository (prj));
+    optional<bool> uncommitted;           // Absent if unrecognized VCS.
+
+    if (git_repo)
+    {
+      git_repository_status st (git_status (prj));
+      uncommitted = st.unstaged || st.staged;
+
+      if (*uncommitted &&
+          o.force ().find ("uncommitted") == o.force ().end ())
+        fail << "project directory has uncommitted changes" <<
+          info << "run 'git status' for details" <<
+          info << "use 'git stash' to temporarily hide the changes" <<
+          info << "use --force=uncommitted to publish anyway";
+    }
+
     for (package_location& pl: pkg_locs)
     {
       package_name n (move (pl.name));
@@ -131,8 +152,31 @@ namespace bdep
       // For example, is it correct to consider a "between betas" snapshot a
       // beta version?
       //
-      if (v.snapshot ())
-        fail << "package " << n << " version " << v << " is a snapshot";
+      // Seems nothing wrong with submitting snapshots generally. We do this
+      // for staging most of the time. The below logic of choosing a default
+      // section requires no changes. Also, the submission service can
+      // probably have some policy of version acceptance, rejecting some
+      // version types for some sections. For example, rejecting pre-release
+      // for stable section and snapshots for any section.
+      //
+      // Note, however, that if the project is under an unrecognized VCS or
+      // the snapshot is uncommitted, then we make it non-forcible. Failed
+      // that, we might end up publishing distinct snapshots under the same
+      // temporary version. (To be more precise, we should have checked for
+      // the latest snapshot but that would require getting the original
+      // version from the package manifest, which feels too hairy for now).
+      //
+      bool non_forcible;
+      if (v.snapshot () &&
+          ((non_forcible = (!uncommitted || *uncommitted)) ||
+           o.force ().find ("snapshot") == o.force ().end ()))
+      {
+        diag_record dr (fail);
+        dr << "package " << n << " version " << v << " is a snapshot";
+
+        if (!non_forcible)
+          dr << info << "use --force=snapshot to publish anyway";
+      }
 
       // Per semver we treat zero major versions as alpha.
       //
@@ -201,11 +245,18 @@ namespace bdep
       // directly to prepare the distribution. If/when we have bpkg-pkg-dist,
       // we may want to switch to that.
       //
+      // We need to specify config.dist.uncommitted=true for a snapshot since
+      // build2's version module by default does not allow distribution of
+      // uncommitted projects.
+      //
       run_b (o,
              "dist:", (dir_path (cfg) /= p.name.string ()).representation (),
              "config.dist.root=" + dr.representation (),
              "config.dist.archives=tar.gz",
-             "config.dist.checksums=sha256");
+             "config.dist.checksums=sha256",
+             (uncommitted && *uncommitted
+              ? "config.dist.uncommitted=true"
+              : nullptr));
 
       // This is the canonical package archive name that we expect dist to
       // produce.
@@ -297,7 +348,7 @@ namespace bdep
     //
     // See if this is a VCS repository we recognize.
     //
-    if (ctrl && git_repository (prj))
+    if (ctrl && git_repo)
     {
       // Checkout the build2-control branch into a separate working tree not
       // to interfere with the user's stuff.
diff --git a/bdep/release.cli b/bdep/release.cli
index 651288c..c90ec29 100644
--- a/bdep/release.cli
+++ b/bdep/release.cli
@@ -2,6 +2,8 @@
 // copyright : Copyright (c) 2014-2019 Code Synthesis Ltd
 // license   : MIT; see accompanying LICENSE file
 
+include <set>;
+
 include <bdep/common.cli>;
 
 "\section=1"
@@ -151,6 +153,13 @@ namespace bdep
       "Open the development cycle with the next major version."
     }
 
+    std::set<string> --force
+    {
+      "<check>",
+      "Force releasing, disabling the specified check. Repeat this option to
+       disable multiple checks."
+    }
+
     bool --yes|-y
     {
       "Don't prompt for confirmation before releasing."
diff --git a/bdep/release.cxx b/bdep/release.cxx
index dfa092c..cb4cac9 100644
--- a/bdep/release.cxx
+++ b/bdep/release.cxx
@@ -66,17 +66,19 @@ namespace bdep
   static void
   plan_tag (const cmd_release_options& o, project& prj, const status& st)
   {
-    // While there is nothing wrong with having uncommitted changes while
-    // tagging, in our case we may end up with a wrong tag if the version in
-    // the (modified) manifest does not correspond to the latest commit.
+    // Note: not forcible. While there is nothing wrong with having
+    // uncommitted changes while tagging, in our case we may end up with a
+    // wrong tag if the version in the (modified) manifest does not correspond
+    // to the latest commit.
     //
     // Note that if we are called as part of releasing a new revision, then
-    // the project will have some changes staged (see plan_revision() for
+    // the project may have some changes staged (see plan_revision() for
     // details).
     //
     if ((st.staged && !o.revision ()) || st.unstaged)
       fail << "project directory has uncommitted changes" <<
-        info << "run 'git status' for details";
+        info << "run 'git status' for details" <<
+        info << "use 'git stash' to temporarily hide the changes";
 
     // All the versions are the same sans the revision. Note that our version
     // can be either current (--tag mode) or release (part of the release).
@@ -86,12 +88,26 @@ namespace bdep
                                 ? *pkg.release_version
                                 :  pkg.current_version);
 
-    if (cv.snapshot ())
-      fail << "current version " << cv << " is a snapshot";
+    // It could be desirable to tag a snapshot for some kind of a pseudo-
+    // release. For example, tag a snapshot for QA or some such. Note: can't
+    // happen as part of a release or revision.
+    //
+    if (cv.snapshot () && o.force ().find ("snapshot") == o.force ().end ())
+      fail << "current version " << cv << " is a snapshot"<<
+        info << "use --force=snapshot to tag anyway";
 
     // Canonical version tag without epoch or revision.
     //
-    prj.tag = "v" + cv.string_project ();
+    // For tagging a snapshot we need to use the actual package version
+    // (replacing .z with the actual snapshot information). Note: for
+    // snapshots we are always tagging the current version (see above).
+    //
+    const standard_version& v (
+      cv.latest_snapshot ()
+      ? package_version (o, pkg.manifest.directory ())
+      : cv);
+
+    prj.tag = "v" + v.string_project ();
 
     // Replace the existing tag only if this is a revision.
     //
@@ -104,9 +120,14 @@ namespace bdep
     // There could be changes already added to the index but otherwise the
     // repository should be clean.
     //
-    if (st.unstaged)
+    // Note: not forcible. Generally, we don't touch unstanged changes (which
+    // include untracked files).
+    //
+    if (st.unstaged && !o.no_commit ())
       fail << "project directory has unstaged changes" <<
-        info << "run 'git status' for details";
+        info << "run 'git status' for details" <<
+        info << "use 'git add' to add the changes to this commit" <<
+        info << "use 'git stash' to temporarily hide the changes";
 
     // All the versions are the same sans the revision. Note that our version
     // can be either current (--open mode) or release (part of the release).
@@ -151,10 +172,16 @@ namespace bdep
                                "" /* snapshot_id */);
     };
 
+    // Note: not forcible. The following (till the end of the function) checks
+    // prevent skipping a release. Hard to think of a use case but we probably
+    // could allow this as it doesn't break anything (released versions need
+    // not be contiguous). Let's, however, postpone this until a real use
+    // case comes up in order not to complicate things needlessly.
+    //
     if (cv.snapshot ())
     {
-      // The cv variable refers the current version as the release version
-      // cannot be a snapshot.
+      // The cv variable refers to the current version since the release
+      // version cannot be a snapshot.
       //
       assert (!pkg.release_version);
 
@@ -209,9 +236,14 @@ namespace bdep
     // There could be changes already added to the index but otherwise the
     // repository should be clean.
     //
-    if (st.unstaged)
+    // Note: not forcible. Generally, we don't touch unstanged changes (which
+    // include untracked files).
+    //
+    if (st.unstaged && !o.no_commit ())
       fail << "project directory has unstaged changes" <<
-        info << "run 'git status' for details";
+        info << "run 'git status' for details" <<
+        info << "use 'git add' to add the changes to this commit" <<
+        info << "use 'git stash' to temporarily hide the changes";
 
     // All the current versions are the same sans the revision.
     //
@@ -219,6 +251,9 @@ namespace bdep
 
     // Change the release version to the next (pre-)release.
     //
+    // Note: not forcible. The following (till the end of the function) checks
+    // prevent skipping a release. The same reasoning as in plan_open().
+    //
     standard_version rv;
     if (cv.snapshot ())
     {
@@ -319,19 +354,35 @@ namespace bdep
     // There must be changes already added to the index but otherwise the
     // repository should be clean.
     //
-    if (st.unstaged)
+    // Note: not forcible. Generally, we don't touch unstanged changes (which
+    // include untracked files).
+    //
+    if (st.unstaged && !o.no_commit ())
       fail << "project directory has unstaged changes" <<
-        info << "run 'git status' for details";
+        info << "run 'git status' for details" <<
+        info << "use 'git add' to add the changes to this commit" <<
+        info << "use 'git stash' to temporarily hide the changes";
 
-    if (!st.staged)
+    // Note: the use-case for forcing would be to create a commit to be
+    // squashed later.
+    //
+    // Also, don't complain if we are not committing.
+    //
+    if (!st.staged      &&
+        !o.no_commit () &&
+        o.force ().find ("unchanged") == o.force ().end ())
       fail << "project directory has no staged changes" <<
         info << "revision increment must be committed together with "
-           << "associated changes";
+           << "associated changes" <<
+        info << "use --force=unchanged to release anyway";
 
     // All the current versions are the same sans the revision.
     //
     const standard_version& cv (prj.packages.front ().current_version);
 
+    // Note: not forcible. Doesn't seem to make sense to release a revision
+    // for snapshot (just release another snapshot).
+    //
     if (cv.snapshot ())
       fail << "current version " << cv << " is a snapshot";
 
@@ -460,8 +511,12 @@ namespace bdep
           return true;
         }));
 
+      //
       // Validate the *-file manifest values expansion.
       //
+      // Note: not forcible. Allowing releasing broken packages we are just
+      // asking for trouble and long mailing list threads.
+      //
       m.load_files ([&f] (const string& n, const path& p)
       {
         path vf (f.directory () / p);
@@ -696,8 +751,7 @@ namespace bdep
         catch (const io_error& e)
         {
           fail << "unable to read/write " << p.manifest << ": " << e <<
-            info << "run 'git -C " << prj.path << " checkout -- ./' to revert "
-               << "any changes and try again";
+            info << "use 'git checkout' to revert any changes and try again";
         }
       }
 
@@ -782,14 +836,12 @@ namespace bdep
         {
           // If we are releasing, then the release/revision version have
           // already been written to the manifests and the changes have been
-          // committed.  Thus, the user should re-try with the --open option
+          // committed. Thus, the user should re-try with the --open option
           // in this case.
           //
           fail << "unable to read/write " << p.manifest << ": " << e <<
-            info << "run 'git -C " << prj.path << " checkout -- ./' to revert "
-               << "any changes and try again" << (pkg.release_version
-                                                  ? " with --open"
-                                                  : "");
+            info << "use 'git checkout' to revert any changes and try again"
+               << (pkg.release_version ? " with --open" : "");
         }
       }