Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Fix compilation for deprecated API on macOS Mojave preview#30716

Merged
maryamariyan merged 2 commits into
dotnet:masterfrom
maryamariyan:help-mojave
Jun 28, 2018
Merged

Fix compilation for deprecated API on macOS Mojave preview#30716
maryamariyan merged 2 commits into
dotnet:masterfrom
maryamariyan:help-mojave

Conversation

@maryamariyan
Copy link
Copy Markdown

@maryamariyan maryamariyan commented Jun 28, 2018

Relates to: #30599

cc: @danmosemsft @bartonjs

@maryamariyan maryamariyan added the os-mac-os-x OS-X aka Mac OS label Jun 28, 2018
@maryamariyan maryamariyan self-assigned this Jun 28, 2018
@danmoseley
Copy link
Copy Markdown
Member

This is only a temporary workaround... Is it possible to add a comment in this file with the issue you are using to track checking in your correct fix?

@danmoseley
Copy link
Copy Markdown
Member

I assume that issue is not #30599 since this issue is currently set to close it

@maryamariyan
Copy link
Copy Markdown
Author

@danmosemsft changing from "fixes" to "related to", so closing the PR won't close the issue.

Comment thread src/Native/Unix/CMakeLists.txt Outdated

add_compile_options(-Wno-format-nonliteral)
add_compile_options(-Wno-disabled-macro-expansion)
add_compile_options(-Wno-deprecated-declarations)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are there multiple projects that need this? If it's just the crypto shim then I'd expect this to be in that specific file.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should be moved to src/Native/Unix/System.Security.Cryptography.Native.Apple/CMakeLists.txt

Copy link
Copy Markdown

@ghost ghost Jun 28, 2018

Choose a reason for hiding this comment

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

Currently there is only one place that defines these: this file. To be consistent with the current norm, should it be kept here?
When the second project will require it, nobody will remember that it is also in a project-specific file to move at global location.


The second one-off approach is in the .c file, such as:

#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wpadded"
typedef struct
{
uint64_t length;
uint8_t* data;
} PAL_GssBuffer;
#pragma clang diagnostic pop

and

#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wsign-compare"
#endif
return CMSG_NXTHDR(mhdr, cmsg);
#ifndef __GLIBC__
#pragma clang diagnostic pop
#endif

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Currently there is only one place that defines these: this file. To be consistent with the current norm, should it be kept here?

I believe that the intention is that it will be deleted fairly soon, and if everything else is compiling without that flag then there won't be a second place.

// just ignore it for now.
}
else if (CFEqual(keyString, CFSTR("NonEmptySubject")))
else if (CFEqual(keyString, CFSTR("NonEmptySubject")) || CFEqual(keyString, CFSTR("GrayListedKey")))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@danmosemsft This line we'll want to send to release/2.1 at whatever rate we say we support Mojave

@maryamariyan maryamariyan merged commit 5e46fe1 into dotnet:master Jun 28, 2018
@maryamariyan maryamariyan deleted the help-mojave branch June 28, 2018 23:20
maryamariyan added a commit to maryamariyan/corefx that referenced this pull request Jun 28, 2018
)

* Fix compilation for deprecated API on macOS Mojave preview

Fixes: #30599

* Fixing tests on macOS Mojave
maryamariyan added a commit that referenced this pull request Jul 5, 2018
…30744)

* Fix compilation for deprecated API on macOS Mojave preview

Fixes: #30599
@karelz karelz added this to the 3.0 milestone Jul 8, 2018
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
…refx#30716)

* Fix compilation for deprecated API on macOS Mojave preview

Fixes: dotnet/corefx#30599

* Fixing tests on macOS Mojave


Commit migrated from dotnet/corefx@5e46fe1
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