[libphonenumber] Change DEPS to track new github mirror, uprev library

[libphonenumber] Change DEPS to track new github mirror, uprev library

Changes to build files and include paths.

BUG=585286, 585244
TBR=tedchoc

Committed: https://crrev.com/09e9b25eea4777845bf56d53948d123212f5efa7
Cr-Commit-Position: refs/heads/master@{#374841}

[Mathieu, 2016-02-10 02:53:28]

mathp@chromium.org changed reviewers:
+ rouslan@chromium.org

[Mathieu, 2016-02-10 02:53:28]

Hi Rouslan, PTAL.

Please make sure I didn't forget to update anything in
third_party/libphonenumber

[please use gerrit instead, 2016-02-10 18:21:33]

I'm fairly sure that we need to clone libphonenumber into
third_party/libphonenumber/src. That way Chrome's files for libphonenumber can
live in third_party/libphonenumber/. For example, that's where BUILD.gn lives.

[Mathieu, 2016-02-10 19:21:51]

Patchset #5 (id:80001) has been deleted

[Mathieu, 2016-02-10 20:08:33]

Patchset #4 (id:60001) has been deleted

[Mathieu, 2016-02-10 20:08:40]

Patchset #6 (id:140001) has been deleted

[Mathieu, 2016-02-10 20:08:54]

Patchset #5 (id:120001) has been deleted

[Mathieu, 2016-02-10 20:09:05]

Patchset #3 (id:40001) has been deleted

[Mathieu, 2016-02-10 22:13:22]

On 2016/02/10 18:21:33, Rouslan wrote:
> I'm fairly sure that we need to clone libphonenumber into
> third_party/libphonenumber/src. That way Chrome's files for libphonenumber can
> live in third_party/libphonenumber/. For example, that's where BUILD.gn lives.

I've rebased now that the license issue is resolved. PTAL.

[please use gerrit instead, 2016-02-10 22:18:04]

lgtm after .gitignore is fixed.

https://codereview.chromium.org/1674373003/diff/160001/.gitignore
File .gitignore (right):

https://codereview.chromium.org/1674373003/diff/160001/.gitignore#newcode352
.gitignore:352: /third_party/libphonenumber
/third_party/libphonenumber/src

[Mathieu, 2016-02-10 22:19:33]

Description was changed from

==========
[libphonenumber] Change DEPS to track new github mirror, uprev library

Changes to build files and include paths.

BUG=585286,585244
==========

to

==========
[libphonenumber] Change DEPS to track new github mirror, uprev library

Changes to build files and include paths.

BUG=585286,585244
TBR=tedchoc
==========

[Mathieu, 2016-02-10 22:19:42]

mathp@chromium.org changed reviewers:
+ tedchoc@chromium.org

[Mathieu, 2016-02-10 22:19:54]

Thanks!

TBR tedchoc for include path change.

https://codereview.chromium.org/1674373003/diff/160001/.gitignore
File .gitignore (right):

https://codereview.chromium.org/1674373003/diff/160001/.gitignore#newcode352
.gitignore:352: /third_party/libphonenumber
On 2016/02/10 22:18:04, Rouslan wrote:
> /third_party/libphonenumber/src

Done.

[Mathieu, 2016-02-10 22:20:02]

The CQ bit was checked by mathp@chromium.org

[Mathieu, 2016-02-10 22:20:03]

The patchset sent to the CQ was uploaded after l-g-t-m from rouslan@chromium.org
Link to the patchset: https://codereview.chromium.org/1674373003/#ps80002
(title: ".gitignore")

[commit-bot: I haz the power, 2016-02-10 22:21:48]

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

[Mathieu, 2016-02-11 00:53:35]

Patchset #6 (id:180001) has been deleted

[Mathieu, 2016-02-11 00:54:59]

The CQ bit was checked by mathp@chromium.org

[Mathieu, 2016-02-11 00:54:59]

The patchset sent to the CQ was uploaded after l-g-t-m from rouslan@chromium.org
Link to the patchset: https://codereview.chromium.org/1674373003/#ps200001
(title: "README file")

[commit-bot: I haz the power, 2016-02-11 00:56:41]

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

[commit-bot: I haz the power, 2016-02-11 02:28:09]

Description was changed from

==========
[libphonenumber] Change DEPS to track new github mirror, uprev library

Changes to build files and include paths.

BUG=585286,585244
TBR=tedchoc
==========

to

==========
[libphonenumber] Change DEPS to track new github mirror, uprev library

Changes to build files and include paths.

BUG=585286,585244
TBR=tedchoc
==========

[commit-bot: I haz the power, 2016-02-11 02:28:10]

Committed patchset #6 (id:200001)

[Nico, 2016-02-11 02:41:33]

thakis@chromium.org changed reviewers:
+ thakis@chromium.org

[Nico, 2016-02-11 02:41:34]

This doesn't build on clang/win:

https://build.chromium.org/p/chromium.fyi/builders/CrWinClang%28dbg%29/builds...

..\..\third_party\libphonenumber\src\cpp\src\phonenumbers/base/synchronization/lock.h(47,3)
:  error: constructor for 'i18n::phonenumbers::Lock' must explicitly initialize
the const member 'thread_checker_'
  Lock() {}
  ^
..\..\third_party\libphonenumber\src\cpp\src\phonenumbers/base/synchronization/lock.h(60,23)
:  note: 'thread_checker_' declared here
  const ThreadChecker thread_checker_;
                      ^
1 error generated.

This is because C++11 requires that const objects have explicit default ctors,
and I guess ThreadChecker doesn't have one. (Note: `= default` doesn't count, it
needs to be `{}`). Can we please revert this until this is resolved?

[Nico, 2016-02-11 02:44:05]

A revert of this CL (patchset #6 id:200001) has been created in
https://codereview.chromium.org/1687243002/ by thakis@chromium.org.

The reason for reverting is: Needs some tweaks to build on clang/win, see
comment 24. (Checked with mathp on chat.).

[Mathieu, 2016-02-11 13:36:38]

On 2016/02/11 02:44:05, Nico wrote:
> A revert of this CL (patchset #6 id:200001) has been created in
> https://codereview.chromium.org/1687243002/ by mailto:thakis@chromium.org.
> 
> The reason for reverting is: Needs some tweaks to build on clang/win, see
> comment 24. (Checked with mathp on chat.).

Hi Nico, 

Thanks for reverting. Last change to the problematic file was in 2013 in
libphonenumber, and I thought our previous DEPS was younger than that. Did it
compile before? Is it just a problem for windows clang?

Let me know the best way to repro this compile failure and how best to fix it
(send them a patch?). This no-argument constructor [1] won't do?

[1]
https://github.com/googlei18n/libphonenumber/blob/master/cpp/src/phonenumbers...

[Nico, 2016-02-11 14:06:00]

huh, that should do it, weird. let me take a quick look.

[Nico, 2016-02-11 17:59:54]

https://codereview.chromium.org/1674373003/diff/200001/DEPS
File DEPS (left):

https://codereview.chromium.org/1674373003/diff/200001/DEPS#oldcode241
DEPS:241: Var('chromium_git') + '/external/libphonenumber/resources.git' + '@' +
'b6dfdc7952571ff7ee72643cd88c988cbe966396',
I'm playing around with this a bit, and after this CL I have both
third_party\libphonenumber\src with the new code and
third_party\libphonenumber\src\phonenumbers with the old code. Did you test that
moving a collection of repos deps'd in like so works correctly?

[Nico, 2016-02-11 18:08:07]

The errors on
https://build.chromium.org/p/chromium.fyi/builders/CrWinClang%28dbg%29/builds...
refer to the old file paths. I can't repro locally, but I wonder if that's
related...

[Mathieu, 2016-02-11 18:16:58]

https://codereview.chromium.org/1674373003/diff/200001/DEPS
File DEPS (left):

https://codereview.chromium.org/1674373003/diff/200001/DEPS#oldcode241
DEPS:241: Var('chromium_git') + '/external/libphonenumber/resources.git' + '@' +
'b6dfdc7952571ff7ee72643cd88c988cbe966396',
On 2016/02/11 17:59:54, Nico wrote:
> I'm playing around with this a bit, and after this CL I have both
> third_party\libphonenumber\src with the new code and
> third_party\libphonenumber\src\phonenumbers with the old code. Did you test
that
> moving a collection of repos deps'd in like so works correctly?

When you are done gclient sync with my change, it should say "you are free to
remove third_party/libphonenumber/src/phonenumbers" etc. No?

[Nico, 2016-02-11 18:19:23]

On 2016/02/11 18:16:58, Mathieu Perreault wrote:
> https://codereview.chromium.org/1674373003/diff/200001/DEPS
> File DEPS (left):
> 
> https://codereview.chromium.org/1674373003/diff/200001/DEPS#oldcode241
> DEPS:241: Var('chromium_git') + '/external/libphonenumber/resources.git' + '@'
+
> 'b6dfdc7952571ff7ee72643cd88c988cbe966396',
> On 2016/02/11 17:59:54, Nico wrote:
> > I'm playing around with this a bit, and after this CL I have both
> > third_party\libphonenumber\src with the new code and
> > third_party\libphonenumber\src\phonenumbers with the old code. Did you test
> that
> > moving a collection of repos deps'd in like so works correctly?
> 
> When you are done gclient sync with my change, it should say "you are free to
> remove third_party/libphonenumber/src/phonenumbers" etc. No?

Maybe? `git clean` didn't remove them since they're git repos. And the bot has a
build error in the old file location, or am I misreading that?

[commit-bot: I haz the power, 2016-02-16 22:34:11]

Description was changed from

==========
[libphonenumber] Change DEPS to track new github mirror, uprev library

Changes to build files and include paths.

BUG=585286,585244
TBR=tedchoc
==========

to

==========
[libphonenumber] Change DEPS to track new github mirror, uprev library

Changes to build files and include paths.

BUG=585286,585244
TBR=tedchoc

Committed: https://crrev.com/09e9b25eea4777845bf56d53948d123212f5efa7
Cr-Commit-Position: refs/heads/master@{#374841}
==========

[commit-bot: I haz the power, 2016-02-16 22:34:14]

Patchset 6 (id:??) landed as
https://crrev.com/09e9b25eea4777845bf56d53948d123212f5efa7
Cr-Commit-Position: refs/heads/master@{#374841}