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

Remove platform restriction from Hosting.WindowsServices #59037

Closed
Tratcher opened this issue Sep 13, 2021 · 1 comment · Fixed by #59039
Closed

Remove platform restriction from Hosting.WindowsServices #59037

Tratcher opened this issue Sep 13, 2021 · 1 comment · Fixed by #59039
Assignees
Milestone

Comments

@Tratcher
Copy link
Member

<SupportedOSPlatforms>windows</SupportedOSPlatforms>

This project is windows specific, but it does its own platform checks and no-ops. This enables development on one platform and publishing to another.

The original change was here: https://github.com/dotnet/runtime/pull/53941/files

Some discussion offline with @jeffhandley and @davidfowl:

@jeffhandley said:

I don’t think we have precedent on such a scenario yet. The attributes are meant to take a runtime failure (PNSE) and turn them into a build-time warning/error. My opinion is to two with your “on the other side” argument—if the API no-ops on non-Windows platforms, then I’m not sure the attribute gains anything. The UseWindowsService() / UseSystemd() pairing for local/deployed is nice too.

Any objections to setting the precedent here that we can remove/omit attributes in cases where the API connotes the platform support by name or docs summary, and no-ops on other platforms?

@davidfowl said:

I think this makes the most sense. The API that takes control into their own hands by doing manual platform tests should be able to suppress the warning so that consumers of that library don't get the warning.

@ghost
Copy link

ghost commented Sep 13, 2021

Tagging subscribers to this area: @eerhardt, @maryamariyan
See info in area-owners.md if you want to be subscribed.

Issue Details

<SupportedOSPlatforms>windows</SupportedOSPlatforms>

This project is windows specific, but it does its own platform checks and no-ops. This enables development on one platform and publishing to another.

The original change was here: https://github.com/dotnet/runtime/pull/53941/files

Some discussion offline with @jeffhandley and @davidfowl:

@jeffhandley said:

I don’t think we have precedent on such a scenario yet. The attributes are meant to take a runtime failure (PNSE) and turn them into a build-time warning/error. My opinion is to two with your “on the other side” argument—if the API no-ops on non-Windows platforms, then I’m not sure the attribute gains anything. The UseWindowsService() / UseSystemd() pairing for local/deployed is nice too.

Any objections to setting the precedent here that we can remove/omit attributes in cases where the API connotes the platform support by name or docs summary, and no-ops on other platforms?

@davidfowl said:

I think this makes the most sense. The API that takes control into their own hands by doing manual platform tests should be able to suppress the warning so that consumers of that library don't get the warning.

Author: Tratcher
Assignees: eerhardt
Labels:

area-Extensions-Hosting

Milestone: -

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Sep 13, 2021
eerhardt added a commit to eerhardt/runtime that referenced this issue Sep 13, 2021
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Sep 13, 2021
@eerhardt eerhardt removed the untriaged New issue has not been triaged by the area owner label Sep 13, 2021
@eerhardt eerhardt added this to the 6.0.0 milestone Sep 13, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Sep 13, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Nov 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants