Skip to content

Comments

migrate github.com/docker/docker/pkg/signal (take 2)#70

Merged
AkihiroSuda merged 98 commits intomoby:masterfrom
thaJeztah:integrate_moby_signal_take2
Jul 22, 2021
Merged

migrate github.com/docker/docker/pkg/signal (take 2)#70
AkihiroSuda merged 98 commits intomoby:masterfrom
thaJeztah:integrate_moby_signal_take2

Conversation

@thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Jul 15, 2021

same as #69, but from moby/moby#42641 (not yet merged)

relates to containerd/containerd#5402

Migrating this package from commit (moby/moby#42641):
moby/moby@9a6ff68

Strategy taken:

# install filter-repo (https://github.com/newren/git-filter-repo/blob/main/INSTALL.md)
brew install git-filter-repo

cd ~/projects

# create a temporary clone of docker
git clone https://github.com/docker/docker.git moby_signal
cd moby_signal

# remove all code, except for pkg/signal, and rename to /signal
git filter-repo  --path pkg/signal --path-rename pkg/signal:signal

# exclude the _deprecated.go and README.md
git filter-repo --path-glob 'signal/*.md' --path-glob 'signal/*_deprecated.go' --invert-paths

# go to the target github.com/moby/sys repository
cd ~/projects/moby-sys

# create a branch to work with
git checkout -b integrate_moby_signal_take2

# add the temporary repository as an upstream and make sure it's up-to-date
git remote add moby_signal ~/projects/moby_signal
git fetch moby_signal

# merge the upstream code
git merge --allow-unrelated-histories --signoff -S moby_signal/master

After this:

  • signal: update import-paths
  • signal: add go.mod
  • Makefile: enable tests for signals

creack and others added 30 commits March 10, 2014 17:36
Docker-DCO-1.1-Signed-off-by: Guillaume J. Charmes <[email protected]> (github: creack)
Docker-DCO-1.1-Signed-off-by: Guillaume J. Charmes <[email protected]> (github: creack)
Docker-DCO-1.1-Signed-off-by: Guillaume J. Charmes <[email protected]> (github: creack)
Docker-DCO-1.1-Signed-off-by: Kato Kazuyoshi <[email protected]> (github: kzys)
Docker-DCO-1.1-Signed-off-by: Guillaume J. Charmes <[email protected]> (github: creack)
Docker-DCO-1.1-Signed-off-by: Andrew Page <[email protected]> (github: tianon)
Fix various MAINTAINERS format inconsistencies
Docker-DCO-1.1-Signed-off-by: Michael Crosby <[email protected]> (github: crosbymichael)
Docker-DCO-1.1-Signed-off-by: Michael Crosby <[email protected]> (github: crosbymichael)
Docker-DCO-1.1-Signed-off-by: Michael Crosby <[email protected]> (github: crosbymichael)
Docker-DCO-1.1-Signed-off-by: Michael Crosby <[email protected]> (github: crosbymichael)
as a maintainer.

Best of luck on your e-commerce business Guillaume, and thanks for all
the great contributions!

Docker-DCO-1.1-Signed-off-by: Solomon Hykes <[email protected]> (github: shykes)
…me_on_his_new_business_and_no_longer_available_as_a_maintainer
Cleanup: refactor shutdown and signal handling facility
Signed-off-by: Michael Crosby <[email protected]>
thaJeztah and others added 9 commits February 11, 2020 00:06
…64el

error log :
signal_test.go:20: assertion failed: error is not nil: Invalid signal: SIGEMT
signal_test.go:22: assertion failed:
When "ParseSignal" function parse sigStr from SignalMap, it find the signal object with key ("SIG"+sigStr). But  EMT signal named "SIGEMT" in SignalMap structrue, so the real key is "SIGSIGEMT" , and cannot find the target signal.
modify "SIGEMT" to "EMT" in SignalMap structrue.

Signed-off-by: liuxiaodong <[email protected]>
TestCatchAll, TestStopCatch: remove unneeded goroutine
Do not handle SIGURG on Linux, as in go1.14+, the go runtime issues
SIGURG as an interrupt to support preemptable system calls on Linux.

This issue was caught in TestCatchAll, which could fail when updating to Go 1.14 or above;

    === Failed
    === FAIL: pkg/signal TestCatchAll (0.01s)
        signal_linux_test.go:32: assertion failed: urgent I/O condition (string) != continued (string)
        signal_linux_test.go:32: assertion failed: continued (string) != hangup (string)
        signal_linux_test.go:32: assertion failed: hangup (string) != child exited (string)
        signal_linux_test.go:32: assertion failed: child exited (string) != illegal instruction (string)
        signal_linux_test.go:32: assertion failed: illegal instruction (string) != floating point exception (string)

Signed-off-by: Sebastiaan van Stijn <[email protected]>
Other Unix platforms (e.g. Darwin) are also affected by the Go
runtime sending SIGURG.

This patch changes how we match the signal by just looking for the
"URG" name, which should handle any platform that has this signal
defined in the SignalMap.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
It is not directly related to signal-handling, so can well live
in its own package.

Also added a variant that doesn't take a directory to write files
to, for easier consumption / better match to how it's used.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
It's the only location where this is used, and it's quite specific
to dockerd (not really a reusable function for external use), so
moving it into that package.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
Migrating this package from commit:
moby/moby@9a6ff68

Strategy taken:

    # install filter-repo (https://github.com/newren/git-filter-repo/blob/main/INSTALL.md)
    brew install git-filter-repo

    cd ~/projects

    # create a temporary clone of docker
    git clone https://github.com/docker/docker.git moby_signal
    cd moby_signal

    # remove all code, except for pkg/signal, and rename to /signal
    git filter-repo  --path pkg/signal --path-rename pkg/signal:signal

    # exclude the _deprecated.go and README.md
    git filter-repo --path-glob 'signal/*.md' --path-glob 'signal/*_deprecated.go' --invert-paths

    # go to the target github.com/moby/sys repository
    cd ~/projects/moby-sys

    # create a branch to work with
    git checkout -b integrate_moby_signal_take2

    # add the temporary repository as an upstream and make sure it's up-to-date
    git remote add moby_signal ~/projects/moby_signal
    git fetch moby_signal

    # merge the upstream code
    git merge --allow-unrelated-histories --signoff -S moby_signal/master

Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah force-pushed the integrate_moby_signal_take2 branch from a0e49de to 1e8ecd8 Compare July 20, 2021 07:18
@thaJeztah thaJeztah marked this pull request as ready for review July 20, 2021 07:18
@thaJeztah
Copy link
Member Author

Some old commits were missing a proper DCO, so I manually marked it to "pass"

Commit sha: 6057481, Author: Guillaume J. Charmes, Committer: Guillaume J. Charmes; The sign-off is missing.
Commit sha: 108a12b, Author: Guillaume J. Charmes, Committer: Guillaume J. Charmes; The sign-off is missing.
Commit sha: aa57f84, Author: Guillaume J. Charmes, Committer: Guillaume J. Charmes; The sign-off is missing.
Commit sha: 5fcf5fa, Author: Kato Kazuyoshi, Committer: Kato Kazuyoshi; The sign-off is missing.
Commit sha: 530bcc0, Author: Guillaume J. Charmes, Committer: Guillaume J. Charmes; The sign-off is missing.
Commit sha: a3e2247, Author: Tianon Gravi, Committer: Tianon Gravi; The sign-off is missing.
Commit sha: 2f0e890, Author: Solomon Hykes, Committer: Solomon Hykes; The sign-off is missing.
Commit sha: 57a9d66, Author: Phil Estes, Committer: Phil Estes; The sign-off is missing.
Commit sha: 860f076, Author: Amit Krishnan, Committer: Amit Krishnan; Expected "Amit Krishnan [email protected]", but got "Amit Krishnan [email protected]".

@thaJeztah
Copy link
Member Author

moby/moby#42641 was merged, so updated this one and moved it out of draft

@cpuguy83 @kolyshkin @samuelkarp ptal

Copy link
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

@cpuguy83
Copy link
Member

I was on the fence if we should make a separate module for this, but there is nothing else in moby/sys that this is related to in any way other than it is a more system-y thing, and it probably makes sense to have a different release for it so 👍

@thaJeztah
Copy link
Member Author

Yes, it's nearing "left-pad" territory, but with recent hassles around go modules, and this package likely to be reasonably "stable", I think it's fine to have it separate. (shouldn't be too much overhead of having it as a separate module) (famous last words)

@AkihiroSuda AkihiroSuda merged commit 9b0136d into moby:master Jul 22, 2021
@thaJeztah thaJeztah deleted the integrate_moby_signal_take2 branch July 22, 2021 13:04
@thaJeztah thaJeztah mentioned this pull request Oct 15, 2024
79 tasks
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.