Merge pull request #3806 from ipfs/lgierth-patch-1

commands: improve name and key helptexts

Author: whyrusleeping

cmd/ipfs/daemon.go

@@ -11,7 +11,9 @@ import (
 	"sort"
 	"sync"
 
-	cmds "github.com/ipfs/go-ipfs/commands"
+	"github.com/ipfs/go-ipfs-cmds/cmdsutil"
+
+	cmds "github.com/ipfs/go-ipfs-cmds"
 	"github.com/ipfs/go-ipfs/core"
 	commands "github.com/ipfs/go-ipfs/core/commands"
 	corehttp "github.com/ipfs/go-ipfs/core/corehttp"
@@ -54,7 +56,7 @@ const (
 )
 
 var daemonCmd = &cmds.Command{
-	Helptext: cmds.HelpText{
+	Helptext: cmdsutil.HelpText{
 		Tagline: "Run a network-connected IPFS node.",
 		ShortDescription: `
 'ipfs daemon' runs a persistent ipfs daemon that can serve commands
@@ -145,24 +147,25 @@ Headers.
 `,
 	},
 
-	Options: []cmds.Option{
-		cmds.BoolOption(initOptionKwd, "Initialize ipfs with default settings if not already initialized").Default(false),
-		cmds.StringOption(routingOptionKwd, "Overrides the routing option").Default("dht"),
-		cmds.BoolOption(mountKwd, "Mounts IPFS to the filesystem").Default(false),
-		cmds.BoolOption(writableKwd, "Enable writing objects (with POST, PUT and DELETE)").Default(false),
-		cmds.StringOption(ipfsMountKwd, "Path to the mountpoint for IPFS (if using --mount). Defaults to config setting."),
-		cmds.StringOption(ipnsMountKwd, "Path to the mountpoint for IPNS (if using --mount). Defaults to config setting."),
-		cmds.BoolOption(unrestrictedApiAccessKwd, "Allow API access to unlisted hashes").Default(false),
-		cmds.BoolOption(unencryptTransportKwd, "Disable transport encryption (for debugging protocols)").Default(false),
-		cmds.BoolOption(enableGCKwd, "Enable automatic periodic repo garbage collection").Default(false),
-		cmds.BoolOption(adjustFDLimitKwd, "Check and raise file descriptor limits if needed").Default(true),
-		cmds.BoolOption(offlineKwd, "Run offline. Do not connect to the rest of the network but provide local API.").Default(false),
-		cmds.BoolOption(migrateKwd, "If true, assume yes at the migrate prompt. If false, assume no."),
-		cmds.BoolOption(enableFloodSubKwd, "Instantiate the ipfs daemon with the experimental pubsub feature enabled."),
-		cmds.BoolOption(enableMultiplexKwd, "Add the experimental 'go-multiplex' stream muxer to libp2p on construction.").Default(true),
+	Options: []cmdsutil.Option{
+		cmdsutil.BoolOption(initOptionKwd, "Initialize ipfs with default settings if not already initialized").Default(false),
+		cmdsutil.StringOption(routingOptionKwd, "Overrides the routing option").Default("dht"),
+		cmdsutil.BoolOption(mountKwd, "Mounts IPFS to the filesystem").Default(false),
+		cmdsutil.BoolOption(writableKwd, "Enable writing objects (with POST, PUT and DELETE)").Default(false),
+		cmdsutil.StringOption(ipfsMountKwd, "Path to the mountpoint for IPFS (if using --mount). Defaults to config setting."),
+		cmdsutil.StringOption(ipnsMountKwd, "Path to the mountpoint for IPNS (if using --mount). Defaults to config setting."),
+		cmdsutil.BoolOption(unrestrictedApiAccessKwd, "Allow API access to unlisted hashes").Default(false),
+		cmdsutil.BoolOption(unencryptTransportKwd, "Disable transport encryption (for debugging protocols)").Default(false),
+		cmdsutil.BoolOption(enableGCKwd, "Enable automatic periodic repo garbage collection").Default(false),
+		cmdsutil.BoolOption(adjustFDLimitKwd, "Check and raise file descriptor limits if needed").Default(true),
+		cmdsutil.BoolOption(offlineKwd, "Run offline. Do not connect to the rest of the network but provide local API.").Default(false),
+		cmdsutil.BoolOption(migrateKwd, "If true, assume yes at the migrate prompt. If false, assume no."),
+		cmdsutil.BoolOption(enableFloodSubKwd, "Instantiate the ipfs daemon with the experimental pubsub feature enabled."),
+		cmdsutil.BoolOption(enableMultiplexKwd, "Add the experimental 'go-multiplex' stream muxer to libp2p on construction."),
+
 		// TODO: add way to override addresses. tricky part: updating the config if also --init.
-		// cmds.StringOption(apiAddrKwd, "Address for the daemon rpc API (overrides config)"),
-		// cmds.StringOption(swarmAddrKwd, "Address for the swarm socket (overrides config)"),
+		// cmdsutil.StringOption(apiAddrKwd, "Address for the daemon rpc API (overrides config)"),
+		// cmdsutil.StringOption(swarmAddrKwd, "Address for the swarm socket (overrides config)"),
 	},
 	Subcommands: map[string]*cmds.Command{},
 	Run:         daemonFunc,
@@ -181,7 +184,7 @@ func defaultMux(path string) corehttp.ServeOption {
 
 var fileDescriptorCheck = func() error { return nil }
 
-func daemonFunc(req cmds.Request, res cmds.Response) {
+func daemonFunc(req cmds.Request, re cmds.ResponseEmitter) {
 	// Inject metrics before we do anything
 
 	err := mprome.Inject()
@@ -221,7 +224,7 @@ func daemonFunc(req cmds.Request, res cmds.Response) {
 	// running in an uninitialized state.
 	initialize, _, err := req.Option(initOptionKwd).Bool()
 	if err != nil {
-		res.SetError(err, cmds.ErrNormal)
+		re.SetError(err, cmdsutil.ErrNormal)
 		return
 	}
 
@@ -234,7 +237,7 @@ func daemonFunc(req cmds.Request, res cmds.Response) {
 		if !util.FileExists(req.InvocContext().ConfigRoot) {
 			err := initWithDefaults(os.Stdout, req.InvocContext().ConfigRoot)
 			if err != nil {
-				res.SetError(err, cmds.ErrNormal)
+				re.SetError(err, cmdsutil.ErrNormal)
 				return
 			}
 		}
@@ -245,7 +248,7 @@ func daemonFunc(req cmds.Request, res cmds.Response) {
 	repo, err := fsrepo.Open(req.InvocContext().ConfigRoot)
 	switch err {
 	default:
-		res.SetError(err, cmds.ErrNormal)
+		re.SetError(err, cmdsutil.ErrNormal)
 		return
 	case fsrepo.ErrNeedMigration:
 		domigrate, found, _ := req.Option(migrateKwd).Bool()
@@ -258,7 +261,7 @@ func daemonFunc(req cmds.Request, res cmds.Response) {
 		if !domigrate {
 			fmt.Println("Not running migrations of fs-repo now.")
 			fmt.Println("Please get fs-repo-migrations from https://dist.ipfs.io")
-			res.SetError(fmt.Errorf("fs-repo requires migration"), cmds.ErrNormal)
+			re.SetError(fmt.Errorf("fs-repo requires migration"), cmdsutil.ErrNormal)
 			return
 		}
 
@@ -268,13 +271,13 @@ func daemonFunc(req cmds.Request, res cmds.Response) {
 			fmt.Printf("  %s\n", err)
 			fmt.Println("If you think this is a bug, please file an issue and include this whole log output.")
 			fmt.Println("  https://github.com/ipfs/fs-repo-migrations")
-			res.SetError(err, cmds.ErrNormal)
+			re.SetError(err, cmdsutil.ErrNormal)
 			return
 		}
 
 		repo, err = fsrepo.Open(req.InvocContext().ConfigRoot)
 		if err != nil {
-			res.SetError(err, cmds.ErrNormal)
+			re.SetError(err, cmdsutil.ErrNormal)
 			return
 		}
 	case nil:
@@ -283,7 +286,7 @@ func daemonFunc(req cmds.Request, res cmds.Response) {
 
 	cfg, err := ctx.GetConfig()
 	if err != nil {
-		res.SetError(err, cmds.ErrNormal)
+		re.SetError(err, cmdsutil.ErrNormal)
 		return
 	}
 
@@ -305,14 +308,14 @@ func daemonFunc(req cmds.Request, res cmds.Response) {
 
 	routingOption, _, err := req.Option(routingOptionKwd).String()
 	if err != nil {
-		res.SetError(err, cmds.ErrNormal)
+		re.SetError(err, cmdsutil.ErrNormal)
 		return
 	}
 	switch routingOption {
 	case routingOptionSupernodeKwd:
 		servers, err := cfg.SupernodeRouting.ServerIPFSAddrs()
 		if err != nil {
-			res.SetError(err, cmds.ErrNormal)
+			re.SetError(err, cmdsutil.ErrNormal)
 			repo.Close() // because ownership hasn't been transferred to the node
 			return
 		}
@@ -332,14 +335,14 @@ func daemonFunc(req cmds.Request, res cmds.Response) {
 	case routingOptionNoneKwd:
 		ncfg.Routing = core.NilRouterOption
 	default:
-		res.SetError(fmt.Errorf("unrecognized routing option: %s", routingOption), cmds.ErrNormal)
+		re.SetError(fmt.Errorf("unrecognized routing option: %s", routingOption), cmdsutil.ErrNormal)
 		return
 	}
 
 	node, err := core.NewNode(req.Context(), ncfg)
 	if err != nil {
 		log.Error("error from node construction: ", err)
-		res.SetError(err, cmds.ErrNormal)
+		re.SetError(err, cmdsutil.ErrNormal)
 		return
 	}
 	node.SetLocal(false)
@@ -370,32 +373,43 @@ func daemonFunc(req cmds.Request, res cmds.Response) {
 	// construct api endpoint - every time
 	err, apiErrc := serveHTTPApi(req)
 	if err != nil {
-		res.SetError(err, cmds.ErrNormal)
+		re.SetError(err, cmdsutil.ErrNormal)
 		return
 	}
 
+	// construct http gateway - if it is set in the config
+	var gwErrc <-chan error
+	if len(cfg.Addresses.Gateway) > 0 {
+		var err error
+		err, gwErrc = serveHTTPGateway(req)
+		if err != nil {
+			re.SetError(err, cmdsutil.ErrNormal)
+			return
+		}
+	}
+
 	// construct fuse mountpoints - if the user provided the --mount flag
 	mount, _, err := req.Option(mountKwd).Bool()
 	if err != nil {
-		res.SetError(err, cmds.ErrNormal)
+		re.SetError(err, cmdsutil.ErrNormal)
 		return
 	}
 	if mount && offline {
-		res.SetError(errors.New("mount is not currently supported in offline mode"),
-			cmds.ErrClient)
+		re.SetError(errors.New("mount is not currently supported in offline mode"),
+			cmdsutil.ErrClient)
 		return
 	}
 	if mount {
 		if err := mountFuse(req); err != nil {
-			res.SetError(err, cmds.ErrNormal)
+			re.SetError(err, cmdsutil.ErrNormal)
 			return
 		}
 	}
 
 	// repo blockstore GC - if --enable-gc flag is present
 	err, gcErrc := maybeRunGC(req, node)
 	if err != nil {
-		res.SetError(err, cmds.ErrNormal)
+		re.SetError(err, cmdsutil.ErrNormal)
 		return
 	}
 
@@ -419,7 +433,7 @@ func daemonFunc(req cmds.Request, res cmds.Response) {
 	for err := range merge(apiErrc, gwErrc, gcErrc) {
 		if err != nil {
 			log.Error(err)
-			res.SetError(err, cmds.ErrNormal)
+			re.SetError(err, cmdsutil.ErrNormal)
 		}
 	}
 	return

cmd/ipfs/init.go

@@ -1,28 +1,30 @@
 package main
 
 import (
+	"context"
 	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
 	"os"
 	"path"
 
-	context "context"
 	assets "github.com/ipfs/go-ipfs/assets"
 	cmds "github.com/ipfs/go-ipfs/commands"
 	core "github.com/ipfs/go-ipfs/core"
 	namesys "github.com/ipfs/go-ipfs/namesys"
 	config "github.com/ipfs/go-ipfs/repo/config"
 	fsrepo "github.com/ipfs/go-ipfs/repo/fsrepo"
+
+	"github.com/ipfs/go-ipfs-cmds/cmdsutil"
 )
 
 const (
 	nBitsForKeypairDefault = 2048
 )
 
 var initCmd = &cmds.Command{
-	Helptext: cmds.HelpText{
+	Helptext: cmdsutil.HelpText{
 		Tagline: "Initializes ipfs config file.",
 		ShortDescription: `
 Initializes ipfs configuration files and generates a new keypair.
@@ -34,17 +36,17 @@ environment variable:
     export IPFS_PATH=/path/to/ipfsrepo
 `,
 	},
-	Arguments: []cmds.Argument{
-		cmds.FileArg("default-config", false, false, "Initialize with the given configuration.").EnableStdin(),
+	Arguments: []cmdsutil.Argument{
+		cmdsutil.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),
+	Options: []cmdsutil.Option{
+		cmdsutil.IntOption("bits", "b", "Number of bits to use in the generated RSA private key.").Default(nBitsForKeypairDefault),
+		cmdsutil.BoolOption("empty-repo", "e", "Don't add and pin help files to the local storage.").Default(false),
 
 		// TODO need to decide whether to expose the override as a file or a
 		// directory. That is: should we allow the user to also specify the
 		// name of the file?
-		// TODO cmds.StringOption("event-logs", "l", "Location for machine-readable event logs."),
+		// TODO cmdsutil.StringOption("event-logs", "l", "Location for machine-readable event logs."),
 	},
 	PreRun: func(req cmds.Request) error {
 		daemonLocked, err := fsrepo.LockedByOtherProcess(req.InvocContext().ConfigRoot)
@@ -63,19 +65,19 @@ environment variable:
 	},
 	Run: func(req cmds.Request, res cmds.Response) {
 		if req.InvocContext().Online {
-			res.SetError(errors.New("init must be run offline only!"), cmds.ErrNormal)
+			res.SetError(errors.New("init must be run offline only!"), cmdsutil.ErrNormal)
 			return
 		}
 
 		empty, _, err := req.Option("e").Bool()
 		if err != nil {
-			res.SetError(err, cmds.ErrNormal)
+			res.SetError(err, cmdsutil.ErrNormal)
 			return
 		}
 
 		nBitsForKeypair, _, err := req.Option("b").Int()
 		if err != nil {
-			res.SetError(err, cmds.ErrNormal)
+			res.SetError(err, cmdsutil.ErrNormal)
 			return
 		}
 
@@ -85,19 +87,19 @@ environment variable:
 		if f != nil {
 			confFile, err := f.NextFile()
 			if err != nil {
-				res.SetError(err, cmds.ErrNormal)
+				res.SetError(err, cmdsutil.ErrNormal)
 				return
 			}
 
 			conf = &config.Config{}
 			if err := json.NewDecoder(confFile).Decode(conf); err != nil {
-				res.SetError(err, cmds.ErrNormal)
+				res.SetError(err, cmdsutil.ErrNormal)
 				return
 			}
 		}
 
 		if err := doInit(os.Stdout, req.InvocContext().ConfigRoot, empty, nBitsForKeypair, conf); err != nil {
-			res.SetError(err, cmds.ErrNormal)
+			res.SetError(err, cmdsutil.ErrNormal)
 			return
 		}
 	},

cmd/ipfs/ipfs.go

@@ -3,7 +3,9 @@ package main
 import (
 	"fmt"
 
-	cmds "github.com/ipfs/go-ipfs/commands"
+	cmds "github.com/ipfs/go-ipfs-cmds"
+
+	oldcmds "github.com/ipfs/go-ipfs/commands"
 	commands "github.com/ipfs/go-ipfs/core/commands"
 )
 
@@ -22,7 +24,7 @@ var commandsClientCmd = commands.CommandsCmd(Root)
 // They can override subcommands in commands.Root by defining a subcommand with the same name.
 var localCommands = map[string]*cmds.Command{
 	"daemon":   daemonCmd,
-	"init":     initCmd,
+	"init":     cmds.NewCommand(initCmd),
 	"commands": commandsClientCmd,
 }
 var localMap = make(map[*cmds.Command]bool)
@@ -31,8 +33,14 @@ func init() {
 	// setting here instead of in literal to prevent initialization loop
 	// (some commands make references to Root)
 	Root.Subcommands = localCommands
+	Root.OldSubcommands = map[string]*oldcmds.Command{}
 
 	// copy all subcommands from commands.Root into this root (if they aren't already present)
+	for k, v := range commands.Root.OldSubcommands {
+		if _, found := Root.OldSubcommands[k]; !found {
+			Root.OldSubcommands[k] = v
+		}
+	}
 	for k, v := range commands.Root.Subcommands {
 		if _, found := Root.Subcommands[k]; !found {
 			Root.Subcommands[k] = v
@@ -95,17 +103,16 @@ func (d *cmdDetails) usesRepo() bool                 { return !d.doesNotUseRepo
 // not being able to run on all the same contexts. This map describes these
 // properties so that other code can make decisions about whether to invoke a
 // command or return an error to the user.
-var cmdDetailsMap = map[*cmds.Command]cmdDetails{
-	initCmd: {doesNotUseConfigAsInput: true, cannotRunOnDaemon: true, doesNotUseRepo: true},
-
-	// daemonCmd allows user to initialize the config. Thus, it may be called
-	// without using the config as input
-	daemonCmd:                             {doesNotUseConfigAsInput: true, cannotRunOnDaemon: true},
-	commandsClientCmd:                     {doesNotUseRepo: true},
-	commands.CommandsDaemonCmd:            {doesNotUseRepo: true},
-	commands.VersionCmd:                   {doesNotUseConfigAsInput: true, doesNotUseRepo: true}, // must be permitted to run before init
-	commands.LogCmd:                       {cannotRunOnClient: true},
-	commands.ActiveReqsCmd:                {cannotRunOnClient: true},
-	commands.RepoFsckCmd:                  {cannotRunOnDaemon: true},
-	commands.ConfigCmd.Subcommand("edit"): {cannotRunOnDaemon: true, doesNotUseRepo: true},
+//
+// TODO WIP keks : switch to paths as keys
+//var cmdDetailsMap = map[*cmds.Command]cmdDetails{
+var cmdDetailsMap = map[string]cmdDetails{
+	"init":        {doesNotUseConfigAsInput: true, cannotRunOnDaemon: true, doesNotUseRepo: true},
+	"daemon":      {doesNotUseConfigAsInput: true, cannotRunOnDaemon: true},
+	"commands":    {doesNotUseRepo: true},
+	"version":     {doesNotUseConfigAsInput: true, doesNotUseRepo: true}, // must be permitted to run before init
+	"log":         {cannotRunOnClient: true},
+	"diag/cmds":   {cannotRunOnClient: true},
+	"repo/fsck":   {cannotRunOnDaemon: true},
+	"config/edit": {cannotRunOnDaemon: true, doesNotUseRepo: true},
 }