-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[Solaris] Fix #1637 #1638
[Solaris] Fix #1637 #1638
Conversation
Did you check this doesn't break anything on newer versions? I won't have a SunOS box to test against anytime soon so I'd basically merge this blindly. (also please update HISTORY.rst) |
4388b64
to
75d236d
Compare
I've checked it against the latest Solaris 5.10 box I have access to and the preprocessor code is the same as on master. I'm asking around to get access to a 5.11 box but can't ensure I'll get it. There is indeed one possible pitfail: if latest SunOS latest History updated 👍 |
OK, Based on issue #421 and on this serverfault question I'd suggest to pass Solaris update version DC the compiler and activate I'll do it on Monday! |
75d236d
to
d20b32f
Compare
@giampaolo I've checked this against all of the solaris boxes I have access to (eg Initial Release, Update 2, Update 3, Update 5, Update 11). The compilation fails on master on the boxes Initial Release, Update 2, Update 3 as described in the issue. With this PR, psutil will install on all the boxes with no pb. Update5 and 11 do have the support of MIB (checked the gcc -E code manually). |
77bacea
to
e98cb6a
Compare
@@ -6,7 +6,6 @@ | |||
* Functions specific for Process.environ(). | |||
*/ | |||
|
|||
#define NEW_MIB_COMPLIANT 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As said in the commit message, I'm pretty sure this is dead code :)
@@ -12,12 +12,8 @@ | |||
/* fix compilation issue on SunOS 5.10, see: | |||
* https://github.com/giampaolo/psutil/issues/421 | |||
* https://github.com/giampaolo/psutil/issues/1077 | |||
* http://us-east.manta.joyent.com/jmc/public/opensolaris/ARChive/PSARC/2010/111/materials/s10ceval.txt | |||
* | |||
* Because LEGACY_MIB_SIZE defined in the same file there is no way to make autoconfiguration =\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not seem relevant anymore.
*/ | ||
|
||
#define NEW_MIB_COMPLIANT 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Defined for updates after 4 by gcc.
setup.py
Outdated
@@ -268,6 +269,17 @@ def get_ethtool_macro(): | |||
if SUNOS: | |||
posix_extension.libraries.append('socket') | |||
if platform.release() == '5.10': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean psutil is not supported on Solaris 5.11 BTW?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's supported, I guess it's NEW_MIB_COMPLIANT
and I need to move up my macro one level!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, if I am not mistaken that's in place because getifaddrs()
syscall is not available on Solaris 5.10, therefore it means psutil SunOS support is supposed to start with SunOS version 5.10 (+ newer versions including 5.11).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job. I added some comments inline. =)
setup.py
Outdated
@@ -268,6 +269,17 @@ def get_ethtool_macro(): | |||
if SUNOS: | |||
posix_extension.libraries.append('socket') | |||
if platform.release() == '5.10': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, if I am not mistaken that's in place because getifaddrs()
syscall is not available on Solaris 5.10, therefore it means psutil SunOS support is supposed to start with SunOS version 5.10 (+ newer versions including 5.11).
if update is None or (int(update.group(0)) < 4): | ||
new_mib_compliant = False | ||
if new_mib_compliant: | ||
posix_extension.define_macros.append(('NEW_MIB_COMPLIANT', 1)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about shortening it a bit?
# Detect Solaris 5.10, update >= 4, see:
# https://github.com/giampaolo/psutil/pull/1638
# For an explanation of Solaris /etc/release see:
# https://serverfault.com/q/524883
with open('/etc/release') as f:
update = re.search(r'(?<=s10s_u)[0-9]{1,2}', f.readline())
if update is None or (int(update.group(0)) >= 4):
posix_extension.define_macros.append(('NEW_MIB_COMPLIANT', 1))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right! But I still need to add the macro for the other platforms.
Question, is it on purpose that you've let the condition as if update is None or …
(which now means that when no update is found or the update is greater than 4, the platform is new mib compliant)? Difference with my version is that I assumed when no update is found, the platform is not mib compliant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question, is it on purpose that you've let the condition as if update is None or … (which now means that when no update is found or the update is greater than 4, the platform is new mib compliant)?
No I just copy/pasted your code.
Detect Solaris MIB compliancy based on Update number This allows setting NEW_MIB_COMPLIANT macro in the compile line. Consider initial 5.10 release as non mib compliant
286ed93
to
6b2e155
Compare
Sorry, should have tested before pushing ^^ |
The macro NEW_MIDS_COMPLIANT is now passed by the compiler anyway.
6b2e155
to
f7b900f
Compare
Here are my tests results:
As a reference (on SunOS 5.10U11 as 5.10U3 is not supported):
Though it does not seem really stable. BTW, I've added a |
Merged. While we're at it, could you paste the test output of latest solaris? Just to get an idea of what is broken. |
@giampaolo Could you confirm you received the log I've sent yesterday by email? |
I did not. Did you send it to g.rodola AT gmail? |
Can’t you just attach it here? |
Right, hadn't seen I could simply do a drag&drop ^^ |
Except for a couple, most of these failures are related to tests themselves and not real psutil issues. I pushed 2 commits which should fix something: 508ca73 and 04bcd8d. As for the others, I'll get around them when I'll put hands on a SunOS box (or feel free to make a PR if you wish). Regardless, thanks for this PR Étienne. |
Here is my latest result:
I'll check the plain log a bit more tomorrow. Cheers |
Adam Johnson (1): Reference exact CVE in HISTORY for 5.6.6 (#1653) Anselm Kruis (1): Fix a compile error in _psutil_linux.c if PSUTIL_HAVE_IOPRIO is false (#1695) Caleb Bassi (1): Remove links to abandoned psutil ports in readme (#1669) Giampaolo Rodola (129): fix compilation err on win Readme tidelift update (#1636) SunOS, fix #1642: PID 0 raises FileNotFoundError HERE: use realpath() instead of abspath() because of failures seen in giampaolo/psutil#1638 (comment) update HISTORY for #1646 setup.py: print instructions if C compiler is not installed credits for #1648 lint update/fix wait_procs() doc bump version windows: move net_connections() in socks.c win: split code into new process_utils.c highlight cmd.exe warnings/errs from VS Drop windows XP support (#1652) small refactoring move custom exceptions in _common.py #1652: also drop support for Windows Server 2003 Windows: split C modules (#1655) #1652: remove inet_ntop.c winmake / uninstall: remove installation path from easy-install.pth file move AF_INET6 def in global.h use HeapAlloc() instead of malloc() around GetAdaptersAddresses rename global.c -> globals.c fix compiler warning fix compiler warning + remove cruft win: move ObjectNameInformation in ntextapi.h win: provide alias for HeapAlloc() cleanup ntextapi.h fix open_files() tests broken on windows due to case sensitiveness winmake.py: accept builtiple targets/args include ntextapi.h from globals.h so that we won't have to import it ever windows: move get_process_info() into process_info.c to make room for Cygwin fix compiler warnings; move some defss into process_info.h move PEB structs into ntextapi.h rm duplicated C code refactor win C code: use original WinAPI functions and remove psuil_ prefix move send_signal() code into windows module refactoring + remove dead code Win: fix compilation err on python 32 bit Win: fix segfault cause by FREE/MALLOC macros winmake.py: use argparse fix #1656: [Windows] Process.memory_full_info() raises AccessDenied even for the current user and os.getpid() fix some win tests just move stuff around get rid of globals.c; move stuff in _psutil_common.c #1652 / win / XP support: remove routine to collect files on win < vista refactoring test_contracts.py #1652: ionice(), remove code checking if we're on Win Vista+ add contract tests for IOPRIO_ win constants add more types tests #1020: remove doc part mentionin open_files() on win is only able to list files living on the C drive [Windows] rewrite of open_files() (#1660) #1652: remove win XP code path checking avilability of GetTickCount64 #1652 remove Windows Vista references update setup.py classifiers print/set syscall origin when raising NSP or AD exec make install before 2 targets check MALLOC_ZERO ret code properly cleanup C thread fix #1662: QueryFullProcessImageNameW may fail with error code = 0 (Win API bug?) AD script: print AD percentage + elapsed time update HISTORY Add *new_only* parameter for process_iter() (#1667) set proper NTSTATUS error code [Windows] psutil_handle_from_pid() refactoring (#1668) Add CI testing for FreeBSD (#1671) fix Cirrus failure Cirrus: enable python 2 run py2 tests on cirrus OpenBSD fixes (#1673) Properly handle PID type in C (#1672) update HISTORY small C refactoring [Windows] use NtQuerySystemInformation to determine process exe() (#1677) print-ad.py: double sort [Windows] connections() refactoring (#1678) #1610: clarify in the doc what cpu_count means [Windows] speedup connections (#1679) fix #1674, SunOS: disk_partitions() may raise OSError. update doc fix #1650 [Linux] sensors_temperatures() no longer emit warnings on file not found (print debug msg instead) [Linux] disk_io_counters() ValueError when parsing /sys/block (#1684) #1672, #1682: SIZEOF_INT is not available on pypy3; assume that on systems where pid_t size can't be determined at runtime pid_t is an int Fix test errors for PYPY. skip memleak tests on PyPy: they are unreliable probably because of the JIT fix #1681 / linux / disk_partitions: show swap small refact fix #1627: [Linux] Process.memory_maps() can raise KeyError fix #1576: remove code duplicate for create_time / boot_time try to fix travis test merge from master windows: convert pid_t to DWORD Add support for PyPy on Windows (#1686) refactor doc fix pypy on Linux refactoring #1672: determine pid_t size at runtime in setup.py fix #1688: use python3 in GIT commit hook hashbang small refactoring to accomodate #1691 #1672: warning pre-processor directive don't work on win + py2; also if struct.calcsize('l') < 8 assume int [Windows] increase precision of boot_time() and proc create_time() (#1693) refactoring: get rid of duplicated code; use one function to return (user, sys, create time) rename method remove unnecessary wrap_exceptions deco bind cpu_times() and create_time() with oneshot() rename var remove cache for proc_times(): unnecessary because create_time() is already cached refactoring: get rid of duplicated code; use one function to return (user, sys, create time) #1681, revert 00a3398 small refactoring #1659: provide error message in case of bugged PYPY2 version fix tests divide test_system.py unit tests in multiple classes #1693: also increase precision of users()'s login time update doc update doc update doc update history for #1695 update doc refactor print colors utils point all shebangs to python 3 get rid of pip_install() code for py2; move everything in runner.py remove deprecation test: it fails intermittently because warnings uses a global state #1053 fix syntax incompatible with py2.6 fix Makefile for freebsd revert #1667 process_iter() new_only param revert process_iter() exactly how it was pre #1667 Add C linter script (#1698) fix KeyError Javad Karabi (1): sensors_temperatures: also search /sys/devices/platform/coretemp.* for temperatures (#1648) Jon Dufresne (1): Remove use of deprecated setuptools test_suite & tests_require (#1696) Kamil Rytarowski (1): Add special case for psutil_proc_cwd/NetBSD (#1538) Mike Hommey (1): Future-proof disk_io_counters on Linux. (#1665) Po-Chuan Hsieh (1): Fix Process on FreeBSD 12.0+ i386 (#1646) Thomas Güttler (1): total physical memory (exclusive swap). (#1634) Tim Gates (1): Fix simple typo: whish -> wish (#1640) johnthagen (1): Enable PyPy3 on CI (#1682) vser1 (1): [Solaris] Fix #1637 (#1638)
This add support for SunOS 5.10 update prior to 4.
It shouldn't change anything for conforming targets but add support for older targets.