Skip to content

Allow build on Solaris 11.4 #2453

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Allow build on Solaris 11.4 #2453

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ if(WIN32)
set(CMAKE_C_FLAGS "/Zc:preprocessor /W3 /wd4711 /w14013 /utf-8 ${CMAKE_C_FLAGS}")
else()
# global C flags
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Wextra -Wpedantic -std=c11")
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Wextra -Wpedantic")
Copy link
Member

@michalvasko michalvasko Nov 25, 2025

Choose a reason for hiding this comment

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

Sorry, but the C standard needs to be specified because various compilers use different default standards. And before deciding the exact solution I would like to know the details of the issue. Is the compiler too old on Solaris that does not support this C revision?

Copy link
Author

@psumbera psumbera Nov 26, 2025

Choose a reason for hiding this comment

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

Without this, declarations for vasprintf and strndup are missing:

/scratch/psumbera/libyang/src/ly_common.c:704:11: error: implicit declaration of function 'vasprintf'; did you mean 'vsprintf'? [-Wimplicit-function-declaration]
  704 |     len = vasprintf(&addition, format, fp);
      |           ^~~~~~~~~
      |           vsprintf
/scratch/psumbera/libyang/src/ly_common.c: In function 'ly_parse_int':
/scratch/psumbera/libyang/src/ly_common.c:733:11: error: implicit declaration of function 'strndup'; did you mean 'strncmp'? [-Wimplicit-function-declaration]
  733 |     str = strndup(val_str, val_len);
      |           ^~~~~~~
      |           strncmp
/scratch/psumbera/libyang/src/ly_common.c:733:9: error: assignment to 'char *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
  733 |     str = strndup(val_str, val_len);
      |         ^
/scratch/psumbera/libyang/src/ly_common.c: In function 'ly_parse_uint':
/scratch/psumbera/libyang/src/ly_common.c:773:9: error: assignment to 'char *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
  773 |     str = strndup(val_str, val_len);
      |         ^

Note that you can play with Solaris via https://github.com/vmactions/shell-solaris .

Copy link
Member

@michalvasko michalvasko Nov 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Seems like there are some macro definitions missing, you can learn about those in the man pages. The VM seems nice but to have a lasting effect we would need to add it into our automatic actions, which does not seem possible.

Copy link
Author

@psumbera psumbera Nov 26, 2025

Choose a reason for hiding this comment

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

As for macros. This is also possibiltiy:

--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -77,6 +77,9 @@ if(WIN32)
 else()
     # global C flags
     set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Wextra -Wpedantic -std=c11")
+    if(${CMAKE_SYSTEM_NAME} STREQUAL "SunOS")
+        set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -D__EXTENSIONS__")
+    endif()
 endif()
 
 include_directories(${PROJECT_BINARY_DIR}/libyang ${PROJECT_SOURCE_DIR}/src ${PROJECT_SOURCE_DIR}/src/plugins_exts)

As for Solaris CI. It's possible: https://github.com/rust-lang/libc/blob/d58c1f4f9ae01648b342d7dead291d08655677c4/.github/workflows/ci.yaml#L278

Copy link
Member

@michalvasko michalvasko Nov 27, 2025

Choose a reason for hiding this comment

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

Fine although the macro looks very non-standard. Better adding macros than changing the C standard. As for the CI, I may take a look at it later and add an action for Solaris in our CI.

endif()

include_directories(${PROJECT_BINARY_DIR}/libyang ${PROJECT_SOURCE_DIR}/src ${PROJECT_SOURCE_DIR}/src/plugins_exts)
Expand Down
1 change: 1 addition & 0 deletions CMakeModules/FindPCRE2.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ else()
pcre2.h
PATHS
/usr/include
/usr/include/pcre
/usr/local/include
/opt/local/include
/sw/include
Expand Down
8 changes: 8 additions & 0 deletions src/tree_schema.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2370,7 +2370,11 @@ lys_search_localfile_file_type(const struct dirent *file, const char *wd, struct

*skip = 0;

#ifdef __sun__
if (1) {
#else
if ((file->d_type == DT_UNKNOWN) || (file->d_type == DT_LNK)) {
#endif
Comment on lines +2373 to +2377
Copy link
Member

@michalvasko michalvasko Nov 25, 2025

Choose a reason for hiding this comment

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

Please explain because just looking at this I do not understand.

Copy link
Author

@psumbera psumbera Nov 26, 2025

Choose a reason for hiding this comment

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

d_type is not part of struct dirent on Solaris.

Copy link
Member

@michalvasko michalvasko Nov 26, 2025

Choose a reason for hiding this comment

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

I see, then it should ideally be properly detected in by cmake and then handled here. I can add that and it may be faster, if you want.

/* FS does not support this field or its a symbolic link, need to call stat */
if (asprintf(&str, "%s/%s", wd, file->d_name) == -1) {
LOGMEM(NULL);
Expand All @@ -2387,13 +2391,17 @@ lys_search_localfile_file_type(const struct dirent *file, const char *wd, struct
/* stat - file */
is_reg = 1;
}
#ifdef __sun__
}
#else
} else if (file->d_type == DT_DIR) {
/* dirent - dir */
is_dir = 1;
} else if (file->d_type == DT_REG) {
/* dirent - file */
is_reg = 1;
}
#endif

if (is_dir && (dirs->count || !implicit_cwd)) {
/* we have another subdirectory in searchpath to explore,
Expand Down