From d6b4ed9cc7f6b27c9180627e7d1fec4d698af28c Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Thu, 17 Jan 2019 19:46:28 +0300 Subject: Add support for --force option in bdep-release and bdep-publish --- bdep/ci.cxx | 12 ++++++- bdep/project.cxx | 87 ++++++++++++++++++++++++++++------------------- bdep/project.hxx | 6 +++- bdep/publish.cli | 9 +++++ bdep/publish.cxx | 59 +++++++++++++++++++++++++++++--- bdep/release.cli | 9 +++++ bdep/release.cxx | 102 +++++++++++++++++++++++++++++++++++++++++-------------- 7 files changed, 218 insertions(+), 66 deletions(-) (limited to 'bdep') 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 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 ; + include ; "\section=1" @@ -125,6 +127,13 @@ namespace bdep for details." } + std::set --force + { + "", + "Force publishing, disabling the specified check. Repeat this option to + disable multiple checks." + } + string --simulate { "", 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 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 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 ; + include ; "\section=1" @@ -151,6 +153,13 @@ namespace bdep "Open the development cycle with the next major version." } + std::set --force + { + "", + "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" : ""); } } -- cgit v1.1