command line order is problematic

[zandr]

allmydata-tahoe put -u ​http://localhost:port LOCAL_FILE REMOTE_FILE

Assuming we're going to have to specify the -u at all (can we stash that in ~/.allmydata or similar?), it seems that options that are generic regardless of the command should be *before* the verb.

This is made more annoying by the fact that the usage/help specifies "put LOCAL_FILE REMOTE_FILE" with no meniton of -u, and the error if you get that wrong is an unhelpful "wrong number of arguments."

[warner]

As of an hour ago, you no longer need the '-u' .. all CLI commands default to using ~/.tahoe , including 'create-client' and 'start'. See #120 for details.

The other criticisms are still outstanding though. The short answer is that not all commands accept those "generic" parameters, so they live after the verb. Also, the directory to use (for start/stop/restart) can come as an actual argument (as opposed to an option/parameter/flag), so 'tahoe start node1' works. This is specifically useful for 'tahoe restart -m node*', to kick a whole directory full of nodes at once.

That said, enough of the CLI commands take that argument that maybe it *should* be moved before the verb. And certainly the silent confusion with LOCAL_FILE REMOTE_FILE is a big problem.

[zooko]

I kind of think the node-admin commands -- create-introducer, create-client, start, stop, and restart ought to be under a separate executable name than the "CLI" commands -- ls, put, get, mv, rm, etc..

This would fix Zandr's original complaint, and it would fit my brain. The first command could be tahoeadmin and the second one could be tahoe, for example.

[warner]

sounds reasonable to me. That matches how svn/svnadmin works too.

I'll work on this if nobody else beats me to it.

[warner]

hm, or how about 'tahoe ls' vs 'tahoe admin create-client' ? That way we only install a single executable to /usr/bin/tahoe (reducing user-perceived complexity), while still hiding the admin-only commands from a normal 'tahoe --help' or bare 'tahoe' command.

If we write man pages, we could have separate ones for tahoe(1) and tahoe_admin(1). A couple tools I've been looking at recently do it this way, like imapfilter.

There are other projects that use this approach; the ones that come to mind are more distributed and make it more likely that regular users will be using the "admin"-oriented commands. darcs and mercurial both have 'init' and various repair/optimize operations as part of their normal subcommand set (they install exactly one executable), but still have a flat command namespace.

Hm, so I guess this is a flat-vs-nested question as well as a UI-affordance (keeping the 'tahoe help' listing short enough to find the interesting commands) question. So the three options here are:

1. 'tahoe ls' vs 'tahoe create-client' (the 0.6.1 behavior)
2. 'tahoe ls' vs 'tahoeadmin create-client'
3. 'tahoe ls' vs 'tahoe admin create-client'

I'm leaning towards number three right now.. thoughts?

[zandr]

I'm with you, 3. is the way to go. Attention should be paid to producing meaningful error/usage info, as described in the initial report.

[zooko]

I'm happy with 3. I asked my friend Seb about it in e-mail, and his comment was something like "order of options shouldn't matter -- I didn't realize how much I cared about this until I met git, where order of options matters and it's a pain".

So, I don't know if tahoe can be made insensitive to order of options, but let it be known that there is at least one person out there who would like that.

[zooko]

We're focussing on an imminent v0.7.0 (see ​the roadmap) which hopefully has Small Distributed Mutable Files and also a fix for bad SHA-256. So I'm bumping less urgent tickets to v0.7.1.

[zooko]

As far as I understand the current cli is good enough for Zandr to use for Allmydata 3.0 rollout, so I'm bumping this ticket to "undecided" milestone and if Zandr wants something different then he should speak up.

[davidsarah]

Putting options after the verb creates a security hazard on Unix platforms: if the user tries to use wildcards to specify multiple local files (for tahoe cp, say), then a local filename may begin with - and be treated as an argument. If options had to be before the verb, that couldn't happen.

Although this hazard can be worked around either by prefixing each wildcard argument with ./ or (for most commands) using -- before any filename arguments, it's very easy to forget to do that. Also, more users know about the -- workaround than ./, and Tahoe does not currently appear to support --.

OTOH, requiring tahoe -r cp would look backwards to many users. I don't know how to resolve that.

(Windows doesn't have this problem because the shell doesn't automatically glob arguments.)

[davidsarah]

Riastradh on irc:

