Page MenuHomePhabricator

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

Authored by martinvonz on Aug 30 2019, 2:55 AM.

Details

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.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

martinvonz created this revision.Aug 30 2019, 2:55 AM
pulkit accepted this revision.Aug 31 2019, 1:55 PM
This revision is now accepted and ready to land.Aug 31 2019, 1:55 PM
yuja added a subscriber: yuja.Aug 31 2019, 11:02 PM
  • 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.

In 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.