GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-78469: Declare missing sethostname for Solaris 10 by kulikjak · Pull Request #109447 · python/cpython

Conversation

@kulikjak

@kulikjak kulikjak commented Sep 15, 2023 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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.

  • Issue: Declare sethostname in socketmodule.c for SOLARIS #78469

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
kulikjak added 2 commits September 15, 2023 11:54
@kulikjak
Add OS version specific macro for Solaris
e71d8fb
@kulikjak
declare sethostname in socketmodule.c for Solaris 10
5593f21
@kulikjak
kulikjak requested review from corona10 and erlend-aasland as code owners September 15, 2023 13:03
@bedevere-app bedevere-app Bot added the awaiting review label Sep 15, 2023
@bedevere-app bedevere-app Bot mentioned this pull request Sep 15, 2023
Declare sethostname in socketmodule.c for SOLARIS #78469
Closed
vstinner
vstinner reviewed Sep 15, 2023
View reviewed changes
Comment thread Modules/socketmodule.c Outdated

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

@vstinner vstinner Sep 15, 2023

Copy link
Copy Markdown
Member

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread configure.ac
# 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.])

@vstinner vstinner Sep 15, 2023

Copy link
Copy Markdown
Member

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 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 :-(

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@vstinner vstinner Sep 15, 2023

Copy link
Copy Markdown
Member

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@erlend-aasland erlend-aasland Sep 18, 2023

Copy link
Copy Markdown
Contributor

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@kulikjak kulikjak Oct 5, 2023

Copy link
Copy Markdown
Contributor Author

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 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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@vstinner

vstinner commented Sep 15, 2023

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@kulikjak
rename SUNOS_VERSION to Py_SUNOS_VERSION (PEP 7 naming conventions)
bc5523e
@kulikjak

kulikjak commented Oct 5, 2023

Copy link
Copy Markdown
Contributor Author

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
vstinner
vstinner reviewed Oct 5, 2023
View reviewed changes
Comment thread configure.ac Resolved

Uh oh!

There was an error while loading. Please reload this page.
kulikjak and others added 2 commits October 6, 2023 14:15
@kulikjak @vstinner
Update configure.ac
3a96e29 
Co-authored-by: Victor Stinner <[email protected]>
@kulikjak
Update configure
691441c
erlend-aasland
erlend-aasland approved these changes Oct 6, 2023
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting core review and removed awaiting review labels Oct 6, 2023
@erlend-aasland

erlend-aasland commented Oct 6, 2023

Copy link
Copy Markdown
Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner
Run make regen-configure
f8f49e4
@vstinner

vstinner commented Oct 6, 2023

Copy link
Copy Markdown
Member

@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).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner
vstinner enabled auto-merge (squash) October 6, 2023 13:51
vstinner
vstinner reviewed Oct 6, 2023
View reviewed changes
Comment thread Modules/socketmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
vstinner
vstinner approved these changes Oct 6, 2023
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

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.

LGTM.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@vstinner
vstinner disabled auto-merge October 6, 2023 13:51
@vstinner
Update Modules/socketmodule.c
5970802
@vstinner vstinner added the skip news label Oct 6, 2023
@vstinner
vstinner enabled auto-merge (squash) October 6, 2023 13:52
@vstinner

vstinner commented Oct 6, 2023

Copy link
Copy Markdown
Member

@kulikjak: I also replaced Py_SUNOS_VERSION == 510 with Py_SUNOS_VERSION <= 510.
kulikjak reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@kulikjak

kulikjak commented Oct 9, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@erlend-aasland
Merge branch 'main' into Solaris10-sethostname
9cb0a31
@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 only security fixes label Oct 9, 2023
@vstinner
vstinner merged commit 3b1580a into python:main Oct 9, 2023
@miss-islington

miss-islington commented Oct 9, 2023

Copy link
Copy Markdown
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!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the awaiting core review label Oct 9, 2023
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 9, 2023
@kulikjak @miss-islington
pythongh-78469: Declare missing sethostname for Solaris 10 (pythonGH-…
12f0640 
…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

bedevere-app Bot commented Oct 9, 2023

Copy link
Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.12 only security fixes label Oct 9, 2023
@bedevere-app

bedevere-app Bot commented Oct 9, 2023

Copy link
Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@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
@kulikjak @miss-islington
pythongh-78469: Declare missing sethostname for Solaris 10 (pythonGH-…
2d973c1 
…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

bedevere-bot commented Oct 9, 2023

Copy link
Copy Markdown

⚠️⚠️⚠️ 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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
erlend-aasland pushed a commit that referenced this pull request Oct 10, 2023
@miss-islington @kulikjak
[3.12] gh-78469: Declare missing sethostname for Solaris 10 (GH-109447)…
190660a 
… (#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
@miss-islington @kulikjak
[3.11] gh-78469: Declare missing sethostname for Solaris 10 (GH-109447)…
1942721 
… (#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
@kulikjak @Glyphack
pythongh-78469: Declare missing sethostname for Solaris 10 (python#10…
dc2bf28 
…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

Reviewers

@vstinner vstinner vstinner approved these changes
@erlend-aasland erlend-aasland erlend-aasland approved these changes
@corona10 corona10 Awaiting requested review from corona10 corona10 is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@kulikjak @vstinner @erlend-aasland @miss-islington @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.