Pants options cannot come after the target spec

areitz commented 9 years ago

This example works fine:

[prometheus pants (master)]$ ./pants gen.protoc --lang=java examples/src/protobuf/com/pants/examples/distance

17:21:31 00:00 [main]
               (To run a reporting server: ./pants server)
17:21:31 00:00   [bootstrap]
17:21:31 00:00   [setup]
17:21:31 00:00     [parse]
               Executing tasks in goals: bootstrap -> imports -> unpack-jars -> deferred-sources -> gen
17:21:31 00:00   [bootstrap]
17:21:31 00:00     [bootstrap-jvm-tools]
17:21:31 00:00   [imports]
17:21:31 00:00     [ivy-imports]
17:21:31 00:00   [unpack-jars]
17:21:31 00:00     [unpack-jars]
17:21:31 00:00   [deferred-sources]
17:21:31 00:00     [deferred-sources]
17:21:31 00:00   [gen]
17:21:31 00:00     [thrift]
17:21:31 00:00     [scrooge]
17:21:31 00:00     [protoc]
                   Invalidated 1 target containing 1 payload file.
17:21:31 00:00     [antlr]
17:21:31 00:00     [ragel]
17:21:31 00:00     [jaxb]
17:21:31 00:00     [wire]
17:21:31 00:00     [aapt]
               SUCCESS

The same thing, but putting the option at the end, dies horribly:

[prometheus pants (master)]$ ./pants gen.protoc examples/src/protobuf/com/pants/examples/distance --lang=java
Traceback (most recent call last):
  File "/Users/areitz/workspace/pants/pants.pex/.bootstrap/_pex/pex.py", line 272, in execute
    self.execute_entry(entry_point, args)
  File "/Users/areitz/workspace/pants/pants.pex/.bootstrap/_pex/pex.py", line 320, in execute_entry
    runner(entry_point)
  File "/Users/areitz/workspace/pants/pants.pex/.bootstrap/_pex/pex.py", line 343, in execute_pkg_resources
    runner()
  File "/Users/areitz/workspace/pants/pants.pex/pants/bin/pants_exe.py", line 66, in main
  File "/Users/areitz/workspace/pants/pants.pex/pants/bin/pants_exe.py", line 60, in _run
  File "/Users/areitz/workspace/pants/pants.pex/pants/bin/goal_runner.py", line 107, in setup
  File "/Users/areitz/workspace/pants/pants.pex/pants/bin/goal_runner.py", line 133, in _expand_goals_and_specs
  File "/Users/areitz/workspace/pants/pants.pex/pants/option/options.py", line 211, in for_global_scope
  File "/Users/areitz/workspace/pants/pants.pex/pants/option/options.py", line 192, in for_scope
  File "/Users/areitz/workspace/pants/pants.pex/pants/option/parser.py", line 147, in parse_args
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/argparse.py", line 1691, in parse_args
    self.error(msg % ' '.join(argv))
  File "/Users/areitz/workspace/pants/pants.pex/pants/option/parser.py", line 23, in error
ParseError: unrecognized arguments: --lang=java

Using the combined option name does work though:

[prometheus pants (master)]$ ./pants gen.protoc examples/src/protobuf/com/pants/examples/distance --gen-protoc-lang=java

17:26:26 00:00 [main]
               (To run a reporting server: ./pants server)
17:26:26 00:00   [bootstrap]
17:26:26 00:00   [setup]
17:26:26 00:00     [parse]
               Executing tasks in goals: bootstrap -> imports -> unpack-jars -> deferred-sources -> gen
17:26:26 00:00   [bootstrap]
17:26:26 00:00     [bootstrap-jvm-tools]
17:26:26 00:00   [imports]
17:26:26 00:00     [ivy-imports]
17:26:26 00:00   [unpack-jars]
17:26:26 00:00     [unpack-jars]
17:26:26 00:00   [deferred-sources]
17:26:26 00:00     [deferred-sources]
17:26:26 00:00   [gen]
17:26:26 00:00     [thrift]
17:26:26 00:00     [scrooge]
17:26:26 00:00     [protoc]
17:26:26 00:00     [antlr]
17:26:26 00:00     [ragel]
17:26:26 00:00     [jaxb]
17:26:26 00:00     [wire]
17:26:26 00:00     [aapt]
               SUCCESS

Maybe we should just get rid of the short form (that follows the task)? I think that the current situation is pretty confusing.

jsirois commented 9 years ago

The short form was discussed over several summits and via pants-devel and design doc'd etc at length. It would have been advantageous to express reservations then.

ericzundel commented 9 years ago

to be fair, reservations to the short form were raised many times (by me). Benjy recorded an action item that may may help mitigate Andy's concern, which is to list the long form of the option alongside the sort form of the option.

https://docs.google.com/a/squareup.com/document/d/1CEv9T7KkxS1HFcCU4NaTgXzqImpB6L_k22qX5_jVjhU/edit#bookmark=id.wslgv7ujwpus

ericzundel commented 9 years ago

@areitz The changes discussed at the summit are now in. Can we close this issue?

areitz commented 9 years ago

While I don't get a stack trace, it still feels confusing. An obvious thing to do (to me), if you're starting off with pants is something like this:

[drift pants (master)]$ ./pants compile --help
...
  --[no-]compile-java-warnings
  --[no-]warnings         Compile with all configured warnings enabled.
                          (default: True)
...

You see --[no-]warnings in the help, and think "cool, let's disable the warnings":

[drift pants (master)]$ ./pants compile examples/src/java/com/pants/examples/hello/greet --no-warnings

Exception message: unrecognized arguments: --no-warnings

I think at a minimum, we could improve this error message to include a link to a doc that explains short vs. long arguments. It might also be possible to go a bit farther, and have pants spit out something that says:

Sorry, the argument '--no-warnings' is either ambiguous or doesn't exist. Did you mean:
  --[no-]compile-scala-warnings
  --[no-]compile-apt-warnings
  --[no-]compile-java-warnings

I have no idea how hard it would be to implement that, but I think it would pants a lot more understandable for newcomers (and would even be nice for old-timers).

ericzundel commented 9 years ago

The short form of the options has been removed from help.

pantsbuild / pants

Pants options cannot come after the target spec #1024