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

Solaris 10 #364

Merged
merged 3 commits into from
May 26, 2021
Merged

Solaris 10 #364

merged 3 commits into from
May 26, 2021

Conversation

pekdon
Copy link
Contributor

@pekdon pekdon commented Apr 30, 2021

A few compile fixes for Solaris 10.

@coveralls
Copy link

coveralls commented May 1, 2021

Coverage Status

Coverage decreased (-0.004%) to 71.856% when pulling 42cbb47 on pekdon:solaris-10 into b3243a7 on p11-glue:master.

configure.ac Outdated
@@ -173,6 +173,15 @@ if test "$os_unix" = "yes"; then
AC_MSG_RESULT([yes])],
[AC_MSG_RESULT([no])])

# Check if SUN_LEN is defined
AC_LINK_IFELSE([AC_LANG_PROGRAM([[#include <sys/un.h>]],
Copy link
Member

Choose a reason for hiding this comment

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

I would like to minimize the number of checks in configure, as we also maintain meson build. Is the #ifdef SUN_LEN check in compat.h insufficient for any reason?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should be sufficient if sys/un.h is included in compat or always included before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the PR branch with that change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ueno any more feedback on this one, something you want me to fix?

Copy link
Member

Choose a reason for hiding this comment

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

Thank you for the update and apologies for the delay; I somehow missed the notification.

@pekdon
Copy link
Contributor Author

pekdon commented May 7, 2021

Ok with this PR or are you waiting for me to provide something else?

common/compat.h Outdated
@@ -39,6 +39,7 @@

#include <sys/types.h>
#include <sys/stat.h>
#include <sys/un.h>
Copy link
Member

Choose a reason for hiding this comment

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

In the mingw CI:

../common/compat.h:42:10: fatal error: sys/un.h: No such file or directory
   42 | #include <sys/un.h>
      |          ^~~~~~~~~~

I suppose a configure check is nevertheless needed :-(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I'll update the PR

pekdon added 3 commits May 25, 2021 18:43
cp -v does not work on Solaris 10, nor does $() with /bin/sh so instead
go for non verbose as -e will echo the commands performed anyway and
use backticks.
Defining _XOPEN_SOURCE 700 on Solaris makes the compile fail due to
a feature check in sys/feature_tests.h, C99 compiler in combination
with < XPG6 feature set is not supported.
SUN_LEN is not available on Solaris 10, provide a fallback.
@ueno ueno merged commit 96c7b93 into p11-glue:master May 26, 2021
@ueno
Copy link
Member

ueno commented May 26, 2021

Thank you!

@ueno ueno added this to the 0.24.0 milestone Jun 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants