[PATCH v3 16/17] CLI/git: add safety checks for checkout and commit

Subject: [PATCH v3 16/17] CLI/git: add safety checks for checkout and commit

Date: Sat, 4 Jun 2022 14:23:12 -0300

To: notmuch@notmuchmail.org

Cc:

From: David Bremner


Commits or checkouts that modify a large fraction of the messages in
the database should be relatively rare (and in some automated process,
probably non-existent). For initial setup, where such operations are
expected, the user can pass --force.
---
 doc/man1/notmuch-config.rst |  7 ++++
 doc/man1/notmuch-git.rst    | 14 ++++++--
 notmuch-git.py              | 47 ++++++++++++++++++++++--
 test/T850-git.sh            | 71 ++++++++++++++++++++++++++++++++++---
 4 files changed, 130 insertions(+), 9 deletions(-)

diff --git a/doc/man1/notmuch-config.rst b/doc/man1/notmuch-config.rst
index e2e9a632..388315f6 100644
--- a/doc/man1/notmuch-config.rst
+++ b/doc/man1/notmuch-config.rst
@@ -111,6 +111,13 @@ paths are presumed relative to `$HOME` for items in section
 
     Default location for git repository for :any:`notmuch-git`.
 
+.. nmconfig:: git.safe_fraction
+
+   Some :any:`notmuch-git` operations check that the fraction of
+   messages changed (in the database or in git, as appropriate) is not
+   too large. This item controls what fraction of total messages is
+   considered "not too large".
+
 .. nmconfig:: git.tag_prefix
 
     Default tag prefix (filter) for :any:`notmuch-git`.
diff --git a/doc/man1/notmuch-git.rst b/doc/man1/notmuch-git.rst
index ad859b80..fa7a748e 100644
--- a/doc/man1/notmuch-git.rst
+++ b/doc/man1/notmuch-git.rst
@@ -73,13 +73,18 @@ Dump a tar archive of a committed tag set using 'git archive'. See
    :manpage:`git-archive(1)`. Arguments to `git-archive` are reordered
    so that *tree-ish* comes last.
 
-.. option:: checkout
+.. option:: checkout [-f|--force]
 
 Update the notmuch database from Git.
 
 This is mainly useful to discard your changes in notmuch relative
 to Git.
 
+   .. describe:: [-f|--force]
+
+   Override checks that prevent modifying tags for large fractions of
+   messages in the database. See also :nmconfig:`git.safe_fraction`.
+
 .. option:: clone <repository>
 
 Create a local `notmuch git` repository from a remote source.
@@ -94,7 +99,7 @@ upstreams.
     section of :manpage:`git-clone(1)` for more information on
     specifying repositories.
 
-.. option:: commit [message]
+.. option:: commit [-f|--force] [message]
 
 Commit prefix-matching tags from the notmuch database to Git.
 
@@ -102,6 +107,11 @@ Commit prefix-matching tags from the notmuch database to Git.
 
    Optional text for the commit message.
 
+   .. describe:: -f|--force
+
+   Override checks that prevent modifying tags for large fractions of
+   messages in the database. See also :nmconfig:`git.safe_fraction`.
+
 .. option:: fetch [remote]
 
 Fetch changes from the remote repository.
diff --git a/notmuch-git.py b/notmuch-git.py
index 35785336..24ab3e5c 100644
--- a/notmuch-git.py
+++ b/notmuch-git.py
@@ -239,6 +239,16 @@ def _tag_query(prefix=None):
         prefix = TAG_PREFIX
     return '(tag (starts-with "{:s}"))'.format(prefix.replace('"','\\\"'))
 
+def count_messages(prefix=None):
+    "count messages with a given prefix."
+    (status, stdout, stderr) = _spawn(
+        args=['notmuch', 'count', '--query=sexp', _tag_query(prefix)],
+        stdout=_subprocess.PIPE, wait=True)
+    if status != 0:
+        _LOG.error("failed to run notmuch config")
+        sys.exit(1)
+    return int(stdout.rstrip())
+
 def get_tags(prefix=None):
     "Get a list of tags with a given prefix."
     (status, stdout, stderr) = _spawn(
@@ -357,7 +367,26 @@ class CachedIndex:
         _git(args=['read-tree', self.current_treeish], wait=True)
 
 
-def commit(treeish='HEAD', message=None):
+def check_safe_fraction(status):
+    safe = 0.1
+    conf = _notmuch_config_get ('git.safe_fraction')
+    if conf and conf != '':
+        safe=float(conf)
+
+    total = count_messages (TAG_PREFIX)
+    if total == 0:
+        _LOG.error('No existing tags with given prefix, stopping.'.format(safe))
+        _LOG.error('Use --force to override.')
+        exit(1)
+    change = len(status['added'])+len(status['deleted'])+len(status['missing'])
+    fraction = change/total
+    _LOG.debug('total messages {:d}, change: {:d}, fraction: {:f}'.format(total,change,fraction))
+    if fraction > safe:
+        _LOG.error('safe fraction {:f} exceeded, stopping.'.format(safe))
+        _LOG.error('Use --force to override or reconfigure git.safe_fraction.')
+        exit(1)
+
+def commit(treeish='HEAD', message=None, force=False):
     """
     Commit prefix-matching tags from the notmuch database to Git.
     """
@@ -368,6 +397,9 @@ def commit(treeish='HEAD', message=None):
         _LOG.warning('Nothing to commit')
         return
 
+    if not force:
+        check_safe_fraction (status)
+
     with CachedIndex(NOTMUCH_GIT_DIR, treeish) as index:
         try:
             _update_index(status=status)
@@ -445,7 +477,7 @@ def init(remote=None):
         wait=True)
 
 
-def checkout():
+def checkout(force=None):
     """
     Update the notmuch database from Git.
 
@@ -453,6 +485,10 @@ def checkout():
     to Git.
     """
     status = get_status()
+
+    if not force:
+        check_safe_fraction(status)
+
     with _spawn(
             args=['notmuch', 'tag', '--batch'], stdin=_subprocess.PIPE) as p:
         for id, tags in status['added'].items():
@@ -943,6 +979,10 @@ if __name__ == '__main__':
                 help=(
                     "Argument passed through to 'git archive'.  Set anything "
                     'before <tree-ish>, see git-archive(1) for details.'))
+        elif command == 'checkout':
+            subparser.add_argument(
+                '-f', '--force', action='store_true',
+                help='checkout a large fraction of tags.')
         elif command == 'clone':
             subparser.add_argument(
                 'repository',
@@ -951,6 +991,9 @@ if __name__ == '__main__':
                     'URLS section of git-clone(1) for more information on '
                     'specifying repositories.'))
         elif command == 'commit':
+            subparser.add_argument(
+                '-f', '--force', action='store_true',
+                help='commit a large fraction of tags.')
             subparser.add_argument(
                 'message', metavar='MESSAGE', default='', nargs='?',
                 help='Text for the commit message.')
diff --git a/test/T850-git.sh b/test/T850-git.sh
index f8537064..7ea50939 100755
--- a/test/T850-git.sh
+++ b/test/T850-git.sh
@@ -7,6 +7,9 @@ if [ $NOTMUCH_HAVE_SFSEXP -ne 1 ]; then
     test_done
 fi
 
+# be very careful using backup_database / restore_database in this
+# file, as they fool the cache invalidation checks in notmuch-git.
+
 add_email_corpus
 
 git config --global user.email notmuch@example.org
@@ -25,12 +28,54 @@ test_expect_equal "$output" "true"
 test_begin_subtest "clone"
 test_expect_success "notmuch git -p '' -C tags.git clone remote.git"
 
+test_begin_subtest "initial commit needs force"
+test_expect_code 1 "notmuch git -C tags.git commit"
+
+test_begin_subtest "committing new prefix requires force"
+notmuch git -C force-prefix.git init
+notmuch tag +new-prefix::foo id:20091117190054.GU3165@dottiness.seas.harvard.edu
+test_expect_code 1 "notmuch git -l debug -p 'new-prefix::' -C force-prefix.git commit"
+notmuch tag -new-prefix::foo id:20091117190054.GU3165@dottiness.seas.harvard.edu
+
+test_begin_subtest "committing new prefix works with force"
+notmuch tag +new-prefix::foo id:20091117190054.GU3165@dottiness.seas.harvard.edu
+notmuch git -l debug -p 'new-prefix::' -C force-prefix.git commit --force
+git -C force-prefix.git ls-tree -r --name-only HEAD | xargs dirname | sort -u | sed s,tags/,id:, > OUTPUT
+notmuch tag -new-prefix::foo id:20091117190054.GU3165@dottiness.seas.harvard.edu
+cat <<EOF>EXPECTED
+id:20091117190054.GU3165@dottiness.seas.harvard.edu
+EOF
+test_expect_equal_file_nonempty EXPECTED OUTPUT
+
+test_begin_subtest "checkout new prefix requires force"
+test_expect_code 1 "notmuch git -l debug -p 'new-prefix::' -C force-prefix.git checkout"
+
+test_begin_subtest "checkout new prefix works with force"
+notmuch dump > BEFORE
+notmuch git -l debug -p 'new-prefix::' -C force-prefix.git checkout --force
+notmuch dump --include=tags id:20091117190054.GU3165@dottiness.seas.harvard.edu | grep -v '^#' > OUTPUT
+notmuch restore < BEFORE
+cat <<EOF > EXPECTED
++inbox +new-prefix%3a%3afoo +signed +unread -- id:20091117190054.GU3165@dottiness.seas.harvard.edu
+EOF
+test_expect_equal_file_nonempty EXPECTED OUTPUT
+
 test_begin_subtest "commit"
