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

Fix illumos build regressions #55916

Merged
merged 9 commits into from
Aug 5, 2021
Merged

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.

@dotnet-issue-labeler
Copy link

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!

Co-authored-by: Adam Sitnik <adam.sitnik@gmail.com>
@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