Skip to content

Use unix.SignalNum in ParseSignal on unix platform#3063

Merged
crosbymichael merged 2 commits intocontainerd:masterfrom
zhsj:fix-mipsx
Mar 4, 2019
Merged

Use unix.SignalNum in ParseSignal on unix platform#3063
crosbymichael merged 2 commits intocontainerd:masterfrom
zhsj:fix-mipsx

Conversation

@zhsj
Copy link
Copy Markdown
Contributor

@zhsj zhsj commented Mar 3, 2019

This removes the signalMap on unix platform, since
the signalMap on different architectures is not same,
especially it's wrong on mipsx.

golang.org/x/sys/unix now has a SignalNum func to convert
signal name to a number, thus there's no need to keep
this redundant map.

Windows platform still needs to have a signalMap, since
golang.org/x/sys/windows doesn't have corresponding
functions.

Address: #3061

@zhsj
Copy link
Copy Markdown
Contributor Author

zhsj commented Mar 3, 2019

Just noted, currently containerd still can't build on mipsx, since the vendored docker/docker is too old. I'm not confident to update the docker/docker vendor right now.

But it builds with no_cri tag, since only cri needs docker/docker

This removes the signalMap on unix platform, since
the signalMap on different architectures is not same,
especially it's wrong on mipsx.

golang.org/x/sys/unix now has a SignalNum func to convert
signal name to a number, thus there's no need to keep
this redundant map.

Windows platform still needs to have a signalMap, since
golang.org/x/sys/windows doesn't have corresponding
functions.

Address: containerd#3061

Signed-off-by: Shengjing Zhu <[email protected]>
@codecov-io
Copy link
Copy Markdown

codecov-io commented Mar 3, 2019

Codecov Report

Merging #3063 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3063   +/-   ##
=======================================
  Coverage   43.87%   43.87%           
=======================================
  Files         102      102           
  Lines       10903    10903           
=======================================
  Hits         4784     4784           
  Misses       5384     5384           
  Partials      735      735
Flag Coverage Δ
#linux 47.48% <ø> (ø) ⬆️
#windows 41.08% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5840ecc...f0d5dd3. Read the comment docs.

@Ace-Tang
Copy link
Copy Markdown
Contributor

Ace-Tang commented Mar 4, 2019

Looks make sense on unix part

Copy link
Copy Markdown
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

@crosbymichael
Copy link
Copy Markdown
Member

LGTM

@crosbymichael crosbymichael merged commit 30b6f46 into containerd:master Mar 4, 2019
@zhsj zhsj deleted the fix-mipsx branch March 4, 2019 17:32
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.

5 participants