New issue
Advanced search Search tips

Issue 601216 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner: ----
Closed: Apr 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug



Sign in to add a comment

Cannot Launch Editor during 'git cl upload'

Project Member Reported by robliao@chromium.org, Apr 6 2016

Issue description

This seems to have regressed recently and I've been using an older depot_tools checkout to get around this in the meantime.

This fails with:

git cl upload
Using 50% similarity for rename/copy detection. Override with --similarity.
Running presubmit upload checks ...

Presubmit checks passed.
''C:' is not recognized as an internal or external command,
operable program or batch file.
Running editor failed

>git config core.editor
'C:/Users/robliao/AppData/Local/GitNPP/notepad++.exe' -multiInst -notabbar -nosession -noPlugin

The subprocess2.Popen arguments are passed in as a string instead of an array as far as I can tell for the editor.

Known Good: 18209cbb99caf94a1c350cb40160bf81490c2fb9
Date:   Wed Mar 30 18:03:52 2016 +0000

Bad: 4d7b7975c2fb5d235062e94d6117bf2af6d9928a
Date:   Wed Apr 6 20:28:10 2016 +0000
 
Labels: Infra-Git Infra-CodeReview
I should also note that git seems to be able to launch the editor.

The git update appears to be involved as that's when it breaks:
commit 534b85903cc93f3b9fbeff9b43747ba34fad7320
Date:   Thu Mar 31 01:36:44 2016 +0000

    Make git 2.7.4 the default version
Printf debugging at subprocess2.communicate seems to indicate this difference:

Old Git:
env 'C:/Users/robliao/AppData/Local/GitNPP/notepad++.exe' -multiInst -notabbar -nosession -noPlugin [Path to CL Description]

New Git:
'C:/Users/robliao/AppData/Local/GitNPP/notepad++.exe' -multiInst -notabbar -nosession -noPlugin [Path to CL Description]

Note the missing env.

Looks like the git update has changed some assumptions here:

def RunEditor(content, git, git_editor=None):
  """Opens up the default editor in the system to get the CL description."""
  [...]
  try:
    editor = GetEditor(git, git_editor=git_editor)
    if not editor:
      return None
    cmd = '%s %s' % (editor, filename)
    # <----------- Line of interest after this point
    if sys.platform == 'win32' and os.environ.get('TERM') == 'msys':
      # Msysgit requires the usage of 'env' to be present.
      cmd = 'env ' + cmd
    [...]
Status: WontFix (was: Untriaged)
env --> D:\depot_tools\git-2.7.4-64_bin\usr\bin\env.exe
Looks like env was doing some rudimentary quote handling.

In git 1.9.5, the quotes were needed to get the 'git commit' command to fire up an editor opened to the commit message, otherwise you'd get a mysterious '$@' doesn't exist.

In git 2.7.4, launching without quotes appears to be fixed.

Removing the quotes from my core.editor setting fixes git cl and git commit is usable on 2.7.4.

Comment 5 by aga...@chromium.org, Apr 27 2016

Components: Infra>Codereview
Labels: -Infra-Codereview

Comment 6 by aga...@chromium.org, Apr 27 2016

Labels: -Infra

Comment 7 by benhenry@google.com, Apr 27 2016

Components: Infra>Git
Labels: -Infra-Git

Sign in to add a comment