chiark / gitweb /
git-debpush: Support forcing individual checks
authorSean Whitton <spwhitton@spwhitton.name>
Fri, 19 Jul 2019 16:11:22 +0000 (17:11 +0100)
committerIan Jackson <ijackson@chiark.greenend.org.uk>
Sat, 20 Jul 2019 16:12:05 +0000 (17:12 +0100)
While using git-debpush interactively, if the user sees that checks
have failed which they know to be safe to override, they typically
just use --force or -f.

However, it is useful for scripts to be able to skip single checks,
and if the user knows that a check will fail in advance of running
git-debpush, it is convenient to be able to specify that but still see
whether other checks fail, where those failures were unexpected.  This
can avoid the user having to run git-debpush more than once.

The list of any checks which failed but were overridden is not stored
in the generated tag.  git-debpush's checks are for the convenience of
the local user only, and the list of failed but overridden checks is
not considered to be metadata for the upload.  Not recording failed
but overridden checks in the git tag keeps git-debpush's checking
independent of its wrapping of git-tag and git-push, which makes
git-debpush simpler and easier to understand.  The complexity is on
the server side, in tag2upload.

git-debpush(1): We want the list of checks that can be overridden to
be at the bottom of the list of options because most users will not
need to look at it.  We also want the description of --force|-f to be
adjacent to the description of --force=<check>.  So move the
description of --force|-f to the end of the list of options.

Signed-off-by: Sean Whitton <spwhitton@spwhitton.name>
Closes: #932459
git-debpush
git-debpush.1.pod
tests/tests/tagupl

index e2ea2b64adf939a03686db14518ba2a11f12029d..dee358b2c43370a8fcca42b76abe85749f457642 100755 (executable)
@@ -65,10 +65,16 @@ get_file_from_ref () {
 
 failed_check=false
 fail_check () {
-    if $force; then
-        echo >&2 "$us: warning: $*"
+    local check=$1; shift
+    local check_is_forced=false
+
+    case ",$force," in
+        *",$check,"*) check_is_forced=true ;;
+    esac
+    if $force_all || $check_is_forced; then
+        echo >&2 "$us: warning: $* ('$check' check)"
     else
-        echo >&2 "$us: $*"
+        echo >&2 "$us: $* ('$check' check)"
         failed_check=true
     fi
 }
@@ -89,7 +95,7 @@ find_last_tag () {
 # **** Parse command line ****
 
 getopt=$(getopt -s bash -o 'nfu:' \
-              -l 'no-push,force,branch:,remote:,distro:,upstream:,quilt:,gbp,dpm,\
+              -l 'no-push,force::,branch:,remote:,distro:,upstream:,quilt:,gbp,dpm,\
 baredebian,baredebian+git,baredebian+tarball' \
               -n "$us" -- "$@")
 eval "set - $getopt"
@@ -97,7 +103,8 @@ set -e$DGIT_TEST_DEBPUSH_DEBUG
 
 git_tag_opts=()
 pushing=true
-force=false
+force_all=false
+force=""
 distro=debian
 quilt_mode=""
 branch="HEAD"
@@ -106,7 +113,7 @@ while true; do
     case "$1" in
         '-n'|'--no-push') pushing=false;           shift;   continue ;;
        '-u')             git_tag_opts+=(-u "$2"); shift 2; continue ;;
-        '-f'|'--force')   force=true;              shift;   continue ;;
+        '-f')             force_all=true;          shift;   continue ;;
         '--gbp')          quilt_mode='gbp';        shift;   continue ;;
         '--dpm')          quilt_mode='dpm';        shift;   continue ;;
         '--branch')       branch=$2;               shift 2; continue ;;
@@ -121,6 +128,18 @@ while true; do
             fail "--baredebian+tarball quilt mode not supported"
             ;;
 
+        # we require the long form of the option to skip individual
+        # checks, not permitting `-f check`, to avoid problems if we
+        # later want to introduce positional args
+        '--force')
+            case "$2" in
+                '')
+                    force_all=true                         ;;
+                *)
+                    force="$force,$2"                      ;;
+            esac
+            shift 2; continue ;;
+
         '--') shift; break ;;
        *) badusage "unknown option $1" ;;
     esac
@@ -252,7 +271,7 @@ fi
 # ---- UNRELEASED suite
 
 if [ "$target" = "UNRELEASED" ]; then
-    fail_check "UNRELEASED changelog"
+    fail_check unreleased "UNRELEASED changelog"
 fi
 
 # ---- Pushing dgit view to maintainer view
@@ -263,7 +282,7 @@ if ! [ "x$last_debian_tag" = "x" ] && ! [ "x$last_archive_tag" = "x" ]; then
     if ! [ "$last_debian_tag_c" = "$last_archive_tag_c" ] \
             && git merge-base --is-ancestor \
                    "$last_debian_tag" "$last_archive_tag"; then
-        fail_check \
+        fail_check dgit-view \
 "looks like you might be trying to push the dgit view to the maintainer branch?"
     fi
 fi
@@ -280,7 +299,7 @@ if ! [ "x$last_debian_tag" = "x" ]; then
     trap - EXIT
 
     if ! [ "$prev_target" = "$target" ] && ! [ "$target" = "UNRELEASED" ]; then
-        fail_check \
+        fail_check suite \
 "last upload targeted $prev_target, now targeting $target; might be a mistake?"
     fi
 fi
@@ -290,14 +309,19 @@ fi
 if ! [ "x$upstream_tag" = "x" ] \
         && ! git merge-base --is-ancestor "$upstream_tag" "$branch" \
         && ! [ "$quilt_mode" = "baredebian" ]; then
-    fail_check \
+    fail_check upstream-nonancestor \
  "upstream tag $upstream_tag is not an ancestor of $branch; probably a mistake"
 fi
 
 # ---- Summary
 
-if ! $force && $failed_check; then
-    fail "some checks failed; you can override with --force"
+if $failed_check; then
+    # We don't mention the --force=check options here as those are
+    # mainly for use by scripts, or when you already know what check
+    # is going to fail before you invoke git-debpush.  Keep the
+    # script's terminal output as simple as possible.  No "see the
+    # manpage"!
+    fail "some check(s) failed; you can pass --force to ignore them"
 fi
 
 # **** Create the git tag ****
index 242bf677ffdadfefdf022dc33278f2cb0a946526..429148d40181023c23d04e8099763ac6bdb6e7e9 100644 (file)
@@ -146,10 +146,6 @@ upload your package.
 
 Just tag, don't push.
 
-=item B<--force>|B<-f>
-
-Ignore the results of all checks designed to prevent broken uploads.
-
 =item B<-u> I<keyid>
 
 Passed on to git-tag(1).
@@ -182,6 +178,40 @@ git would use if you typed "git push BRANCH".
 What distribution name to embed in the signed tag.  Defaults to
 "debian".
 
+=item B<--force>|B<-f>
+
+Ignore the results of all checks designed to prevent broken uploads.
+
+=item B<--force>=I<check>[,I<check>] ...
+
+Override individual checks designed to prevent broken uploads.  May be
+specified more than once.  Valid values for I<check> are:
+
+=over 4
+
+=item B<suite>
+
+Permit uploading to a different suite than the target of the most
+recent upload made with B<git-debpush> (e.g. when uploading to
+Debian unstable after uploading to Debian experimental).
+
+=item B<upstream-nonancestor>
+
+Ignore the fact that the upstream tag is not an ancestor of the branch
+to be tagged (skipping this check is implied by B<--quilt=baredebian>).
+
+=item B<unreleased>
+
+Permit upload to a suite called UNRELEASED.
+
+=item B<dgit-view>
+
+Ignore apparently pushing the dgit view of a package (as produced by
+B<dgit clone>) to the maintainer branch, where the dgit view and the
+maintainer view of the package are not identical.
+
+=back
+
 =back
 
 =head1 SEE ALSO
index e11ea5af7f463b0e2f753571a6c4404932e0e6c5..09ca53a8c542e36576ac74cd93f9488b7ff05373 100755 (executable)
@@ -29,7 +29,13 @@ tagname=test-dummy/$v
 
 t-expect-fail "upstream tag $upstreamtag is not an ancestor of refs/heads/master" \
 t-tagupl-test --quilt=gbp --upstream=$upstreamtag
-t-tagupl-test --quilt=gbp --force --upstream=$upstreamtag
+
+t-expect-fail "upstream tag $upstreamtag is not an ancestor of refs/heads/master" \
+t-tagupl-test --quilt=gbp --force=suite --force=no-such-force-option --upstream=$upstreamtag
+
+t-tagupl-test --quilt=gbp --force=suite --force=no-such-force-option-1 \
+              --force=upstream-nonancestor,no-such-force-option-2 \
+              --upstream=$upstreamtag
 t-pushed-good master
 
 # todo: test each miss/rejection