In CMake conversioned TDE modules, USE_SOLARIS is still used for Solaris support codes.
Then, Solaris support codes will not be used on Solaris.
Steps to reproduce
build TDE modules with CMake on Solaris
OS depending features will not work well
Screenshots
<!--
This is a comment.
Please fill in the required fields below.
The comments provide instructions on how to do so.
Note: You do not need to remove comments.
-->
## Basic information
- TDE version: N/A <!-- such as R14.0.10 - see tde-config -v -->
- Distribution: Solaris <!-- such as Debian Buster - see lsb_release -sd -->
- Hardware: N/A <!-- amd64 / i386 / armhf / ... -->
<!--
Use SL/* labels to set the severity level.
Please do not set a milestone.
-->
## Description
With autoconf/automake, `USE_SOLARIS` is defined as CPPFLAGS for Solaris:
https://mirror.git.trinitydesktop.org/gitea/TDE/tde-common-admin/src/branch/master/acinclude.m4.in#L4341
But with CMake, it is not defined.
In CMake conversioned TDE modules, `USE_SOLARIS` is still used for Solaris support codes.
Then, Solaris support codes will not be used on Solaris.
## Steps to reproduce
1. build TDE modules with CMake on Solaris
2. OS depending features will not work well
## Screenshots
<!-- If it seems useful, please provide provide one or more screenshots. -->
#ifdef USE_SOLARIS can be safely replaced with #if (defined(sun) || defined(__sun)) && (defined(__SVR4) || defined(__svr4__))
But #ifdef __sun should be good enough.
`#ifdef USE_SOLARIS` can be safely replaced with
`#if (defined(sun) || defined(__sun)) && (defined(__SVR4) || defined(__svr4__))`
But `#ifdef __sun` should be good enough.
I understand correctly that instead of adding the definition of USE_SOLARIS at the global level of TDE CMake macros, we will solve the use of #ifdef __sun in modules where it is needed?
I understand correctly that instead of adding the definition of `USE_SOLARIS` at the global level of TDE CMake macros, we will solve the use of `#ifdef __sun` in modules where it is needed?
Hi Obata-san,
maybe I misunderstood what you wanted to do.
I see USE_SOLARIS being used in admin module, when we build packages with autotools/automake. I don't see USE_SOLARIS in cmake-trinity, so I thought you intended to add support for it in USE_SOLARIS.
Re-reading your comment again, I now understand you are suggesting to replace USE_SOLARIS everywhere in the code with #if (defined(sun) || defined(__sun)) && (defined(__SVR4) || defined(__svr4__)).
Is that correct? If so, I can work on it in coming days but I won't be able to test. Would you like me to prepare PR for you to test each individual module or are you happy enough if I push to master/R14.0.x and you just rebuild?
Hi Obata-san,
maybe I misunderstood what you wanted to do.
I see USE_SOLARIS being used in admin module, when we build packages with autotools/automake. I don't see USE_SOLARIS in cmake-trinity, so I thought you intended to add support for it in USE_SOLARIS.
Re-reading your comment again, I now understand you are suggesting to replace USE_SOLARIS everywhere in the code with ```#if (defined(sun) || defined(__sun)) && (defined(__SVR4) || defined(__svr4__))```.
Is that correct? If so, I can work on it in coming days but I won't be able to test. Would you like me to prepare PR for you to test each individual module or are you happy enough if I push to master/R14.0.x and you just rebuild?
I just found the issue, but not ready to test it, so created this ticket for the reminder (intend to be meta ticket).
So I'm happy enough with just changed codes.
defined(sun) is for ancient compiler(?), defined(__SVR4) || defined(__svr4__) is "is SVR4" i.e. "not for SunOS 4", then defined(__sun) should be used here.
I just found the issue, but not ready to test it, so created this ticket for the reminder (intend to be meta ticket).
So I'm happy enough with just changed codes.
`defined(sun)` is for ancient compiler(?), `defined(__SVR4) || defined(__svr4__)` is "is SVR4" i.e. "not for SunOS 4", then `defined(__sun)` should be used here.
In the code there is also _OS_SOLARIS_ and Q_OS_SOLARIS.
The first one is not defined anywhere, so I guess we can replace it with __sun as well. The second one is defined in TQt3 if __sun is defined. So the question is whether to leave Q_OS_SOLARIS as is or simply replace it with __sun.
What do you think? (@SlavekB @obache)
In the code there is also ```_OS_SOLARIS_``` and ```Q_OS_SOLARIS```.
The first one is not defined anywhere, so I guess we can replace it with ```__sun``` as well. The second one is defined in TQt3 if ```__sun``` is defined. So the question is whether to leave ```Q_OS_SOLARIS``` as is or simply replace it with ```__sun```.
What do you think? (@SlavekB @obache)
As far as I know, TQt3 defines Q_OS... to make it certainty that it is uniform and unambiguous. Therefore, a good idea seems to leave these definitions in TQt3. However, in other code it seems that these Q_<something> constants are used rarely. It is hard to decide whether to prioritize these constants specific to TQt or whether to stick to standard constants, although quite inconsistent.
As far as I know, TQt3 defines `Q_OS...` to make it certainty that it is uniform and unambiguous. Therefore, a good idea seems to leave these definitions in TQt3. However, in other code it seems that these `Q_<something>` constants are used rarely. It is hard to decide whether to prioritize these constants specific to TQt or whether to stick to standard constants, although quite inconsistent.
Thanks @SlavekB. Over the night I thought it may be good to use Q_OS_* across the code. So leave the Q_OS_SOLARIS define in TQt3 and replace __sun/__solaris__ with Q_OS_SOLARIS in the other modules.
The reason for this is to have a unique central place where we detect the OS and use it uniformly across the rest of the code base.
What do you think?
Thanks @SlavekB. Over the night I thought it may be good to use Q_OS_* across the code. So leave the Q_OS_SOLARIS define in TQt3 and replace ```__sun/__solaris__``` with Q_OS_SOLARIS in the other modules.
The reason for this is to have a unique central place where we detect the OS and use it uniformly across the rest of the code base.
What do you think?
Basic information
Description
With autoconf/automake,
USE_SOLARIS
is defined as CPPFLAGS for Solaris:https://mirror.git.trinitydesktop.org/gitea/TDE/tde-common-admin/src/branch/master/acinclude.m4.in#L4341
But with CMake, it is not defined.
In CMake conversioned TDE modules,
USE_SOLARIS
is still used for Solaris support codes.Then, Solaris support codes will not be used on Solaris.
Steps to reproduce
Screenshots
#ifdef USE_SOLARIS
can be safely replaced with#if (defined(sun) || defined(__sun)) && (defined(__SVR4) || defined(__svr4__))
But
#ifdef __sun
should be good enough.Hi Obata-san,
could you please prepare a PR for this fix? Thanks a lot 😄
Can someone create the list affected modules?
I understand correctly that instead of adding the definition of
USE_SOLARIS
at the global level of TDE CMake macros, we will solve the use of#ifdef __sun
in modules where it is needed?Hi Obata-san,
maybe I misunderstood what you wanted to do.
I see USE_SOLARIS being used in admin module, when we build packages with autotools/automake. I don't see USE_SOLARIS in cmake-trinity, so I thought you intended to add support for it in USE_SOLARIS.
Re-reading your comment again, I now understand you are suggesting to replace USE_SOLARIS everywhere in the code with
#if (defined(sun) || defined(__sun)) && (defined(__SVR4) || defined(__svr4__))
.Is that correct? If so, I can work on it in coming days but I won't be able to test. Would you like me to prepare PR for you to test each individual module or are you happy enough if I push to master/R14.0.x and you just rebuild?
I just found the issue, but not ready to test it, so created this ticket for the reminder (intend to be meta ticket).
So I'm happy enough with just changed codes.
defined(sun)
is for ancient compiler(?),defined(__SVR4) || defined(__svr4__)
is "is SVR4" i.e. "not for SunOS 4", thendefined(__sun)
should be used here.Ok, thanks @obache. I will work on replacing
#ifdef USE_SOLARIS
with#ifdef __sun
in coming days/weeks.@obache could you please check TDE/smb4k#4 to make sure what I didn is ok for you?
In the code there is also
_OS_SOLARIS_
andQ_OS_SOLARIS
.The first one is not defined anywhere, so I guess we can replace it with
__sun
as well. The second one is defined in TQt3 if__sun
is defined. So the question is whether to leaveQ_OS_SOLARIS
as is or simply replace it with__sun
.What do you think? (@SlavekB @obache)
As far as I know, TQt3 defines
Q_OS...
to make it certainty that it is uniform and unambiguous. Therefore, a good idea seems to leave these definitions in TQt3. However, in other code it seems that theseQ_<something>
constants are used rarely. It is hard to decide whether to prioritize these constants specific to TQt or whether to stick to standard constants, although quite inconsistent.Thanks @SlavekB. Over the night I thought it may be good to use Q_OS_* across the code. So leave the Q_OS_SOLARIS define in TQt3 and replace
__sun/__solaris__
with Q_OS_SOLARIS in the other modules.The reason for this is to have a unique central place where we detect the OS and use it uniformly across the rest of the code base.
What do you think?
Yes, the use of constants
Q_OS_<something>
seems like a good idea because the names of these constants are well consistent.