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

Update mkwinsyscall and make stand alone tool #248

Merged
merged 3 commits into from
Jun 16, 2022

Conversation

helsaawy
Copy link
Contributor

Updated mkwinsyscall to latest version, and created a standalone tool that can be called from go-winio and hcsshim.
Used latest version from:
https://github.com/golang/sys/blob/bc2c85ada10aa9b6aa9607e9ac9ad0761b95cf1d/windows/mkwinsyscall/mkwinsyscall.go

Applied changes from both go-winio (forced UTF-16, go-winio import) and hcsshim (UTF-16, HResult error return type)

Added (temporary?) flag to disable sorting so that generated syscall file diffs in hcsshim are easier to read.

Added build constraint comment to generated file, in case file names do not have one.

Allowed input files can be specified with a glob pattern.
This allows hcsshim/internal/winapi to avoid listing all the files within it:
//go:generate go run github.com/Microsoft/go-winio/tools/mkwinsyscall -output zsyscall_windows.go ./*.go

PR has two commits so that changes can be rebased on newer versions of golang.org/x/sys/windows/mkwinsyscall, but it may make more sense to create a dedicated branch on this repo with those commits

Signed-off-by: Hamza El-Saawy hamzaelsaawy@microsoft.com

Added hr error return type, cmdline flags for winio import, utf16, and
sorting.

Generated file has build constrain comment.

Input files can be specified with a glob pattern.

Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
syscall.go Outdated Show resolved Hide resolved
@dcantah
Copy link
Contributor

dcantah commented Jun 2, 2022

This is great that we're converging on one copy. Have you verified vendoring a local copy of winio with this change to hcsshim has everything work alright?

@helsaawy
Copy link
Contributor Author

helsaawy commented Jun 2, 2022

This is great that we're converging on one copy. Have you verified vendoring a local copy of winio with this change to hcsshim has everything work alright?

None of the tests fail and the diffs dont look too egregious here: microsoft/hcsshim#1409.
Unfortunately, I dont think we have really good testing around our syscalls

@dcantah dcantah self-assigned this Jun 3, 2022
Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
@helsaawy helsaawy merged commit 35837cf into microsoft:master Jun 16, 2022
@helsaawy helsaawy deleted the he/mkwinsyscall branch August 22, 2022 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants