Add real variable support and split --variable in 3 different flags.

Add real variable support and split --variable in 3 different flags.

Standardize version number handling and bump .isolated file version to 1.1.

Make valid variable names explicit.

Add explicit support fo _SUFFIX variables that won't be matched for path prefix.

Clarify subtle subdir behavior w.r.t. variables.

This removes a lot of the internal chromium specific hacks.

R=vadimsh@chromium.org
BUG=chromium:324911

Committed: https://code.google.com/p/swarming/source/detail?repo=client&r=1c1edd6

[M-A, 2013-12-04 18:49:07]

The change is relatively tricky but good code coverage helped here.

The next client roll in chromium will require a change to build/isolate.gypi so
the '-V' will be '--config-variable' or '--path-variable' accordingly.

This fixes a long standing issue about which variable should be considered a
configuration variable. This CL fixes this.

[vadimsh, 2013-12-04 21:16:40]

Mostly lgtm, though I'm not very familiar with isolate.py, so I could miss
something...

https://codereview.appspot.com/37180043/diff/60001/isolate.py
File isolate.py (right):

https://codereview.appspot.com/37180043/diff/60001/isolate.py#newcode54
isolate.py:54: VALID_VARIABLE = '[A-Za-z_]+'
no digits?

https://codereview.appspot.com/37180043/diff/60001/isolate.py#newcode370
isolate.py:370: """Bases the file on the most restrictive variable."""
I don't see where 'most restrictive' part is implemented... It just picks
whatever applies first in sorted path_variables list.

https://codereview.appspot.com/37180043/diff/60001/isolate.py#newcode388
isolate.py:388: if f.startswith(value):
Is there a guarantee that 'value' ends with '/'?

Imagine value = '/some/path' and f = '/some/path_with_suffix'. Converted result
would be '<(VARIABLE)_with_suffix'.

https://codereview.appspot.com/37180043/diff/60001/isolate.py#newcode1293
isolate.py:1293: raise isolateserver.ConfigError(
I assume in that case saved file will just be ignored, right?

https://codereview.appspot.com/37180043/diff/60001/isolate.py#newcode1504
isolate.py:1504: return None
Maybe raise ExecutionError instead? This property both raises exceptions and
returns None to indicate 'something is wrong'. It's better to keep only one.

(Also I don't fully understand how root_dir can become '').

https://codereview.appspot.com/37180043/diff/60001/isolate.py#newcode1681
isolate.py:1681: help='Filters to a subdirectory. It\'s behavior changes
depending if it '
nit: 'Its', not 'it's'

[M-A, 2013-12-04 21:54:56]

https://codereview.appspot.com/37180043/diff/60001/isolate.py
File isolate.py (right):

https://codereview.appspot.com/37180043/diff/60001/isolate.py#newcode54
isolate.py:54: VALID_VARIABLE = '[A-Za-z_]+'
On 2013/12/04 21:16:40, vadimsh wrote:
> no digits?

Nope.

https://codereview.appspot.com/37180043/diff/60001/isolate.py#newcode370
isolate.py:370: """Bases the file on the most restrictive variable."""
On 2013/12/04 21:16:40, vadimsh wrote:
> I don't see where 'most restrictive' part is implemented... It just picks
> whatever applies first in sorted path_variables list.

That's true, fixed around line 384.

https://codereview.appspot.com/37180043/diff/60001/isolate.py#newcode388
isolate.py:388: if f.startswith(value):
On 2013/12/04 21:16:40, vadimsh wrote:
> Is there a guarantee that 'value' ends with '/'?
> 
> Imagine value = '/some/path' and f = '/some/path_with_suffix'. Converted
result
> would be '<(VARIABLE)_with_suffix'.

No, but that's fine. There are cases where the path name is some form of
executable name.

https://codereview.appspot.com/37180043/diff/60001/isolate.py#newcode1293
isolate.py:1293: raise isolateserver.ConfigError(
On 2013/12/04 21:16:40, vadimsh wrote:
> I assume in that case saved file will just be ignored, right?

Yes and added unit test in isolate_test.py just to be safe.

https://codereview.appspot.com/37180043/diff/60001/isolate.py#newcode1504
isolate.py:1504: return None
On 2013/12/04 21:16:40, vadimsh wrote:
> Maybe raise ExecutionError instead? This property both raises exceptions and
> returns None to indicate 'something is wrong'. It's better to keep only one.
> 
> (Also I don't fully understand how root_dir can become ''). 

Removed this code, this happened only when I had broken the code elsewhere.

https://codereview.appspot.com/37180043/diff/60001/isolate.py#newcode1681
isolate.py:1681: help='Filters to a subdirectory. It\'s behavior changes
depending if it '
On 2013/12/04 21:16:40, vadimsh wrote:
> nit: 'Its', not 'it's'

Fixed and extracted in a function so the same help text can be used in CMDcheck.

[vadimsh, 2013-12-04 22:08:38]

lgtm

https://codereview.appspot.com/37180043/diff/80001/isolate.py
File isolate.py (right):

https://codereview.appspot.com/37180043/diff/80001/isolate.py#newcode389
isolate.py:389: value = path_variables[key]
This line is not needed anymore.

[M-A, 2013-12-06 14:13:46]

Committed patchset #4 manually as r1c1edd6 (presubmit successful).