-
-
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
psutil fails to build on Solaris 10 #609
Comments
According to /usr/include/net/if.h on both Solaris 10 and 11:
the ifreq struct doesn't have the mtu member, while the new non-obsolete lifreq struct does. I committed a fix to switch to lifreq here: Infinidat@e8c06de
(this file doesn't exist on Solaris 10 and also on AIX so it doesn't entirely belong to the "posix" module...) I think the only way to fix this problem is by #ifdef-ing out and not supporting the function that uses ifaddrs.h (psutil_net_if_addrs). What do you think? |
Can you make a PR for Infinidat/psutil@e8c06de?
I think it makes sense. Can you make a PR for this as well? |
The problem with disabling this function is that there is no way to tell Solaris 10 and 11 apart according to the predefined macros (see https://gist.github.com/basman/587684). My current commit to fix this is here: Infinidat@006d277 |
If by default there's no way to distinguish Solaris 10 from 11+ in C (are we 100% sure? it's weird) then I'd look into how to do this as a macro defined in |
I may be out of my depth here, so feel free to point and laugh, but there is a Solaris C system call for 10 and 11 in sys/utsname.h. We built our Python 2.7.8 on Sun Studio 12 cc rather than gcc because gcc wouldn't build the Python libpython shared object, although it did build a viable Python executable. |
Thanks for pointing it out, but the system call is not good enough because we need to distinguish between the two operating systems in compile time, not run time ('#include ifaddrs.h' is parsed during preprocessing). |
Again, feel free to laugh, but wouldn’t the simple execution of a “uname –r” tell you? I’m not sufficiently knowledgeable about the compilation process to know whether or not this is possible. Sorry. I’m not sure if the modifications have been pushed by my colleague yet. We’re still testing internally. The reason we made those changes was we could not get the psutil Python module to work properly on our Solaris 10 systems. For some reason, it couldn’t find the Solaris 10 network interfaces. Michael Peoples (mp4783) Principal Applications Developer From: wiggin15 [mailto:[email protected]] Thanks for pointing it out, but the system call is not good enough because we need to distinguish between the two operating systems in compile time, not run time ('#include ifaddrs.h' is parsed during preprocessing). — |
The compiler can't run commands like 'uname' - it's not a shell. |
This is supposed to have been fixed by #610. |
I think this may not have been fixed fully since I am seeing the original error ifr_mtu not existing and from examining the code I do not see anything which address's this issue in _psutil_posix.c. Using release-5.0.0 tag from github https://github.com/giampaolo/psutil I am building on a Solaris 10 update 8 on Intel in a VirtualBox, but see the same issue on Sparc with similar level of OS, with current OpenCSW tools installed on both. Command to reproduce (relevant excerpts from bash script):
Log output:
Wondering if I missed something, like should posix even being getting built, my environment is set to en_US.UTF-8. |
problem still occurs with fixed include form above, had typo,extra 'i' in -I/opt/csw/includei corrected below:
|
FYI - I checked #610 and recent code to make sure no over-writing of fix, but from what I ascertain the original fixes (610 et al.) did not fix the issue with mtu |
What if you grep for "ifr_mtu" into /usr/include? Is it defined somewhere? |
@giampaolo no, in none of the /opt or /usr, for example: |
What's defined in Solaris is this:
In Solaris it's lifr, not ifr, and that's what we use in the Solaris implementation of net_if_stats: https://github.com/giampaolo/psutil/blob/master/psutil/_psutil_sunos.c#L1252 (this is the fix from #610) For some reason the compilation tries to compile _psutil_posix.c with the wrong definition of ifr_mtu, but the line with this definition is wrapped with an ifdef:
so the compiler shouldn't really get there.... I suspect you're not using the latest version from this repository: your compiler hits the error on psutil/_psutil_posix.c:272 but this is a blank line in the current implementation: https://github.com/giampaolo/psutil/blob/master/psutil/_psutil_posix.c#L272 |
@wiggin15 AFAICT the Line 282 in a26d896
It looks like in order to fix this problem we should be doing this:
@dmurphy18 can you try that? |
I see. I looked at psutil_net_if_stats and not at psutil_net_if_mtu, which is relatively new in _psutil_posix.c (after the fix in #610, so it's not fixed for Solaris 10). Your suggestion looks good. |
suggest using #ifdef PSUTIL_SUNOS10 as at the start of the file, I can make the changes I feel needed, test it and let you know. |
The following compiles cleanly on Solaris, it could be optimized leveraging offsetof definitions, but preferred to keep it straight-forward. Changes ifdef'd by PSUTIL_SUNOS10.
|
By the way, I'm having issues compiling psutil on Solaris. Do any of you guys have a solution? |
I should have fixed this in 405c5fe. |
@giampaolo wondering about ifr on a Solaris system, will throw compile errors since not defined +#ifdef PSUTIL_SUNOS10
Have not compiled since error is plain. |
Right. That's what happens when you're not able to test a change on the actual system. =) |
Taking another look and will try on Solaris box in VirtualBox |
@giampaolo it compiled fine and the code looks good. Thanks for fixing this so quickly. |
Cool. I will make a release soon. |
The text was updated successfully, but these errors were encountered: