Skip to content
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

fix build and tests on Solaris 10 #610

Merged
merged 8 commits into from
Sep 5, 2015
Merged

Conversation

wiggin15
Copy link
Collaborator

No description provided.

@wiggin15
Copy link
Collaborator Author

Note: there are still 3 tests that fail on Solaris 10, 2 of them because of the issue described in #517.

@giampaolo
Copy link
Owner

Hey, I will travel to NYC soon so I won't time to look into this for a while (I hope not too much). Please be patient.

@jliverman
Copy link

We experienced similar issues (and produced a similar patch) as lymanepp in #517.

We also found that the variable PSUTIL_VERSION was not defined and so we supplied it in the _psutil_sunos.h header file (resulting in a one-line patch). We just gave it a value of 221 for 2.2.1, the version from https://pypi.python.org/simple/ for easy_install.

For the most up-to-date version of psutil (here on github) there seemed to be additional issues. Namely, the function psutil_net_if_stats seems to rely on a struct typedef called ifreq from net/if.h. However, our versions of Solaris 10 specifically say:

/*
 * OBSOLETE: Replaced by struct lifreq. Supported for compatibility.
 *
 * Interface request structure used for socket
 * ioctl's.  All interface ioctl's must have parameter
 * definitions which begin with ifr_name.  The
 * remainder may be interface specific.
 */
struct  ifreq {
...

within the net/if.h header file.

Also, the ifreq struct seems to be missing a member called "ifr_mtu" that is referenced in the _psutil_sunos.c code. The suggested replacement (i.e. struct typedef lifreq) has a member called "lifru_mtu" that seems like the likely replacement. However, I just hard-coded the value to 0 to get everything to compile as I did not feel comfortable working with the types or required casts, if any (ifreq relies on standard types like int, short and char while lifreq relies on derived types like uint_t and uint64_t).

At this time we're getting by with our little hacks. But we're hoping that the maintainer can address them soon. We have a fairly critical application that depends on it. But alas, we are not exactly "C people" if you know what I mean...

@wiggin15
Copy link
Collaborator Author

@jliverman The lifreq issue was reported in #609, the fix is in this pull request.
@giampaolo any chance to review this and get this merged? Thanks.

@giampaolo
Copy link
Owner

I'm still in US, still having very few time to look at anything other than work (sigh!). Sorry.

@wiggin15
Copy link
Collaborator Author

wiggin15 commented Sep 4, 2015

Hi. I rebased and updated this pull request. Can you please review it?
Note that merging this pull request will also resolve #609, #620 and #621.
Also note that there are still 12 (!) tests that fail on Solaris 10 now, which I didn't look into yet (most are new tests that didn't exist when I first opened this pull request).

Arnon Yaari added 6 commits September 5, 2015 11:50
ifaddrs doesn't exist on this platform, so I added an
implementation of it for use in _psutil_posix.c
os.path.exists will fail if the link is broken. see comment about errno.ENOENT
in memory_maps in _pssunos.py
@wiggin15
Copy link
Collaborator Author

wiggin15 commented Sep 5, 2015

I updated the branch, with support for net_if_addrs, and now we're down to 9 failing tests. I found and fixed the problem with 4 of the failing tests but the fixes are not Solaris specific so I will open a separate pull request for them. 3 tests fail because of the issue reported in #517. I'm not sure about the other 2.

@giampaolo
Copy link
Owner

what's the test output?
Il 05/set/2015 11:13, "wiggin15" [email protected] ha scritto:

I updated the branch, with support for net_if_addrs, and now we're down to
9 failing tests. I found and fixed the problem with 4 of the failing tests
but the fixes are not Solaris specific so I will open a separate pull
request for them. 3 tests fail because of the issue reported in #517
#517. I'm not sure about the
other 2.


Reply to this email directly or view it on GitHub
#610 (comment).

@wiggin15
Copy link
Collaborator Author

wiggin15 commented Sep 5, 2015

I uploaded the output here: https://gist.github.com/wiggin15/ae62963b33716da3a503

I created this branch: https://github.com/Infinidat/psutil/commits/test_fixes
to fix 4 of the tests (test_cpu_count and the 3 TestUnicode tests). I'm working on making it pass the CI.
The ones with "couldn't find any network interface" are #517 related.
test_connections and test_prog_w_funky_name are left.

@@ -1385,7 +1385,10 @@ def test_create_time(self):
def test_terminal(self):
terminal = psutil.Process().terminal()
if sys.stdin.isatty():
self.assertEqual(terminal, sh('tty'))
tty = sh('tty')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could simply use tty = os.path.realpath(sh('tty'))

#include <net/if.h>

#ifdef _SUNOS10
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename this to PSUTIL_SUNOS10

@giampaolo
Copy link
Owner

Good work! I added some comments. Please also do not forget to update HISTORY.rst and CREDITS files.

@wiggin15
Copy link
Collaborator Author

wiggin15 commented Sep 5, 2015

Thanks for the review. I fixed your comments.

giampaolo added a commit that referenced this pull request Sep 5, 2015
fix build and tests on Solaris 10
@giampaolo giampaolo merged commit 7ca207d into giampaolo:master Sep 5, 2015
@giampaolo giampaolo mentioned this pull request Feb 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants