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

Define dirent d_type for Solaris based OS #34263

Merged
merged 1 commit into from
Apr 4, 2020
Merged

Define dirent d_type for Solaris based OS #34263

merged 1 commit into from
Apr 4, 2020

Conversation

am11
Copy link
Member

@am11 am11 commented Mar 29, 2020

  1. On Solaris-like operating systems such as SmartOS, struct dirent does not contain a member called d_type. Although we have a comment about it:

    // Handle symlinks and file systems that do not support d_type
    but that itself is in a switch block over d_type. This delta defines d_type's DT_UNKNOWN and friends for Solaris.

  2. _X is defined in one of the system header, so undef it before #define:

    /usr/include/iso/ctype_iso.h:60:0: note: this is the location of the previous definition
     #define _X 0x00000080 /* heXadecimal digit */
    
  3. GCC build of installer was broken even on Linux, due to missing dl link in test project. Align it with what we are already using in src projects:

    target_link_libraries (${DOTNET_PROJECT_NAME} "dl")

Contributes to #4173.

@am11 am11 marked this pull request as ready for review March 29, 2020 22:00
@am11
Copy link
Member Author

am11 commented Mar 29, 2020

With these changes:

src/installer/corehost/build.sh -apphostver "5.0.0-dev" -hostver "5.0.0-dev" \
  -fxrver "5.0.0-dev" -policyver "5.0.0-dev" -commithash $(git rev-parse HEAD) -gcc

succeeds on SmartOS amd64. There is another layer of shared configuration for SunOS, which is based on top of #34211, I will create a separate PR once these are merged.

cc @jkotas

@jkotas jkotas requested review from lpereira and elinor-fung March 30, 2020 03:12
@am11
Copy link
Member Author

am11 commented Mar 30, 2020

Libraries tests failures are related to #28553.

eng/native/configurecompiler.cmake Show resolved Hide resolved
src/installer/corehost/cli/hostmisc/pal.unix.cpp Outdated Show resolved Hide resolved
src/installer/corehost/cli/test/nativehost/CMakeLists.txt Outdated Show resolved Hide resolved
@am11 am11 changed the title Use POSIX compliant way of checking file stat Define dirent d_type for Solaris based OS Mar 31, 2020
@am11
Copy link
Member Author

am11 commented Apr 1, 2020

Resolved merge conflict and updated title to reflect what PR is doing now.

Copy link
Contributor

@lpereira lpereira left a comment

Choose a reason for hiding this comment

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

LGTM with a suggestion

src/installer/corehost/cli/common.cmake Outdated Show resolved Hide resolved
@am11
Copy link
Member Author

am11 commented Apr 4, 2020

@jkotas, seems like two CI legs are green on AzDo but GitHub hasn't sync in past two days:

System.Net.Security.Tests.ClientAsyncAuthenticateTest.ClientAsyncAuthenticate_ServerNoEncryption_NoConnect failure from Libraries Test Run release coreclr Windows_NT x86 Release leg seems unrelated, as PR does not touch Libraries (even :/eng/native/configurecompiler.cmake is not used by libraries - yet). However, there was no issue reported for it previously so I have filed #34545.

@jkotas jkotas merged commit 9f0c540 into dotnet:master Apr 4, 2020
@am11 am11 deleted the feature/solaris/installer-port branch April 4, 2020 16:28
@ghost ghost locked as resolved and limited conversation to collaborators Dec 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants