From 820040ec284f80533903c379a9784fc9ec8b7fe3 Mon Sep 17 00:00:00 2001 From: marwan-at-work Date: Mon, 5 Aug 2019 14:04:37 -0400 Subject: [PATCH 1/8] add context.Context to cli.Context --- context.go | 12 ++++++++++++ go.mod | 3 +++ 2 files changed, 15 insertions(+) create mode 100644 go.mod diff --git a/context.go b/context.go index 9802594a7f..0109f45512 100644 --- a/context.go +++ b/context.go @@ -1,11 +1,14 @@ package cli import ( + "context" "errors" "flag" "os" + "os/signal" "reflect" "strings" + "syscall" ) // Context is a type that is passed through to @@ -13,6 +16,7 @@ import ( // can be used to retrieve context-specific args and // parsed command-line options. type Context struct { + context.Context App *App Command *Command shellComplete bool @@ -24,6 +28,14 @@ type Context struct { // NewContext creates a new context. For use in when invoking an App or Command action. func NewContext(app *App, set *flag.FlagSet, parentCtx *Context) *Context { c := &Context{App: app, flagSet: set, parentContext: parentCtx} + ctx, cancel := context.WithCancel(context.Background()) + go func() { + defer cancel() + sigs := make(chan os.Signal, 1) + signal.Notify(sigs, syscall.SIGINT, syscall.SIGTERM) + <-sigs + }() + c.Context = ctx if parentCtx != nil { c.shellComplete = parentCtx.shellComplete diff --git a/go.mod b/go.mod new file mode 100644 index 0000000000..7ee92dd616 --- /dev/null +++ b/go.mod @@ -0,0 +1,3 @@ +module gopkg.in/urfave/fli.v2 + +go 1.12 From 006fad30ca33bb1882b6de91d0df38409cfd67ed Mon Sep 17 00:00:00 2001 From: marwan-at-work Date: Mon, 5 Aug 2019 15:03:26 -0400 Subject: [PATCH 2/8] fix go.mod --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 7ee92dd616..c21d59cbd7 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,3 @@ -module gopkg.in/urfave/fli.v2 +module gopkg.in/urfave/cli.v2 go 1.12 From cee005ee62349b316defe3cb9c72099f9b118c52 Mon Sep 17 00:00:00 2001 From: marwan-at-work Date: Tue, 6 Aug 2019 12:55:29 -0400 Subject: [PATCH 3/8] only create context once --- context.go | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/context.go b/context.go index 0109f45512..427f73751c 100644 --- a/context.go +++ b/context.go @@ -28,17 +28,21 @@ type Context struct { // NewContext creates a new context. For use in when invoking an App or Command action. func NewContext(app *App, set *flag.FlagSet, parentCtx *Context) *Context { c := &Context{App: app, flagSet: set, parentContext: parentCtx} - ctx, cancel := context.WithCancel(context.Background()) - go func() { - defer cancel() - sigs := make(chan os.Signal, 1) - signal.Notify(sigs, syscall.SIGINT, syscall.SIGTERM) - <-sigs - }() - c.Context = ctx - if parentCtx != nil { + if parentCtx.Context != nil { + parentCtx.Context = context.Background() + } + c.Context = parentCtx.Context c.shellComplete = parentCtx.shellComplete + } else { + ctx, cancel := context.WithCancel(context.Background()) + go func() { + defer cancel() + sigs := make(chan os.Signal, 1) + signal.Notify(sigs, syscall.SIGINT, syscall.SIGTERM) + <-sigs + }() + c.Context = ctx } return c From 1f7d1684b85350633d7b049d7b709f30412a221b Mon Sep 17 00:00:00 2001 From: marwan-at-work Date: Tue, 6 Aug 2019 13:57:00 -0400 Subject: [PATCH 4/8] fix nil check --- context.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/context.go b/context.go index 427f73751c..1e5414e7d8 100644 --- a/context.go +++ b/context.go @@ -29,7 +29,7 @@ type Context struct { func NewContext(app *App, set *flag.FlagSet, parentCtx *Context) *Context { c := &Context{App: app, flagSet: set, parentContext: parentCtx} if parentCtx != nil { - if parentCtx.Context != nil { + if parentCtx.Context == nil { parentCtx.Context = context.Background() } c.Context = parentCtx.Context From 98e64f4507588a67659d0f9d9e762c0ac4496e0e Mon Sep 17 00:00:00 2001 From: marwan-at-work Date: Tue, 6 Aug 2019 14:04:51 -0400 Subject: [PATCH 5/8] add propagation tests --- app_test.go | 5 +---- context_test.go | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/app_test.go b/app_test.go index 796f66597a..dfb3a888a3 100644 --- a/app_test.go +++ b/app_test.go @@ -236,7 +236,7 @@ func ExampleApp_Run_shellComplete() { os.Args = []string{"greet", fmt.Sprintf("--%s", genCompName())} app := &App{ - Name: "greet", + Name: "greet", EnableShellCompletion: true, Commands: []*Command{ { @@ -503,7 +503,6 @@ func TestApp_Float64Flag(t *testing.T) { } func TestApp_ParseSliceFlags(t *testing.T) { - var parsedOption, firstArg string var parsedIntSlice []int var parsedStringSlice []string @@ -518,8 +517,6 @@ func TestApp_ParseSliceFlags(t *testing.T) { Action: func(c *Context) error { parsedIntSlice = c.IntSlice("p") parsedStringSlice = c.StringSlice("ip") - parsedOption = c.String("option") - firstArg = c.Args().First() return nil }, }, diff --git a/context_test.go b/context_test.go index 0509488e20..948af57090 100644 --- a/context_test.go +++ b/context_test.go @@ -1,6 +1,7 @@ package cli import ( + "context" "flag" "sort" "testing" @@ -262,3 +263,24 @@ func TestContext_lookupFlagSet(t *testing.T) { t.Fail() } } + +// TestContextPropagation tests that +// *cli.Context always has a valid +// context.Context +func TestContextPropagation(t *testing.T) { + ctx := NewContext(nil, nil, nil) + if ctx.Context == nil { + t.Fatal("expected a non nil context when no parent is present") + } + parent := NewContext(nil, nil, nil) + parent.Context = context.WithValue(context.Background(), "key", "val") + ctx = NewContext(nil, nil, parent) + val := ctx.Value("key") + if val == nil { + t.Fatal("expected a parent context to be inherited but got nil") + } + valstr, _ := val.(string) + if valstr != "val" { + t.Fatalf("expected the context value to be %q but got %q", "val", valstr) + } +} From 2fb0dc188f59d42961fc3bf3e7a99a06f56ec162 Mon Sep 17 00:00:00 2001 From: marwan-at-work Date: Tue, 6 Aug 2019 14:14:12 -0400 Subject: [PATCH 6/8] ignore compile errors --- app_test.go | 5 +++++ go.mod | 5 +++++ go.sum | 5 +++++ 3 files changed, 15 insertions(+) create mode 100644 go.sum diff --git a/app_test.go b/app_test.go index dfb3a888a3..84631fdf9e 100644 --- a/app_test.go +++ b/app_test.go @@ -503,6 +503,7 @@ func TestApp_Float64Flag(t *testing.T) { } func TestApp_ParseSliceFlags(t *testing.T) { + var parsedOption, firstArg string var parsedIntSlice []int var parsedStringSlice []string @@ -517,11 +518,15 @@ func TestApp_ParseSliceFlags(t *testing.T) { Action: func(c *Context) error { parsedIntSlice = c.IntSlice("p") parsedStringSlice = c.StringSlice("ip") + parsedOption = c.String("option") + firstArg = c.Args().First() return nil }, }, }, } + var _ = parsedOption + var _ = firstArg app.Run([]string{"", "cmd", "-p", "22", "-p", "80", "-ip", "8.8.8.8", "-ip", "8.8.4.4", "my-arg"}) diff --git a/go.mod b/go.mod index c21d59cbd7..b8f4fa3a40 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,8 @@ module gopkg.in/urfave/cli.v2 go 1.12 + +require ( + github.com/BurntSushi/toml v0.3.1 + gopkg.in/yaml.v2 v2.2.2 +) diff --git a/go.sum b/go.sum new file mode 100644 index 0000000000..9a73308140 --- /dev/null +++ b/go.sum @@ -0,0 +1,5 @@ +github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ= +github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw= +gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= From c8863d0b304ec3a200d6a3a0e12e3e59fcbfdd94 Mon Sep 17 00:00:00 2001 From: marwan-at-work Date: Tue, 6 Aug 2019 14:20:21 -0400 Subject: [PATCH 7/8] PR updates --- context.go | 6 ++---- context_test.go | 19 ++++++++++++++----- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/context.go b/context.go index 1e5414e7d8..f1a01b41d7 100644 --- a/context.go +++ b/context.go @@ -29,12 +29,10 @@ type Context struct { func NewContext(app *App, set *flag.FlagSet, parentCtx *Context) *Context { c := &Context{App: app, flagSet: set, parentContext: parentCtx} if parentCtx != nil { - if parentCtx.Context == nil { - parentCtx.Context = context.Background() - } c.Context = parentCtx.Context c.shellComplete = parentCtx.shellComplete - } else { + } + if c.Context == nil { ctx, cancel := context.WithCancel(context.Background()) go func() { defer cancel() diff --git a/context_test.go b/context_test.go index 948af57090..7333ae099d 100644 --- a/context_test.go +++ b/context_test.go @@ -264,17 +264,20 @@ func TestContext_lookupFlagSet(t *testing.T) { } } -// TestContextPropagation tests that -// *cli.Context always has a valid -// context.Context -func TestContextPropagation(t *testing.T) { +func TestNonNilContext(t *testing.T) { ctx := NewContext(nil, nil, nil) if ctx.Context == nil { t.Fatal("expected a non nil context when no parent is present") } +} + +// TestContextPropagation tests that +// *cli.Context always has a valid +// context.Context +func TestContextPropagation(t *testing.T) { parent := NewContext(nil, nil, nil) parent.Context = context.WithValue(context.Background(), "key", "val") - ctx = NewContext(nil, nil, parent) + ctx := NewContext(nil, nil, parent) val := ctx.Value("key") if val == nil { t.Fatal("expected a parent context to be inherited but got nil") @@ -283,4 +286,10 @@ func TestContextPropagation(t *testing.T) { if valstr != "val" { t.Fatalf("expected the context value to be %q but got %q", "val", valstr) } + parent = NewContext(nil, nil, nil) + parent.Context = nil + ctx = NewContext(nil, nil, parent) + if ctx.Context == nil { + t.Fatal("expected context to not be nil even if the parent's context is nil") + } } From be6dbba47b3a4455f5c8022f6fdecd14f6dd310b Mon Sep 17 00:00:00 2001 From: marwan-at-work Date: Tue, 6 Aug 2019 14:21:13 -0400 Subject: [PATCH 8/8] rm modules --- go.mod | 8 -------- go.sum | 5 ----- 2 files changed, 13 deletions(-) delete mode 100644 go.mod delete mode 100644 go.sum diff --git a/go.mod b/go.mod deleted file mode 100644 index b8f4fa3a40..0000000000 --- a/go.mod +++ /dev/null @@ -1,8 +0,0 @@ -module gopkg.in/urfave/cli.v2 - -go 1.12 - -require ( - github.com/BurntSushi/toml v0.3.1 - gopkg.in/yaml.v2 v2.2.2 -) diff --git a/go.sum b/go.sum deleted file mode 100644 index 9a73308140..0000000000 --- a/go.sum +++ /dev/null @@ -1,5 +0,0 @@ -github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ= -github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= -gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw= -gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=