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

Windows: win32 Filesystem Impl uses POSIX subsystem, refactor to purely win32 APIs #11655

Closed
sunjayBhatia opened this issue Jun 19, 2020 · 8 comments

Comments

@sunjayBhatia
Copy link
Member

The win32 Filesystem impl code uses POSIX substem functions and should be refactored to use win32 APIs instead. The File IoError class should also be converted to use the platform agnostic Envoy::errorDetails function instead of strerror as we will no longer be using POSIX on Windows.

We believe coalescing on all win32 APIs will make error handling and reasoning about these operations easier and more consistent with the rest of the codebase. In #11565 we introduce a standardized mechanism for handling errors from winsock2 which are disjoint from POSIX errors. If we can eliminate as many uses of the POSIX subsystem we can, it should make for an easier time writing portable code. We only need a translation from win32 to generic Envoy errors on Windows and POSIX to generic Envoy errors on other platforms, the alternative being that Windows requires conversion of win32 errors to POSIX which is not possible in all cases and can be misleading when debugging.

@sunjayBhatia
Copy link
Member Author

cc @wrowe @davinci26 @nigriMSFT

@sunjayBhatia
Copy link
Member Author

/assign @sunjayBhatia

@sunjayBhatia
Copy link
Member Author

/assign @wrowe

@stale
Copy link

stale bot commented Jul 20, 2020

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or other activity occurs. Thank you for your contributions.

@stale stale bot added the stale stalebot believes this issue/PR has not been touched recently label Jul 20, 2020
@sunjayBhatia
Copy link
Member Author

bumping to get rid of the stale tag

@stale stale bot removed the stale stalebot believes this issue/PR has not been touched recently label Jul 21, 2020
@stale
Copy link

stale bot commented Aug 23, 2020

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or other activity occurs. Thank you for your contributions.

@stale stale bot added the stale stalebot believes this issue/PR has not been touched recently label Aug 23, 2020
@sunjayBhatia
Copy link
Member Author

Bumping to remove stale tag, will do this refactor after more pressing concerns of Envoy Windows Alpha are covered

@stale stale bot removed the stale stalebot believes this issue/PR has not been touched recently label Aug 26, 2020
davinci26 pushed a commit to davinci26/envoy that referenced this issue Sep 14, 2020
@mattklein123
Copy link
Member

Fixed by #13077

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants