@cryptotaxi247 / kubo / commits / 408fadc8b

fix(2/main) don't check for updates when running init

@jbenet @mappum Yeah, there's some duplicated work. But there's also a separation of concerns. In one case, we check to determine where the command should run. In the other case, we check to determine which hooks should run. Having these actions separated reduces complexity in a nice way. License: MIT Signed-off-by: Brian Tiger Chow <brian@perfmode.com>

Brian Tiger Chow committed Nov 14, 2014 at 16:51 UTC 408fadc8beb952d67021244d4868228382fa58f4
2 files changed +75 -19
cmd/ipfs2/ipfs.go
+11 -4
@@ -55,6 +55,13 @@ type cmdDetails struct {
55 cannotRunOnClient bool
56 cannotRunOnDaemon bool
57 doesNotUseRepo bool
58 +
59 + // initializesConfig describes commands that initialize the config.
60 + // pre-command hooks that require configs must not be run before this
61 + // command
62 + initializesConfig bool
63 +
64 + preemptsUpdates bool
65 }
66
67 func (d *cmdDetails) String() string {
@@ -71,14 +78,14 @@ func (d *cmdDetails) usesRepo() bool { return !d.doesNotUseRepo }
78 // properties so that other code can make decisions about whether to invoke a
79 // command or return an error to the user.
80 var cmdDetailsMap = map[*cmds.Command]cmdDetails{
74 - initCmd: cmdDetails{cannotRunOnDaemon: true, doesNotUseRepo: true},
81 + initCmd: cmdDetails{initializesConfig: true, cannotRunOnDaemon: true, doesNotUseRepo: true},
82 daemonCmd: cmdDetails{cannotRunOnDaemon: true},
83 commandsClientCmd: cmdDetails{doesNotUseRepo: true},
84 commands.CommandsDaemonCmd: cmdDetails{doesNotUseRepo: true},
85 commands.DiagCmd: cmdDetails{cannotRunOnClient: true},
86 commands.VersionCmd: cmdDetails{doesNotUseRepo: true},
80 - commands.UpdateCmd: cmdDetails{cannotRunOnDaemon: true},
81 - commands.UpdateCheckCmd: cmdDetails{},
82 - commands.UpdateLogCmd: cmdDetails{},
87 + commands.UpdateCmd: cmdDetails{preemptsUpdates: true, cannotRunOnDaemon: true},
88 + commands.UpdateCheckCmd: cmdDetails{preemptsUpdates: true},
89 + commands.UpdateLogCmd: cmdDetails{preemptsUpdates: true},
90 commands.LogCmd: cmdDetails{cannotRunOnClient: true},
91 }
cmd/ipfs2/main.go
+64 -15
@@ -20,6 +20,7 @@ import (
20 daemon "github.com/jbenet/go-ipfs/daemon2"
21 updates "github.com/jbenet/go-ipfs/updates"
22 u "github.com/jbenet/go-ipfs/util"
23 + "github.com/jbenet/go-ipfs/util/debugerror"
24 )
25
26 // log is the command logger
@@ -201,21 +202,61 @@ func (i *cmdInvocation) requestedHelp() (short bool, long bool, err error) {
202 return longHelp, shortHelp, nil
203 }
204
205 +func callPreCommandHooks(details cmdDetails, req cmds.Request, root *cmds.Command) error {
206 +
207 + log.Debug("Calling pre-command hooks...")
208 +
209 + // some hooks only run when the command is executed locally
210 + daemon, err := commandShouldRunOnDaemon(details, req, root)
211 + if err != nil {
212 + return err
213 + }
214 +
215 + // check for updates when 1) commands is going to be run locally, 2) the
216 + // command does not initialize the config, and 3) the command does not
217 + // pre-empt updates
218 + if !daemon && !details.initializesConfig && !details.preemptsUpdates {
219 +
220 + log.Debug("Calling hook: Check for updates")
221 +
222 + cfg, err := req.Context().GetConfig()
223 + if err != nil {
224 + return err
225 + }
226 + // Check for updates and potentially install one.
227 + if err := updates.CliCheckForUpdates(cfg, req.Context().ConfigRoot); err != nil {
228 + return err
229 + }
230 + }
231 +
232 + return nil
233 +}
234 +
235 func callCommand(req cmds.Request, root *cmds.Command) (cmds.Response, error) {
236 var res cmds.Response
237
207 - useDaemon, err := commandShouldRunOnDaemon(req, root)
238 + details, err := commandDetails(req.Path(), root)
239 + if err != nil {
240 + return nil, err
241 + }
242 +
243 + useDaemon, err := commandShouldRunOnDaemon(*details, req, root)
244 if err != nil {
245 return nil, err
246 }
247
212 - cfg, err := req.Context().GetConfig()
248 + err = callPreCommandHooks(*details, req, root)
249 if err != nil {
250 return nil, err
251 }
252
253 if useDaemon {
254
255 + cfg, err := req.Context().GetConfig()
256 + if err != nil {
257 + return nil, err
258 + }
259 +
260 addr, err := ma.NewMultiaddr(cfg.Addresses.API)
261 if err != nil {
262 return nil, err
@@ -237,11 +278,6 @@ func callCommand(req cmds.Request, root *cmds.Command) (cmds.Response, error) {
278 } else {
279 log.Info("Executing command locally")
280
240 - // Check for updates and potentially install one.
241 - if err := updates.CliCheckForUpdates(cfg, req.Context().ConfigRoot); err != nil {
242 - return nil, err
243 - }
244 -
281 // this sets up the function that will initialize the node
282 // this is so that we can construct the node lazily.
283 ctx := req.Context()
@@ -267,13 +303,11 @@ func callCommand(req cmds.Request, root *cmds.Command) (cmds.Response, error) {
303 return res, nil
304 }
305
270 -func commandShouldRunOnDaemon(req cmds.Request, root *cmds.Command) (bool, error) {
271 - path := req.Path()
272 - // root command.
273 - if len(path) < 1 {
274 - return false, nil
275 - }
276 -
306 +// commandDetails returns a command's details for the command given by |path|
307 +// within the |root| command tree.
308 +//
309 +// Returns an error if the command is not found in the Command tree.
310 +func commandDetails(path []string, root *cmds.Command) (*cmdDetails, error) {
311 var details cmdDetails
312 // find the last command in path that has a cmdDetailsMap entry
313 cmd := root
@@ -281,7 +315,7 @@ func commandShouldRunOnDaemon(req cmds.Request, root *cmds.Command) (bool, error
315 var found bool
316 cmd, found = cmd.Subcommands[cmp]
317 if !found {
284 - return false, fmt.Errorf("subcommand %s should be in root", cmp)
318 + return nil, debugerror.Errorf("subcommand %s should be in root", cmp)
319 }
320
321 if cmdDetails, found := cmdDetailsMap[cmd]; found {
@@ -289,6 +323,21 @@ func commandShouldRunOnDaemon(req cmds.Request, root *cmds.Command) (bool, error
323 }
324 }
325 log.Debugf("cmd perms for +%v: %s", path, details.String())
326 + return &details, nil
327 +}
328 +
329 +// commandShouldRunOnDaemon determines, from commmand details, whether a
330 +// command ought to be executed on an IPFS daemon.
331 +//
332 +// It returns true if the command should be executed on a daemon and false if
333 +// it should be executed on a client. It returns an error if the command must
334 +// NOT be executed on either.
335 +func commandShouldRunOnDaemon(details cmdDetails, req cmds.Request, root *cmds.Command) (bool, error) {
336 + path := req.Path()
337 + // root command.
338 + if len(path) < 1 {
339 + return false, nil
340 + }
341
342 if details.cannotRunOnClient && details.cannotRunOnDaemon {
343 return false, fmt.Errorf("command disabled: %s", path[0])