X Tutup
The Wayback Machine - https://web.archive.org/web/20250624143627/https://github.com/python/cpython/pull/24226
Skip to content

bpo-42896 Allow for Solaris 11.4 crle output not containing ELF #24226

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

Closed
wants to merge 2 commits into from

Conversation

dmurphy18
Copy link

@dmurphy18 dmurphy18 commented Jan 16, 2021

Pull Request title

bp0-42896: Allow for Solaris 11.4 crle output

The output of crle has changed with Solaris 11 and '(ELF)' has been removed from the 'Default Library Path' string

https://bugs.python.org/issue42896

@dmurphy18 dmurphy18 changed the title Allow for Solaris 11.4 crle output BPO-42896 Allow for Solaris 11.4 crle output not containing ELF Jan 19, 2021
@dmurphy18 dmurphy18 changed the title BPO-42896 Allow for Solaris 11.4 crle output not containing ELF bpo-42896 Allow for Solaris 11.4 crle output not containing ELF Jan 19, 2021
@github-actions
Copy link

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Feb 19, 2021
Copy link
Contributor

@ZackerySpytz ZackerySpytz left a comment

Choose a reason for hiding this comment

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

Unit tests?

@@ -0,0 +1 @@
Allow for Solaris 11.4 crle output not containing ELF
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use full sentences in news entries. Also, this entry is too vague -- please add more details.

Copy link
Author

Choose a reason for hiding this comment

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

Will try to update the PR title once but would have thought while terse, it stated the issue well enough, and take a look at unit tests for this ( simple enough to test, just another test framework to learn :) )

Copy link
Author

Choose a reason for hiding this comment

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

PR title updated

Copy link
Author

Choose a reason for hiding this comment

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

Looking at existing tests at the end of util.py and ctypes/test

util.py def test

344     if os.name == "posix":
345         # find and load_version
346         print(find_library("m"))

will fail and print nothing on Solaris 11.4, for example:
without the fix

root@sol114:/export/home/david# python3
Python 3.7.8 (default, Jan 14 2021, 23:45:01) 
[GCC 7.3.0] on sunos5
Type "help", "copyright", "credits" or "license" for more information.
>>> import ctypes.util
>>> ctypes.util.find_library('m')
>>> 

with the fix

root@sol114:/export/home/david# python3
Python 3.7.8 (default, Jan 14 2021, 23:45:01) 
[GCC 7.3.0] on sunos5
Type "help", "copyright", "credits" or "license" for more information.
>>> import ctypes.util
>>> ctypes.util.find_library('m')
'libm.so.2'
>>> 

Similarly the same occurs finding the library 'c'. Hence I believe that the setupModule of ctypes/test/test_loading.py catches this case but I am perplexed by class LoaderTest.test_load where if libc is None, skiptest ??. Surely this should be an error if libc cannot be found on a system then something is wrong. Similarly LoaderTest.test_find, libraries 'c' and 'm' would not be found on Solaris 11.4 but no action.
Maybe there are some platforms where Python is used and no libc would be available by default.

I include the lines as example

from ctypes.util import find_library
libc_name = None
def setUpModule():
global libc_name
if os.name == "nt":
libc_name = find_library("c")
elif sys.platform == "cygwin":
libc_name = "cygwin1.dll"
else:
libc_name = find_library("c")
if test.support.verbose:
print("libc_name is", libc_name)
class LoaderTest(unittest.TestCase):
unknowndll = "xxrandomnamexx"
def test_load(self):
if libc_name is None:
self.skipTest('could not find libc')
CDLL(libc_name)
CDLL(os.path.basename(libc_name))
self.assertRaises(OSError, CDLL, self.unknowndll)
def test_load_version(self):
if libc_name is None:
self.skipTest('could not find libc')
if os.path.basename(libc_name) != 'libc.so.6':
self.skipTest('wrong libc path for test')
cdll.LoadLibrary("libc.so.6")
# linux uses version, libc 9 should not exist
self.assertRaises(OSError, cdll.LoadLibrary, "libc.so.9")
self.assertRaises(OSError, cdll.LoadLibrary, self.unknowndll)
def test_find(self):
for name in ("c", "m"):
lib = find_library(name)
if lib:
cdll.LoadLibrary(lib)
CDLL(lib)

Existing tests are already catching the condition but are effectively ignored, hence unsure what my test cases would contribute to this when existing tests are ignoring the condition, especially for such a simple fix.

Copy link
Author

Choose a reason for hiding this comment

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

@ZackerySpytz wondering about a reply concerning my point about unit tests for this.

@github-actions github-actions bot removed the stale Stale PR or inactive for long period of time. label Aug 3, 2022
@encukou
Copy link
Member

encukou commented Apr 5, 2024

I'll close this PR as the OS is unsupported.
The issue will be listed on the GitHub project for anyone who wants to support the platform unofficially or bring support back.

@encukou encukou closed this Apr 5, 2024
@dmurphy18
Copy link
Author

@encukou The Solaris 11 support is community supported in repo https://github.com/vmware/salt-native-minion-for-solaris-11. This could be handled there.

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.

6 participants
X Tutup