[devil] Upgrade parser/serializer for command line flags

[devil] Upgrade parser/serializer for command line flags

Upgrade the code to parse/serialize flags in the command line file.
Internally flags are now stored unquoted (to match the behaviour of
desktop flags) and only parded/serialized when reading/writing from
the file system.

The algorithms now match the behaviour of Chrome's own command line
parsing code.

API changes:
- Clients should now supply flags unquoted (but to ease the transition
  flags that appear to be quoted are not double-quoted).
- There is a new GetCurrentFlags method to read the current flags
  stored on the device.
- All methods to set/update flags now return the final set of
  flags stored on the device.

BUG=catapult:#3112

Review-Url: https://codereview.chromium.org/2633133002
Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/c4ea5aba503c326e243e7b35a110a35fa5d47cd6

[perezju, 2017-01-16 14:07:23]

Description was changed from

==========
[devil] Upgrade parser/serializer for command line flags

Upgrade the code to parse/serialize flags in the command line file.
Internally flags are now stored unquoted (to match the behaviour of
desktop flags) and only parded/serialized when reading/writing from
the file system.

The algorithms now match the behaviour of Chrome's own command line
parsing code.

API changes:
- Clients should now supply flags unquoted (but to ease the transition
  flags that appear to be quoted are not double-quoted).
- There is a new GetCurrentFlags method to read the current flags
  stored on the device.
- All methods to set/update flags now return the final set of
  flags stored on the device.

BUG=catapult:#3028
==========

to

==========
[devil] Upgrade parser/serializer for command line flags

Upgrade the code to parse/serialize flags in the command line file.
Internally flags are now stored unquoted (to match the behaviour of
desktop flags) and only parded/serialized when reading/writing from
the file system.

The algorithms now match the behaviour of Chrome's own command line
parsing code.

API changes:
- Clients should now supply flags unquoted (but to ease the transition
  flags that appear to be quoted are not double-quoted).
- There is a new GetCurrentFlags method to read the current flags
  stored on the device.
- All methods to set/update flags now return the final set of
  flags stored on the device.

BUG=catapult:#3112
==========

[perezju, 2017-01-16 14:12:18]

perezju@chromium.org changed reviewers:
+ jbudorick@chromium.org, rnephew@chromium.org

[perezju, 2017-01-16 14:12:19]

https://codereview.chromium.org/2633133002/diff/1/devil/devil/android/file_ch...
File devil/devil/android/file_changer_devicetest.py (left):

https://codereview.chromium.org/2633133002/diff/1/devil/devil/android/file_ch...
devil/devil/android/file_changer_devicetest.py:1: #!/usr/bin/env python
Didn't notice on my previous CL, file should be named
*flag*_changer_devicetest.py

[perezju, 2017-01-18 11:26:58]

The CQ bit was checked by perezju@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-18 11:27:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[perezju, 2017-01-18 11:27:23]

Friendly ping. :)

Hope you can have a look.

[commit-bot: I haz the power, 2017-01-18 11:47:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-18 11:47:18]

Dry run: This issue passed the CQ dry run.

[jbudorick, 2017-01-18 14:56:28]

lgtm

Any idea why rietveld thinks you've deleted and readded the tests?

https://codereview.chromium.org/2633133002/diff/20001/devil/devil/android/fla...
File devil/devil/android/flag_changer.py (right):

https://codereview.chromium.org/2633133002/diff/20001/devil/devil/android/fla...
devil/devil/android/flag_changer.py:14: _RE_NEEDS_QUOTING =
re.compile(r'[^\w-]')  # Not in: alphanumeric or hyphens.
I'm surprised that this doesn't also include underscores.

https://codereview.chromium.org/2633133002/diff/20001/devil/devil/android/fla...
devil/devil/android/flag_changer.py:181: It's a direct port of
CommandLine.java::tokenizeQuotedArguments.
somewhat amusingly, it's misspelled in CommandLine.java as
"tokenizeQuotedAruments"

[perezju, 2017-01-18 15:19:31]

On 2017/01/18 14:56:28, jbudorick wrote:
> lgtm
> 
> Any idea why rietveld thinks you've deleted and readded the tests?

I renamed the files "file_changer" -> "flag_changer", not sure how I missed that
when writing the first CL that added these tests.

https://codereview.chromium.org/2633133002/diff/20001/devil/devil/android/fla...
File devil/devil/android/flag_changer.py (right):

https://codereview.chromium.org/2633133002/diff/20001/devil/devil/android/fla...
devil/devil/android/flag_changer.py:14: _RE_NEEDS_QUOTING =
re.compile(r'[^\w-]')  # Not in: alphanumeric or hyphens.
On 2017/01/18 14:56:28, jbudorick wrote:
> I'm surprised that this doesn't also include underscores.

\w already includes underscores, from the docs: "this is equivalent to the set
[a-zA-Z0-9_]"

https://codereview.chromium.org/2633133002/diff/20001/devil/devil/android/fla...
devil/devil/android/flag_changer.py:181: It's a direct port of
CommandLine.java::tokenizeQuotedArguments.
On 2017/01/18 14:56:28, jbudorick wrote:
> somewhat amusingly, it's misspelled in CommandLine.java as
> "tokenizeQuotedAruments"

yeah :-/ noted that too, wasn't sure whether to spread the misspelling here or
whether that one should be fixed over there.

[perezju, 2017-01-18 15:22:16]

The CQ bit was checked by perezju@chromium.org

[commit-bot: I haz the power, 2017-01-18 15:22:23]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-18 15:24:19]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484752936058410,
"parent_rev": "c3ec514d8726530643844182677304ce7f15a5c2", "commit_rev":
"c4ea5aba503c326e243e7b35a110a35fa5d47cd6"}

[commit-bot: I haz the power, 2017-01-18 15:24:22]

Description was changed from

==========
[devil] Upgrade parser/serializer for command line flags

Upgrade the code to parse/serialize flags in the command line file.
Internally flags are now stored unquoted (to match the behaviour of
desktop flags) and only parded/serialized when reading/writing from
the file system.

The algorithms now match the behaviour of Chrome's own command line
parsing code.

API changes:
- Clients should now supply flags unquoted (but to ease the transition
  flags that appear to be quoted are not double-quoted).
- There is a new GetCurrentFlags method to read the current flags
  stored on the device.
- All methods to set/update flags now return the final set of
  flags stored on the device.

BUG=catapult:#3112
==========

to

==========
[devil] Upgrade parser/serializer for command line flags

Upgrade the code to parse/serialize flags in the command line file.
Internally flags are now stored unquoted (to match the behaviour of
desktop flags) and only parded/serialized when reading/writing from
the file system.

The algorithms now match the behaviour of Chrome's own command line
parsing code.

API changes:
- Clients should now supply flags unquoted (but to ease the transition
  flags that appear to be quoted are not double-quoted).
- There is a new GetCurrentFlags method to read the current flags
  stored on the device.
- All methods to set/update flags now return the final set of
  flags stored on the device.

BUG=catapult:#3112

Review-Url: https://codereview.chromium.org/2633133002
Committed:
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...
==========

[commit-bot: I haz the power, 2017-01-18 15:24:23]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...

[jbudorick, 2017-01-18 15:25:10]

On 2017/01/18 15:19:31, perezju wrote:
> On 2017/01/18 14:56:28, jbudorick wrote:
> > lgtm
> > 
> > Any idea why rietveld thinks you've deleted and readded the tests?
> 
> I renamed the files "file_changer" -> "flag_changer", not sure how I missed
that
> when writing the first CL that added these tests.

Clearly I also missed that when reviewing this CL :)

> 
>
https://codereview.chromium.org/2633133002/diff/20001/devil/devil/android/fla...
> File devil/devil/android/flag_changer.py (right):
> 
>
https://codereview.chromium.org/2633133002/diff/20001/devil/devil/android/fla...
> devil/devil/android/flag_changer.py:14: _RE_NEEDS_QUOTING =
> re.compile(r'[^\w-]')  # Not in: alphanumeric or hyphens.
> On 2017/01/18 14:56:28, jbudorick wrote:
> > I'm surprised that this doesn't also include underscores.
> 
> \w already includes underscores, from the docs: "this is equivalent to the set
> [a-zA-Z0-9_]"

huh, didn't realize that.

> 
>
https://codereview.chromium.org/2633133002/diff/20001/devil/devil/android/fla...
> devil/devil/android/flag_changer.py:181: It's a direct port of
> CommandLine.java::tokenizeQuotedArguments.
> On 2017/01/18 14:56:28, jbudorick wrote:
> > somewhat amusingly, it's misspelled in CommandLine.java as
> > "tokenizeQuotedAruments"
> 
> yeah :-/ noted that too, wasn't sure whether to spread the misspelling here or
> whether that one should be fixed over there.

Fixed there, imo, but that's out of scope.