Closed Bug 1577826 Opened 5 months ago Closed 5 months ago

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

Categories

(Firefox Build System :: Lint and Formatting, defect)

defect
Not set

Tracking

(firefox70 fixed, firefox71 fixed)

VERIFIED FIXED
mozilla71
Tracking Status
firefox70 --- fixed
firefox71 --- fixed

People

(Reporter: mayhemer, Assigned: ahal)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

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

Type: task → defect

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

Flags: needinfo?(jaws)

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.

Flags: needinfo?(jaws)

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

Blocks: mach-busted
Flags: needinfo?(honzab.moz)
Summary: `hg amend` after modifying some js tests fails → Running eslint: "TypeError: environment can only contain strings" (`hg amend` fails after modifying some js tests)
See Also: → 1578198
Duplicate of this bug: 1578110

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.

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.

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

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

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.

(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?

Flags: needinfo?(jorgk)

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

Flags: needinfo?(jorgk)

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

Flags: needinfo?(honzab.moz)

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 :-(

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

Flags: needinfo?(jorgk)
Attachment #9089888 - Attachment is obsolete: true

My suggestion above was based on various searches:

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

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.

Flags: needinfo?(jorgk)

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.

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?

Assignee: nobody → ahal
Status: NEW → ASSIGNED

(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!

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

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

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

Pushed by ahalberstadt@mozilla.com:
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
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71

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

Status: RESOLVED → VERIFIED
Blocks: 1580533

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
Attachment #9090386 - Flags: approval-mozilla-beta?

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.

Attachment #9090386 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.