1577826 - Running eslint: "TypeError: environment can only contain strings" (`hg amend` fails after modifying some js tests)

Status: VERIFIED FIXED

(Developer Infrastructure :: Lint and Formatting, defect)

Description

[Honza Bambas (:mayhemer)]

win10, mingw, up-to-date bootstrapped m-c

$ hg amend
eslint-plugin-jest v22.15.1 should be v22.15.2.
eslint-config-prettier v6.0.0 should be v6.1.0.
babel-eslint v10.0.2 should be v10.0.3.
eslint v6.1.0 should be v6.2.2.
Installing eslint for mach using "c:\Users\<usename>\.mozbuild\node\npm.CMD install --loglevel=error"...
Traceback (most recent call last):
  File "c:\Mozilla\src\mozilla-central3\python/mozlint\mozlint\roller.py", line 140, in setup
    res = findobject(linter['setup'])(**setupargs)
  File "c:\Mozilla\src\mozilla-central3\tools\lint\eslint\__init__.py", line 47, in setup
    return setup_helper.eslint_maybe_setup()
  File "c:\Mozilla\src\mozilla-central3\tools\lint\eslint\setup_helper.py", line 66, in eslint_maybe_setup
    eslint_setup(needs_clobber)
  File "c:\Mozilla\src\mozilla-central3\tools\lint\eslint\setup_helper.py", line 76, in eslint_setup
    package_setup(get_project_root(), 'eslint', should_clobber=should_clobber)
  File "c:\Mozilla\src\mozilla-central3\tools\lint\eslint\setup_helper.py", line 146, in package_setup
    result = call_process(package_name, cmd, append_env={'PATH': os.pathsep.join(path)})
  File "c:\Mozilla\src\mozilla-central3\tools\lint\eslint\setup_helper.py", line 170, in call_process
    subprocess.check_call(cmd, cwd=cwd, stdout=fnull, env=env)
  File "c:\Mozilla\mozilla-build\python\lib\subprocess.py", line 185, in check_call
    retcode = call(*popenargs, **kwargs)
  File "c:\Mozilla\mozilla-build\python\lib\subprocess.py", line 172, in call
    return Popen(*popenargs, **kwargs).wait()
  File "c:\Mozilla\mozilla-build\python\lib\subprocess.py", line 394, in __init__
    errread, errwrite)
  File "c:\Mozilla\mozilla-build\python\lib\subprocess.py", line 644, in _execute_child
    startupinfo)
TypeError: environment can only contain strings
error: problem with lint setup, skipping eslint
Error running mach:

    ['eslint', '--fix', 'netwerk/test/unit/test_redirect-caching_failure.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:

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

  File "c:\Mozilla\src\mozilla-central3\tools/lint/mach_commands.py", line 96, in eslint
    argv=extra_args, **kwargs)
  File "c:\Mozilla\src\mozilla-central3\python/mach\mach\registrar.py", line 176, in dispatch
    return self._run_command_handler(handler, context=context, **kwargs)
  File "c:\Mozilla\src\mozilla-central3\python/mach\mach\registrar.py", line 133, in _run_command_handler
    result = fn(**kwargs)
  File "c:\Mozilla\src\mozilla-central3\tools/lint/mach_commands.py", line 77, in lint
    return cli.run(*runargs, **lintargs)
  File "c:\Mozilla\src\mozilla-central3\python/mozlint\mozlint\cli.py", line 200, in run
    result = lint.roll(paths, outgoing=outgoing, workdir=workdir)
  File "c:\Mozilla\src\mozilla-central3\python/mozlint\mozlint\roller.py", line 194, in roll
    raise LintersNotConfigured

$ which npm
/c/Mozilla/mozilla-build/node-v8.11.1-win-x64/npm
$ which eslint
/c/Mozilla/mozilla-build/node-v8.11.1-win-x64/eslint
$ eslint --version
v6.2.2

but it's using npm from ~/.mozbuild

Comment 1

[Mark Banner (:standard8)]

I think Jared might have had this issue, I don't know if he found a solution for it or not.

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Yeah, I was getting an error with "TypeError: environment can only contain strings". As far as I can tell, it happened because I was trying to update eslint from 6.2.0(?) to 6.2.2 (I missed one of the updates in between).

Trying to figure out the cause, I deleted my node_modules directory and ran hg bisect with an older m-c merge changeset and m-c tip. In doing this, I inadvertently updated through each update of eslint and the problem went away.

Comment 3