I typed `tahoe -d . ls root:' and it crashed, complaining that I have no ~/.tahoe directory, not complaining that I put the option in the wrong place.

So the problem is also that the error message when you get the argument order wrong can be quite unhelpful.

[davidsarah]

Replying to davidsarah:

So the problem is also that the error message when you get the argument order wrong can be quite unhelpful.

Another example of that, from munderwo on IRC:

tahoe create-alias default -d infra-client
.../bin/tahoe:  Wrong number of arguments.

(should be "tahoe create-alias -d infra-client default").

[davidsarah]

Greg Troxel wrote on tahoe-dev:

I'm trying to gradually get more serious in my tahoe usage - private grid and backups, not just playing with the pubgrid.

I now on several machines have client directories ~/.tahoe-pubgrid and ~/.tahoe-gdt. I have separate server processes on server machines.

So, I find myself doing

tahoe deep-check -d ~/.tahoe-gdt --add-lease --repair
tahoe deep-check -d ~/.tahoe-pubgrid --add-lease --repair

and naturally would like to have aliases

  tahoep () { tahoe -d ${HOME}/.tahoe-pubgrid $*; }
  tahoeg () { tahoe -d ${HOME}/.tahoe-gdt $*; }

but that won't work, because one must do "tahoe command [options]".

Since -d is about everything, it would be nice if it were accepted at the beginning.

[warner]

#1751 was another aspect of this: --node-directory is ignored when it appears before the subcommand. In general, the tahoe command/subcommand parser looks for arguments in two places: tahoe --ARG1 SUBCOMMAND --ARG2, but how they're combined is up to the subcommand.

What do people think about the following proposal?:

• the generic arguments (--quiet, --version, --version-and-path, --node-directory/-d) will only be accepted in the ARG1 slot, and all commands that use them (i.e. anything that needs a node directory to work with) will honor them from there.
• the tahoe create-*/start/stop/restart NODEDIR shortcuts will continue to work
• tahoe create-*/start/stop/restart will continue to accept a --basedir/-C option in the ARG2 slot
• only one of ARG1--node-directory / ARG2--basedir / NODEDIR will be accepted. Providing more than one will produce an error.

Note that there are several tahoe commands that don't need a node-directory (everything under "tahoe debug" and "tahoe admin"), but seeing how those are in the minority, I think it's better to put --node-directory in the ARG1 slot and have those commands just ignore it, rather than putting --node-directory in the ARG2 slot for every single command that can use it.

[davidsarah]

+1 for the comment:18 proposal.

[warner]

Hm, actually, would anyone mind if I removed the --basedir/-C option too? It's never been accepted in the ARG1 position (only in the ARG2 position for create/start/stop/restart), so it'd be simpler to remove it completely than to move it to a new place.

[davidsarah]

I think users probably have scripts using --basedir/-C in the ARG2 position for create/start/stop/restart, so that should continue to be accepted. It's easy in that case; just ignore the option.

[warner]

fix handling of --node-directory

[warner]

Ok, that patch makes the changes proposed in comment:18, leaves --basedir in place according to comment:21 (but pays attention to --basedir rather than ignoring it). It also requires that --version, --version-and-path, and --quiet live in ARG1, rejecting them if they appear in ARG2. Tests that exercise command-line parsing have been updated to match.

The patch is a bit big because it changes the way we use base classes in src/allmydata/scripts/ . Previously common.BaseOptions provided --version and --node-directory, and both the ARG1-handling runner.Options and all the ARG2-handling parsers inherited from it, and then the basedir-needing parsers additionally inherited from common.BasedirMixin.

In the new world, BaseOptions is much smaller (it forbids --version and sets up .command_name), BasedirOptions is a parent class instead of a mixin, the ARG1 parser inherits from neither (but now contains the --node-directory handler), and the ARG2 parsers all inherit from BaseOptions, or BasedirOptions if they need --basedir/BASEDIR (create/start/stop/restart).

[davidsarah]

Will review.

[davidsarah]

typo: 'argumnent' (on line 49 of the patch)

[warner]

improved patch (add default nodedir to --help in all commands, fix typo)

[davidsarah]

After discussing this on IRC we ended up with the following unresolved issues:

• --help for subcommands no longer mentions the global options
• because the new version accepts -d/--node-directory only before the command, and the old version accepted it only after the command (and ignored it if before), there is no way to write a script that works with both.

We considered allowing -d both before and after but could not decide whether that was worth the complexity. Zooko?

[warner]

add "[global-opts]" to command synopses

[warner]

latest patch (also visible in ​https://github.com/warner/tahoe-lafs/tree/166-args) fixes the first issue (mentioning global options)

So the remaining question is -d before the subcommand. I'm not hugely motivated to make it possible to write new scripts that work with old versions (unlike py2/py3 I think once you've upgrated to a new tahoe version, you'll just stick with it). And I prefer the simplicity of having fewer options (I still want to remove the -C/--basedir option from the node-admin commands, but want to respect David-Sarah's opinion that people may have scripts that will break).

[Zancas]

I believed that I could pass options to the tahoe utility instead of its subcommand so I attempted to run: >tahoe -d /home/arc/myLAEgateway/ backup /home/arc/sandbox/KandRC/ KandR: which failed (with an ugly traceback).

[warner]

On the dev call this morning, we agreed that davidsarah will look at the 166-3.diff patch and if they don't raise any major objections, warner will land it this coming weekend. Hopefully we can resolve the -d issue in the 1.11 timeframe.

[zooko]

Replying to davidsarah:

After discussing this on IRC we ended up with the following unresolved issues:

• --help for subcommands no longer mentions the global options

This seems like an important issue to me. Users have a hard enough time that if they ask for help we should really give it.

• because the new version accepts -d/--node-directory only before the command, and the old version accepted it only after the command (and ignored it if before), there is no way to write a script that works with both.

We considered allowing -d both before and after but could not decide whether that was worth the complexity. Zooko?

Hm... I'm not sure if there are any script-writers who are going to care about this. I guess since I don't have any evidence either way, I'd like to err on the side of ease-of-development on this. Especially since I'm reviewing this patch right now for inclusion in Tahoe-LAFS v1.10. So I think we should not worry about this compatibility-break.

[zooko]

Replying to warner:

And I prefer the simplicity of having fewer options (I still want to remove the -C/--basedir option from the node-admin commands, but want to respect David-Sarah's opinion that people may have scripts that will break).

FWIW, I would prefer that simplicity too. I think Tahoe-LAFS currently has too much complexity in various ways, but one of those ways is the command-line syntax. I don't know whether anyone has scripts that would break, and if they do, maybe it would be not a big deal for them to detect the breakage and update their scripts. David-Sarah: is there any process by which we can prune old or redundant command-line syntax with sufficient grace period? Is it worth the effort to emit a "deprecation warning" sort of thing for options that we would like to remove in the future?

[zooko]

reviewed ​https://github.com/warner/tahoe-lafs/commit/c7e8d6907abc873a60279e2eef48769fe9c10b9f

(There is very little to say about it. It is an important patch, but a very simple one. ☺ There was one typo that Brian found and this patch fixes it.)

[zooko]

reviewed: ​https://github.com/warner/tahoe-lafs/commit/96200cf7d26f1b0fd6f39856c403063f3f0f0211

Once again, this was *very* easy to review, since it simply renames a variable. Thanks to warner for making patches which are easy to review!

[zooko]

reviewed: ​https://github.com/warner/tahoe-lafs/commit/de15710cc29bb8104f9ee0f9e365f28c7ec7d24d#commitcomment-2972281

Good work!

[warner]

Rebased to 3ee950f09ed8b7f6 and landed.

[daira]

The following are filesystem command options but should probably be global options:

  -u, --node-url=     Specify the URL of the Tahoe gateway node, such as
                      'http://127.0.0.1:3456'. This overrides the URL found in
                      the --node-directory .
      --dir-cap=      Specify which dirnode URI should be used as the 'tahoe'
                      alias.

Also, in e.g. tahoe ls --help, --version is listed as a command option but does not work in that position:

[...]/bin/tahoe:  --version not allowed on subcommands

[daira]

Some commands, such as tahoe ls, don't show "[global-opts]" in their --help synopsis.

[Daira Hopwood <david-sarah@…>]

In 57e997809055960a:

Add "[global-opts]" to help synopsis for tahoe ls. refs #166

Signed-off-by: Daira Hopwood <david-sarah@…>

[daira]

Replying to daira:

Some commands, such as tahoe ls, don't show "[global-opts]" in their --help synopsis.

tahoe ls was the only one.

[daira]

NEWS entry added in becae165d2971bd3.