Added brotli enc/ and tools/ directories.

Added brotli enc/ and tools/ directories.

BUG=332521
Committed: https://crrev.com/cdf50b7c6ae3ba7c64c5f619d46bf9b157b07cfb
Cr-Commit-Position: refs/heads/master@{#393898}

[smaier, 2016-05-06 19:00:01]

This review is missing the file "third_party/brotli/enc/static_dict_lut.h". It
is a large file (~1MB), and according to
https://groups.google.com/a/chromium.org/forum/#!msg/infra-dev/tSnGrouea8s/VN...
, I am going to have to land the file manually. This means that I expect bots
not to be happy with the change until this large file can be pushed in.

[smaier, 2016-05-06 19:02:37]

Description was changed from

==========
Added brotli enc/ and tools/ directories.

BUG=332521
==========

to

==========
Added brotli enc/ and tools/ directories.

BUG=332521
==========

[smaier, 2016-05-06 19:02:37]

smaier@chromium.org changed reviewers:
+ bashi@chromium.org, ksakamoto@chromium.org

[smaier, 2016-05-06 19:03:28]

smaier@chromium.org changed reviewers:
+ phajdan.jr@chromium.org

[smaier, 2016-05-06 19:03:29]

phajdan.jr@chromium.org: Please review changes in PRESUBMIT.py

[bashi, 2016-05-09 00:42:58]

rubber-stamp lgtm.

[eustas, 2016-05-09 09:04:30]

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

[eustas, 2016-05-09 09:04:31]

rfc-format.py and version.h are not used.

[Paweł Hajdan Jr., 2016-05-10 14:24:14]

phajdan.jr@chromium.org changed reviewers:
+ brettw@chromium.org, jam@chromium.org

[Paweł Hajdan Jr., 2016-05-10 14:24:15]

+brettw,jam for the PRESUBMIT change ; it affects checkdeps, so wanted to get
your advice

[jam, 2016-05-10 18:11:54]

On 2016/05/10 14:24:15, Paweł Hajdan Jr. wrote:
> +brettw,jam for the PRESUBMIT change ; it affects checkdeps, so wanted to get
> your advice

brettw is better reviewer than me, he wrote checkdeps

[brettw, 2016-05-11 17:55:54]

lgtm

[smaier, 2016-05-11 18:00:48]

The CQ bit was checked by smaier@chromium.org

[smaier, 2016-05-11 18:00:49]

The patchset sent to the CQ was uploaded after l-g-t-m from bashi@chromium.org
Link to the patchset: https://codereview.chromium.org/1956893002/#ps10001
(title: "Removed unused files")

[commit-bot: I haz the power, 2016-05-11 18:01:13]

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

[commit-bot: I haz the power, 2016-05-11 18:29:28]

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

[commit-bot: I haz the power, 2016-05-11 18:29:29]

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

[agrieve, 2016-05-12 01:38:31]

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

[agrieve, 2016-05-12 01:38:33]

https://codereview.chromium.org/1956893002/diff/20001/third_party/brotli/BUIL...
File third_party/brotli/BUILD.gn (right):

https://codereview.chromium.org/1956893002/diff/20001/third_party/brotli/BUIL...
third_party/brotli/BUILD.gn:94: configs -= [ "//build/config/gcc:no_exceptions"
]
Found in brotli/setup.py that it sets /EHsc to enable exception support. There's
no config to remove for win because looks like there's no exception support by
default.

So, I'd guess the bot fix is to add:

} else if (is_win) {
  cflags_cc = [ "/EHsc" ]
}

[smaier, 2016-05-12 18:35:10]

The CQ bit was checked by smaier@chromium.org

[smaier, 2016-05-12 18:35:11]

The patchset sent to the CQ was uploaded after l-g-t-m from brettw@chromium.org,
bashi@chromium.org
Link to the patchset: https://codereview.chromium.org/1956893002/#ps50001
(title: "Presubmit passing in both ways it can be run")

[commit-bot: I haz the power, 2016-05-12 18:35:30]

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

[smaier, 2016-05-12 18:36:46]

https://codereview.chromium.org/1956893002/diff/20001/third_party/brotli/BUIL...
File third_party/brotli/BUILD.gn (right):

