diff --git a/commands/cli/parse.go b/commands/cli/parse.go index 7d77e49fc4f..452f51adf99 100644 --- a/commands/cli/parse.go +++ b/commands/cli/parse.go @@ -296,10 +296,17 @@ func parseArgs(inputs []string, stdin *os.File, argDefs []cmds.Argument, recursi stringArgs, inputs = append(stringArgs, inputs[0]), inputs[1:] } else { if stdin != nil && argDef.SupportsStdin && !fillingVariadic { - if err := printReadInfo(stdin, msgStdinInfo); err == nil { - fileArgs[stdin.Name()] = files.NewReaderFile("", stdin.Name(), stdin, nil) - stdin = nil + fname := "" + istty, err := isTty(stdin) + if err != nil { + return nil, nil, err + } + if istty { + fname = "*stdin*" } + + fileArgs[stdin.Name()] = files.NewReaderFile(fname, "", stdin, nil) + stdin = nil } } case cmds.ArgFile: @@ -417,15 +424,24 @@ func appendFile(fpath string, argDef *cmds.Argument, recursive, hidden bool) (fi // Inform the user if a file is waiting on input func printReadInfo(f *os.File, msg string) error { - fInfo, err := f.Stat() + isTty, err := isTty(f) if err != nil { - log.Error(err) return err } - if (fInfo.Mode() & os.ModeCharDevice) != 0 { + if isTty { fmt.Fprintf(os.Stderr, msg, f.Name()) } return nil } + +func isTty(f *os.File) (bool, error) { + fInfo, err := f.Stat() + if err != nil { + log.Error(err) + return false, err + } + + return (fInfo.Mode() & os.ModeCharDevice) != 0, nil +} diff --git a/commands/cli/parse_test.go b/commands/cli/parse_test.go index a1a23dc6527..5b86a3f243e 100644 --- a/commands/cli/parse_test.go +++ b/commands/cli/parse_test.go @@ -4,7 +4,6 @@ import ( "io" "io/ioutil" "os" - "path/filepath" "runtime" "strings" "testing" @@ -178,49 +177,32 @@ func TestArgumentParsing(t *testing.T) { commands.StringArg("b", true, false, "another arg"), }, }, - "FileArg": { + "stdinenabled": { Arguments: []commands.Argument{ - commands.FileArg("a", true, false, "some arg"), + commands.StringArg("a", true, true, "some arg").EnableStdin(), }, }, - "FileArg+Variadic": { - Arguments: []commands.Argument{ - commands.FileArg("a", true, true, "some arg"), - }, - }, - "FileArg+Stdin": { - Arguments: []commands.Argument{ - commands.FileArg("a", true, true, "some arg").EnableStdin(), - }, - }, - "StringArg+FileArg": { - Arguments: []commands.Argument{ - commands.StringArg("a", true, false, "some arg"), - commands.FileArg("a", true, false, "some arg"), - }, - }, - "StringArg+FileArg+Stdin": { + "stdinenabled2args": &commands.Command{ Arguments: []commands.Argument{ commands.StringArg("a", true, false, "some arg"), - commands.FileArg("a", true, true, "some arg").EnableStdin(), + commands.StringArg("b", true, true, "another arg").EnableStdin(), }, }, - "StringArg+FileArg+Variadic": { + "stdinenablednotvariadic": &commands.Command{ Arguments: []commands.Argument{ - commands.StringArg("a", true, false, "some arg"), - commands.FileArg("a", true, true, "some arg"), + commands.StringArg("a", true, false, "some arg").EnableStdin(), }, }, - "StringArg+FileArg+Variadic+Stdin": { + "stdinenablednotvariadic2args": &commands.Command{ Arguments: []commands.Argument{ commands.StringArg("a", true, false, "some arg"), - commands.FileArg("a", true, true, "some arg"), + commands.StringArg("b", true, false, "another arg").EnableStdin(), }, }, }, } - test := func(cmd words, f *os.File, exp words) { + test := func(cmd words, f *os.File, res words) { if f != nil { if _, err := f.Seek(0, os.SEEK_SET); err != nil { t.Fatal(err) @@ -230,18 +212,8 @@ func TestArgumentParsing(t *testing.T) { if err != nil { t.Errorf("Command '%v' should have passed parsing: %v", cmd, err) } - - parsedWords := make([]string, len(req.Arguments())) - copy(parsedWords, req.Arguments()) - - if files := req.Files(); files != nil { - for file, err := files.NextFile(); err != io.EOF; file, err = files.NextFile() { - parsedWords = append(parsedWords, file.FullPath()) - } - } - - if !sameWords(parsedWords, exp) { - t.Errorf("Arguments parsed from '%v' are '%v' instead of '%v'", cmd, parsedWords, exp) + if !sameWords(req.Arguments(), res) { + t.Errorf("Arguments parsed from '%v' are '%v' instead of '%v'", cmd, req.Arguments(), res) } } @@ -281,52 +253,59 @@ func TestArgumentParsing(t *testing.T) { testFail([]string{"reversedoptional"}, nil, "didn't provide any args, 1 required") testFail([]string{"reversedoptional", "value1", "value2", "value3"}, nil, "provided too many args, only takes 1") - // Since FileArgs are presently stored ordered by Path, the enum string - // is used to construct a predictably ordered sequence of filenames. - tmpFile := func(t *testing.T, enum string) *os.File { - f, err := ioutil.TempFile("", enum) + // Use a temp file to simulate stdin + fileToSimulateStdin := func(t *testing.T, content string) *os.File { + fstdin, err := ioutil.TempFile("", "") if err != nil { t.Fatal(err) } - fn, err := filepath.EvalSymlinks(f.Name()) - if err != nil { - t.Fatal(err) - } - f.Close() - f, err = os.Create(fn) - if err != nil { + defer os.Remove(fstdin.Name()) + + if _, err := io.WriteString(fstdin, content); err != nil { t.Fatal(err) } - - return f + return fstdin } - file1 := tmpFile(t, "1") - file2 := tmpFile(t, "2") - file3 := tmpFile(t, "3") - defer os.Remove(file3.Name()) - defer os.Remove(file2.Name()) - defer os.Remove(file1.Name()) - - test([]string{"noarg"}, file1, []string{}) - test([]string{"FileArg", file1.Name()}, nil, []string{file1.Name()}) - test([]string{"FileArg+Variadic", file1.Name(), file2.Name()}, nil, - []string{file1.Name(), file2.Name()}) - test([]string{"FileArg+Stdin"}, file1, []string{file1.Name()}) - test([]string{"FileArg+Stdin", "-"}, file1, []string{file1.Name()}) - test([]string{"FileArg+Stdin", file1.Name(), "-"}, file2, - []string{file1.Name(), file2.Name()}) - test([]string{"StringArg+FileArg", - "foo", file1.Name()}, nil, []string{"foo", file1.Name()}) - test([]string{"StringArg+FileArg+Variadic", - "foo", file1.Name(), file2.Name()}, nil, - []string{"foo", file1.Name(), file2.Name()}) - test([]string{"StringArg+FileArg+Stdin", - "foo", file1.Name(), "-"}, file2, - []string{"foo", file1.Name(), file2.Name()}) - test([]string{"StringArg+FileArg+Variadic+Stdin", - "foo", file1.Name(), file2.Name()}, file3, - []string{"foo", file1.Name(), file2.Name()}) - test([]string{"StringArg+FileArg+Variadic+Stdin", - "foo", file1.Name(), file2.Name(), "-"}, file3, - []string{"foo", file1.Name(), file2.Name(), file3.Name()}) + + test([]string{"stdinenabled", "value1", "value2"}, nil, []string{"value1", "value2"}) + + fstdin := fileToSimulateStdin(t, "stdin1") + test([]string{"stdinenabled"}, fstdin, []string{"stdin1"}) + test([]string{"stdinenabled", "value1"}, fstdin, []string{"value1"}) + test([]string{"stdinenabled", "value1", "value2"}, fstdin, []string{"value1", "value2"}) + + fstdin = fileToSimulateStdin(t, "stdin1\nstdin2") + test([]string{"stdinenabled"}, fstdin, []string{"stdin1", "stdin2"}) + + fstdin = fileToSimulateStdin(t, "stdin1\nstdin2\nstdin3") + test([]string{"stdinenabled"}, fstdin, []string{"stdin1", "stdin2", "stdin3"}) + + test([]string{"stdinenabled2args", "value1", "value2"}, nil, []string{"value1", "value2"}) + + fstdin = fileToSimulateStdin(t, "stdin1") + test([]string{"stdinenabled2args", "value1"}, fstdin, []string{"value1", "stdin1"}) + test([]string{"stdinenabled2args", "value1", "value2"}, fstdin, []string{"value1", "value2"}) + test([]string{"stdinenabled2args", "value1", "value2", "value3"}, fstdin, []string{"value1", "value2", "value3"}) + + fstdin = fileToSimulateStdin(t, "stdin1\nstdin2") + test([]string{"stdinenabled2args", "value1"}, fstdin, []string{"value1", "stdin1", "stdin2"}) + + test([]string{"stdinenablednotvariadic", "value1"}, nil, []string{"value1"}) + + fstdin = fileToSimulateStdin(t, "stdin1") + test([]string{"stdinenablednotvariadic"}, fstdin, []string{"stdin1"}) + test([]string{"stdinenablednotvariadic", "value1"}, fstdin, []string{"value1"}) + + test([]string{"stdinenablednotvariadic2args", "value1", "value2"}, nil, []string{"value1", "value2"}) + + fstdin = fileToSimulateStdin(t, "stdin1") + test([]string{"stdinenablednotvariadic2args", "value1"}, fstdin, []string{"value1", "stdin1"}) + test([]string{"stdinenablednotvariadic2args", "value1", "value2"}, fstdin, []string{"value1", "value2"}) + testFail([]string{"stdinenablednotvariadic2args"}, fstdin, "cant use stdin for non stdin arg") + + fstdin = fileToSimulateStdin(t, "stdin1") + test([]string{"noarg"}, fstdin, []string{}) + + fstdin = fileToSimulateStdin(t, "stdin1") + test([]string{"optionalsecond", "value1", "value2"}, fstdin, []string{"value1", "value2"}) } diff --git a/commands/command.go b/commands/command.go index 8c783636af1..907e43dd08f 100644 --- a/commands/command.go +++ b/commands/command.go @@ -206,7 +206,7 @@ func (c *Command) GetOptions(path []string) (map[string]Option, error) { } func (c *Command) CheckArguments(req Request) error { - args := req.Arguments() + args := req.(*request).arguments // count required argument definitions numRequired := 0 @@ -218,7 +218,7 @@ func (c *Command) CheckArguments(req Request) error { // iterate over the arg definitions valueIndex := 0 // the index of the current value (in `args`) - for _, argDef := range c.Arguments { + for i, argDef := range c.Arguments { // skip optional argument definitions if there aren't // sufficient remaining values if len(args)-valueIndex <= numRequired && !argDef.Required || @@ -235,6 +235,11 @@ func (c *Command) CheckArguments(req Request) error { valueIndex++ } + // in the case of a non-variadic required argument that supports stdin + if !found && len(c.Arguments)-1 == i && argDef.SupportsStdin { + found = true + } + err := checkArgValue(v, found, argDef) if err != nil { return err diff --git a/commands/request.go b/commands/request.go index 621ef35396f..06294a83a78 100644 --- a/commands/request.go +++ b/commands/request.go @@ -170,6 +170,16 @@ func (r *request) SetOptions(opts OptMap) error { // Arguments returns the arguments slice func (r *request) Arguments() []string { + if r.haveVarArgsFromStdin() { + err := r.VarArgs(func(s string) error { + r.arguments = append(r.arguments, s) + return nil + }) + if err != nil && err != io.EOF { + log.Error(err) + } + } + return r.arguments } @@ -189,10 +199,22 @@ func (r *request) Context() context.Context { return r.rctx } +func (r *request) haveVarArgsFromStdin() bool { + // we expect varargs if we have a variadic required argument and no arguments to + // fill it + if len(r.cmd.Arguments) == 0 { + return false + } + + last := r.cmd.Arguments[len(r.cmd.Arguments)-1] + return last.SupportsStdin && last.Type == ArgString && + len(r.arguments) < len(r.cmd.Arguments) +} + func (r *request) VarArgs(f func(string) error) error { var i int for i = 0; i < len(r.cmd.Arguments); i++ { - if r.cmd.Arguments[i].Variadic { + if r.cmd.Arguments[i].Variadic || r.cmd.Arguments[i].SupportsStdin { break } } @@ -208,19 +230,27 @@ func (r *request) VarArgs(f func(string) error) error { return nil } else { - fi, err := r.files.NextFile() - if err != nil { - return err - } - - scan := bufio.NewScanner(fi) - for scan.Scan() { - err := f(scan.Text()) + if r.files != nil { + fi, err := r.files.NextFile() if err != nil { return err } + + if fi.FileName() == "*stdin*" { + fmt.Fprintln(os.Stderr, "ipfs: Reading from stdin; send Ctrl-d to stop.") + } + + scan := bufio.NewScanner(fi) + for scan.Scan() { + err := f(scan.Text()) + if err != nil { + return err + } + } + return nil + } else { + return fmt.Errorf("expected more arguments from stdin") } - return nil } } diff --git a/core/commands/bitswap.go b/core/commands/bitswap.go index 4ac2441dd86..854a2a9a199 100644 --- a/core/commands/bitswap.go +++ b/core/commands/bitswap.go @@ -31,7 +31,7 @@ var unwantCmd = &cmds.Command{ Tagline: "Remove a given block from your wantlist.", }, Arguments: []cmds.Argument{ - cmds.StringArg("key", true, true, "Key(s) to remove from your wantlist."), + cmds.StringArg("key", true, true, "Key(s) to remove from your wantlist.").EnableStdin(), }, Run: func(req cmds.Request, res cmds.Response) { nd, err := req.InvocContext().GetNode() diff --git a/core/commands/block.go b/core/commands/block.go index e910e6fc74d..7a0e61a717f 100644 --- a/core/commands/block.go +++ b/core/commands/block.go @@ -55,7 +55,7 @@ on raw ipfs blocks. It outputs the following to stdout: }, Arguments: []cmds.Argument{ - cmds.StringArg("key", true, false, "The base58 multihash of an existing block to get."), + cmds.StringArg("key", true, false, "The base58 multihash of an existing block to stat.").EnableStdin(), }, Run: func(req cmds.Request, res cmds.Response) { b, err := getBlockForKey(req, req.Arguments()[0]) @@ -88,7 +88,7 @@ It outputs to stdout, and is a base58 encoded multihash. }, Arguments: []cmds.Argument{ - cmds.StringArg("key", true, false, "The base58 multihash of an existing block to get."), + cmds.StringArg("key", true, false, "The base58 multihash of an existing block to get.").EnableStdin(), }, Run: func(req cmds.Request, res cmds.Response) { b, err := getBlockForKey(req, req.Arguments()[0]) diff --git a/core/commands/bootstrap.go b/core/commands/bootstrap.go index ffdcfb2ad1e..1eebd7550cc 100644 --- a/core/commands/bootstrap.go +++ b/core/commands/bootstrap.go @@ -47,7 +47,7 @@ in the bootstrap list). }, Arguments: []cmds.Argument{ - cmds.StringArg("peer", false, true, peerOptionDesc), + cmds.StringArg("peer", false, true, peerOptionDesc).EnableStdin(), }, Options: []cmds.Option{ @@ -55,30 +55,13 @@ in the bootstrap list). }, Run: func(req cmds.Request, res cmds.Response) { - inputPeers, err := config.ParseBootstrapPeers(req.Arguments()) - if err != nil { - res.SetError(err, cmds.ErrNormal) - return - } - - r, err := fsrepo.Open(req.InvocContext().ConfigRoot) - if err != nil { - res.SetError(err, cmds.ErrNormal) - return - } - defer r.Close() - cfg, err := r.Config() - if err != nil { - res.SetError(err, cmds.ErrNormal) - return - } - deflt, _, err := req.Option("default").Bool() if err != nil { res.SetError(err, cmds.ErrNormal) return } + var inputPeers []config.BootstrapPeer if deflt { // parse separately for meaningful, correct error. defltPeers, err := config.DefaultBootstrapPeers() @@ -87,7 +70,15 @@ in the bootstrap list). return } - inputPeers = append(inputPeers, defltPeers...) + inputPeers = defltPeers + } else { + parsedPeers, err := config.ParseBootstrapPeers(req.Arguments()) + if err != nil { + res.SetError(err, cmds.ErrNormal) + return + } + + inputPeers = parsedPeers } if len(inputPeers) == 0 { @@ -95,6 +86,18 @@ in the bootstrap list). return } + r, err := fsrepo.Open(req.InvocContext().ConfigRoot) + if err != nil { + res.SetError(err, cmds.ErrNormal) + return + } + defer r.Close() + cfg, err := r.Config() + if err != nil { + res.SetError(err, cmds.ErrNormal) + return + } + added, err := bootstrapAdd(r, cfg, inputPeers) if err != nil { res.SetError(err, cmds.ErrNormal) diff --git a/core/commands/pin.go b/core/commands/pin.go index 25035eea055..c9c95d06300 100644 --- a/core/commands/pin.go +++ b/core/commands/pin.go @@ -61,25 +61,13 @@ var addPinCmd = &cmds.Command{ return } - out := make(chan interface{}) - go func(ctx context.Context) { - defer close(out) - err := req.VarArgs(func(arg string) error { - added, err := corerepo.Pin(n, ctx, []string{arg}, recursive) - if err != nil { - return err - } - - out <- &PinOutput{added} - return nil - }) + added, err := corerepo.Pin(n, req.Context(), req.Arguments(), recursive) + if err != nil { + res.SetError(err, cmds.ErrNormal) + return + } - if err != nil { - res.SetError(err, cmds.ErrNormal) - return - } - }(req.Context()) - res.SetOutput((<-chan interface{})(out)) + res.SetOutput(&PinOutput{added}) }, Marshalers: cmds.MarshalerMap{ cmds.Text: func(res cmds.Response) (io.Reader, error) { @@ -91,28 +79,17 @@ var addPinCmd = &cmds.Command{ pintype = "directly" } - marshalPinOutput := func(po *PinOutput) io.Reader { - buf := new(bytes.Buffer) - for _, k := range po.Pins { - fmt.Fprintf(buf, "pinned %s %s\n", k, pintype) - } - return buf - } - - out, ok := res.Output().(<-chan interface{}) + po, ok := res.Output().(*PinOutput) if !ok { return nil, u.ErrCast() } - marshal := func(i interface{}) (io.Reader, error) { - return marshalPinOutput(i.(*PinOutput)), nil + buf := new(bytes.Buffer) + for _, k := range po.Pins { + fmt.Fprintf(buf, "pinned %s %s\n", k, pintype) } + return buf, nil - return &cmds.ChannelMarshaler{ - Res: res, - Marshaler: marshal, - Channel: out, - }, nil }, }, } @@ -147,47 +124,26 @@ collected if needed. (By default, recursively. Use -r=false for direct pins) return } - out := make(chan interface{}) - go func() { - defer close(out) - err = req.VarArgs(func(arg string) error { - removed, err := corerepo.Unpin(n, req.Context(), req.Arguments(), recursive) - if err != nil { - return err - } - - out <- &PinOutput{removed} - return nil - }) - if err != nil { - res.SetError(err, cmds.ErrNormal) - return - } - }() + removed, err := corerepo.Unpin(n, req.Context(), req.Arguments(), recursive) + if err != nil { + res.SetError(err, cmds.ErrNormal) + return + } - res.SetOutput((<-chan interface{})(out)) + res.SetOutput(&PinOutput{removed}) }, Marshalers: cmds.MarshalerMap{ cmds.Text: func(res cmds.Response) (io.Reader, error) { - outch, ok := res.Output().(<-chan interface{}) + added, ok := res.Output().(*PinOutput) if !ok { return nil, u.ErrCast() } - marshal := func(i interface{}) (io.Reader, error) { - added := i.(*PinOutput) - buf := new(bytes.Buffer) - for _, k := range added.Pins { - fmt.Fprintf(buf, "unpinned %s\n", k) - } - return buf, nil + buf := new(bytes.Buffer) + for _, k := range added.Pins { + fmt.Fprintf(buf, "unpinned %s\n", k) } - - return &cmds.ChannelMarshaler{ - Res: res, - Marshaler: marshal, - Channel: outch, - }, nil + return buf, nil }, }, } diff --git a/test/sharness/t0085-pins.sh b/test/sharness/t0085-pins.sh new file mode 100755 index 00000000000..45b61e98f52 --- /dev/null +++ b/test/sharness/t0085-pins.sh @@ -0,0 +1,52 @@ +#!/bin/sh +# +# Copyright (c) 2016 Jeromy Johnson +# MIT Licensed; see the LICENSE file in this repository. +# + +test_description="Test ipfs pinning operations" + +. lib/test-lib.sh + + +test_pins() { + test_expect_success "create some hashes" ' + HASH_A=$(echo "A" | ipfs add -q --pin=false) && + HASH_B=$(echo "B" | ipfs add -q --pin=false) && + HASH_C=$(echo "C" | ipfs add -q --pin=false) && + HASH_D=$(echo "D" | ipfs add -q --pin=false) && + HASH_E=$(echo "E" | ipfs add -q --pin=false) && + HASH_F=$(echo "F" | ipfs add -q --pin=false) && + HASH_G=$(echo "G" | ipfs add -q --pin=false) + ' + + test_expect_success "put all those hashes in a file" ' + echo $HASH_A > hashes && + echo $HASH_B >> hashes && + echo $HASH_C >> hashes && + echo $HASH_D >> hashes && + echo $HASH_E >> hashes && + echo $HASH_F >> hashes && + echo $HASH_G >> hashes + ' + + test_expect_success "pin those hashes via stdin" ' + cat hashes | ipfs pin add + ' + + test_expect_success "unpin those hashes" ' + cat hashes | ipfs pin rm + ' +} + +test_init_ipfs + +test_pins + +test_launch_ipfs_daemon --offline + +test_pins + +test_kill_ipfs_daemon + +test_done