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 ↗

(On Diff #210443)

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 ↗

(On Diff #210446)

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 ↗

(On Diff #210446)

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 ↗

(On Diff #210443)

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

453 ↗

(On Diff #210446)

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):

107 ↗

(On Diff #210443)

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

[ymandel]

llvm/utils/git-svn/git-llvm

116 ↗

(On Diff #210481)

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 ↗

(On Diff #210646)

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 ↗

(On Diff #210446)

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 ↗

(On Diff #210646)

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

453 ↗

(On Diff #210446)

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 ↗

(On Diff #210646)

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

llvm/trunk/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.lower() not in ['y','n', '']:
+           print('Expect y or n!')
+           continue
+        return query.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.