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 a regression of windows build issue of undefined symbol #66

Merged
merged 1 commit into from
Sep 30, 2020

Conversation

thehajime
Copy link
Contributor

This commit fixes the following build failure.

  • mingw32-make.exe binaries
  • bin/ctr.exe
  • bin/containerd.exe
    github.com/containerd/containerd/vendor/github.com/containerd/go-runc
    [error]vendor\github.com\containerd\go-runc\runc.go:66:16: undefined: unix.Signal
    [error]mingw32-make: *** [Makefile.windows:28: bin/containerd.exe] Error 2
    [error]Process completed with exit code 2.

Fixes #65.
Fixes: 421b4ca ("Change the type of PdeathSignal")

Signed-off-by: Hajime Tazaki thehajime@gmail.com

This commit fixes the following build failure.

+ mingw32-make.exe binaries
+ bin/ctr.exe
+ bin/containerd.exe
github.com/containerd/containerd/vendor/github.com/containerd/go-runc
[error]vendor\github.com\containerd\go-runc\runc.go:66:16: undefined: unix.Signal
[error]mingw32-make: *** [Makefile.windows:28: bin/containerd.exe] Error 2
[error]Process completed with exit code 2.

Fixes containerd#65.
Fixes: 421b4ca ("Change the type of PdeathSignal")

Signed-off-by: Hajime Tazaki <thehajime@gmail.com>
Copy link
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@AkihiroSuda
Copy link
Member

I'm not sure go-runc should support Windows builds, unless it is really functional on Windows.

Do you plan to use this for porting runU to Windows?

@estesp
Copy link
Member

estesp commented Sep 30, 2020

I don't think the package needs to function on Windows, but it is imported into containerd and the file in question is compiled for all os/arch combinations. That means this package (currently) can't be vendor updated in containerd for valid os/arch (e.g. Linux) because it breaks CI on Windows.

@AkihiroSuda
Copy link
Member

Would it be trivial to remove codes that import this package for Windows builds?

@thehajime
Copy link
Contributor Author

thehajime commented Sep 30, 2020

Do you plan to use this for porting runU to Windows?

no, in a short timeframe.

again, as @estesp kindly explained, this is not related to darwin port at all; just trying to fix errors which I came across...

Would it be trivial to remove codes that import this package for Windows builds?

:~/.go/src/github.com/containerd/containerd% grep runc vendor/github.com/Microsoft/hcsshim/pkg/go-runhcs/*               
vendor/github.com/Microsoft/hcsshim/pkg/go-runhcs/NOTICE:go-runhcs is a fork of go-runc
vendor/github.com/Microsoft/hcsshim/pkg/go-runhcs/NOTICE:The following is runc's legal notice.
vendor/github.com/Microsoft/hcsshim/pkg/go-runhcs/NOTICE:runc
vendor/github.com/Microsoft/hcsshim/pkg/go-runhcs/runhcs.go:    "github.com/containerd/go-runc"
vendor/github.com/Microsoft/hcsshim/pkg/go-runhcs/runhcs.go:            ec, err := runc.Monitor.Start(cmd)
vendor/github.com/Microsoft/hcsshim/pkg/go-runhcs/runhcs.go:            status, err := runc.Monitor.Wait(cmd, ec)
vendor/github.com/Microsoft/hcsshim/pkg/go-runhcs/runhcs.go:    ec, err := runc.Monitor.Start(cmd)
vendor/github.com/Microsoft/hcsshim/pkg/go-runhcs/runhcs.go:    status, err := runc.Monitor.Wait(cmd, ec)
vendor/github.com/Microsoft/hcsshim/pkg/go-runhcs/runhcs_create.go:     runc "github.com/containerd/go-runc"
vendor/github.com/Microsoft/hcsshim/pkg/go-runhcs/runhcs_create.go:     runc.IO
vendor/github.com/Microsoft/hcsshim/pkg/go-runhcs/runhcs_create.go:     ec, err := runc.Monitor.Start(cmd)
vendor/github.com/Microsoft/hcsshim/pkg/go-runhcs/runhcs_create.go:             if c, ok := opts.IO.(runc.StartCloser); ok {
vendor/github.com/Microsoft/hcsshim/pkg/go-runhcs/runhcs_create.go:     status, err := runc.Monitor.Wait(cmd, ec)
vendor/github.com/Microsoft/hcsshim/pkg/go-runhcs/runhcs_exec.go:       "github.com/containerd/go-runc"
vendor/github.com/Microsoft/hcsshim/pkg/go-runhcs/runhcs_exec.go:       runc.IO
vendor/github.com/Microsoft/hcsshim/pkg/go-runhcs/runhcs_exec.go:       ec, err := runc.Monitor.Start(cmd)
vendor/github.com/Microsoft/hcsshim/pkg/go-runhcs/runhcs_exec.go:               if c, ok := opts.IO.(runc.StartCloser); ok {
vendor/github.com/Microsoft/hcsshim/pkg/go-runhcs/runhcs_exec.go:       status, err := runc.Monitor.Wait(cmd, ec)

I would say no, though somebody may raise his/her hand to fix.
But I think this would be another patchset to refactor the existing code.

@AkihiroSuda AkihiroSuda merged commit 7c5957f into containerd:master Sep 30, 2020
@thehajime thehajime deleted the fix-win-build branch September 30, 2020 05:17
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.

failure on windows build (undefined: unix.Signal)
3 participants