search

containerd / go-runc

runc bindings for Go
Apache License 2.0
161 stars 71 forks source link

failure on windows build (undefined: unix.Signal) #65

Closed thehajime closed 3 years ago

thehajime commented 3 years ago

Regression of #61 ?

I'm trying to bump the version of go-runc at containerd (as a part of https://github.com/containerd/containerd/pull/4526, related to #64), but run into a failure on windows build due to the change in this PR.

The error detail:

https://github.com/ukontainer/containerd/runs/1104376893?check_suite_focus=true#step:7:16

+ export CGO_ENABLED=1
+ CGO_ENABLED=1
+ cd src/github.com/containerd/containerd
+ 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.
thehajime commented 3 years ago

@KentaTada ping ?

KentaTada commented 3 years ago

@thehajime IANAM and I'm not familiar with your work. I think the RCA in this issue was due to building runc.go for Windows although Pdeathsig is only supported for Linux. So the fix of this issue is not to change the type of Pdeathsig but to modify supported platform of runc.go.

thehajime commented 3 years ago

@KentaTada Thanks for the message.

Let me explain a bit of context of this issue.

As I described in the message above, I was trying to bump up the version of go-runc at https://github.com/containerd/containerd/pull/4526 in order to reflect my patch (#64), but the containerd/containerd uses slightly old version of go-runc (7016d3ce2328dd2cb1192b2076ebd565c4e8df0c).

So, my patch is not directly involved with the changed in #61, just faced a build failure when pulling the go-runc into the latest containerd.

Why is the patches #61 not pulled in containerd even the associated patches (https://github.com/containerd/containerd/pull/4317) are in?

KentaTada commented 3 years ago

@thehajime As I've already said above, I think we need to modify the incorrect "type Runc struct" because it includes Linux-specific member like PdeathSignal. It should not be used for Windows build.

But I have no concrete idea how to modify it now. Should runc.go be separated for each platform? It should be discussed and fixed at first.