-
Notifications
You must be signed in to change notification settings - Fork 256
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Drop _emerge/getloadavg.py #1383
base: master
Are you sure you want to change the base?
Conversation
Portage's custom getloadavg.py wrapper was added with 8607a39 ("If necessary, use /proc/loadavg to emulate os.getloadavg()."). All platforms we care about provide os.getloadavg() now, so this can be dropped. Signed-off-by: Florian Schmaus <[email protected]>
I searched a bit and found comment related to #589:
As I recall, the getloadavg function was added in 8607a39 due to the function being missing for python built with uclibc back then. I've found that this is still an issue with current autobuilds found in http://ftp.romnet.org/gentoo/releases/amd64/autobuilds/current-stage3-amd64-uclibc-vanilla/, for example this is with stage3-amd64-uclibc-vanilla-20200619.tar.bz2:
```
Python 3.7.7 (default, May 31 2020, 21:22:59)
[GCC 9.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import os
>>> os.getloadavg
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
AttributeError: module 'os' has no attribute 'getloadavg'
```
We should be able to remove the /proc/loadaverage usage if all of your supported libc implementations have
getloadaverage(). It was lacking in uclibc at the time of 8607a39.
Note that `os.getloadavg()` is already preferred when it is detected in `lib/_emerge/getloadavg.py` here:
```python
from portage import os
getloadavg = getattr(os, "getloadavg", None)
if getloadavg is None:
def getloadavg():
```
It would be basically harmless to keep this getloadavg fallback implementation, in case we ever encounter python with `os.getloadavg()` missing again.
It could be helpful for people running prefix portage with exotic kernel / libc / python implementations. We could remove it and then add it back again if anyone ever finds that they need it, but what's the motivation to remove it given that it's basically harmless whenever a true `os.getloadavg()` is available?
If we remove it from the master branch, then the prefix branch might want to keep it in order to preserve maximum flexibility here. @grobian, @heroxbd, @chewi, and @thesamesam how do you guys feel about this?
|
Hi @zmedico os.getloadavg() returns data everywhere I tried. Should it be unavailable, I think we should just ignore it, and disable or bark or something when load average limits are used. It's not like loadaverage means the same on every platform anyway. As for keeping it in Prefix: I'd say not. /proc is only available on Linux this way AFAICT, so it's hardly useful on other platforms. |
Sounds good to me, although I don't do much with alternative libcs. |
CC @zmedico You added the
getloadavg()
wrapper to portage with 8607a39dc9d9. However, the commit message lacks a rationale why such a wrapper became necessary. This PR assumes that the reasons for the wrapper are now no longer existent. Could you please clarify if this is true? Otherwise we should consider adding a code comment to_emerge/getloadavg.py
explaining the reason for the wrapper.