[Mark Banner (:standard8)]

Honza, does it help if you remove the node_modules directory and then re-run?

Comment 5

[Jorg K (CEST = GMT+2)]

Check bug 1578110 comment #0 for a workaround.

Comment 6

[Jorg K (CEST = GMT+2)]

If you remove the node_modules directory, the "eslint-plugin-jest v22.15.1 should be v22.15.2." changes to something along the lines of "really need to install this and that" and then it fails just the same. Anyway, analysis over in the other bug.

Comment 7

[Andrew Halberstadt [:ahal]]

Holiday today so can't debug deeper until tomorrow, but there's a good chance this was regressed by bug 1473498 (would be appreciated if someone can verify).

Though I'm not really sure how.. mach lint / mach eslint should continue to run with Python 2 and the hook uses Python 2 explicitly:
https://searchfox.org/mozilla-central/source/tools/lint/hooks.py#30

So afaict, nothing should change. If anyone can add:

import sys
print(sys.version_info)

just before the traceback, that would also help with debugging. Otherwise I'll take a look tomorrow.

Comment 8

[Jorg K (CEST = GMT+2)]

OK, I renamed node_modules so it tries to install everything from scratch. Adding the print, I see (see below). As I said over in bug 1578110 comment #0, the 'PATH' in the env containing a "u" throws it.

[some more packages deleted]
sax v1.2.4 needs to be installed locally.
eslint v6.2.2 needs to be installed locally.
eslint-plugin-fetch-options v0.0.5 needs to be installed locally.
eslint-plugin-import v2.18.2 needs to be installed locally.
prettier v1.17.0 needs to be installed locally.
sys.version_info(major=2, minor=7, micro=15, releaselevel='final', serial=0)
sys.version_info(major=2, minor=7, micro=15, releaselevel='final', serial=0)
Installing eslint for mach using "c:\mozilla-source\.mozbuild\node\npm.cmd install --loglevel=error"...
sys.version_info(major=2, minor=7, micro=15, releaselevel='final', serial=0)
Traceback (most recent call last):
  File "c:\mozilla-source\comm-central\python/mozlint\mozlint\roller.py", line 140, in setup
    res = findobject(linter['setup'])(**setupargs)
  File "c:\mozilla-source\comm-central\tools\lint\eslint\__init__.py", line 47, in setup
    return setup_helper.eslint_maybe_setup()
  File "c:\mozilla-source\comm-central\tools\lint\eslint\setup_helper.py", line 66, in eslint_maybe_setup
    eslint_setup(needs_clobber)
  File "c:\mozilla-source\comm-central\tools\lint\eslint\setup_helper.py", line 76, in eslint_setup
    package_setup(get_project_root(), 'eslint', should_clobber=should_clobber)
  File "c:\mozilla-source\comm-central\tools\lint\eslint\setup_helper.py", line 146, in package_setup
    result = call_process(package_name, cmd, append_env={'PATH': os.pathsep.join(path)})
  File "c:\mozilla-source\comm-central\tools\lint\eslint\setup_helper.py", line 170, in call_process
    subprocess.check_call(cmd, cwd=cwd, stdout=fnull, env=env)
  File "c:\mozilla-build\python\lib\subprocess.py", line 185, in check_call
    retcode = call(*popenargs, **kwargs)
  File "c:\mozilla-build\python\lib\subprocess.py", line 172, in call
    return Popen(*popenargs, **kwargs).wait()
  File "c:\mozilla-build\python\lib\subprocess.py", line 394, in __init__
    errread, errwrite)
  File "c:\mozilla-build\python\lib\subprocess.py", line 648, in _execute_child
    startupinfo)
TypeError: environment can only contain strings
error: problem with lint setup, skipping eslint

Comment 9

[Andrew Halberstadt [:ahal]]

On my phone, but reading the stack it looks like eslint might be invoking clobber which probably makes this a dupe of 1578198.

Comment 10

[Jorg K (CEST = GMT+2)]

I hope linting isn't invoking clobbering ;-) - BTW, clobbering still worked before the push mentioned in bug 1578198 comment #2. So Gijs could be right.

Comment 11

[Mark Banner (:standard8)]

