1411994 - UnicodeDecodeError: 'charmap' codec can't decode byte 0x81 in position 62: character maps to <undefined>

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Richard Marti (:Paenglab)]

Attachment: error.log

I installed VS 2017 with the settings from wiki after I uninstalled VS 2015. When I build I get the error in the attached log.

Ted thinks, that that code wasn't prepared to handle the non-ascii output from my compiler.

python -c 'import locale; print locale.getpreferredencoding()' gives cp1252.

I'm using mozilla-build v3.0

Comment 1

[Richard Marti (:Paenglab)]

Switched VS to English and now it's building. So it is a problem when it is set to German.

Comment 2

[Gregory Szorc [:gps]]

I think I can code up a fix real quick...

Comment 3

[Gregory Szorc [:gps]]

Attachment: Bug 1411994 - Ignore Unicode errors when logging;

LineIO currently attempts to convert bytes to unicode using the
system preferred encoding. This can obviously fail for some byte
sequences.

In many cases, the conversion is happening for purposes of logging.
In these cases, Unicode validation shouldn't be that important to
us.

So, this commit teaches LineIO to accept an argument defining its
Unicode error mode. We also teach all users of LineIO doing logging
to switch the default mode from "strict" to "replace" so invalid
Unicode byte sequences can be logged with the Unicode replacement
character.

FWIW, I /think/ all instances of LineIO are used for logging. We
/might/ be able to change the default error mode to "replace."
However, I don't like the idea of introducing subtle data loss and
think "strict" is the saner default. But if LineIO is only supposed
to be used for logging and we don't care about byte-for-byte
preservation, I'd support documenting that in its docstring and
changing the default to "replace."

Review commit: https://reviewboard.mozilla.org/r/193624/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/193624/

Comment 4

[Mike Hommey [:glandium]]

Comment on attachment 8922549 [details]
Bug 1411994 - Ignore Unicode errors when logging;

https://reviewboard.mozilla.org/r/193624/#review199876

One one end, this solves the bug as filed, on the other end, I'm thinking that LineIO is not necessarily the only way we can get such errors, and that the encoding that happens in LineIO should probably moved one layer down, to the log.* functions.

::: build/moz.configure/init.configure:190
(Diff revision 1)
>          elif 'PYTHON' in mozconfig['vars']['added']:
>              python = mozconfig['vars']['added']['PYTHON']
>          elif 'PYTHON' in mozconfig['vars']['modified']:
>              python = mozconfig['vars']['modified']['PYTHON'][1]
>  
> -    with LineIO(lambda l: log.error(l)) as out:
> +    with LineIO(lambda l: log.error(l), 'replace') as out:

verify_python_version is not going to write more than valid ascii, you can skip replace here.

::: build/moz.configure/rust.configure:193
(Diff revision 1)
>          out_fd, out_path = mkstemp(prefix='conftest', suffix='.rlib')
>          os.close(out_fd)
>          try:
>              source = 'pub extern fn hello() { println!("Hello world"); }'
>              log.debug('Creating `%s` with content:', in_path)
> -            with LineIO(lambda l: log.debug('| %s', l)) as out:
> +            with LineIO(lambda l: log.debug('| %s', l), 'replace') as out:

the written string is a controlled, unicode string. No need for replace.

::: build/moz.configure/util.configure:221
(Diff revision 1)
>      fd, path = mkstemp(prefix='conftest.', suffix=suffix)
>      try:
>          source = source.encode('ascii', 'replace')
>  
>          log.debug('Creating `%s` with content:', path)
> -        with LineIO(lambda l: log.debug('| %s', l)) as out:
> +        with LineIO(lambda l: log.debug('| %s', l), 'replace') as out:

see 2 lines above, the written buffer is the result of an encoding to ascii, so you don't need to further replace back.

Comment 5

[Gregory Szorc [:gps]]

Comment on attachment 8922549 [details]
Bug 1411994 - Ignore Unicode errors when logging;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/193624/diff/1-2/

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6f9a71b5312b
Ignore Unicode errors when logging; r=glandium

Comment 7

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/6f9a71b5312b