Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change logic of stdin input to be last and only if required #2822

Merged
merged 4 commits into from
Jun 15, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 30 additions & 7 deletions cmd/ipfs/init.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package main

import (
"encoding/json"
"errors"
"fmt"
"io"
Expand Down Expand Up @@ -33,7 +34,9 @@ environment variable:
export IPFS_PATH=/path/to/ipfsrepo
`,
},

Arguments: []cmds.Argument{
cmds.FileArg("default-config", false, false, "Initialize with the given configuration.").EnableStdin(),
},
Options: []cmds.Option{
cmds.IntOption("bits", "b", "Number of bits to use in the generated RSA private key.").Default(nBitsForKeypairDefault),
cmds.BoolOption("empty-repo", "e", "Don't add and pin help files to the local storage.").Default(false),
Expand Down Expand Up @@ -76,7 +79,24 @@ environment variable:
return
}

if err := doInit(os.Stdout, req.InvocContext().ConfigRoot, empty, nBitsForKeypair); err != nil {
var conf *config.Config

f := req.Files()
if f != nil {
confFile, err := f.NextFile()
if err != nil {
res.SetError(err, cmds.ErrNormal)
return
}

conf = &config.Config{}
if err := json.NewDecoder(confFile).Decode(conf); err != nil {
res.SetError(err, cmds.ErrNormal)
return
}
}

if err := doInit(os.Stdout, req.InvocContext().ConfigRoot, empty, nBitsForKeypair, conf); err != nil {
res.SetError(err, cmds.ErrNormal)
return
}
Expand All @@ -88,10 +108,10 @@ Reinitializing would overwrite your keys.
`)

func initWithDefaults(out io.Writer, repoRoot string) error {
return doInit(out, repoRoot, false, nBitsForKeypairDefault)
return doInit(out, repoRoot, false, nBitsForKeypairDefault, nil)
}