-notmuch git -C tags.git commit
+notmuch git -C tags.git commit --force
 git -C tags.git ls-tree -r --name-only HEAD | xargs dirname | sort -u | sed s,tags/,id:, > OUTPUT
 notmuch search --output=messages '*' | sort > EXPECTED
 test_expect_equal_file_nonempty EXPECTED OUTPUT
 
+test_begin_subtest "commit --force succeeds"
+notmuch git -C force.git init
+test_expect_success "notmuch git -C force.git commit --force"
+
+test_begin_subtest "changing git.safe_fraction succeeds"
+notmuch config set git.safe_fraction 1
+notmuch git -C force2.git init
+test_expect_success "notmuch git -C force2.git commit"
+notmuch config set git.safe_fraction
+
 test_begin_subtest "commit, with quoted tag"
 notmuch git -C clone2.git clone tags.git
 git -C clone2.git ls-tree -r --name-only HEAD | grep /inbox > BEFORE
@@ -64,12 +109,12 @@ test_expect_equal_file_nonempty EXPECTED OUTPUT
 
 test_begin_subtest "commit (change prefix)"
 notmuch tag +test::one id:20091117190054.GU3165@dottiness.seas.harvard.edu
-notmuch git -C tags.git -p 'test::' commit
+notmuch git -C tags.git -p 'test::' commit --force
 git -C tags.git ls-tree -r --name-only HEAD |
     grep 20091117190054 | sort > OUTPUT
 echo "--------------------------------------------------" >> OUTPUT
 notmuch tag -test::one id:20091117190054.GU3165@dottiness.seas.harvard.edu
-notmuch git -C tags.git commit
+notmuch git -C tags.git commit --force
 git -C tags.git ls-tree -r --name-only HEAD |
     grep 20091117190054 | sort >> OUTPUT
 cat <<EOF > EXPECTED
@@ -81,10 +126,26 @@ tags/20091117190054.GU3165@dottiness.seas.harvard.edu/unread
 EOF
 test_expect_equal_file_nonempty EXPECTED OUTPUT
 
+backup_database
+test_begin_subtest "large checkout needs --force"
+notmuch tag -inbox '*'
+test_expect_code 1 "notmuch git -C tags.git checkout"
+restore_database
+
+test_begin_subtest "checkout (git.safe_fraction)"
+notmuch git -C force3.git clone tags.git
+notmuch dump > BEFORE
+notmuch tag -inbox '*'
+notmuch config set git.safe_fraction 1
+notmuch git -C force3.git checkout
+notmuch config set git.safe_fraction
+notmuch dump > AFTER
+test_expect_equal_file_nonempty BEFORE AFTER
+
 test_begin_subtest "checkout"
 notmuch dump > BEFORE
 notmuch tag -inbox '*'
-notmuch git -C tags.git checkout
+notmuch git -C tags.git checkout --force
 notmuch dump > AFTER
 test_expect_equal_file_nonempty BEFORE AFTER
 
@@ -122,7 +183,7 @@ test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "fetch"
 notmuch tag +test2 id:20091117190054.GU3165@dottiness.seas.harvard.edu
-notmuch git -C remote.git commit
+notmuch git -C remote.git commit --force
 notmuch tag -test2 id:20091117190054.GU3165@dottiness.seas.harvard.edu
 notmuch git -C tags.git fetch
 notmuch git -C tags.git status > OUTPUT
-- 
2.35.2

_______________________________________________
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-leave@notmuchmail.org

Thread: