Ignoring bad switches

[Evan Stade]

Hi chromiumites,

tl;dr: I want to add --abort-on-bad-flag to chrome. Any objections?

Long version:

Chrome currently ignores bad command line switches. For example,

$ google-chrome --foo

is equivalent to

$ google-chrome

for unknown switch foo. Picky users may want to know when their
switches are being ignored, but I understand that in most cases we
don't want to choke on bad switches. I propose adding another switch
--abort-on-bad-flag which causes chrome to abort if an unrecognized
switch is passed (and possibly output an error message).

Besides being useful to alert careful users when their switch is no
longer supported (for example, when something like --enable-extensions
goes away), it would also help for using switches with unknown chrome
versions. I know this won't work retroactively (unless we merge it to
stable/beta branches, which is unlikely), but consider the
hypothetical utility of this flag *if it had been included in earlier
versions of chrome*, and the future utility as more use cases like the
following example crop up:

I recently added a --version switch to posix chrome. Older versions
don't have it, so for them,

$ google-chrome --version

will launch a new chrome instance instead of just printing the version
and exiting. That seems bad. It would be better to be able to do

$ google-chrome --version --abort-on-bad-flag

which would at least not launch a new chrome instance.

-- Evan Stade

[Albert J. Wong (王重傑)]

+1

Having been bitten a number of times by mistyping a flag, I like this idea a lot.

-Albert

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
   http://groups.google.com/a/chromium.org/group/chromium-dev

[Peter Kasting]

On Mon, Jun 14, 2010 at 7:58 PM, Evan Stade <est...@chromium.org> wrote:

Hi chromiumites,

tl;dr: I want to add --abort-on-bad-flag to chrome. Any objections?

I think I tried to implement something like this a while back and ended up bailing because it was too hard.  Among the problems was that there are many lists of valid flags, not one, some of which are only recognized by certain processes or only used in particular unit tests.

I support the idea, but really doing this right may be tricky.

PK

[Peter Kasting]

On Mon, Jun 14, 2010 at 8:02 PM, Peter Kasting <pkas...@google.com> wrote:

On Mon, Jun 14, 2010 at 7:58 PM, Evan Stade <est...@chromium.org> wrote:

Hi chromiumites,

tl;dr: I want to add --abort-on-bad-flag to chrome. Any objections?

I support the idea, but really doing this right may be tricky.

BTW, you should look at http://code.google.com/p/chromium/issues/detail?id=35564 .  It reminds me that another issue is that some switches aren't listed _anywhere_, they're just raw string comparisons buried in various places.

I still would love to see that bug get fixed -- I think it's a much better experience than --abort-on-bad-flag, although perhaps you could temporarily implement that to try and test the infrastructure...

PK 

[Mark Mentovai]

I’d love it if we adopted gflags instead of the command-line flag
architecture (mess) we use now. It wouldn’t be a fun or easy project,
but it would probably help with some of the concerns raised on this
thread.

http://crbug.com/26821

[William Chan (陈智昌)]

+1 to gflags. Failing that, +1 to estade's proposal.

[Zachary Hirsch]

On Mon, Jun 14, 2010 at 8:19 PM, Mark Mentovai <ma...@chromium.org> wrote:

I'd like to help out with this.  Is there anything I need to be aware of (other than carefully making sure that the command line syntax doesn't change on Linux/Windows/Mac)?

Thanks,

-Zach

[Mark Mentovai]

Zachary Hirsch wrote:
> I'd like to help out with this.  Is there anything I need to be aware of
> (other than carefully making sure that the command line syntax doesn't
> change on Linux/Windows/Mac)?

The big thing will be the wchar_t/wstring issue. Command-line
processing is one area of Chrome that hasn’t really seen a resolution.
If you look in base/command_line.{h|cc}, you’ll find a mess of
std::string and std::wstring. Before you wind up too deeply involved
in a Chrome-gflags project, you should have a careful plan, and that
plan may wind up involving extending gflags.

Mark

[Evan Martin]

To shorten what some others have said, have fun trying to track all
these guys down:

http://www.google.com/codesearch?hl=en&sa=N&filter=0&q=package:chromium+CommandLine.*Switch.*%22

:(

On Mon, Jun 14, 2010 at 7:58 PM, Evan Stade <est...@chromium.org> wrote:

[Darin Fisher]

Using gflags does sound tempting.  Does gflags use static initializers to register everything?  How costly is it?

-Darin

On Mon, Jun 14, 2010 at 8:19 PM, Mark Mentovai <ma...@chromium.org> wrote:

[Mark Mentovai]

Darin Fisher wrote:
> Using gflags does sound tempting.  Does gflags use static initializers to
> register everything?

Yes.

> How costly is it?

It doesn’t appear to be very expensive.

Mark

[William Chan (陈智昌)]

Yes, it uses static initializers. I don't think it'd be that costly,
but perhaps it's worth testing out first.

[Darin Fisher]

I think it is clear that CommandLine could use some string cleanup.  There are a

lot of cleanup TODOs.

It is nice that CommandLine has support for returning

FilePath values.

How well does gflags work on Windows?  Does it use GetCommandLineW, or

does it attempt to do everything with char variants?

-Darin

[Evan Stade]

I'm unfamiliar with gflags. Would it replace CommandLine entirely, or
just the parsing code?

I'll see if I can't get rid of the string literal flags. Seems like a
necessary step even if we do use gflags.

-- Evan Stade

[Fred Akalin]

Also worth mentioning that using gflags would make it a lot easier to
move to using google-glog (which itself uses gflags).

[Mark Mentovai]

Fred Akalin wrote:
> Also worth mentioning that using gflags would make it a lot easier to
> move to using google-glog (which itself uses gflags).

That’s http://crbug.com/26820, and I also think it’d be worthwhile.

Mark

[Robert Sesek]

I threw together a script that generates a report of all the switches and where they're used. It also extracts the comments from chrome_switches.cc to document them.  I have an interest in the gflags conversion project, so I figured this would be a good first step.

If you're internal, you can take a look at http://www/~rsesek/switch-scan/

I've also uploaded a ZIP of the report here: http://cl.ly/1O0N

rsesek / @chromium.org

--

[Peter Kasting]

On Tue, Jun 15, 2010 at 4:15 PM, Robert Sesek <rse...@chromium.org> wrote:

I threw together a script that generates a report of all the switches and where they're used.

Which switches does this enumerate?  Everything in chrome_switches.h?  Everything in the "switches" namespace?  Every argument to any of the CommandLine functions?

(I'm trying to determine what this does for the "hard" cases, which range from "defined in a separate header" to "raw string comparisons in unit tests".)

PK

[Robert Sesek]

It uses chrome_switches.cc to create the list and gather the docs, and then it searches the code for the associated constants.

rsesek / @chromium.org

[Marc-Antoine Ruel]

Please don't use non-POD static initializer.

Beside code bloat, it will fail on some command line parsing that
happens *before* main().

M-A

[Jim Roskind]

Perhaps a less hostile solution than "failing" on unexpected flags (and yet able to handle funky embedded command-line queries) would be to have an about:flags page that listed all flags that were:

a) Tested for during the process startup, and found on the command line. (We might even show the values).

b) Never tested for during the process startup, but found on the command line (this is the stuff that folks wanted to fail on).

Since both lists would be small (I hope), the memory usage or perf-cost of recording dynamically what was checked for should be small.

For me, as a developer, it would be sufficient to see that I typed the command-line flags correctly... and then adding or removing a flag would not be as hostile to on old setup.  (example: a developer that added a flag eons ago, and suddenly finds that Chrome won't start anymore... but perhaps the proposal included a pop-up explanation).

Jim

[Peter Kasting]

On Tue, Jun 15, 2010 at 6:23 PM, Jim Roskind <j...@chromium.org> wrote:

Perhaps a less hostile solution than "failing" on unexpected flags (and yet able to handle funky embedded command-line queries) would be to have an about:flags page that listed all flags that were:

a) Tested for during the process startup, and found on the command line. (We might even show the values).

b) Never tested for during the process startup, but found on the command line (this is the stuff that folks wanted to fail on).

This requires the same underlying machinery as the other solutions (which is the hard part), but requires a greater UI investment, and fails to achieve the primary use case, which is to auto-warn users who use outdated, invalid, or misspelled command-line flags (these users will not be checking an "about:flags" page).

"Hostility" to such flags is precisely the point.

PK

[Robert Sesek]

Thanks. I've updated the report to include switches from the following files:

SWITCHES = [

  './src/app/app_switches.cc',

  './src/base/base_switches.cc',

  './src/chrome/common/chrome_switches.cc',

  './src/ipc/ipc_switches.cc',

  './src/media/base/media_switches.cc',

  './src/webkit/glue/plugins/plugin_switches.cc',

]

rsesek / @chromium.org

On Wed, Jun 16, 2010 at 12:21 AM, Viet-Trung Luu <viettr...@gmail.com> wrote:

That presumably misses some switches in base/base_switches.cc. (This is easy to fix, of course.)

Robert Sesek wrote:

It uses chrome_switches.cc to create the list and gather the docs, and then it searches the code for the associated constants.

rsesek / @chromium.org <http://chromium.org>

On Tue, Jun 15, 2010 at 7:39 PM, Peter Kasting <pkas...@google.com <mailto:pkas...@google.com>> wrote:

   On Tue, Jun 15, 2010 at 4:15 PM, Robert Sesek <rse...@chromium.org

   <mailto:rse...@chromium.org>> wrote:

       I threw together a script that generates a report of all the
       switches and where they're used.


   Which switches does this enumerate?  Everything in
   chrome_switches.h?  Everything in the "switches" namespace?  Every
   argument to any of the CommandLine functions?

   (I'm trying to determine what this does for the "hard" cases, which
   range from "defined in a separate header" to "raw string comparisons
   in unit tests".)

   PK

[Robert Shield]

and now, from the correct address.. 

On Wed, Jun 16, 2010 at 10:35 AM, Robert Sesek <rse...@chromium.org> wrote:

Thanks. I've updated the report to include switches from the following files:

SWITCHES = [

  './src/app/app_switches.cc',

  './src/base/base_switches.cc',

  './src/chrome/common/chrome_switches.cc',

  './src/ipc/ipc_switches.cc',

  './src/media/base/media_switches.cc',

  './src/webkit/glue/plugins/plugin_switches.cc',

]

 

Please note that the installer also leans heavily on the CommandLine machinery and has switches defined in './src/chrome/installer/util/util_constants.h'.

[Darin Fisher]

Yeah... I'm concerned that gflags may not be a great fit.  It obviously wasn't designed with Windows in mind, and that might cause some problems.  I haven't investigated yet...

-Darin