func doInit(out io.Writer, repoRoot string, empty bool, nBitsForKeypair int) error {
func doInit(out io.Writer, repoRoot string, empty bool, nBitsForKeypair int, conf *config.Config) error {
if _, err := fmt.Fprintf(out, "initializing ipfs node at %s\n", repoRoot); err != nil {
return err
}
Expand All @@ -104,9 +124,12 @@ func doInit(out io.Writer, repoRoot string, empty bool, nBitsForKeypair int) err
return errRepoExists
}

conf, err := config.Init(out, nBitsForKeypair)
if err != nil {
return err
if conf == nil {
var err error
conf, err = config.Init(out, nBitsForKeypair)
if err != nil {
return err
}
}

if err := fsrepo.Init(repoRoot, conf); err != nil {
Expand Down
59 changes: 25 additions & 34 deletions commands/cli/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,15 +244,6 @@ func parseArgs(inputs []string, stdin *os.File, argDefs []cmds.Argument, recursi
stdin = nil
}

// check if stdin is coming from terminal or is being piped in
if stdin != nil {
if term, err := isTerminal(stdin); err != nil {
return nil, nil, err
} else if term {
stdin = nil // set to nil so we ignore it
}
}

// count required argument definitions
numRequired := 0
for _, argDef := range argDefs {
Expand Down Expand Up @@ -293,9 +284,18 @@ func parseArgs(inputs []string, stdin *os.File, argDefs []cmds.Argument, recursi
numRequired--
}

fillingVariadic := argDefIndex+1 > len(argDefs)

var err error
if argDef.Type == cmds.ArgString {
if stdin == nil {
if len(inputs) > 0 {
// If argument is "-" use stdin
if inputs[0] == "-" && argDef.SupportsStdin {
stringArgs, stdin, err = appendStdinAsString(stringArgs, stdin)
if err != nil {
return nil, nil, err
}
}
// add string values
stringArgs, inputs = appendString(stringArgs, inputs)
} else if !argDef.SupportsStdin {
Expand All @@ -307,35 +307,39 @@ func parseArgs(inputs []string, stdin *os.File, argDefs []cmds.Argument, recursi

stringArgs, inputs = appendString(stringArgs, inputs)
} else {
if len(inputs) > 0 {
// don't use stdin if we have inputs
stdin = nil
} else {
if stdin != nil && argDef.Required && !fillingVariadic {
// if we have a stdin, read it in and use the data as a string value
stringArgs, stdin, err = appendStdinAsString(stringArgs, stdin)
if err != nil {
return nil, nil, err
}
} else {
break
}
}
} else if argDef.Type == cmds.ArgFile {
if stdin == nil || !argDef.SupportsStdin {
if len(inputs) > 0 {
// treat stringArg values as file paths
fpath := inputs[0]
inputs = inputs[1:]
file, err := appendFile(fpath, argDef, recursive, hidden)
var file files.File
var err error
if fpath == "-" {
file = files.NewReaderFile("", "", stdin, nil)
} else {
file, err = appendFile(fpath, argDef, recursive, hidden)
}
if err != nil {
return nil, nil, err
}

fileArgs[fpath] = file
} else {
if len(inputs) > 0 {
// don't use stdin if we have inputs
stdin = nil
} else {
// if we have a stdin, create a file from it
if stdin != nil && argDef.SupportsStdin &&
argDef.Required && !fillingVariadic {
fileArgs[""] = files.NewReaderFile("", "", stdin, nil)
} else {
break
}
}
}
Expand Down Expand Up @@ -431,16 +435,3 @@ func appendFile(fpath string, argDef *cmds.Argument, recursive, hidden bool) (fi

return files.NewSerialFile(path.Base(fpath), fpath, hidden, stat)
}

// isTerminal returns true if stdin is a Stdin pipe (e.g. `cat file | ipfs`),
// and false otherwise (e.g. nothing is being piped in, so stdin is
// coming from the terminal)
func isTerminal(stdin *os.File) (bool, error) {
stat, err := stdin.Stat()
if err != nil {
return false, err
}

// if stdin is a CharDevice, return true
return ((stat.Mode() & os.ModeCharDevice) != 0), nil
}
1 change: 1 addition & 0 deletions test/bin/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@
!checkflags
!continueyn
!verify-go-fmt.sh
!time-out
83 changes: 83 additions & 0 deletions test/bin/time-out
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
#!/bin/bash
Copy link
Contributor

Choose a reason for hiding this comment

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

I am a bit concerned that this script is written in bash.

Copy link
Member Author

Choose a reason for hiding this comment

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

@chriscool we removed this script in #3152

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I saw it afterwards. Thanks for having written something more portable!

#
# The Bash shell script executes a command with a time-out.
# Upon time-out expiration SIGTERM (15) is sent to the process. If the signal
# is blocked, then the subsequent SIGKILL (9) terminates it.
#
# Based on the Bash documentation example.
Copy link
Member

Choose a reason for hiding this comment

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

@Kubuxu did you write this? if not, what is the license?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is Public Domain, I had previously credited the original author but removed it with other comments as for @RichardLitt 's request. I shouldn't have removed the author line, it was a signature for a short letter there.

Copy link
Member

@jbenet jbenet Aug 28, 2016

Choose a reason for hiding this comment

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

licensing and author lines need to be back before release.

Copy link
Member Author

Choose a reason for hiding this comment

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

This should address the license issue: #3152


scriptName="${0##*/}"

declare -i DEFAULT_TIMEOUT=9
declare -i DEFAULT_INTERVAL=1
declare -i DEFAULT_DELAY=1

# Timeout.
declare -i timeout=DEFAULT_TIMEOUT
# Interval between checks if the process is still alive.
declare -i interval=DEFAULT_INTERVAL
# Delay between posting the SIGTERM signal and destroying the process by SIGKILL.
declare -i delay=DEFAULT_DELAY

function printUsage() {
cat <<EOF

Synopsis
$scriptName [-t timeout] [-i interval] [-d delay] command
Execute a command with a time-out.
Upon time-out expiration SIGTERM (15) is sent to the process. If SIGTERM
signal is blocked, then the subsequent SIGKILL (9) terminates it.

-t timeout
Number of seconds to wait for command completion.
Default value: $DEFAULT_TIMEOUT seconds.

-i interval
Interval between checks if the process is still alive.
Positive integer, default value: $DEFAULT_INTERVAL seconds.

-d delay
Delay between posting the SIGTERM signal and destroying the
process by SIGKILL. Default value: $DEFAULT_DELAY seconds.

As of today, Bash does not support floating point arithmetic (sleep does),
therefore all delay/time values must be integers.
Copy link
Contributor

Choose a reason for hiding this comment

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

We have go-sleep that accepts floating point delays.

EOF
}

# Options.
while getopts ":t:i:d:" option; do
case "$option" in
t) timeout=$OPTARG ;;
i) interval=$OPTARG ;;
d) delay=$OPTARG ;;
*) printUsage; exit 1 ;;
esac
done
shift $((OPTIND - 1))

# $# should be at least 1 (the command to execute), however it may be strictly
# greater than 1 if the command itself has options.
if (($# == 0 || interval <= 0)); then
printUsage
exit 1
fi

# kill -0 pid Exit code indicates if a signal may be sent to $pid process.
(
((t = timeout))

while ((t > 0)); do
sleep $interval
kill -0 $$ || exit 0
((t -= interval))
done

# Be nice, post SIGTERM first.
# The 'exit 0' below will be executed if any preceeding command fails.
kill -s SIGTERM $$ && kill -0 $$ || exit 0
sleep $delay
kill -s SIGKILL $$
) 2> /dev/null &

exec "$@"
55 changes: 55 additions & 0 deletions test/sharness/t0022-init-default.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
#!/bin/sh
#
# Copyright (c) 2014 Christian Couder
# MIT Licensed; see the LICENSE file in this repository.
#

test_description="Test init command with default config"

. lib/test-lib.sh

cfg_key="Addresses.API"
cfg_val="/ip4/0.0.0.0/tcp/5001"

# test that init succeeds
test_expect_success "ipfs init succeeds" '
export IPFS_PATH="$(pwd)/.ipfs" &&
echo "IPFS_PATH: \"$IPFS_PATH\"" &&
BITS="2048" &&
ipfs init --bits="$BITS" >actual_init ||
test_fsh cat actual_init
'

test_expect_success ".ipfs/config has been created" '
test -f "$IPFS_PATH"/config ||
test_fsh ls -al .ipfs
'

test_expect_success "ipfs config succeeds" '
ipfs config $cfg_flags "$cfg_key" "$cfg_val"
'

test_expect_success "ipfs read config succeeds" '
IPFS_DEFAULT_CONFIG=$(cat "$IPFS_PATH"/config)
'

test_expect_success "clean up ipfs dir" '
rm -rf "$IPFS_PATH"
'

test_expect_success "ipfs init default config succeeds" '
echo $IPFS_DEFAULT_CONFIG | ipfs init - >actual_init ||
test_fsh cat actual_init
'

test_expect_success "ipfs config output looks good" '
echo "$cfg_val" >expected &&
ipfs config "$cfg_key" >actual &&
test_cmp expected actual
'

test_launch_ipfs_daemon

test_kill_ipfs_daemon

test_done
6 changes: 6 additions & 0 deletions test/sharness/t0500-issues-and-regressions-offline.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,10 @@ test_init_ipfs

# Tests go here

test_expect_success "ipfs init with occupied input works - #2748" '
export IPFS_PATH="ipfs_path"
echo "" | time-out ipfs init &&
rm -rf ipfs_path
'

test_done