Upgrade psutil Python package from version 3.1.1 to 5.4.3

RESOLVED FIXED in Firefox 60

Status

()

Core
Build Config
P3
normal
RESOLVED FIXED
15 days ago
3 days ago

People

(Reporter: whimboo, Assigned: whimboo)

Tracking

(Depends on: 1 bug)

60 Branch
mozilla60
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox60 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

15 days ago
To run the mozprocess unit tests on Windows the psutil Python package is required. While we have the sources of this package in-tree [1], we cannot easily compile it due to the following requirements:

> <@ted> it has a C module, and those are a PITA to build on windows
> <@ted> because they have to be built with a version of MSVC that matches what python was built with
> <@ted> and for python 2.x that's like VC9

The alternative could be:

> <@ted> i assume it should be possible to publish a psutil wheel, and maybe there already is one?
> <@ted> https://pypi.python.org/pypi/psutil shows wheels of the latest release, certainly
> <ahal> we'd need still need the source for linux/mac + 2 windows wheels per python version we want to support

Gregory, given that you added the package via bug 783727, do you have an idea what might be best to do here? I never touched the build system at this level, so more suggestions are welcome.

[1] https://hg.mozilla.org/mozilla-central/file/tip/third_party/python/psutil
Flags: needinfo?(gps)
(Assignee)

Comment 1

15 days ago
Also we are using a very outdated version of psutil: 3.1.1 vs. 5.4.3.

I tried to install the wheel locally for py27, and was only able to do so for the win32 version. The amd64 wheel failed with unsupported platform even I have Win7 64bit.
(Assignee)

Comment 2

14 days ago
I tried to upgrade to version 5.4.3 and it looks all fine except that test machines miss the python(3)-dev package:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8edcedcd659574e1eaee6e81e1e8b3f1b71216a2
(Assignee)

Updated

14 days ago
Depends on: 1436941
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 5

14 days ago
Actually we don't want that package to be installed. The problems were related to two cases with a reference to version 3.1.1 of this package in the tree. Fixing those made it work:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=eb8e6990bf20f62793d28178ca465bf9c8a82bad

Interestingly I noticed that with the new version we don't need the pre-compiled code in the tree.

So the only question which remains now is how we could handle Windows, and if we could easily just use the wheel.
(Assignee)

Comment 6

14 days ago
Turns out that just unpacking the wheel and putting the compiled C extension for Windows into the psutil folder works just fine. With that I can run the mozprocess tests.

I will upload a patch which includes a temporary commit to enable mozprocess unit tests on Windows.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

14 days ago
Depends on: 1437163
Comment hidden (mozreview-request)
(Assignee)

Updated

14 days ago
Attachment #8949710 - Attachment is obsolete: true
(Assignee)

Comment 10

14 days ago
With the mozprocess patch it failed to detect the compiled cext as valid DLL. Not sure why this happens because it works fine locally. Also other test jobs want to retrieve psutil via pip and fail, because the version 5.4.3 has not been made available yet on our internal PyPI mirror. I requested an upload. Once this is done I will request another try build.

Also after spoken to Andrew we might want to get rid of psutil in mozprocess, which is only used to check if a pid exists. On Windows we could perfectly do that via ctypes.
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
Priority: -- → P3
(Assignee)

Updated

14 days ago
Flags: needinfo?(gps)
(Assignee)

Updated

14 days ago
Summary: psutil Python package under third party misses compiled C extension for Windows → Upgrade psutil Python package from version 3.1.1 to 5.4.3
(Assignee)

Updated

14 days ago
No longer blocks: 892902
(Assignee)

Updated

14 days ago
Attachment #8949655 - Flags: review?(gps)

Comment 11

11 days ago
mozreview-review
Comment on attachment 8949655 [details]
Bug 1436857 - Upgrade psutil from version 3.1.1 to 5.4.3.

https://reviewboard.mozilla.org/r/219006/#review225356

Yay - thank you for upgrading this dated package!

One nit: the commit message says the Windows binary extension is committed. That isn't true. But I'm OK with that: if we commit the binary extension, it should be in its own commit, because that will likely require changes.

Also, historically it has been a PITA to get wheels working in CI. Blame the physical Windows buildbot machines, which I believe still to this day run a seriously ancient pip version that doesn't support wheels. If we get everything using the build-derived mozharness.zip file (or source checkouts), we could use the virtualenv/pip that is vendored in mozilla-central. See bug 1436612 for recent developments here.
Attachment #8949655 - Flags: review?(gps) → review+
(Assignee)

Comment 12

10 days ago
mozreview-review-reply
Comment on attachment 8949655 [details]
Bug 1436857 - Upgrade psutil from version 3.1.1 to 5.4.3.

https://reviewboard.mozilla.org/r/219006/#review225356

Oh, yes this extra message is wrong. The compiled C extension has been removed again because it caused problems. I will remove that from the commit description.
Comment hidden (mozreview-request)

Comment 14

10 days ago
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f3bfda748e33
Upgrade psutil from version 3.1.1 to 5.4.3. r=gps

Comment 15

9 days ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f3bfda748e33
Status: ASSIGNED → RESOLVED
Last Resolved: 9 days ago
status-firefox60: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in before you can comment on or make changes to this bug.