(In reply to Andrew Halberstadt [:ahal] from comment #9)

On my phone, but reading the stack it looks like eslint might be invoking clobber which probably makes this a dupe of 1578198.

That would be a clobbering of the node_modules directory, rather than the main mach clobber.

From what I'm seeing via searching around, I think the possible fix would be:

Change here: https://searchfox.org/mozilla-central/rev/9415ecf29ba1acbef9381335e0ecde5916ca4073/tools/lint/eslint/setup_helper.py#146

- result = call_process(package_name, cmd, append_env={'PATH': os.pathsep.join(path)})
+ result = call_process(package_name, cmd, append_env={str('PATH'): str(os.pathsep.join(path))})

Not sure why we need it now, but worth a try maybe. Jorg, could you try it please?

Comment 12

[Jorg K (CEST = GMT+2)]

That fixes it. It creates a whole new node_modules directory.

Comment 13

[Mark Banner (:standard8)]

Attachment: Bug 1577826 - Ensure strings passed to the environment are actually strings for ESLint's setup_helper.py. r?glandium,ahal

Comment 14

[Honza Bambas (:mayhemer)]

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

Honza, does it help if you remove the node_modules directory and then re-run?

sorry for late reply. no, it doesn't. and it's not reinstalled with just 1) modify a js test file 2) run hg amend. still getting an error, looking same as from comment 0.

Comment 15

[Honza Bambas (:mayhemer)]

confirming that (151) ⚙ D44393 Bug 1577826 - Ensure strings passed to the environment are actually strings for ESLint's setup_helper.py. r?glandium,ahal fixes that.

Comment 16

[Jorg K (CEST = GMT+2)]

I got the error also in C:\mozilla-build\python3\Lib\subprocess.py when running mach clang-format.

Dumpling out the env variable, I see heaps of b's prepended: {b'!::': b'::\', b'!EXITCODE': b'00000001' ...

I guess that's another bug, but perhaps we can fix the issue globally. What about the mach clobber from bug 1578198?

EDIT: In this case the workaround was calling with None instead of env . But it's mildly annoying that various tools are broken so you need to fix them first before doing "real" work :-(

Comment 17

[Andrew Halberstadt [:ahal]]

(In reply to Jorg K (GMT+2) from comment #16)

What about the mach clobber from bug 1578198?

I believe this bug is a separate issue from the clobber one (though stem from similar places). I'm still confused about why this one started happening (but have decent understanding on the clobber one).

Jorg, you are running from comm-central right? It's possible the Python detection for commands isn't working quite right in that context. Can you verify the version of Python that is being run by adding:

import sys
print(sys.version_info)

somewhere before you see the traceback? I would expect it to be 2.7 for both eslint and clang-format. If it's saying anything else, that's the root issue and you can ignore all the other errors you see.

Comment 18

[Mark Banner (:standard8)]

My suggestion above was based on various searches:

• https://github.com/jazzband/pip-tools/issues/209
  • This suggests it is caused by the inclusion of unicode literals from the future import
  • Fixed with the str() work around: https://github.com/jazzband/pip-tools/commit/3a439045c71851151c51953b79f5994d3a12d9fe
• https://github.com/peritus/bumpversion/issues/135
• https://github.com/andrivet/ADVbumpversion/commit/bb63e2188aee08c8a110f9b9146452db5ec9c1c0
• str() is possibly the wrong fix for paths that contain non-ascii strings: https://trac.osgeo.org/grass/ticket/2532 (I only just found this).

I'll leave working out the full fix to someone with more experience in this area.

Comment 19

[Jorg K (CEST = GMT+2)]

I'm not running from comm-central. Our directory structure is mozilla-central/comm, and we run all mach commands from the M-C level. That it says comm-central in the trace in comment #8 is because, confusingly, my M-C directory is called comm-central for historical reasons, so where you see comm-central, it's really mozilla-central. The historical reason is that we were operating with comm-central/mozilla before and I had/have a separate mozilla-central tree for Firefox development.

I already dumped out the Python version for mach lint in comment #8, it was 2.7.

As for the mach clang-format:

As you can see in comment #16, is failed in C:\mozilla-build\python3\Lib\subprocess.py, so that appeared to be Python 3, no? Running it with a typo in the command gives:

$ mach clang-formaf -p comm/mailnews/compose/src/
sys.version_info(major=3, minor=6, micro=5, releaselevel='final', serial=0)
sys.version_info(major=3, minor=6, micro=5, releaselevel='final', serial=0)
sys.version_info(major=3, minor=6, micro=5, releaselevel='final', serial=0)
We're assuming the 'clang-formaf' command is 'clang-format' and we're executing it for you.

sys.version_info(major=3, minor=6, micro=5, releaselevel='final', serial=0)
Processing 48 file(s)...

However, running it without the typo gives:

$ mach clang-format -p comm/mailnews/compose/src/
Processing 48 file(s)...

So it doesn't run through the Python 3 file. It's likely that I typoed last night triggering the Python 3 path.

Comment 20

[Andrew Halberstadt [:ahal]]

Ah ok, that makes more sense. Yeah the fact that typoed commands were running with Python 3 was a known issue that is now fixed on the latest central, see bug 1577908.

Comment 21

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1577826 - [eslint] Ensure setup runs subprocess with byte strings in env

Comment 22

[Andrew Halberstadt [:ahal]]

Ok, so I still have no idea what regressed this. I'm almost tempted to think that this has always been an issue and the fact this bug was filed around the same time bug 1473498 landed is just a massive coincidence (I don't actually believe this.. just tempted to believe it).

