Closed Bug 1439727 Opened 2 years ago Closed 2 years ago

After upgrade to psutil 5.4.3 certain mach commands show: AttributeError: 'module' object has no attribute 'ERROR_INVALID_NAME'

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox-esr52 unaffected, firefox58 unaffected, firefox59 unaffected, firefox60 fixed)

RESOLVED FIXED
mozilla60
Tracking Status
firefox-esr52 --- unaffected
firefox58 --- unaffected
firefox59 --- unaffected
firefox60 --- fixed

People

(Reporter: jaws, Assigned: gps)

References

Details

(Keywords: regression, Whiteboard: [workaround in comment 17])

Attachments

(4 files)

Seen on Windows 10 64-bit at changeset 861067332bac.

Jared Wein@LAPTOP-BRR0AH2E /c/fx
$ mach mochitest toolkit/components/payments/test/browser/browser_host_name.js
Error running mach:

    ['mochitest', 'toolkit/components/payments/test/browser/browser_host_name.js']

The error occurred in mach itself. This is likely a bug in mach itself or a
fundamental problem with a loaded module.

Please consider filing a bug against mach by going to the URL:

    https://bugzilla.mozilla.org/enter_bug.cgi?product=Core&component=mach


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

The details of the failure are as follows:

AttributeError: 'module' object has no attribute 'ERROR_INVALID_NAME'

  File "c:\fx\python/mach\mach\main.py", line 359, in run
    return self._run(argv)
  File "c:\fx\python/mach\mach\main.py", line 414, in _run
    args = parser.parse_args(argv)
  File "c:\mozilla-build\python\lib\argparse.py", line 1701, in parse_args
    args, argv = self.parse_known_args(args, namespace)
  File "c:\mozilla-build\python\lib\argparse.py", line 1733, in parse_known_args
    namespace, args = self._parse_known_args(args, namespace)
  File "c:\mozilla-build\python\lib\argparse.py", line 1942, in _parse_known_args
    stop_index = consume_positionals(start_index)
  File "c:\mozilla-build\python\lib\argparse.py", line 1898, in consume_positionals
    take_action(action, args)
  File "c:\mozilla-build\python\lib\argparse.py", line 1807, in take_action
    action(self, namespace, argument_values, option_string)
  File "c:\fx\python/mach\mach\dispatcher.py", line 172, in __call__
    if handler.parser:
  File "c:\fx\python/mach\mach\decorators.py", line 76, in parser
    self._parser = self._parser()
  File "c:\fx\testing/mochitest/mach_commands.py", line 226, in setup_argument_parser
    ('.py', 'r', imp.PY_SOURCE))
  File "c:/fx/objdir-frontend\_tests\testing\mochitest\runtests.py", line 80, in <module>
    import psutil
  File "c:\fx\build/mach_bootstrap.py", line 363, in __call__
    module = self._original_import(name, globals, locals, fromlist, level)
  File "c:\fx\third_party/python/psutil\psutil\__init__.py", line 146, in <module>
    from . import _pswindows as _psplatform
  File "c:\fx\build/mach_bootstrap.py", line 363, in __call__
    module = self._original_import(name, globals, locals, fromlist, level)
  File "c:\fx\third_party/python/psutil\psutil\_pswindows.py", line 84, in <module>
    NO_SUCH_SERVICE_ERRSET = frozenset([cext.ERROR_INVALID_NAME,

Jared Wein@LAPTOP-BRR0AH2E /c/fx
$ hg wip
@  404547:5fd1a590d684 jwein tip Bug 1427947 - Dispatch `shippingoptionchange` when the shipping option is changed. r?mao    404546:861067332bac ffxbld central No bug, Automated blocklist update from host bld-linux64-spot-302 - a=blocklist-:\
: \
Blocks: 1436857
Flags: needinfo?(hskupin)
Keywords: regression
Verified regression due to bug 1436857 via local backout.
So with my patch we no longer have the compiled C extension as part of the tree. Nothing in automation was dependent on it, or maybe functionality is hiding silently? When I check the following line in runtests.py for mochitests, I wonder if we should check for more than ImportError, or really have to include the compiled C extension again.

I think without the compiled extension we might not be able to safely kill a process tree anymore, and fallback to `os.kill()`:
https://dxr.mozilla.org/mozilla-central/rev/994a684a7564c2735d98d6910a78d079a68f0b25/testing/mochitest/runtests.py#328

Greg, what do you think? Do we have to include the compiled c extension, or should that maybe be a part of mozilla-build?
Flags: needinfo?(hskupin) → needinfo?(gps)
Hm, I have an old Mozilla-Build release installed on my machine (version 2.2.0) and there the compiled extension is contained.
I can verify that starting with Mozilla-Build 3.0 we no longer ship the psutil Python package incl the compiled c-extension anymore. Ryan is that on purpose or just an accident by switching to the 64bit version of Python?
Flags: needinfo?(ryanvm)
Did we ship the compiled psutil in MozillaBuild? If so, I completely forgot about that!

Anyway, psutil is supposed to be an optional package for developers - we can make it required in CI if we want. Code in testing/mochitest/runtests.py already attempts to handle import failures. But it looks like an additional exception is occurring and we're not catching that. We may need to fix this throughout the repo :/

Alternatively, we could make psutil mandatory - possibly on some platforms. This is a bridge I've been reluctant to cross because supporting compiled Python C extensions is a can of worms. I think the easiest solution is to vendor .whl files. I'm not a fan of vendoring binaries. But it probably is the simplest solution. It will almost certainly require changes to VirtualenvManager to support installing from wheels.
Flags: needinfo?(gps)
Could be fallout from switching to 64bit. Definitely wasn't an intentional decision made along the way.
Flags: needinfo?(ryanvm)
I have mozilla build version 3.1.1 installed. I'm using artifact builds, I bet that's related.
(In reply to Ryan VanderMeulen [:RyanVM] from comment #6)
> Could be fallout from switching to 64bit. Definitely wasn't an intentional
> decision made along the way.

Gregory, do you think we should add this back? If yes, I can file a bug for mozilla-build. Otherwise I'm happy to check which places need an update regarding this breakage.
Flags: needinfo?(gps)
What does 32 versus 64 bit have to do with things? AFAIK we never checked in a binary version of psutil in MozillaBuild nor in mozilla-central. What am I missing here?
Flags: needinfo?(gps)
Can you please add this back? I have to keep rebasing a backout of this patch to run mochitests.
Flags: needinfo?(hskupin)
I also just ran into this. I'm not using artifact builds. I'll do the backout of bug 1436857 for now, but we should definitely get this fixed...
And I hit this just now :/
Product: Core → Firefox Build System
gps, can you please co-sign a backout of the regressing patch. This is becoming an ever annoying burden while it sits in this inactive state.
Flags: needinfo?(gps)
Sorry, but I was out the last days. I will have a look at this patch tomorrow.

I assume that because mozilla-build now uses Python 64bit since the 3.0 release, I only have to include the compiled cext from the 64bit wheel?
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
Flags: needinfo?(hskupin)
Summary: AttributeError: 'module' object has no attribute 'ERROR_INVALID_NAME' → Missing compiled c extension of psutil for Windows causes AttributeError: 'module' object has no attribute 'ERROR_INVALID_NAME'
We never provided a binary distribution of psutil in the repo nor in MozillaBuild AFAICT.

The regression is due to `import psutil` apparently no longer raising ImportError when the C extension can't be loaded. I'm guessing they lazy load the C extensions in a newer version or something.

That being said, if I run `hg purge --all`, `mach python`, then delete the generated .pyd files, `import psutil` does in fact raise an ImportError. I'm not sure how the AttributeError in the stack is happening. Does that reproduce if you purge VCS ignored files from third_party/python/psutil? Run `hg purge --all -I path:third_party/python/` to delete ignored files from the third_party/python directory tree.
Flags: needinfo?(gps)
You are right. I would leave this question for Jared or someone else facing this problem. So far I was not able to setup my development system on Windows which still faces build errors for artifact builds. As such I cannot test that behavior, because the mentioned mach command requires a build being present. Jared, can you please check what Gregory asked? Thanks.
Flags: needinfo?(jaws)
Summary: Missing compiled c extension of psutil for Windows causes AttributeError: 'module' object has no attribute 'ERROR_INVALID_NAME' → After upgrade to psutil 5.4.3 certain mach commands show: AttributeError: 'module' object has no attribute 'ERROR_INVALID_NAME'
Above works for me:

- `hg purge --all -I third_party/python/`
- `./mach mochitest dom/media/test/test_mediarecorder_record_addtracked_stream.html`
- Reinstall all the things scroll (needs VCForPython27 for this to work)
- Tests run again.
I can confirm comment 17 works.
OK. Then this is a problem with a cached .pyd or .pyc file. Also, I forgot that `mach clobber python` exists to do basically what I said to run.

And I see that `mach clobber python` doesn't remove .pyd files.

I'll submit some improvements...
Assignee: hskupin → gps
Component: Mach Core → General
Flags: needinfo?(jaws)
(In reply to Marco Bonardo [::mak] from comment #18)
> I can confirm comment 17 works.

Ditto, I'm back up and running - thanks!
Whiteboard: [workaround in comment 17]
Comment on attachment 8956881 [details]
Bug 1439727 - Purge untracked files in python/ and third_party/python/;

https://reviewboard.mozilla.org/r/225840/#review231720

::: commit-message-78d02:12
(Diff revision 1)
> +ignored/untracked files. Let's purge those as well.
> +
> +Note: if someone has untracked but not ignored files, this will delete
> +them. This is possibly unwanted. But people shouldn't have untracked
> +but not ignored files sitting around in VCS. We don't run this command
> +by default, so I think it is safe to be aggressive in our purging of

I concur.

::: python/mozbuild/mozbuild/mach_commands.py:357
(Diff revision 1)
>          directory (where build output is stored). Some files (like Visual
>          Studio project files) are not removed by default. If you would like
>          to remove the object directory in its entirety, run with `--full`.
>  
>          The `python` target will clean up various generated Python files from
> -        the source directory. Run this to remove .pyc files, compiled C
> +        the source directory and will remove untracked files from directories

This is a little imprecise -- a random directory that may or may not contain Python won't be purged.  Perhaps scope to "a few well known directories containing Python" or something like that?
Attachment #8956881 - Flags: review+
Attachment #8956879 - Flags: review?(core-build-config-reviews)
Attachment #8956880 - Flags: review?(core-build-config-reviews)
Attachment #8956881 - Flags: review?(core-build-config-reviews)
Attachment #8956882 - Flags: review?(core-build-config-reviews)
Attachment #8956881 - Flags: review?(core-build-config-reviews)
Attachment #8956882 - Flags: review?(core-build-config-reviews)
Pushed by gszorc@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f6ea75e2b8e1
Also delete .pyd files; r=nalexander
https://hg.mozilla.org/integration/autoland/rev/85a7612e5ac9
Add a docstring for `mach clobber`; r=nalexander
https://hg.mozilla.org/integration/autoland/rev/31dc1d9be83d
Purge untracked files in python/ and third_party/python/; r=nalexander
https://hg.mozilla.org/integration/autoland/rev/1231cfa429da
Ignore third_party/python/psutil/tmp/ in VCS; r=nalexander
You need to log in before you can comment on or make changes to this bug.