Ask confirmation when `git llvm push` will push multiple commits

[mehdi_amini]

Summary

This can reduce unexpectedly pushing more than expected for users.

[mehdi_amini]

llvm/utils/git-svn/git-llvm

107

This does not work for me, if anyone can help?

[mehdi_amini]

Fix python 2/3 compatibility

[abrachet]

llvm/utils/git-svn/git-llvm

453

Shouldn't the message be more descriptive? Maybe it should be "Multiple commits are about to be pushed. Are you sure?" The mailing list thread was specifically for newer contributors, right? I could imagine someone using git llvm push for the first time would imagine that was a normal message that is always given.

454

Maybe "Did not proceed with pushing multiple commits" or at least "Aborting". Although I'm not sure any message is needed, probably just exiting is ok too.

[abrachet]

llvm/utils/git-svn/git-llvm

107

Are you also getting a NameError? I'm not sure why this is (I don't know much about Python), but I can say that using raw_input works as expected.

[mehdi_amini]

Change abort message to "Aborting"

llvm/utils/git-svn/git-llvm

107

It works for me in the current version with python 2.7 and 3.7 on MacOS

453

The output would be:

Pushing 2 monorepo commits:
  31a4e7393ea7 Ask confirmation when `git llvm push` will push multiple commits
  c5eb9c5e2018 dummy
Are you sure? (y/n):

[ymandel]

llvm/utils/git-svn/git-llvm

116

Maybe default to "no" (and indicate that with "(y/N)" in the query) when the user doesn't enter either one?

[probinson]

+1 for defaulting to No.
As a follow-up patch, maybe the script could offer to squash the commits for you.

[mehdi_amini]

default to No

[mehdi_amini]

@jyknight : ping?

[jyknight]

llvm/utils/git-svn/git-llvm

30

Instead of all the stuff with builtins, you can just do this:

try:
    # Python 2
    input = raw_input
except NameError:
    input = input

and then below just call input().

453

I agree, it'd be better to say:
"Are you sure you want to create %d commits?"

[mehdi_amini]

• Improve output when asking the user a question
• simplify python 2/3 compatibility

[mehdi_amini]

llvm/utils/git-svn/git-llvm

30

I used read_input instead of input for the local variable name otherwise Python complains about using a variable before defining it, PTAL.

453

Output is now:

Pushing 2 monorepo commits:
  b28f3db64111 Fix `git llvm` script when no arguments are supplied on Python 3
  aeef6550de82 Ask confirmation when `git llvm push` will push multiple commits
Are you sure you want to create 2 commits? (y/N):

[jyknight]

llvm/utils/git-svn/git-llvm

30

Yeah -- what I said would only work at the module level, because python variable binding semantics are different inside functions than outside.

llvm/utils/git-svn/git-llvm

 import re
 import shutil
 import subprocess
 import sys
 import time
 assert sys.version_info >= (2, 7)
 
 try:
+    exec("import __builtin__")  # To avoid IDE's grammar check
+except ImportError:
+    import builtins
+
+try:
     dict.iteritems
 except AttributeError:
     # Python 3
     def iteritems(d):
         return iter(d.items())
 else:
     # Python 2
     def iteritems(d):
def log_verbose(*args, **kwargs):
     print(*args, **kwargs)
 
 
 def die(msg):
     eprint(msg)
     sys.exit(1)
 
 
+def ask_confirm(prompt):
+    # Python 2/3 compatibility
+    try:
+        input = eval("__builtin__.raw_input")
+    except NameError:
+        input = builtins.input
+
+    while True:
+        query = input('%s (y/n): ' % (prompt))
+        if query == '' or not query[0].lower() in ['y','n']:
+           print('Expect y or n!')
+           continue
+        return query[0].lower() == 'y'
+
+
 def split_first_path_component(d):
     # Assuming we have a git path, it'll use slashes even on windows...I hope.
     if '/' in d:
         return d.split('/', 1)
     else:
         return (d, None)
 
 
def cmd_push(args):
     revs = get_revs_to_push(rev_range)
     log('%sPushing %d %s commit%s:\n%s' %
         ('[DryRun] ' if dry_run else '', len(revs),
          'split-repo (%s)' % split_repo_path
          if split_repo_path else 'monorepo',
          's' if len(revs) != 1 else '',
          '\n'.join('  ' + git('show', '--oneline', '--quiet', c)
                    for c in revs)))
+
+    # Ask confirmation if multiple commits are about to be pushed
+    if len(revs) != 1:
+        if not ask_confirm("Are you sure?"):
+            die("Aborting")
+
     for r in revs:
         clean_svn(svn_root)
         svn_push_one_rev(svn_root, r, git_to_svn_mapping, dry_run)
 
 
 def lookup_llvm_svn_id(git_commit_hash):
     # Use --format=%b to get the raw commit message, without any extra
     # whitespace.