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

gh-78469: Declare missing sethostname for Solaris 10 #109447

Merged
merged 8 commits into from
Oct 9, 2023

Conversation

kulikjak
Copy link
Contributor

@kulikjak kulikjak commented Sep 15, 2023

As discussed in the issue, sethostname is missing from Solaris 10 headers but is available on Solaris 11.

And because there isn't an OS version specific macro for Solaris, I had to "create" one.

@@ -5652,8 +5652,9 @@ socket_sethostname(PyObject *self, PyObject *args)
Py_buffer buf;
int res, flag = 0;

#ifdef _AIX
/* issue #18259, not declared in any useful header file */
#if defined(_AIX) || (defined(__sun) && defined(__SVR4) && SUNOS_VERSION == 510)
Copy link
Member

Choose a reason for hiding this comment

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

wow, that's very spefic. It was available before and after, but not at version 510? :-)

# in most compilers, so we define one here.
SUNOS_VERSION=`echo $ac_sys_release | tr -d '.'`
AC_DEFINE_UNQUOTED([SUNOS_VERSION], [$SUNOS_VERSION],
[The version of SunOS/Solaris as reported by `uname -r' without the dot.])
Copy link
Member

Choose a reason for hiding this comment

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

How do you distinguish Solaris 5.10 and Solaris 51.0?

Maybe multiply the major version by 100 or 1000? or use a shift of 8 bits? The usage would be:

major = VERSION >> 8
minor = VERSION & 255

The disavantage is the need to write versions in hexadecimal :-(

Copy link
Member

Choose a reason for hiding this comment

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

@erlend-aasland: Should we prefix new pyconfig.h constants by Py_ ? see: capi-workgroup/problems#46

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should follow the PEP-7 recommendations: https://peps.python.org/pep-0007/#naming-conventions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How do you distinguish Solaris 5.10 and Solaris 51.0?

I see what you mean, but that's not happening. The leading 5 was dropped from the name 25 years ago and while e.g. uname and compilation targets still use it, nobody today calls it Solaris/SunOS 5.10; only Solaris 10. If we ever release Solaris 51, uname -r should report it as 5.51.

That said, I am not strongly against changing it if you still prefer your suggestion.

I think we should follow the PEP-7 recommendations: https://peps.python.org/pep-0007/#naming-conventions

Thanks, I missed that. It's fixed now.

@vstinner
Copy link
Member

When was Solaris 10 release? Is it still supported? Does this change better to forks of Solaris?

@kulikjak
Copy link
Contributor Author

kulikjak commented Oct 5, 2023

When was Solaris 10 release? Is it still supported? Does this change better to forks of Solaris?

Sorry for such a long delay. As mentioned in the issue, Solaris 10, while in the "extended support stage" intended mostly for critical fixes, is still supported (ATM at least until January 2025). There are still people using it and possibly installing newer Python onto it.

As for the Solaris forks, this doesn't affect those because the first open version was Solaris 11.

configure.ac Outdated Show resolved Hide resolved
@erlend-aasland
Copy link
Contributor

Is this a bug-fix? If so, we should backport it and add a NEWS entry.

@vstinner
Copy link
Member

vstinner commented Oct 6, 2023

@kulikjak Update configure

Do you use "make regen-configure"? See: https://docs.python.org/dev/using/configure.html#generated-files

@kulikjak: Anyway, I push directly into your branch to fix the configure script (regenerate it).

@vstinner vstinner enabled auto-merge (squash) October 6, 2023 13:51
Modules/socketmodule.c Outdated Show resolved Hide resolved
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM.

@vstinner vstinner disabled auto-merge October 6, 2023 13:51
@vstinner vstinner enabled auto-merge (squash) October 6, 2023 13:52
@vstinner
Copy link
Member

vstinner commented Oct 6, 2023

@kulikjak: I also replaced Py_SUNOS_VERSION == 510 with Py_SUNOS_VERSION <= 510.

@kulikjak
Copy link
Contributor Author

kulikjak commented Oct 9, 2023

Do you use "make regen-configure"? See: https://docs.python.org/dev/using/configure.html#generated-files

Sigh, sorry. Normally, I do, but this time I thought that I could quickly edit it myself, ... and I was wrong. Thanks for fixing it.

Is this a bug-fix? If so, we should backport it and add a NEWS entry.

I guess it is (fixing a build on Solaris <= 10). I can add a NEWS entry, although now there is a skip-news tag, so I am not sure whether I should do so?

Other than that, the build failure doesn't seem to be related, so that's good.

@erlend-aasland erlend-aasland added the needs backport to 3.11 only security fixes label Oct 9, 2023
@erlend-aasland erlend-aasland added the needs backport to 3.12 bug and security fixes label Oct 9, 2023
@vstinner vstinner merged commit 3b1580a into python:main Oct 9, 2023
27 checks passed
@miss-islington
Copy link
Contributor

Thanks @kulikjak for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 9, 2023
…109447)

Add OS version specific macro for Solaris: Py_SUNOS_VERSION.
(cherry picked from commit 3b1580a)

Co-authored-by: Jakub Kulík <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Oct 9, 2023

GH-110580 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 bug and security fixes label Oct 9, 2023
@bedevere-app
Copy link

bedevere-app bot commented Oct 9, 2023

GH-110581 is a backport of this pull request to the 3.11 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.11 only security fixes label Oct 9, 2023
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 9, 2023
…109447)

Add OS version specific macro for Solaris: Py_SUNOS_VERSION.
(cherry picked from commit 3b1580a)

Co-authored-by: Jakub Kulík <[email protected]>
@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot s390x SLES 3.x has failed when building commit 3b1580a.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/540/builds/6801) and take a look at the build logs.
  4. Check if the failure is related to this commit (3b1580a) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/540/builds/6801

Failed tests:

  • test_signal

Failed subtests:

  • test_stress_modifying_handlers - test.test_signal.StressTest.test_stress_modifying_handlers

Summary of the results of the build (if available):

==

Click to see traceback logs
Traceback (most recent call last):
  File "/home/dje/cpython-buildarea/3.x.edelsohn-sles-z/build/Lib/test/test_signal.py", line 1374, in test_stress_modifying_handlers
    self.assertGreater(num_received_signals, 0)
AssertionError: 0 not greater than 0

erlend-aasland pushed a commit that referenced this pull request Oct 10, 2023
… (#110580)

Add OS version specific macro for Solaris: Py_SUNOS_VERSION.
(cherry picked from commit 3b1580a)

Co-authored-by: Jakub Kulík <[email protected]>
erlend-aasland pushed a commit that referenced this pull request Oct 10, 2023
… (#110581)

Add OS version specific macro for Solaris: Py_SUNOS_VERSION.
(cherry picked from commit 3b1580a)

Co-authored-by: Jakub Kulík <[email protected]>
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
…9447)

Add OS version specific macro for Solaris: Py_SUNOS_VERSION.
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.

5 participants