Anyway, the attached patch should fix it. Jorg or Honza, can one of you confirm?

Comment 23

[Honza Bambas (:mayhemer)]

(In reply to Andrew Halberstadt [:ahal] from comment #22)

Ok, so I still have no idea what regressed this. I'm almost tempted to think that this has always been an issue and the fact this bug was filed around the same time bug 1473498 landed is just a massive coincidence (I don't actually believe this.. just tempted to believe it).

Anyway, the attached patch should fix it. Jorg or Honza, can one of you confirm?

Confirming fixed!

Comment 24

[Jorg K (CEST = GMT+2)]

Working for me.

Comment 25

[Mark Banner (:standard8)]

(In reply to Andrew Halberstadt [:ahal] from comment #22)

Ok, so I still have no idea what regressed this. I'm almost tempted to think that this has always been an issue and the fact this bug was filed around the same time bug 1473498 landed is just a massive coincidence (I don't actually believe this.. just tempted to believe it).

Just a thought... we haven't done a ESLint upgrade for a while (several months at least). So if people haven't been installing ESLint into new trees, then they might not have seen this.

Comment 26

[Andrew Halberstadt [:ahal]]

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

Just a thought... we haven't done a ESLint upgrade for a while (several months at least). So if people haven't been installing ESLint into new trees, then they might not have seen this.

Interesting.. add to this the fact that it's only reproducible on a subset of Windows machines (I can't reproduce on mine.. I think it has something to do with the system locale being used) and the fact we only hit this code path with Linux in CI and it's certainly possible it's taken awhile for someone to file.

Comment 27

[Honza Bambas (:mayhemer)]

My Win system locale is en-us, just in case.

Comment 28

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1577826 - [mozbuild] Create an 'ensure_subprocess_env' utility function, r?glandium

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7f09f747bec7
[mozbuild] Create an 'ensure_subprocess_env' utility function, r=glandium
https://hg.mozilla.org/integration/autoland/rev/e6bda07fa9bb
[eslint] Ensure setup runs subprocess with byte strings in env r=glandium

Comment 30

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/7f09f747bec7
https://hg.mozilla.org/mozilla-central/rev/e6bda07fa9bb

Comment 31

[Jorg K (CEST = GMT+2)]

Working when running eslint on a machine that hadn't been used for a while:

$ ../mach eslint mailnews/base/
eslint-plugin-jest v22.15.1 should be v22.15.2.
eslint-config-prettier v6.0.0 should be v6.1.0.
babel-eslint v10.0.2 should be v10.0.3.
eslint v6.1.0 should be v6.2.2.
Installing eslint for mach using "c:\mozilla-source.mozbuild\node\npm.cmd install --loglevel=error"...

eslint installed successfully!

NOTE: Your local eslint binary is at c:/mozilla-source/comm-central\node_modules.bin\eslint

Comment 32

[Mike Hommey [:glandium]]

Comment on attachment 9090386 [details]
Bug 1577826 - [mozbuild] Create an 'ensure_subprocess_env' utility function, r?glandium

Beta/Release Uplift Approval Request

• User impact if declined: Needed for bug 1580533
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Build system change, no-op in practice. Please note that I'm purposely only requesting an uplift of the piece that is needed for bug 1580533.
• String changes made/needed: N/A

Comment 33

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9090386 [details]
Bug 1577826 - [mozbuild] Create an 'ensure_subprocess_env' utility function, r?glandium

Build changes needed for bug 1580533.
OK for uplift for beta 13.

Comment 34

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d64a120a589b
https://hg.mozilla.org/releases/mozilla-beta/rev/89d5aaf64ff7