https://codereview.chromium.org/1956893002/diff/20001/third_party/brotli/BUIL...
third_party/brotli/BUILD.gn:94: configs -= [ "//build/config/gcc:no_exceptions"
]
On 2016/05/12 01:38:33, agrieve wrote:
> Found in brotli/setup.py that it sets /EHsc to enable exception support.
There's
> no config to remove for win because looks like there's no exception support by
> default.
> 
> So, I'd guess the bot fix is to add:
> 
> } else if (is_win) {
>   cflags_cc = [ "/EHsc" ]
> }

Done.

[commit-bot: I haz the power, 2016-05-12 19:42:24]

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

[commit-bot: I haz the power, 2016-05-12 19:42:26]

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

[smaier, 2016-05-12 19:58:58]

The CQ bit was checked by smaier@chromium.org

[commit-bot: I haz the power, 2016-05-12 19:59:26]

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

[commit-bot: I haz the power, 2016-05-12 21:03:53]

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

[commit-bot: I haz the power, 2016-05-12 21:03:55]

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

[smaier, 2016-05-16 18:15:46]

The CQ bit was checked by smaier@chromium.org

[smaier, 2016-05-16 18:15:47]

The patchset sent to the CQ was uploaded after l-g-t-m from brettw@chromium.org,
bashi@chromium.org
Link to the patchset: https://codereview.chromium.org/1956893002/#ps60001
(title: "Updated to most recent build tools")

[commit-bot: I haz the power, 2016-05-16 18:16:08]

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

[commit-bot: I haz the power, 2016-05-16 19:33:45]

Description was changed from

==========
Added brotli enc/ and tools/ directories.

BUG=332521
==========

to

==========
Added brotli enc/ and tools/ directories.

BUG=332521
==========

[commit-bot: I haz the power, 2016-05-16 19:33:47]

Committed patchset #7 (id:60001)

[commit-bot: I haz the power, 2016-05-16 19:35:13]

Description was changed from

==========
Added brotli enc/ and tools/ directories.

BUG=332521
==========

to

==========
Added brotli enc/ and tools/ directories.

BUG=332521

Committed: https://crrev.com/cdf50b7c6ae3ba7c64c5f619d46bf9b157b07cfb
Cr-Commit-Position: refs/heads/master@{#393898}
==========

[commit-bot: I haz the power, 2016-05-16 19:35:14]

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

[brucedawson, 2016-05-16 23:42:20]

On 2016/05/16 19:35:14, commit-bot: I haz the power wrote:
> Patchset 7 (id:??) landed as
> https://crrev.com/cdf50b7c6ae3ba7c64c5f619d46bf9b157b07cfb
> Cr-Commit-Position: refs/heads/master@{#393898}

It appears that this is building for gn only? Is that true? This fails on my
Windows gn build (component, debug, enable_nacl, 32-bit) and appears to not be
built on my Windows gyp build. Warnings/errors are:

c:\src\chromium4\src\third_party\brotli\enc\cluster.h(91) : error C2220: warning
treated as error - no 'object' file generated
c:\src\chromium4\src\third_party\brotli\enc\cluster.h(91) : warning C4701:
potentially uninitialized local variable 'p' used
c:\src\chromium4\src\third_party\brotli\enc\cluster.h(91) : warning C4701:
potentially uninitialized local variable 'p' used

c:\src\chromium4\src\third_party\brotli\enc\hash.h(659): error C2220: warning
treated as error - no 'object' file generated
c:\src\chromium4\src\third_party\brotli\enc\hash.h(659): warning C4146: unary
minus operator applied to unsigned type, result still unsigned

This happens when building the all target or the 'bro' target.

Anybody else seeing this?

[brucedawson, 2016-05-17 00:31:17]

I opened crbug.com/612349 for the Windows build failures. I'm working around
them locally.

[eustas, 2016-05-17 11:00:54]

On 2016/05/17 00:31:17, brucedawson wrote:
> I opened crbug.com/612349 for the Windows build failures. I'm working around
> them locally.

This is fixed in internal brotli repo. Unfortunately, there is big change to
encoder in process -> not published to github repo.