gn: don't reject absolute unix style pathnames in labels

gn: don't reject absolute unix style pathnames in labels

Absolute paths have been suggested by brettw as a way for external products
based upon chromium to include code outside the chromium source tree.  gn
supports absolute paths in theory, but this has not been tried in practice yet.
This patch is the first step towards testing this proposed setup.

Some background details:
https://groups.google.com/a/chromium.org/forum/#!topic/gn-dev/CU7MzRLrQuE

BUG=445454
R=brettw@chromium.org

Committed: https://crrev.com/db6c2cc6aa2ce8bd5b70964fefe7e39f2e5487a9
Cr-Commit-Position: refs/heads/master@{#309753}

[Mostyn Bramley-Moore, 2014-12-29 22:04:31]

mostynb@opera.com changed reviewers:
+ brettw@chromium.org

[Mostyn Bramley-Moore, 2014-12-29 22:04:54]

@brettw: PTAL

[tfarina, 2014-12-29 22:58:11]

Could you describe in the CL description why it shouldn't be rejected?

[Mostyn Bramley-Moore, 2014-12-29 23:21:01]

On 2014/12/29 22:58:11, tfarina wrote:
> Could you describe in the CL description why it shouldn't be rejected?

Done.

[brettw, 2014-12-30 01:25:22]

lgtm

[tfarina, 2014-12-30 02:38:00]

Thanks for including the context/background (gn-dev discussion) in the CL
description.

lgtm

https://codereview.chromium.org/825013003/diff/1/tools/gn/label.cc
File tools/gn/label.cc (left):

https://codereview.chromium.org/825013003/diff/1/tools/gn/label.cc#oldcode40
tools/gn/label.cc:40: if (input[0] == '/' && (input.size() == 1 || input[1] !=
'/')) {
OK. let's try.

[Mostyn Bramley-Moore, 2014-12-30 08:28:52]

The CQ bit was checked by mostynb@opera.com

[commit-bot: I haz the power, 2014-12-30 08:29:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/825013003/1

[commit-bot: I haz the power, 2014-12-30 09:13:34]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2014-12-30 09:14:14]

Patchset 1 (id:??) landed as
https://crrev.com/db6c2cc6aa2ce8bd5b70964fefe7e39f2e5487a9
Cr-Commit-Position: refs/heads/master@{#309753}

[tfarina, 2015-01-07 20:33:30]

Mostyn, did you run gn_unittests on this change?

I'm getting:

$ out/Debug/gn_unittests --gtest_filter=Label*
Note: Google Test filter = Label*
[==========] Running 2 tests from 2 test cases.
[----------] Global test environment set-up.
[----------] 1 test from LabelPattern
[ RUN      ] LabelPattern.PatternParse
[       OK ] LabelPattern.PatternParse (0 ms)
[----------] 1 test from LabelPattern (0 ms total)

[----------] 1 test from Label
[ RUN      ] Label.Resolve
../../tools/gn/label_unittest.cc:75: Failure
Value of: !err.has_error()
  Actual: true
Expected: cur.success
Which is: false
6 /chrome:bar
../../tools/gn/label_unittest.cc:75: Failure
Value of: !err.has_error()
  Actual: true
Expected: cur.success
Which is: false
7 /chrome/:bar
../../tools/gn/label_unittest.cc:75: Failure
Value of: !err.has_error()
  Actual: true
Expected: cur.success
Which is: false
25 :bar(/t/b)
[  FAILED  ] Label.Resolve (1 ms)
[----------] 1 test from Label (1 ms total)

[----------] Global test environment tear-down
[==========] 2 tests from 2 test cases ran. (1 ms total)
[  PASSED  ] 1 test.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] Label.Resolve

 1 FAILED TEST

[Mostyn Bramley-Moore, 2015-01-07 20:50:02]

On 01/07/2015 09:33 PM, tfarina@chromium.org wrote:
> Mostyn, did you run gn_unittests on this change?

No- I assumed the try bots would run the relevant tests automatically 
(and they were all green).

I can take a look in a couple of hours, if you like.

-Mostyn.

> I'm getting:
>
> $ out/Debug/gn_unittests --gtest_filter=Label*
> Note: Google Test filter = Label*
> [==========] Running 2 tests from 2 test cases.
> [----------] Global test environment set-up.
> [----------] 1 test from LabelPattern
> [ RUN      ] LabelPattern.PatternParse
> [       OK ] LabelPattern.PatternParse (0 ms)
> [----------] 1 test from LabelPattern (0 ms total)
>
> [----------] 1 test from Label
> [ RUN      ] Label.Resolve
> ../../tools/gn/label_unittest.cc:75: Failure
> Value of: !err.has_error()
>    Actual: true
> Expected: cur.success
> Which is: false
> 6 /chrome:bar
> ../../tools/gn/label_unittest.cc:75: Failure
> Value of: !err.has_error()
>    Actual: true
> Expected: cur.success
> Which is: false
> 7 /chrome/:bar
> ../../tools/gn/label_unittest.cc:75: Failure
> Value of: !err.has_error()
>    Actual: true
> Expected: cur.success
> Which is: false
> 25 :bar(/t/b)
> [  FAILED  ] Label.Resolve (1 ms)
> [----------] 1 test from Label (1 ms total)
>
> [----------] Global test environment tear-down
> [==========] 2 tests from 2 test cases ran. (1 ms total)
> [  PASSED  ] 1 test.
> [  FAILED  ] 1 test, listed below:
> [  FAILED  ] Label.Resolve
>
>   1 FAILED TEST
>
>
> https://codereview.chromium.org/825013003/


-- 
Mostyn Bramley-Moore
TV and Connected Devices
Opera Software ASA
mostynb@opera.com

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[tfarina, 2015-01-07 21:00:32]

On Wed, Jan 7, 2015 at 6:49 PM, Mostyn Bramley-Moore <mostynb@opera.com>
wrote:

> On 01/07/2015 09:33 PM, tfarina@chromium.org wrote:
>
>> Mostyn, did you run gn_unittests on this change?
>>
>
> No- I assumed the try bots would run the relevant tests automatically (and
> they were all green).
>
> Unfortunately we don't run it on the bots yet. Hence the bots are green
for this change.

I can take a look in a couple of hours, if you like.
>
> sgtm

-- 
Thiago Farina

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Mostyn Bramley-Moore, 2015-01-07 22:00:16]

Fixup pushed for review over here:
https://codereview.chromium.org/827643003/