Accept absolute Windows paths without leading slash in GN commands

Accept absolute Windows paths without leading slash in GN commands

Leading slash is removed before path is passed to GN when command is run
on MSYS shell. See http://www.mingw.org/wiki/Posix_path_conversion.

BUG=590686
Committed: https://crrev.com/006cf0f95a92cbd6780b9d85fad3e415fd97bec9
Cr-Commit-Position: refs/heads/master@{#385686}

[Tomasz Moniuszko, 2016-02-29 09:57:46]

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

[Tomasz Moniuszko, 2016-03-02 11:26:05]

@brettw: Hi, will you accept such patch? It's a kind of workaround but I guess
it's everything we can do. Leading slash is being removed before passing to GN
so we cannot avoid the removal. Many developers find Git Bash more convenient
than Windows command-line and this patch makes their work even easier.

[Tomasz Moniuszko, 2016-03-30 14:44:54]

On 2016/03/02 11:26:05, Tomasz Moniuszko wrote:
> @brettw: Hi, will you accept such patch? It's a kind of workaround but I guess
> it's everything we can do. Leading slash is being removed before passing to GN
> so we cannot avoid the removal. Many developers find Git Bash more convenient
> than Windows command-line and this patch makes their work even easier.

ping :)

This review is still hanging. IsPathAbsolute() understands both "C:/" and "/C:/"
formats. It's convenient if label (and label pattern) also accepts them both.

[brettw, 2016-03-31 17:51:13]

Whoops, sorry I lost this. I'm out for the rest of the week but will look at
this early next week. Please feel free to ping me again.

[brettw, 2016-04-05 00:01:57]

LGTM with test addition.

https://codereview.chromium.org/1742303002/diff/1/tools/gn/label_unittest.cc
File tools/gn/label_unittest.cc (right):

https://codereview.chromium.org/1742303002/diff/1/tools/gn/label_unittest.cc#...
tools/gn/label_unittest.cc:43: { "//chrome/", "C:/chrome:bar",             true,
 "/C:/chrome/",        "bar",  "//t/",   "d" },
Can you add a test here that "C:foo" is parsed as the "foo" target inside the
"C" directory relative to the current one.

[Tomasz Moniuszko, 2016-04-05 12:17:16]

The CQ bit was checked by tmoniuszko@opera.com

[Tomasz Moniuszko, 2016-04-05 12:17:16]

The patchset sent to the CQ was uploaded after l-g-t-m from brettw@chromium.org
Link to the patchset: https://codereview.chromium.org/1742303002/#ps20001
(title: "Add a test according to the review comment")

[commit-bot: I haz the power, 2016-04-05 12:17:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1742303002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1742303002/20001

[commit-bot: I haz the power, 2016-04-05 12:25:39]

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

[commit-bot: I haz the power, 2016-04-05 12:25:40]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Tomasz Moniuszko, 2016-04-05 13:06:21]

On 2016/04/05 12:25:40, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

@brettw: I think it's nice to have the same formatting for all lines in test
files. Thus I ran 'git cl format --full'. It affects also lines that were not
introduced by this CL. Is it OK for you?

[Tomasz Moniuszko, 2016-04-05 13:07:25]

The CQ bit was checked by tmoniuszko@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-04-05 13:07:31]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1742303002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1742303002/40001

[commit-bot: I haz the power, 2016-04-05 13:39:34]

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

[commit-bot: I haz the power, 2016-04-05 13:39:35]

Dry run: This issue passed the CQ dry run.

[brettw, 2016-04-05 17:14:37]

I kind of preferred the table format of the previous code. Even though it breaks
the style guide, it seems easier to follow to me. What do you think?

[Tomasz Moniuszko, 2016-04-06 08:30:16]

On 2016/04/05 17:14:37, brettw wrote:
> I kind of preferred the table format of the previous code. Even though it
breaks
> the style guide, it seems easier to follow to me. What do you think?

I tried to integrate Patch Set 2 first but chromium_presubmit failed. Patch Set
3 differs only in style. It's not landed yet. Table format looks more clear. Is
it possible to avoid presubmit.py failures and integrate Patch Set 2 instead?

[brettw, 2016-04-06 16:26:39]

Hmm, I guess I can live with the reformatted one. LGTM for the full formatting.

[Tomasz Moniuszko, 2016-04-07 06:50:41]

The CQ bit was checked by tmoniuszko@opera.com

[commit-bot: I haz the power, 2016-04-07 06:50:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1742303002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1742303002/40001

[commit-bot: I haz the power, 2016-04-07 07:29:58]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-04-07 07:30:57]

Description was changed from

==========
Accept absolute Windows paths without leading slash in GN commands

Leading slash is removed before path is passed to GN when command is run
on MSYS shell. See http://www.mingw.org/wiki/Posix_path_conversion.

BUG=590686
==========

to

==========
Accept absolute Windows paths without leading slash in GN commands

Leading slash is removed before path is passed to GN when command is run
on MSYS shell. See http://www.mingw.org/wiki/Posix_path_conversion.

BUG=590686

Committed: https://crrev.com/006cf0f95a92cbd6780b9d85fad3e415fd97bec9
Cr-Commit-Position: refs/heads/master@{#385686}
==========

[commit-bot: I haz the power, 2016-04-07 07:30:58]

Patchset 3 (id:??) landed as
https://crrev.com/006cf0f95a92cbd6780b9d85fad3e415fd97bec9
Cr-Commit-Position: refs/heads/master@{#385686}