[rac] Add libaddressinput dependency

[rac] Add libaddressinput dependency

This patch adds a dependency for the testdata/ and cpp/ directory in
libaddressinput repository. The library will live in:
 - src/third_party/libaddressinput/cpp
 - src/third_party/libaddressinput/testdata

BUG=247202
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=234085

[please use gerrit instead, 2013-09-26 01:57:51]

Evan: PTAL.

[Evan Stade, 2013-09-26 02:08:09]

I'm not the right reviewer for this patch. I think ideally you would write this
patch as if the cpp/ directory were ready to go, then just not land it until
that is actually true.

https://codereview.chromium.org/24538003/diff/1/DEPS
File DEPS (right):

https://codereview.chromium.org/24538003/diff/1/DEPS#newcode23
DEPS:23: "libaddressinput_revision": "137",
alphabetical order?

https://codereview.chromium.org/24538003/diff/1/DEPS#newcode225
DEPS:225: (Var("googlecode_url") % "libphonenumber") +
do these lines have the wrong indent?

https://codereview.chromium.org/24538003/diff/1/DEPS#newcode363
DEPS:363: "src/third_party/libaddressinput/testdata": None,
is there some specific order to this block? it seems vaguely alphabetical

[please use gerrit instead, 2013-09-26 16:17:19]

Pawel: would you be the right person to review this patch? The OWNERS file says
"per-file DEPS=*".

https://codereview.chromium.org/24538003/diff/1/DEPS
File DEPS (right):

https://codereview.chromium.org/24538003/diff/1/DEPS#newcode23
DEPS:23: "libaddressinput_revision": "137",
On 2013/09/26 02:08:10, Evan Stade wrote:
> alphabetical order?

The existing order doesn't seem to be alphabetical. Placing address validation
next to phone validation library seems most logical.

https://codereview.chromium.org/24538003/diff/1/DEPS#newcode225
DEPS:225: (Var("googlecode_url") % "libphonenumber") +
On 2013/09/26 02:08:10, Evan Stade wrote:
> do these lines have the wrong indent?

Yes, these lines appear to have 3-space and 4-space indents. The rest of the
file uses 2-space indents.

https://codereview.chromium.org/24538003/diff/1/DEPS#newcode363
DEPS:363: "src/third_party/libaddressinput/testdata": None,
On 2013/09/26 02:08:10, Evan Stade wrote:
> is there some specific order to this block? it seems vaguely alphabetical

Not really alphabetical here, either, sorry. See the sequence below: first
"libsrtp", then "ursctp", and then "libvpx".

[Evan Stade, 2013-09-26 16:31:12]

https://codereview.chromium.org/24538003/diff/1/DEPS
File DEPS (right):

https://codereview.chromium.org/24538003/diff/1/DEPS#newcode23
DEPS:23: "libaddressinput_revision": "137",
On 2013/09/26 16:17:19, Rouslan Solomakhin wrote:
> On 2013/09/26 02:08:10, Evan Stade wrote:
> > alphabetical order?
> 
> The existing order doesn't seem to be alphabetical. Placing address validation
> next to phone validation library seems most logical.

this block of 3 (now 4) related libraries were alphabetical until you added this
one.

https://codereview.chromium.org/24538003/diff/1/DEPS#newcode225
DEPS:225: (Var("googlecode_url") % "libphonenumber") +
On 2013/09/26 16:17:19, Rouslan Solomakhin wrote:
> On 2013/09/26 02:08:10, Evan Stade wrote:
> > do these lines have the wrong indent?
> 
> Yes, these lines appear to have 3-space and 4-space indents. The rest of the
> file uses 2-space indents.

could you fix that?

https://codereview.chromium.org/24538003/diff/1/DEPS#newcode363
DEPS:363: "src/third_party/libaddressinput/testdata": None,
On 2013/09/26 16:17:19, Rouslan Solomakhin wrote:
> On 2013/09/26 02:08:10, Evan Stade wrote:
> > is there some specific order to this block? it seems vaguely alphabetical
> 
> Not really alphabetical here, either, sorry. See the sequence below: first
> "libsrtp", then "ursctp", and then "libvpx". 

if you take out bidichecker, usrsctp and webgpl_conformance then I think the
entire block is alphabetical. Those three mistakes should be fixed rather than
exacerbated.

[please use gerrit instead, 2013-09-26 16:43:57]

Evan: Those are all valid concerns. I've fixed the issues that you pointed out.

https://codereview.chromium.org/24538003/diff/1/DEPS
File DEPS (right):

https://codereview.chromium.org/24538003/diff/1/DEPS#newcode23
DEPS:23: "libaddressinput_revision": "137",
On 2013/09/26 16:31:12, Evan Stade wrote:
> On 2013/09/26 16:17:19, Rouslan Solomakhin wrote:
> > On 2013/09/26 02:08:10, Evan Stade wrote:
> > > alphabetical order?
> > 
> > The existing order doesn't seem to be alphabetical. Placing address
validation
> > next to phone validation library seems most logical.
> 
> this block of 3 (now 4) related libraries were alphabetical until you added
this
> one.

Good point. Sorted.

https://codereview.chromium.org/24538003/diff/1/DEPS#newcode225
DEPS:225: (Var("googlecode_url") % "libphonenumber") +
On 2013/09/26 16:31:12, Evan Stade wrote:
> On 2013/09/26 16:17:19, Rouslan Solomakhin wrote:
> > On 2013/09/26 02:08:10, Evan Stade wrote:
> > > do these lines have the wrong indent?
> > 
> > Yes, these lines appear to have 3-space and 4-space indents. The rest of the
> > file uses 2-space indents.
> 
> could you fix that?

