Skip to content

Conversation

am11
Copy link
Member

@am11 am11 commented Jul 19, 2021

  • gcc looks for fallthrough or break in morph (we could move the break out of condition, but there are a few gotos to that case which I am not sure might have sideeffect, so i kept it simple).
  • SHM_ANON is not available in illumos/Solaris libc (POSIX does not require it), so manually generate name using timespec for shm_open() call.
  • in line with previous fixes made for statfs, SunOS-like OS has deprecated statfs in favor of statvfs. The f_type does not return magic number as other Unices. The more reliable way is to get the stringly name from f_basetype and then resolve the enum value. Assuming performance is not a concern for DriveInfo, kept it simple.

@ghost
Copy link

ghost commented Jul 19, 2021

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@am11 am11 force-pushed the fix/illumos-build branch 2 times, most recently from 0945a73 to 638ffb9 Compare July 19, 2021 15:40
@am11 am11 marked this pull request as ready for review July 19, 2021 15:40
@am11
Copy link
Member Author

am11 commented Jul 19, 2021

cc @janvorli,

docker run -e ROOTFS_DIR=/crossrootfs/x64 -v $(pwd):/runtime \
    mcr.microsoft.com/dotnet-buildtools/prereqs:ubuntu-18.04-cross-illumos-20210714135645-f13d79e \
    /runtime/build.sh -arch x64 -os illumos -cross -gcc

(entire build) continue to succeed with this patch.

@janvorli
Copy link
Member

The changes look good, but I'd like some libraries folks to review the related part of the change. @stephentoub could you please take a look or add someone else to review this?

@terrajobst terrajobst added the community-contribution Indicates that the PR has been added by a community member label Jul 19, 2021
@am11 am11 force-pushed the fix/illumos-build branch 2 times, most recently from 661bd03 to 810b4ae Compare July 20, 2021 10:29
@am11
Copy link
Member Author

am11 commented Jul 30, 2021

@stephentoub, @adamsitnik. the library change for official platforms has no diffs in IL or PAL binaries. I have split a partial class and updated the native implementation using existing macros, which indicates if system has a legacy struct statfs (Solaris deprecated it during the last decade) and uses stat statvfs instead which provides the filesystem name (string) instead of its ID (number). Please take a look. :)

@stephentoub
Copy link
Member

Sorry, I missed the notification when I was tagged. Looking now...

@am11
Copy link
Member Author

am11 commented Jul 31, 2021

Windows x86 failure is unrelated (and fixed in main branch: #56649).

Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

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

System.IO part LGTM.

@am11 big thanks for another great contribution! And kudos for adding extra file system types to our existing enumeration!

@adamsitnik adamsitnik added this to the 6.0.0 milestone Aug 4, 2021
@adamsitnik adamsitnik merged commit 12a012a into dotnet:main Aug 5, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Sep 7, 2021
@am11 am11 deleted the fix/illumos-build branch April 3, 2023 23:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants