Opened 4 years ago

Closed 12 months ago

#26253 closed enhancement (invalid)

Upgrade: psutil 5.8.0

Reported by: Dima Pasechnik Owned by:
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: packages: standard Keywords: upgrade, psutil
Cc: Erik Bray, Samuel Lelièvre, Michael Orlitzky, Matthias Köppe, François Bissey, Antonio Rojas, Isuru Fernando Merged in:
Authors: Reviewers: Michael Orlitzky
Report Upstream: N/A Work issues: Cygwin
Branch: public/ticket/26253 (Commits, GitHub, GitLab) Commit: 0fc3c304287e728be0700c526b7dde4cd847c4e9
Dependencies: Stopgaps:

Status badges

Description (last modified by Samuel Lelièvre)

Cygwin support needs a rebase.

Change History (52)

comment:1 Changed 4 years ago by Dima Pasechnik

The cygwin-specific patch does not apply, needs a rebase. It's removed for the sake of being able to test on other system (needed on FreeBSD, older package does not work there).

Last edited 4 years ago by Dima Pasechnik (previous) (diff)

comment:2 Changed 4 years ago by Dima Pasechnik

Branch: u/dimpase/psutils547
Cc: Erik Bray added
Commit: eee119432df73a72e802f92f6518408c21e42f6d
Description: modified (diff)

New commits:

eee1194updated and removed cygwin patch (needs a rebase)

comment:3 Changed 4 years ago by Dima Pasechnik

Summary: update psutil to version 5.4.7update psutil to version 5.4.8

meanwhile 5.4.8 is out.

comment:4 Changed 4 years ago by Dima Pasechnik

Branch: u/dimpase/psutils547
Commit: eee119432df73a72e802f92f6518408c21e42f6d
Description: modified (diff)

comment:5 Changed 4 years ago by Erik Bray

Milestone: sage-8.4sage-pending

I keep starting and stopping on reworking my Cygwin stuff for psutil. It needs to be refactored pretty badly and every time I start to work on it I quickly get pulled away to other things.

Is there some specific reason to update the psutil version?

comment:6 Changed 4 years ago by Dima Pasechnik

One reason is that the current version does not work on FreeBSD 12.0. How hard is that refactoring? I have a Windows laptop around, which BSODs now and then due to some hardware issue, but probably is good enough for mild Cygwin work.

A related Cygwin issue is on #26052 - any idea about that?

comment:7 Changed 4 years ago by Erik Bray

It's very non-trivial, unfortunately, and requires upstream cooperation (where in this case the psutil maintainer has been helpful and amenable, but that doesn't mean I can just go and do what I want).

Last edited 4 years ago by Erik Bray (previous) (diff)

comment:8 Changed 4 years ago by Dima Pasechnik

Description: modified (diff)
Summary: update psutil to version 5.4.8update psutil to version 5.5.0

meanwhile psutils 5.5.0 is out

comment:9 Changed 4 years ago by Frédéric Chapoton

Keywords: upgrade added

comment:10 Changed 4 years ago by Erik Bray

I'm working on a new (simpler) version of the Cygwin patch for psutil. Hopefully it will be ready soon.

comment:11 Changed 4 years ago by Dima Pasechnik

the latest psutil is 5.6.2.

comment:12 Changed 4 years ago by François Bissey

It would be good to update. Distros likely use a more recent psutil (I do). It causes at least one optional doctest failure (when combined with fricas) due to changes to output

sage -t --long /usr/lib64/python2.7/site-packages/sage/interfaces/fricas.py
**********************************************************************
File "/usr/lib64/python2.7/site-packages/sage/interfaces/fricas.py", line 351, in sage.interfaces.fricas.FriCAS._quit_string
Failed example:
    p = fricas.pid(); pr = psutil.Process(p); pr                  # optional - fricas
Expected:
    <psutil.Process(pid=..., name='AXIOMsys') at ...>
Got:
    psutil.Process(pid=7277, name='AXIOMsys', started='19:52:17')
**********************************************************************

This is with psutil-5.5.0.

comment:13 Changed 3 years ago by Frédéric Chapoton

Branch: public/ticket/26253
Commit: 4d7ce5119935ea51a8316f8d477c59b488a7b78f
Description: modified (diff)

Here is a branch, not tested. I have not removed the patches.


New commits:

4d7ce51update psutil to 5.6.2

comment:14 Changed 3 years ago by git

Commit: 4d7ce5119935ea51a8316f8d477c59b488a7b78f291ca8643de692d63aede3107104a9257b523efc

Branch pushed to git repo; I updated commit sha1. New commits:

291ca86fix psutil fricas doctest

comment:15 Changed 3 years ago by Samuel Lelièvre

New release (2020-02): psutil 5.7.0.

comment:16 Changed 2 years ago by git

Commit: 291ca8643de692d63aede3107104a9257b523efc844fa71cce390793267dbfb37af19d260ea683cc

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

4c042dcupdate psutil to 5.6.2
891175cfix psutil fricas doctest
844fa71build/pkgs/psutil: Update to 5.7.0

comment:17 Changed 2 years ago by Matthias Köppe

The psutil upstream issue on cygwin support is still open: https://github.com/giampaolo/psutil/issues/82

Erik, your help is needed

comment:18 Changed 2 years ago by Matthias Köppe

Summary: update psutil to version 5.5.0update psutil to version 5.7.0
Work issues: Cygwin!!

comment:19 Changed 2 years ago by Matthias Köppe

Milestone: sage-pendingsage-9.3

comment:20 Changed 2 years ago by Dima Pasechnik

perhaps we should look into removing psutils (on cygwin, or perhaps totally)

comment:21 Changed 2 years ago by Erik Bray

I have been thinking lately of spending a couple days updating the cygwin version of psutils. In particular, I think I will start maintaining my own fork of psutils with cygwin support, since last time I tried to upstream these changes it did not go well. If the psutil developer wants to port over any work from my fork he'd still be free to.

I was thinking about this lately because there was yet another issue on the psutils github asking for cygwin support and pointing to my incomplete pull requests...

comment:22 Changed 2 years ago by Erik Bray

If nothing else, I'll try rebasing my existing patches again...

comment:23 Changed 2 years ago by Erik Bray

I've rebased the latest version of my Cygwin patch onto psutil master. Once we update this ticket and decide which psutil version to upgrade to I can update the patch as well.

I might also try submitting this upstream again. The patch is incomplete (many features are missing) but good enough for Sage's purposes for now.

comment:24 Changed 2 years ago by Dima Pasechnik

Description: modified (diff)

5.7.3 is close to master, so hopefully one can apply your patches on top of 5.7.3

comment:25 Changed 2 years ago by git

Commit: 844fa71cce390793267dbfb37af19d260ea683ccbb604c64c65a02e199cf43e954f365cba8157aed

Branch pushed to git repo; I updated commit sha1. New commits:

bb604c6update to 5.7.3

comment:26 Changed 2 years ago by git

Commit: bb604c64c65a02e199cf43e954f365cba8157aedec6c4a6a602f07b19c5fa4bdb336886fc9c13182

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

3e84ad2update psutil to 5.6.2
79624d6build/pkgs/psutil: Update to 5.7.0
ec6c4a6update to 5.7.3

comment:27 Changed 2 years ago by git

Commit: ec6c4a6a602f07b19c5fa4bdb336886fc9c1318246ba2ee06666eb7504e44dfb8e9b036c415442f0

Branch pushed to git repo; I updated commit sha1. New commits:

46ba2eeremove old Cygwin-specific patch

comment:28 Changed 2 years ago by Dima Pasechnik

wrong comment - confused by the timeline, which I didn't update. It's fine.

Last edited 2 years ago by Dima Pasechnik (previous) (diff)

comment:29 Changed 2 years ago by Dima Pasechnik

Status: newneeds_review

this should be tested everywhere, except Cygwin, where Erik has promised a patch.

comment:30 Changed 2 years ago by Dima Pasechnik

Summary: update psutil to version 5.7.0update psutil to version 5.7.3

comment:31 Changed 2 years ago by Erik Bray

I can generate a patch from my latest Cygwin branch at https://github.com/embray/psutil/tree/cygwin/v3

But longer term if we want psutil to support Cygwin someone other than me will have to try to talk sense into its maintainer, who apparently does not have time to work on the project, but also says that he and only he can work on the Cygwin port which I've already spent days bringing to around 90% completion, with frankly a pretty clean patch not requiring a lot of changes. Seems pretty unreasonable, and disappointing.

I continued to work on my branch and he blocked me from the project bc I unthinkingly used his @ in a commit message (perhaps in unfounded optimism that he'd be willing to give the work another look later): https://github.com/giampaolo/psutil/pull/1884#issuecomment-733165286

comment:32 Changed 2 years ago by Dima Pasechnik

one cannot even open issues on the main psutil repo, I don't think there is any hope that patches would be accepted there.

it's not the 1st case we have where our patches are not welcome, I would not be too upset about it (cf. e.g. csdp, cliquer...)

comment:33 Changed 2 years ago by Matthias Köppe

We should not maintain patches that add functionality in Sage. This will create problems later when switch to using standard Python package dependency information for Python packages.

Instead I see two options:

  • Fork psutil, making it available on PyPI under a new name. Only makes sense if you are willing to maintain it, obviously.
  • Just implement Cygwin version of the few things needed from psutil in src/sage/interfaces/gap.py and src/sage/misc/getusage.py

comment:34 in reply to:  33 Changed 2 years ago by Erik Bray

Replying to mkoeppe:

We should not maintain patches that add functionality in Sage. This will create problems later when switch to using standard Python package dependency information for Python packages.

Normally I would agree but that's not really the case here. We're not talking about adding new functionality or bug fixes required for Sage to function in such a way that would be at odds with using system packages.

This is just adding support to the package for a platform it doesn't otherwise support: Which is to say, there is no "standard Python package dependency information" for psutil on Cygwin. In fact, PyPI does not even support cygwin wheels yet (something I plan to lobby on, but it's slightly complicated). There is also no psutil package for Cygwin, again because support for it does not even exist modulo my patch.

Instead I see two options:

  • Fork psutil, making it available on PyPI under a new name. Only makes sense if you are willing to maintain it, obviously.
  • Just implement Cygwin version of the few things needed from psutil in src/sage/interfaces/gap.py and src/sage/misc/getusage.py

What I plan to do is, once my Cygwin port of psutil reaches 100% feature parity then I will add a Cygwin package for it to cygwinports based on my fork. Then there will at least be a standard Cygwin package for psutil and we can drop the patch and/or use my fork as the standard installation source for psutil on all platforms.

I don't think it will require a lot of maintenance since he says he's planning on taking a break from making new psutil releases for a while.

comment:35 Changed 22 months ago by git

Commit: 46ba2ee06666eb7504e44dfb8e9b036c415442f00fc3c304287e728be0700c526b7dde4cd847c4e9

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

6417214update psutil to 5.6.2
3c9ff5dbuild/pkgs/psutil: Update to 5.7.0
b5b584bupdate to 5.7.3
2c2e6dfremove old Cygwin-specific patch
0fc3c30psutil 5.8.0

comment:36 Changed 22 months ago by Dima Pasechnik

Description: modified (diff)
Summary: update psutil to version 5.7.3update psutil to version 5.8.0

comment:37 Changed 22 months ago by Matthias Köppe

Is there any news on Cygwin support?

comment:38 Changed 21 months ago by Erik Bray

I don't know. It would be great if someone would try to talk sense to its maintainer at https://github.com/giampaolo/psutil/pull/1884#issuecomment-733165286.

Otherwise we can still just generate a patch from my existing work in the meantime...

comment:39 Changed 21 months ago by Dima Pasechnik

It sounds as if psutil is ripe for a fork :-)

comment:40 Changed 21 months ago by Matthias Köppe

Milestone: sage-9.3sage-9.4

Sage development has entered the release candidate phase for 9.3. Setting a new milestone for this ticket based on a cursory review of ticket status, priority, and last modification date.

comment:41 Changed 17 months ago by Matthias Köppe

Milestone: sage-9.4sage-9.5

Setting a new milestone for this ticket based on a cursory review.

comment:42 Changed 15 months ago by Samuel Lelièvre

Cc: Samuel Lelièvre added
Description: modified (diff)
Keywords: psutil added
Summary: update psutil to version 5.8.0Upgrade: psutil 5.8.0
Work issues: Cygwin!!Cygwin

Currently our spkg is at psutil 5.2.0, from 2017-03 (with patches).

Python 3 deprecation warnings in psutil were resolved in psutil itself by

With a recent psutil, the first start of Sage on Cygwin could be warnings-free.

Some user reports mentioning the deprecation warnings:

Last edited 15 months ago by Samuel Lelièvre (previous) (diff)

comment:43 Changed 15 months ago by Dima Pasechnik

psutil 5.8 does not support Cygwin, as far as I know.

Last edited 15 months ago by Dima Pasechnik (previous) (diff)

comment:44 Changed 14 months ago by Michael Orlitzky

Cc: Michael Orlitzky Matthias Köppe François Bissey Antonio Rojas Isuru Fernando added

Do we need psutil at all? I think we can delete it. It's used for three things:

  1. To guess at the initial GAP memory pool size, passed to gap via -o and -s. I don't think we need these flags, though: https://www.gap-system.org/Manuals/doc/ref/chap3.html
  2. To set the maximum stack size during PARI initialization. Again, we can just set it to a big number; it's a limit, and we're already guessing.
  3. Two doctests that call get_memory_usage() to check for leaks.

Those two doctests are not worth keeping it around, even as an optional package, considering it won't work on one of our supported platforms. So I say we,

  1. Drop the -o and -s flags to GAP. If the absence of -s causes problems in some cases, we can always pass -s <big_number> since that memory should not actually be allocated. And if that assumption is violated on some platform, then we could special-case that platform.
  2. Pass a big value to PARI in libs/pari/__init__.py for its stack size limit. 1024 times the initial size, or something like that.
  3. Delete those two doctests.
  4. Delete psutil, and sleep easier knowing that we won't be responsible for a fork of it for the rest of our lives.

comment:45 Changed 14 months ago by Dima Pasechnik

+1 to removing psutils

comment:46 Changed 14 months ago by Matthias Köppe

+1 if it can be done without breaking stuff

comment:47 Changed 14 months ago by Matthias Köppe

Status: needs_reviewneeds_work

comment:48 Changed 14 months ago by Samuel Lelièvre

Status: needs_workneeds_info

Consider setting to invalid/wontfix after removal of psutil in #32656.

comment:49 Changed 14 months ago by Dima Pasechnik

Milestone: sage-9.5sage-wishlist

comment:50 Changed 13 months ago by Samuel Lelièvre

Authors: Dima Pasechnik
Milestone: sage-wishlistsage-duplicate/invalid/wontfix
Status: needs_infoneeds_review

Ticket #32656 was merged in Sage 9.5.beta5.

comment:51 Changed 12 months ago by Michael Orlitzky

Reviewers: Michael Orlitzky
Status: needs_reviewpositive_review

comment:52 Changed 12 months ago by Matthias Köppe

Resolution: invalid
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.