Fixed to be 2-space indents after : and 4-space indents after +, which matches
the rest of the DEPS file.

https://codereview.chromium.org/24538003/diff/1/DEPS#newcode363
DEPS:363: "src/third_party/libaddressinput/testdata": None,
On 2013/09/26 16:31:12, Evan Stade wrote:
> On 2013/09/26 16:17:19, Rouslan Solomakhin wrote:
> > On 2013/09/26 02:08:10, Evan Stade wrote:
> > > is there some specific order to this block? it seems vaguely alphabetical
> > 
> > Not really alphabetical here, either, sorry. See the sequence below: first
> > "libsrtp", then "ursctp", and then "libvpx". 
> 
> if you take out bidichecker, usrsctp and webgpl_conformance then I think the
> entire block is alphabetical. Those three mistakes should be fixed rather than
> exacerbated.

Agreed. Sorted everything in this block.

[please use gerrit instead, 2013-09-26 16:46:14]

Pawel: Would it be more convenient for your review if I split this into two
patches? (1: Sort some existing lists and fix indentation for libphonenumber. 2:
Add libaddressinput.)

[Evan Stade, 2013-09-26 16:48:11]

thanks.

https://codereview.chromium.org/24538003/diff/10001/DEPS
File DEPS (right):

https://codereview.chromium.org/24538003/diff/10001/DEPS#newcode225
DEPS:225: # 'googlecode_url' variable here.
imo link to the bug you filed

[please use gerrit instead, 2013-09-26 17:42:22]

Done.

https://codereview.chromium.org/24538003/diff/10001/DEPS
File DEPS (right):

https://codereview.chromium.org/24538003/diff/10001/DEPS#newcode225
DEPS:225: # 'googlecode_url' variable here.
On 2013/09/26 16:48:11, Evan Stade wrote:
> imo link to the bug you filed

-  # There's no internal mirror for libaddressinput, so not using
-  # 'googlecode_url' variable here.
+  # TODO(rouslan): Use googlecode_url variable when there's an internal mirror
+  # for libaddressinput. http://crbug.com/298602.

[please use gerrit instead, 2013-10-14 17:21:34]

Pawel: PTAL.

This CL is now ready for you review, because libaddressinput has the boilerplate
C++ code checked in. 

Let me know if you think that there's a better person to review it.

[Paweł Hajdan Jr., 2013-10-14 17:36:16]

LGTM with a comment.

https://codereview.chromium.org/24538003/diff/20001/DEPS
File DEPS (right):

https://codereview.chromium.org/24538003/diff/20001/DEPS#newcode224
DEPS:224: # TODO(rouslan): Use googlecode_url variable when there's an internal
mirror
Please give it at least a trybot run to make sure it runs fine on our
infrastructure.

While I'm not entirely sure, on some bots (likely gcompute) the mirror may be
required.

[please use gerrit instead, 2013-10-16 21:27:40]

The library has been mirrored. Running try-bots with the mirror-specific
configuration.

[Evan Stade, 2013-10-16 22:30:59]

lgtm

[please use gerrit instead, 2013-10-17 00:35:47]

Brett: PTAL third_party/libaddressinput/{LICENSE,README.chromium,OWNERS} files.

[Daniel Berlin, 2013-10-17 00:35:54]

LGTM for open source review

[please use gerrit instead, 2013-10-17 00:37:28]

Brett: PTAL third_party/libaddressinput/{LICENSE,README.chromium,OWNERS} files.

(Sorry if this is a duplicate email.)

[brettw, 2013-10-20 20:33:18]

lgtm

[jln (very slow on Chromium), 2013-10-25 00:51:36]

security lgtm with nits.

Make sure to get a review when you include the rest of the code. I expect the
review to be expedited if the needed server-side changes happen before then.

https://codereview.chromium.org/24538003/diff/231001/DEPS
File DEPS (right):

https://codereview.chromium.org/24538003/diff/231001/DEPS#newcode222
DEPS:222: "src/third_party/libaddressinput/cpp":
Please add a comment "Get security approval when including more of the code.

[please use gerrit instead, 2013-11-08 21:44:49]

https://codereview.chromium.org/24538003/diff/231001/DEPS
File DEPS (right):

https://codereview.chromium.org/24538003/diff/231001/DEPS#newcode222
DEPS:222: "src/third_party/libaddressinput/cpp":
On 2013/10/25 00:51:36, jln wrote:
> Please add a comment "Get security approval when including more of the code.

Added the comment.

[please use gerrit instead, 2013-11-08 22:08:56]

Stefan: PTAL Patch Set 12.

I've made the directory modification that deps2git requires. Please let me know
what version of deps2git I should specify in DEPS. (As we discussed, this patch
requires rolling the deps2git dependency.)

[please use gerrit instead, 2013-11-08 22:13:46]

Stefan: Patch Set 13 rolls deps2git version to r234015 as you requested.

[szager1, 2013-11-08 22:13:48]

LGTM

[commit-bot: I haz the power, 2013-11-08 22:41:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rouslan@chromium.org/24538003/841001

[commit-bot: I haz the power, 2013-11-09 04:21:53]

Change committed as 234085