Code review comment for lp:~songofacandy/bzr/fix-523746-2

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

INADA Naoki wrote:
>> Martin [gz] wrote:
>>> Ack, was that big a follow up needed? I only intended to suggest something
>> like:
>>> - full_path = osutils.pathjoin(self._root, prefix, relpath)
>>> + full_path = osutils.pathjoin(self._root, prefix, relpath.encode(
>>> + sys.getfilesystemencoding() or 'ascii', 'replace').replace(
>>> + "?", "_"))
>> ^- This should probably be 'osutils._fs_enc' where we already handle fs
>> encoding of None.
>
> subprocess.Popen uses os.execv(), and os.execv() use Py_FileSystemDefaultEncoding (=sys.getfilesystemencoding()) to encode arguments. When filesystemencoding is NULL, fallbacks to defaultencoding.
>
> osutils._fs_enc fallbacks to 'utf-8'. So using osutils._fs_enc may cause UnicodeEncodeError on Popen.
> # I think osutils._fs_enc fallbacks to utf-8 is good because I hope bzr uses
> # utf-8 when I set LANG=C.

If you set LANG=C then we see fs_enc as ascii, not None.

The change was primarily for BSD, where fs_enc was None, and we wanted
UTF-8 as the most reasonable option.

However, if execv is going to fail if we pass something other than
ascii, then fair enough.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkuH/RkACgkQJdeBCYSNAAPZdACgo0Dy748l7zTEzTElE5Gj8tL5
ZnMAoJO+qwFcz1H92xlmlDNt2VoskBII
=W/Bm
-----END PGP SIGNATURE-----