Skip to content

host_os_name value in config-scripts has changed to 'solaris'#704

Merged
michaelrsweet merged 5 commits intoOpenPrinting:masterfrom
l1gi:master
Jun 1, 2023
Merged

host_os_name value in config-scripts has changed to 'solaris'#704
michaelrsweet merged 5 commits intoOpenPrinting:masterfrom
l1gi:master

Conversation

@l1gi
Copy link
Copy Markdown

@l1gi l1gi commented May 18, 2023

Variable host_os_name value has changed on Solaris from sunos to solaris some time ago. This change updates config-scripts to keep values used for Solaris 5.10 (sunos), but also sets relevant options on Solaris 5.11 (solaris). This change decreases the number of configure options necessary to build CUPS on current Solaris.

Please consider merge.

Thank you,
m.

@l1gi
Copy link
Copy Markdown
Author

l1gi commented May 18, 2023

Could I kindly ask you to review this Solaris specific change, please?

Thank you,
m.

@LumioseSil
Copy link
Copy Markdown
Contributor

LumioseSil commented May 18, 2023

Make sure to run autoconf --force @l1gi

@michaelrsweet
Copy link
Copy Markdown
Member

@AtariDreams NEVER run "aclocal" on the CUPS repository - it will damage the config.h.in file and render the local sources unbuildable. We do not use automake or aclocal, just autoconf (autoconf -f) to rebuild the configure script.

@michaelrsweet
Copy link
Copy Markdown
Member

@l1gi I've been in standards meetings all week, but just FYI it is not reasonable to expect code review after 6 hours.

Copy link
Copy Markdown
Member

@michaelrsweet michaelrsweet left a comment

Choose a reason for hiding this comment

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

Please respond to my question about /system. Also, you missed the sunos reference in the cups-threads.m4 file.

Comment thread config-scripts/cups-directories.m4 Outdated
# Darwin (macOS)
CUPS_STATEDIR="$CUPS_SERVERROOT"
], [solaris*], [
CUPS_STATEDIR="/system/volatile/cups"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not having used a recent version of Solaris, this looks like something that needs more checking - certainly the last version of Solaris that CUPS actually supported did not have a /system hierarchy, so I'd expect there to be a directory existence check.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

True, will update this.

@michaelrsweet michaelrsweet self-assigned this May 18, 2023
@michaelrsweet michaelrsweet added enhancement New feature or request priority-low platform issue Issue is specific to an OS or desktop labels May 18, 2023
@l1gi
Copy link
Copy Markdown
Author

l1gi commented May 19, 2023

Hi Michael,

thanks for looking into this. Your objections caused I did more testing on how that behaves on Solaris 10 and I must say I was wrong. Even on Solaris 10 (which is the last Oracle supports) the host_os_name is already solaris. So the question is how sunos got there and whether we should keep it or replace it by solaris. What do you think (or remember)? :)

Solaris 11 builds with or without -D_POSIX_PTHREAD_SEMANTICS, but Solaris 10 needs this. I will take care of this one also.

Thanks for any advice,
m.

@michaelrsweet
Copy link
Copy Markdown
Member

WRT what host_os_name contains, that is controlled by the GNU config.sub/guess scripts which have been updated several times over the years. And prior to us using that we just use the uname output, which probably still reports SunOS on Solaris (but I really don't have a way to test that...)

Anyways, checking the config.sub/guess scripts, it looks like they still report sunos on platforms that are running pre-Solaris builds (if you'd even be able to get working hardware for that!) so for now let's use both sunos and solaris in the configure scripts.

@l1gi
Copy link
Copy Markdown
Author

l1gi commented May 26, 2023

WRT what host_os_name contains, that is controlled by the GNU config.sub/guess scripts which have been updated several times over the years. And prior to us using that we just use the uname output, which probably still reports SunOS on Solaris (but I really don't have a way to test that...)

That's true of course:

$ uname -a
SunOS ulx-0 5.11 11.4.60.147.0 i86pc i386 i86pc non-virtualized

Anyways, checking the config.sub/guess scripts, it looks like they still report sunos on platforms that are running pre-Solaris builds (if you'd even be able to get working hardware for that!) so for now let's use both sunos and solaris in the configure scripts.

Okay, so I have just added solaris branches where necessary leaving sunos as-is and made a directory check for CUPS_STATEDIR.

@LumioseSil
Copy link
Copy Markdown
Contributor

@michaelrsweet Thoughts?

Copy link
Copy Markdown
Member

@michaelrsweet michaelrsweet left a comment

Choose a reason for hiding this comment

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

Still needs a few tweaks...

CUPS_STATEDIR="$CUPS_SERVERROOT"
], [sun* | solaris*], [
AS_IF([test -d /system/volatile], [
CUPS_STATEDIR="/system/volatile/cups"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Need an 'else' clause for this.

Comment thread config-scripts/cups-threads.m4 Outdated
# Solaris requires -D_POSIX_PTHREAD_SEMANTICS to be POSIX-
# compliant... :(
AS_IF([test $host_os_name = sunos], [
AS_IF([test $host_os_name = solaris], [
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Needs to look for both sunos and solaris here, something like:

AS_IF([test $host_os_name = sunos -o $host_os_name = solaris], [

@zdohnal zdohnal added this to the v2.4.x milestone Jun 1, 2023
@l1gi
Copy link
Copy Markdown
Author

l1gi commented Jun 1, 2023

Let me know if it's okay now.

Thank you,
Martin

Copy link
Copy Markdown
Member

@michaelrsweet michaelrsweet left a comment

Choose a reason for hiding this comment

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

Looks good now, thank you!

@michaelrsweet michaelrsweet merged commit 1878df8 into OpenPrinting:master Jun 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request platform issue Issue is specific to an OS or desktop priority-low

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants