Patchwork D6775: py3: convert hg executable path to bytes in missing case in procutil

login
register
mail settings
Submitter phabricator
Date Aug. 30, 2019, 6:55 a.m.
Message ID <differential-rev-PHID-DREV-udkgsk3ulavkofvujlfb-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/41430/
State Superseded
Headers show

Comments

phabricator - Aug. 30, 2019, 6:55 a.m.
martinvonz created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  We (Google) noticed this in our tests when we use chg and a hg wrapper
  script not called 'hg'. The executable then ended up being a native
  string, which then failed in chgserver when trying to convert the
  environment dict to a byte string.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6775

AFFECTED FILES
  mercurial/utils/procutil.py

CHANGE DETAILS




To: martinvonz, #hg-reviewers
Cc: mercurial-devel
Yuya Nishihara - Sept. 1, 2019, 2:53 a.m.
> --- a/mercurial/utils/procutil.py
> +++ b/mercurial/utils/procutil.py
> @@ -246,7 +246,7 @@
>              _sethgexecutable(pycompat.fsencode(mainmod.__file__))
>          else:
>              exe = findexe('hg') or os.path.basename(sys.argv[0])
> -            _sethgexecutable(exe)
> +            _sethgexecutable(pycompat.fsencode(exe))

Perhaps, `pycompat.sysargv` has to be used instead. Applying `fsencode()`
on `sys.argv` might be incorrect on Windows.
phabricator - Sept. 1, 2019, 3:02 a.m.
yuja added a comment.


  > - a/mercurial/utils/procutil.py
  >
  > +++ b/mercurial/utils/procutil.py
  > @@ -246,7 +246,7 @@
  >
  >       _sethgexecutable(pycompat.fsencode(mainmod.__file__))
  >   else:
  >       exe = findexe('hg') or os.path.basename(sys.argv[0])
  >
  > - _sethgexecutable(exe)
  >
  > +            _sethgexecutable(pycompat.fsencode(exe))
  
  Perhaps, `pycompat.sysargv` has to be used instead. Applying `fsencode()`
  on `sys.argv` might be incorrect on Windows.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6775/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D6775

To: martinvonz, #hg-reviewers, pulkit
Cc: yuja, mercurial-devel
phabricator - Sept. 2, 2019, 6:44 a.m.
martinvonz added a comment.


  In D6775#99509 <https://phab.mercurial-scm.org/D6775#99509>, @yuja wrote:
  
  >> - a/mercurial/utils/procutil.py
  >>
  >> +++ b/mercurial/utils/procutil.py
  >> @@ -246,7 +246,7 @@
  >>
  >>       _sethgexecutable(pycompat.fsencode(mainmod.__file__))
  >>   else:
  >>       exe = findexe('hg') or os.path.basename(sys.argv[0])
  >>
  >> - _sethgexecutable(exe)
  >>
  >> +            _sethgexecutable(pycompat.fsencode(exe))
  >
  > Perhaps, `pycompat.sysargv` has to be used instead. Applying `fsencode()`
  > on `sys.argv` might be incorrect on Windows.
  
  Sounds reasonable. I'll send a follow-up patch. Feel free to apply it on top of fold it in.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6775/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D6775

To: martinvonz, #hg-reviewers, pulkit
Cc: yuja, mercurial-devel

Patch

diff --git a/mercurial/utils/procutil.py b/mercurial/utils/procutil.py
--- a/mercurial/utils/procutil.py
+++ b/mercurial/utils/procutil.py
@@ -246,7 +246,7 @@ 
             _sethgexecutable(pycompat.fsencode(mainmod.__file__))
         else:
             exe = findexe('hg') or os.path.basename(sys.argv[0])
-            _sethgexecutable(exe)
+            _sethgexecutable(pycompat.fsencode(exe))
     return _hgexecutable
 
 def _sethgexecutable(path):