1585565 - [Windows] Linter broken - UnicodeDecodeError: 'charmap' codec can't decode byte 0x90 in position 959: character maps to <undefined> / ValueError: embedded null character / AttributeError: 'Process' object has no attribute '_job'

Status: RESOLVED FIXED

(Developer Infrastructure :: Lint and Formatting, defect)

Description

[:Gijs (he/him)]

On https://hg.mozilla.org/mozilla-central/rev/345d89faf40e (centralfrom yesterday), invoking ./mach eslint path/to/file, or making any commit which runs the linter hook gives me:

$ ./mach eslint toolkit/components/printing/content/printPreviewToolbar.js
Traceback (most recent call last):
  File "path\to\mozilla-central\python/mozlint\mozlint\roller.py", line 184, in setup
    res = findobject(linter['setup'])(**setupargs)
  File "path\to\mozilla-central\tools\lint\eslint\__init__.py", line 47, in setup
    return setup_helper.eslint_maybe_setup()
  File "path\to\mozilla-central\tools\lint\eslint\setup_helper.py", line 62, in eslint_maybe_setup
    has_issues, needs_clobber = eslint_module_needs_setup()
  File "path\to\mozilla-central\tools\lint\eslint\setup_helper.py", line 250, in eslint_module_needs_setup
    data = json.load(open(path))
  File "path\to\mozilla-build\python3\lib\json\__init__.py", line 296, in load
    return loads(fp.read(),
  File "path\to\mozilla-build\python3\lib\encodings\cp1252.py", line 23, in decode
    return codecs.charmap_decode(input,self.errors,decoding_table)[0]
UnicodeDecodeError: 'charmap' codec can't decode byte 0x90 in position 959: character maps to <undefined>
error: problem with lint setup, skipping eslint
Error running mach:

    ['eslint', 'toolkit/components/printing/content/printPreviewToolbar.js']

The error occurred in code that was called by the mach command. This is either
a bug in the called code itself or in the way that mach is calling it.
You can invoke |./mach busted| to check if this issue is already on file. If it
isn't, please use |./mach busted file| to report it. If |./mach busted| is
misbehaving, you can also inspect the dependencies of bug 1543241.

If filing a bug, please include the full output of mach, including this error
message.

The details of the failure are as follows:

mozlint.errors.LintersNotConfigured: No linters registered! Use `LintRoller.read` to register a linter.

  File "path\to\mozilla-central\tools/lint/mach_commands.py", line 99, in eslint
    argv=extra_args, **kwargs)
  File "path\to\mozilla-central\python/mach\mach\registrar.py", line 176, in dispatch
    return self._run_command_handler(handler, context=context, **kwargs)
  File "path\to\mozilla-central\python/mach\mach\registrar.py", line 133, in _run_command_handler
    result = fn(**kwargs)
  File "path\to\mozilla-central\tools/lint/mach_commands.py", line 80, in lint
    return cli.run(*runargs, **lintargs)
  File "path\to\mozilla-central\python/mozlint\mozlint\cli.py", line 204, in run
    result = lint.roll(paths, outgoing=outgoing, workdir=workdir)
  File "path\to\mozilla-central\python/mozlint\mozlint\roller.py", line 238, in roll
    raise LintersNotConfigured

Comment 1

[:Gijs (he/him)]

hg bisect says this regressed with bug 1580280. How can I get more info to help debug what's going on here / how to fix it?

Comment 2

[:Gijs (he/him)]

(specifically, commit https://hg.mozilla.org/integration/autoland/rev/fbc81f842739 )

Comment 3

[:Gijs (he/him)]

This seems to be caused by yarn's package.json file which helpfully has some emoji in it: https://github.com/yarnpkg/yarn/blob/1.17-stable/package.json#L7

Comment 4

[:Gijs (he/him)]

Fixing the open call at https://searchfox.org/mozilla-central/rev/f372e8a46ef7659ef61be9938ec2a3ea34d343c6/tools/lint/eslint/setup_helper.py#250 to include encoding="utf-8" produces:

Traceback (most recent call last):
  File "path\to\mozilla-central\python/mozlint\mozlint\roller.py", line 55, in _run_worker
    res = func(paths, config, **lintargs) or []
  File "path\to\mozilla-central\python/mozlint\mozlint\types.py", line 54, in __call__
    return self._lint(paths, config, **lintargs)
  File "path\to\mozilla-central\python/mozlint\mozlint\types.py", line 139, in _lint
    return func(files, config, **lintargs)
  File "path\to\mozilla-central\tools\lint\eslint\__init__.py", line 93, in lint
    proc.run()
  File "path\to\mozilla-central\testing/mozbase/mozprocess\mozprocess\processhandler.py", line 811, in run
    self.proc = self.Process([self.cmd] + self.args, **args)
  File "path\to\mozilla-central\testing/mozbase/mozprocess\mozprocess\processhandler.py", line 123, in __init__
    universal_newlines, startupinfo, creationflags)
  File "path\to\mozilla-build\python3\lib\subprocess.py", line 709, in __init__
    restore_signals, start_new_session)
  File "path\to\mozilla-central\testing/mozbase/mozprocess\mozprocess\processhandler.py", line 309, in _execute_child
    winprocess.EnvironmentBlock(env),
  File "path\to\mozilla-central\testing/mozbase/mozprocess\mozprocess\winprocess.py", line 167, in __init__
    self._as_parameter_ = LPCWSTR("\0".join(values))
ValueError: embedded null character
Exception ignored in: <bound method ProcessHandlerMixin.Process.__del__ of <mozprocess.processhandler.ProcessHandlerMixin.Process object at 0x0000026264AABC50>>
Traceback (most recent call last):
  File "path\to\mozilla-central\testing/mozbase/mozprocess\mozprocess\processhandler.py", line 143, in __del__
    if handle or self._job or self._io_port:
AttributeError: 'Process' object has no attribute '_job'

which is not really an improvement.

Comment 5

[Mark Banner (:standard8)]

I think this line probably wants to refer to python3 rather than python2.7: https://searchfox.org/mozilla-central/rev/f372e8a46ef7659ef61be9938ec2a3ea34d343c6/tools/lint/hooks.py#30

I guess there's probably not a better way to deal with that?

Comment 6

[:Gijs (he/him)]

(In reply to Mark Banner (:standard8) from comment #5)

I think this line probably wants to refer to python3 rather than python2.7: https://searchfox.org/mozilla-central/rev/f372e8a46ef7659ef61be9938ec2a3ea34d343c6/tools/lint/hooks.py#30

I guess there's probably not a better way to deal with that?

mach eslint is also broken, so I don't think that's the problem, though it probably doesn't help.

Comment 7

[:Gijs (he/him)]

Attachment: Bug 1585565 - fix package.json loading in eslint setup to always use utf-8 instead of system encoding, r?ahal

Comment 8

[Mark Banner (:standard8)]

Oh ./mach eslint is working fine for me on Mac, it is just the hook. So maybe both are needed?

Comment 9

[:Gijs (he/him)]

Attachment: Bug 1585565 - clarify we want python3 in the hook, r?ahal

Comment 10

[:Gijs (he/him)]

I mean, I've put up 2 patches but things are still completely non-functional for me. The mozbase problem (comment #4) is something where (a) I'm not really sure how to fix it - just using byte string for "\00" doesn't help because the values we're joining are unicode strings - we just decoded them from the filesystem encoding! But also, (b) we probably would need a mozbase release or something and I have no idea how any of that works.

I would really like future python3 conversions to be better tested on Windows, because it's the primary platform for our users so it's the primary platform on which frontend folks need to work / test stuff, and this is the second time in a month that critical tools break as a result of the python3 conversion. I know this conversion is important and I'm happy it's happening, but having things break like this all the time is really frustrating.

Comment 11

[Andrew Halberstadt [:ahal]]

The hook issue is bug 1585385.. there's a patch just need someone to r+ it (feel free to steal).

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/493072ebb42b
fix package.json loading in eslint setup to always use utf-8 instead of system encoding, r=ahal

Comment 13

[Andrew Halberstadt [:ahal]]

(In reply to :Gijs (he/him) from comment #10)

I mean, I've put up 2 patches but things are still completely non-functional for me. The mozbase problem (comment #4) is something where (a) I'm not really sure how to fix it - just using byte string for "\00" doesn't help because the values we're joining are unicode strings - we just decoded them from the filesystem encoding! But also, (b) we probably would need a mozbase release or something and I have no idea how any of that works.

I'm investigating. No need for releases, it's all in-tree.

I would really like future python3 conversions to be better tested on Windows, because it's the primary platform for our users so it's the primary platform on which frontend folks need to work / test stuff, and this is the second time in a month that critical tools break as a result of the python3 conversion. I know this conversion is important and I'm happy it's happening, but having things break like this all the time is really frustrating.

Duly noted. In this case I did run a few ./mach lint invocations on Windows, but not nearly enough to cover all the various linters / code paths. Furthermore, turns out the eslint tests are disabled on Windows:
https://searchfox.org/mozilla-central/source/tools/lint/test/python.ini#6

facepalm

Comment 14

[Andrew Halberstadt [:ahal]]

I think the ValueError: embedded null character might be a regression in Python:
https://bugs.python.org/issue32745

Trying to come up with a way to avoid hitting it but my clone somehow got in a bad state and hg is taking forever to recover, so haven't been able to attempt reproduction yet.

Comment 15

[Andrew Halberstadt [:ahal]]

Ok, I have a workaround for the eslint case in this bug. But there's a bigger issue here around mozprocess in general. Will get patches and bugs up shortly.

Comment 16

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1585565 - [lint.eslint] Work around mozprocess Windows + Python 3 bug, r?Gijs,Standard8

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/50f95869c616
[lint.eslint] Work around mozprocess Windows + Python 3 bug, r=Gijs

Comment 18

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/493072ebb42b
https://hg.mozilla.org/mozilla-central/rev/50f95869c616

Comment 19

[:Gijs (he/him)]

Oops. Thanks Andrew, I owe you drinks!